Source Code Project Mantis - FSSCP
View Issue Details
0003054FSSCPmodelspublic2014-06-05 08:422014-07-14 06:10
Reporterniffiwan 
Assigned Toniffiwan 
PrioritynormalSeverityminorReproducibilityalways
StatusresolvedResolutionfixed 
PlatformOSOS Version
Product Version3.7.2 RC2 
Target VersionFixed in Version3.7.2 
Summary0003054: campaigns without starting weapons cause crashes
DescriptionAnd the crash is possibly the least helpful error I've seen to date.

ASSERTION FAILED: "Polygon_models[num]->id == model_num" at model/modelread.cpp:2979 Index collision between model warp.pof and requested model 0. Please backtrace and investigate.

Yep. No starting weapons in the campaign and it crashes with a model error, in the loadout screen, when you select the weapons tab. Incidentally, you can launch a mission, but you get no weapons on your ship. The best bit is that FSO helpfully saves this bogus info to your pilot file, meaning I needed to delete my campaign file before the "fixed" campaign would work.

ANYWAY. A campaign without starting weapons is probably an error (i.e. bad data) anyway, but it shouldn't cause crashes. The loadout code needs to handle this more gracefully.
Additional InformationThis is the crashing code, specifically draw_model_rotating():

    if(Wl_icons[Selected_wl_class].model_index != -1) {
        static float WeapSelectScreenWeapRot = 0.0f;
        wl_icon_info *sel_icon = &Wl_icons[Selected_wl_class];
        weapon_info *wip = &Weapon_info[Selected_wl_class];
        draw_model_rotating(sel_icon->model_index,
            weapon_ani_coords[0],
            weapon_ani_coords[1],
            gr_screen.res == 0 ? 202 : 332,
            gr_screen.res == 0 ? 185 : 260,
            &WeapSelectScreenWeapRot,
            &Weapon_info->closeup_pos,
            Weapon_info->closeup_zoom * 0.65f,
            REVOLUTION_RATE,
            MR_IS_MISSILE | MR_LOCK_DETAIL | MR_AUTOCENTER | MR_NO_FOGGING,
            GR_RESIZE_MENU,
            wip->selection_effect);
    }


From a quick look, it seems that the default model_index should be -1 (not 0) in order to skip attempting to draw the wrong model.
TagsNo tags attached.
Attached Filesrar CPVTest.rar (3,915) 2014-06-06 02:48
http://scp.indiegames.us/mantis/file_download.php?file_id=2374&type=bug
patch weapon_select.patch (636) 2014-07-01 16:20
http://scp.indiegames.us/mantis/file_download.php?file_id=2442&type=bug

Notes
(0016007)
ngld   
2014-07-01 16:23   
Since there is no weapon the code can't assign a valid number to Selected_wl_class (see wl_maybe_reset_selected_weapon_class).
It's default value is -1 and once this function is called it tries to evaluate Wl_icons[-1].model_index which for some reason results in 0.

The remaining code seems to have appropriate if(model_index >= 0) checks and assertions. The patch fixes the crash at least.
(0016043)
m_m   
2014-07-09 11:06   
The patch looks good to me.
I don't know if a campaign without starting weapons is actually a bug, I can see why a mission designer could want to do it so just checking if Selected_wl_class is not -1 should be sufficient.
(0016071)
niffiwan   
2014-07-14 06:06   
(Last edited: 2014-07-14 06:10)
looks good to me as well

Committed in r10918


Issue History
2014-06-05 08:42niffiwanNew Issue
2014-06-06 02:48niffiwanFile Added: CPVTest.rar
2014-07-01 16:20ngldFile Added: weapon_select.patch
2014-07-01 16:23ngldNote Added: 0016007
2014-07-05 11:54Echelon9Assigned To => niffiwan
2014-07-05 11:54Echelon9Statusnew => code review
2014-07-09 11:06m_mNote Added: 0016043
2014-07-14 06:06niffiwanAssigned Toniffiwan =>
2014-07-14 06:06niffiwanNote Added: 0016071
2014-07-14 06:10niffiwanNote Edited: 0016071bug_revision_view_page.php?bugnote_id=16071#r870
2014-07-14 06:10niffiwanStatuscode review => resolved
2014-07-14 06:10niffiwanFixed in Version => 3.7.2
2014-07-14 06:10niffiwanResolutionopen => fixed
2014-07-14 06:10niffiwanAssigned To => niffiwan
2014-07-14 06:10niffiwanAssigned Toniffiwan =>
2014-07-14 06:10niffiwanStatusresolved => feedback
2014-07-14 06:10niffiwanResolutionfixed => reopened
2014-07-14 06:10niffiwanStatusfeedback => resolved
2014-07-14 06:10niffiwanResolutionreopened => fixed
2014-07-14 06:10niffiwanAssigned To => niffiwan