View Issue Details

IDProjectCategoryView StatusLast Update
0002859FSSCPPilot datapublic2013-05-13 03:14
ReporterMacfie Assigned Toniffiwan  
PriorityurgentSeverityblockReproducibilityalways
Status resolvedResolutionfixed 
PlatformnightliesOSwindows vista 
Product Version3.6.19 
Target Version3.7.0 
Summary0002859: Game crashes if mod is changed with the same pilot in use.
DescriptionIf I change from one mod to another using the same pilot the game crashes when I select a campaign. It also corrupts the pilot so It can not be used. If you restart the game, the game will crash if you select the pilot, even if you return to the original mod. The pilot becomes unusable.
Steps To ReproduceI tried it with several nightly builds from 9622 to 9665 a long with 3.7.0 RC1. Each time I got the same results even if I had completed the campaign.
Additional InformationThe only time I did not get the crash was if I selected a common campaign within the mod that was also in the other mod (I.E. The main freespace campaign). Then I could go to the new mod and select the campaign within the campaign room without any problem. It appears the crash only occurs if you get the notice that no campaign is selected.
TagsNo tags attached.

Relationships

related to 0002868 resolvedniffiwan Unlocking missions in one campaign unlocks missions in all campaigns in the mission simulator 

Activities

niffiwan

2013-05-01 01:42

developer   ~0015002

Could you please check if the problem occurs in this nightly?
http://www.hard-light.net/forums/index.php?topic=83975.0

(i.e. original commit of pilot code to trunk)

Macfie

2013-05-01 03:18

reporter   ~0015004

Checked 9563 nightly and it exhibited the same behavior. I did find out that if you get the notice that no campaign is selected and you select the main freespace (either FS1 or FS2) campaign you can then go back into the campaign room and select the campaign associated with the mod without a crash.

MjnMixael

2013-05-01 03:36

manager   ~0015005

Woah, glad this one was caught as it would defeat the whole purpose of the new pilot code!

Based on that last comment, (shooting from the hip here), I wonder if 2817's fix might give a decent starting place?

Additionally, this seems to only corrupt the campaign file and not the entire pilot. I was able to delete the associated .csg and then continue just fine.

niffiwan

2013-05-01 08:19

developer   ~0015007

I'm going to need some help reproducing this :)

I've done the following without triggering a crash, r9664 Debug & Release:

1) start FSO with Vassago's Dirge as active mod
2) create a new pilot
3) select VD campaign
4) exit FSO
5) start FSO with ST:R as active mod
6) select same pilot as before
7) get popup about active campaign not being found
8) select go to campaign room
9) select ST:R campaign
10) no crash

I also tried with Transcend instead of ST:R. Do you need to enter a mission at all?

Macfie

2013-05-01 10:28

reporter   ~0015010

Last edited: 2013-05-01 12:59

With all of the pilots involved I have played through a campaign or a portion of a campaign before switching to a new Mod. Some of the pilots I tried this with were created before the pilot code. I did not try it with a new pilot.
Update
I tried a new pilot and did not have the problem it appears that this bug only affects pilots created before the pilot code. It would appear to be a problem with converting pilots.

Yarn

2013-05-01 20:11

developer   ~0015016

Last edited: 2013-05-01 20:14

I found a way to reproduce this in r9668 with a new pilot:

1) Start FSO with the FreeSpace Port as the active mod.
2) Create a new pilot.
3) Make sure that the main FreeSpace campaign is selected.
4) Enter the Ready Room and skip the first three training missions. (This step is necessary to cause the crash.)
5) Exit FSO.
6) Start FSO either without any mods or with only the MediaVPs.
7) Select the pilot that you used earlier.
8) You'll get a message about the current campaign not being found. Select "Go to Campaign Room."
9) Select the FreeSpace 2 campaign, and then click "Select."
10) The game will crash without an error message, even if you're using a debug build.

After following these steps, if you load the pilot again without mods (or with the MediaVPs), the game will crash with no error message before the main hall appears.

Oh, and if you use a debug build, then you'll get this warning just before step 8:

main_hall_init() was passed a blank mainhall name, loading first available mainhall.
ntdll.dll! ZwWaitForSingleObject + 21 bytes
kernel32.dll! WaitForSingleObjectEx + 67 bytes
kernel32.dll! WaitForSingleObject + 18 bytes
fs2_open_3_6_19-DEBUG-20130501_r9668.exe! <no symbol>
fs2_open_3_6_19-DEBUG-20130501_r9668.exe! <no symbol>
fs2_open_3_6_19-DEBUG-20130501_r9668.exe! <no symbol>
fs2_open_3_6_19-DEBUG-20130501_r9668.exe! <no symbol>
fs2_open_3_6_19-DEBUG-20130501_r9668.exe! <no symbol>
fs2_open_3_6_19-DEBUG-20130501_r9668.exe! <no symbol>
fs2_open_3_6_19-DEBUG-20130501_r9668.exe! <no symbol>
fs2_open_3_6_19-DEBUG-20130501_r9668.exe! <no symbol>
fs2_open_3_6_19-DEBUG-20130501_r9668.exe! <no symbol>
fs2_open_3_6_19-DEBUG-20130501_r9668.exe! <no symbol>
fs2_open_3_6_19-DEBUG-20130501_r9668.exe! <no symbol>
kernel32.dll! BaseThreadInitThunk + 18 bytes
ntdll.dll! RtlInitializeExceptionChain + 99 bytes
ntdll.dll! RtlInitializeExceptionChain + 54 bytes

niffiwan

2013-05-02 09:31

developer   ~0015018

Thanks, I can reproduce this now.

niffiwan

2013-05-02 11:42

developer   ~0015020

I think the issue is that FSO reads part of the current (freespace.csg) savefile before it realises that the .fc2 doesn't exist in the current mod. And not all the read data gets reset before loading the new campaign (Freespace2.csg), in particular here that's the num_missions_completed. This results in reading non-existent mission data from .csg, an array index waaaay outside the array size, a write to that location and boom.

I've got a fix (that I want to test some more) that, when reading a .csg, checks that the campaign file can be found before reading any data. I thought this was more foolproof than setting Campaign.num_missions_completed = 0; in mission/missioncampaign.cpp mission_campaign_load() (although that's probably a good idea anyway).

Any comments?

(and here's the patch anyway, note that I've only done basic testing on this so far...)

niffiwan

2013-05-02 11:44

developer  

mantis2859-svn.patch (1,225 bytes)   
Index: code/pilotfile/csg.cpp
===================================================================
--- code/pilotfile/csg.cpp	(revision 9657)
+++ code/pilotfile/csg.cpp	(working copy)
@@ -1328,6 +1328,13 @@
 
 	filename = buf.str().c_str();
 
+	// if campaign file doesn't exist, abort so we don't load irrelevant data
+	buf.str(std::string());
+	buf << base << FS_CAMPAIGN_FILE_EXT;
+	if ( !cf_exists_full((char*)buf.str().c_str(), CF_TYPE_MISSIONS) ) {
+		mprintf(("CSG => Unable to find campaign file '%s'!\n", buf.str().c_str()));
+		return false;
+	}
 
 	// we need to reset this early, in case open fails and we need to create
 	m_data_invalid = false;
Index: code/mission/missioncampaign.cpp
===================================================================
--- code/mission/missioncampaign.cpp	(revision 9657)
+++ code/mission/missioncampaign.cpp	(working copy)
@@ -655,6 +655,7 @@
 	Campaign.prev_mission = -1;
 	Campaign.current_mission = -1;
 	Campaign.loop_mission = CAMPAIGN_LOOP_MISSION_UNINITIALIZED;
+	Campaign.num_missions_completed = 0;
 
 	// loading the campaign will get us to the current and next mission that the player must fly
 	// plus load all of the old goals that future missions might rely on.
mantis2859-svn.patch (1,225 bytes)   

Echelon9

2013-05-02 11:50

developer   ~0015022

Yes, I think I've seen this occur where the num_missions_completed carries across between campaigns. i.e. complete three missions in BP1 with a new pilot, then switch to BP2 and you see three completed missions.

niffiwan

2013-05-04 08:49

developer   ~0015032

I've done some more testing now including:
1) switching mods
2) switching campaigns within a mod
3) switching mods using a converted pilot

I'm happy with this patch now.

However, possibly for a second patch, I need to check the rest of the Campaign struct to see if there's anything else that needs to be reset when a new campaign is loaded.

Echelon9

2013-05-04 23:32

developer   ~0015034

Fix committed to trunk@9673.

Related Changesets

fs2open: trunk r9673

2013-05-04 20:25

Echelon9


Ported: N/A

Details Diff
Fix Mantis 2859: Game crashes if mod is changed with the same pilot in use. From niffiwan. Affected Issues
0002859
mod - /trunk/fs2_open/code/mission/missioncampaign.cpp Diff File
mod - /trunk/fs2_open/code/pilotfile/csg.cpp Diff File

Issue History

Date Modified Username Field Change
2013-04-30 23:59 Macfie New Issue
2013-05-01 01:19 niffiwan Priority high => urgent
2013-05-01 01:19 niffiwan Severity crash => block
2013-05-01 01:19 niffiwan Target Version => 3.7.0
2013-05-01 01:42 niffiwan Note Added: 0015002
2013-05-01 03:18 Macfie Note Added: 0015004
2013-05-01 03:36 MjnMixael Note Added: 0015005
2013-05-01 08:19 niffiwan Note Added: 0015007
2013-05-01 10:28 Macfie Note Added: 0015010
2013-05-01 10:28 Macfie Note Edited: 0015010
2013-05-01 10:29 Macfie Note Edited: 0015010
2013-05-01 12:59 Macfie Note Edited: 0015010
2013-05-01 20:11 Yarn Note Added: 0015016
2013-05-01 20:14 Yarn Note Edited: 0015016
2013-05-01 20:14 Yarn Note Edited: 0015016
2013-05-02 09:31 niffiwan Note Added: 0015018
2013-05-02 09:31 niffiwan Assigned To => niffiwan
2013-05-02 09:31 niffiwan Status new => confirmed
2013-05-02 11:42 niffiwan Note Added: 0015020
2013-05-02 11:44 niffiwan File Added: mantis2859-svn.patch
2013-05-02 11:50 Echelon9 Note Added: 0015022
2013-05-02 11:51 Echelon9 Assigned To niffiwan =>
2013-05-02 11:51 Echelon9 Status confirmed => code review
2013-05-04 08:49 niffiwan Note Added: 0015032
2013-05-04 23:32 Echelon9 Changeset attached => fs2open trunk r9673
2013-05-04 23:32 Echelon9 Note Added: 0015034
2013-05-04 23:32 Echelon9 Status code review => resolved
2013-05-04 23:32 Echelon9 Resolution open => fixed
2013-05-05 01:09 niffiwan Assigned To => niffiwan
2013-05-05 01:09 niffiwan Status resolved => feedback
2013-05-05 01:09 niffiwan Resolution fixed => reopened
2013-05-05 01:09 niffiwan Status feedback => resolved
2013-05-05 01:09 niffiwan Resolution reopened => fixed
2013-05-13 03:14 Echelon9 Relationship added related to 0002868