Source Code Project Mantis - FSSCP
View Issue Details
0002404FSSCPtablespublic2011-02-25 18:122014-11-21 12:56
Assigned ToThe_E 
PlatformOSOS Version
Product Version3.6.13 
Target Version3.7.2Fixed in Version 
Summary0002404: $Weapon Model Draw Distance: does nothing
DescriptionIt's in the wiki, defined and parsed in ship.h and ship.cpp but never used anywhere. Basically it defaults it to 200 and stuffs the value if it exists in ships.tbl but that's it.

If this is going to be implimented it would be nice to be able to tie it to the LOD system as well. So a value of 200 would be multiplied by the LOD factors (default .125 .25, 1, 4, 8). An alternate option would be specifing what LODs the models are visible on.
TagsNo tags attached.
Attached Filespatch 2404.patch (5,727) 2012-12-24 04:50
patch 2404_r11171.patch (5,673) 2014-11-11 20:55

2012-12-20 01:48   
Feature request. Targeted for 3.7.2.
2012-12-21 06:11   
Added a draft patch for this. Please review.
2012-12-24 01:37   
So if I'm reading this correctly it will take whatever you have tabled or the default of 200 and scale it by the defined detail distances and not draw if beyond that. So using retail defaults it would draw weapons out to 25, 50, 200, 800, 1600 based on your detail setting.

I see 2 possible issues here.
First is it appears to only work for primaries unless that info is set somewhere else. When I found this what I was trying to do was cut off rendering of models displayed with show secondaries at distances where they really couldn't be seen attached to the ship anymore.

Second is do we really want to change the behavior if this value isn't specified? I don't have a problem with it but I can see where others might. Maybe it would be better to just fall back to the old behavior if the entry does not exist in the table. Possibly change the default to -1 (it appears the 200 was never used) and if the value is -1 just skip that check and render as it used to.
2012-12-24 04:53   
I've attached a new patch that should work for secondaries as well.

As for the other issue you've raised, I believe that some form of lodding for external weapon models is a good thing to have. If the default distance value is too small, well, then we can discuss raising it, but I am of the opinion that _some_ value should be there. Rendering weapon models when they're very definitely not going to be seen is a bad idea in my opinion.
2014-07-27 06:10   
I tried running a test with Diaspora (which I know displays external weapons by default) and the patch causes a crash when entering the Techroom after pilot selection. It seems that The_mission.ai_profile is a NULL pointer at this stage?

Without the patch, this code prevents the case statement from being executed:
if ( Interp_flags & MR_LOCK_DETAIL ) {
    i = Interp_detail_level_locked+1;
} else {
2014-07-30 06:39   
Surrounding the crashing code with
    if (!(Interp_flags & MR_LOCK_DETAIL)) {
fixes the crash. Given that that flag says that detail scaling should not be done it seems like a good solution to this problem.
2014-08-21 06:14   
Confirmed that change fixed the techroom crash. However, when trying to play Diaspora "The Strike" I hit this assert just after the Theseus jumps in:

Assert((objnum == Interp_objnum && !(flags & MR_ATTACHED_MODEL)));

Changing it to this (which I think matches what the comment says) lets me proceed past that point.

Assert(objnum == Interp_objnum || (flags & MR_ATTACHED_MODEL));

However, I then start hitting other asserts within 30 seconds of mission start, while trying to get a look at the ships external missile models from different distances.

ASSERTION FAILED: "instance_depth<MAX_INSTANCE_DEPTH" at render/3dsetup.cpp:256
ASSERTION FAILED: "GL_modelview_matrix_depth == 2" at graphics/gropengltnl.cpp:1727

(FYI, I applied the patch to r9461 from SVN; I haven't updated it for current trunk yet)
2014-11-11 20:57   
I couldn't get the second assertion to trigger, but then, I tried this with latest trunk. The first assertion was caused by the return skipping a g3_done_instance() call; I flew around in "The Strike" with an executable made from the attached patch for a few minutes shooting raiders without triggering any assertions.
2014-11-21 12:56   
Fix committed to trunk@11178.

Issue History
2011-02-25 18:12FUBAR-BDHRNew Issue
2012-12-20 01:48MjnMixaelNote Added: 0014551
2012-12-20 01:48MjnMixaelSeverityminor => feature
2012-12-20 01:48MjnMixaelTarget Version => 3.7.2
2012-12-21 06:10The_EFile Added: 2404.patch
2012-12-21 06:11The_ENote Added: 0014555
2012-12-21 06:11The_EAssigned To => The_E
2012-12-21 06:11The_EStatusnew => code review
2012-12-24 01:37FUBAR-BDHRNote Added: 0014574
2012-12-24 04:50The_EFile Deleted: 2404.patch
2012-12-24 04:50The_EFile Added: 2404.patch
2012-12-24 04:53The_ENote Added: 0014575
2014-07-27 06:10niffiwanNote Added: 0016138
2014-07-30 06:39m_mNote Added: 0016149
2014-08-21 06:14niffiwanNote Added: 0016246
2014-11-11 20:55MageKing17File Added: 2404_r11171.patch
2014-11-11 20:57MageKing17Note Added: 0016382
2014-11-21 12:56MageKing17Changeset attached => fs2open trunk r11178
2014-11-21 12:56MageKing17Note Added: 0016388
2014-11-21 12:56MageKing17Statuscode review => resolved
2014-11-21 12:56MageKing17Resolutionopen => fixed