View Issue Details

IDProjectCategoryView StatusLast Update
0003054FSSCPmodelspublic2014-07-14 10:10
Reporterniffiwan Assigned Toniffiwan  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version3.7.2 RC2 
Fixed 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.

Activities

niffiwan

2014-06-06 06:48

developer  

CPVTest.rar (3,915 bytes)

ngld

2014-07-01 20:20

reporter  

weapon_select.patch (636 bytes)   
diff --git a/code/missionui/missionweaponchoice.cpp b/code/missionui/missionweaponchoice.cpp
index 06299f7..1990633 100644
--- a/code/missionui/missionweaponchoice.cpp
+++ b/code/missionui/missionweaponchoice.cpp
@@ -2659,7 +2659,7 @@ void weapon_select_do(float frametime)
 		weapon_ani_coords = Wl_weapon_ani_coords[gr_screen.res];
 	}
 
-	if(Wl_icons[Selected_wl_class].model_index != -1) {
+	if(Selected_wl_class != -1 && 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];
weapon_select.patch (636 bytes)   

ngld

2014-07-01 20:23

reporter   ~0016007

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.

m_m

2014-07-09 15:06

developer   ~0016043

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.

niffiwan

2014-07-14 10:06

developer   ~0016071

Last edited: 2014-07-14 10:10

looks good to me as well

Committed in r10918

Issue History

Date Modified Username Field Change
2014-06-05 12:42 niffiwan New Issue
2014-06-06 06:48 niffiwan File Added: CPVTest.rar
2014-07-01 20:20 ngld File Added: weapon_select.patch
2014-07-01 20:23 ngld Note Added: 0016007
2014-07-05 15:54 Echelon9 Assigned To => niffiwan
2014-07-05 15:54 Echelon9 Status new => code review
2014-07-09 15:06 m_m Note Added: 0016043
2014-07-14 10:06 niffiwan Assigned To niffiwan =>
2014-07-14 10:06 niffiwan Note Added: 0016071
2014-07-14 10:10 niffiwan Note Edited: 0016071
2014-07-14 10:10 niffiwan Status code review => resolved
2014-07-14 10:10 niffiwan Fixed in Version => 3.7.2
2014-07-14 10:10 niffiwan Resolution open => fixed
2014-07-14 10:10 niffiwan Assigned To => niffiwan
2014-07-14 10:10 niffiwan Assigned To niffiwan =>
2014-07-14 10:10 niffiwan Status resolved => feedback
2014-07-14 10:10 niffiwan Resolution fixed => reopened
2014-07-14 10:10 niffiwan Status feedback => resolved
2014-07-14 10:10 niffiwan Resolution reopened => fixed
2014-07-14 10:10 niffiwan Assigned To => niffiwan