View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0003051 | FSSCP | gameplay | public | 2014-05-29 22:25 | 2014-06-09 03:28 |
Reporter | Axem | Assigned To | MageKing17 | ||
Priority | high | Severity | minor | Reproducibility | always |
Status | resolved | Resolution | fixed | ||
Product Version | 3.7.0 RC1 | ||||
Target Version | 3.7.2 | Fixed in Version | 3.7.2 | ||
Summary | 0003051: Campaign Variables do not reset on mission restart | ||||
Description | Campaign Persistent variables aren't resetting on a failed or replayed mission from the debriefing screen like they should be. Works properly in 3.6.18 but suddenly has issues in 3.7.0. New pilot code issues maybe? | ||||
Steps To Reproduce | http://lazymodders.fsmods.net/files/CPVTest.rar Run the CPV Test campaign. 5 seconds into the first mission, Variable1 will increment itself by 1. Then jump out. If you don't destroy the container, the campaign will make you replay the mission. On replay the variable should again begin at 0. | ||||
Additional Information | http://www.hard-light.net/forums/index.php?topic=87667.msg1750194;boardseen#new This thread for some enlightening conversation | ||||
Tags | No tags attached. | ||||
|
Couldn't sleep; made a patch that caused the test campaign to function normally. Could really use a more experienced coder looking over what I did (and even if what I did is the correct solution, looking over other usages of mission_campaign_store_goals_and_events_and_variables() to see if they should be similarly changed). |
|
I am an idiot; somehow completely missed the the debrief_accept() function. Updated patch. |
|
cpvars2.patch (2,789 bytes)
Index: code/mission/missioncampaign.cpp =================================================================== --- code/mission/missioncampaign.cpp (revision 10753) +++ code/mission/missioncampaign.cpp (working copy) @@ -983,7 +983,7 @@ /** * Store mission's goals and events in Campaign struct */ -void mission_campaign_store_goals_and_events_and_variables() +void mission_campaign_store_goals_and_events() { char *name; int cur, i, j; @@ -1058,7 +1058,16 @@ } else Int3(); } +} +void mission_campaign_store_variables() +{ + int cur, i, j; + cmission *mission; + + cur = Campaign.current_mission; + mission = &Campaign.missions[cur]; + // Goober5000 - handle campaign-persistent variables ------------------------------------- if (mission->variables != NULL) { vm_free( mission->variables ); @@ -1161,6 +1170,12 @@ // -------------------------------------------------------------------------- } +void mission_campaign_store_goals_and_events_and_variables() +{ + mission_campaign_store_goals_and_events(); + mission_campaign_store_variables(); +} + /** * Called when the player's mission is over. It updates the internal store of goals * and their status then saves the state of the campaign in the campaign file. Index: code/mission/missioncampaign.h =================================================================== --- code/mission/missioncampaign.h (revision 10753) +++ code/mission/missioncampaign.h (working copy) @@ -242,6 +242,12 @@ int mission_load_up_campaign( player *p = NULL ); // stores mission goals and events in Campaign struct +void mission_campaign_store_goals_and_events(); + +// stores campaign-persistent variables +void mission_campaign_store_variables(); + +// does both of the above void mission_campaign_store_goals_and_events_and_variables(); // evaluates next mission and possible loop mission Index: code/missionui/missiondebrief.cpp =================================================================== --- code/missionui/missiondebrief.cpp (revision 10753) +++ code/missionui/missiondebrief.cpp (working copy) @@ -1352,6 +1352,8 @@ // mission that isn't in a campaign. if ( Game_mode & GM_CAMPAIGN_MODE ) { + mission_campaign_store_variables(); + // check for possible mission loop // check for (1) mission loop available, (2) don't have to repeat last mission if(!(Game_mode & GM_MULTIPLAYER)){ @@ -2032,7 +2034,7 @@ if ( (Game_mode & GM_CAMPAIGN_MODE) && ( !MULTIPLAYER_CLIENT ) ) { // MUST store goals and events first - may be used to evaluate next mission // store goals and events - mission_campaign_store_goals_and_events_and_variables(); + mission_campaign_store_goals_and_events(); // evaluate next mission mission_campaign_eval_next_mission(); |
|
|
|
Per IRC discussion, I've attached a simple update to the campaign to test red alert missions. I've confirmed that without the patch, the CPV incorrectly increments when you replay the 2nd mission from the 3rd mission red-alert briefing. edit: Well, my IRC suggestion didn't work. i.e. 1) in red_alert_maybe_move_to_next_mission() change to only store goals & events 2) then in red_alert_button_pressed(), add the "store vars" in the "RA_CONTINUE" case As I feared, the Sexp_variables data is stale by this point. So maybe the approach to fix this issue will be a return to a "save then maybe rollback" style. With consideration given to groundhog day missions per IRC discussion :) |
|
I was originally thinking of trying to save a CPV "backup" at the start of Red Alert missions, but after looking into the code a bit more, I think you're probably right; the easiest thing to do would be to restore the pre-3.7 behavior of storing CPVs on a per-mission basis and just adding compatibility for when the next mission is the same as the current mission. |
|
Either way, it looks like it require a change in the .csg format (and if CPVs are moved back to the missions, some code to load "bugged" campaign saves, unless we want to force everyone to restart every campaign that uses CPVs after this bug gets fixed), which I don't want to even attempt at my current level of understanding. |
|
cpvars3.patch (7,574 bytes)
Index: code/mission/missioncampaign.cpp =================================================================== --- code/mission/missioncampaign.cpp (revision 10758) +++ code/mission/missioncampaign.cpp (working copy) @@ -928,10 +928,25 @@ return 0; Campaign.current_mission = Campaign.prev_mission; + Campaign.prev_mission = -1; Campaign.next_mission = Campaign.current_mission; Campaign.num_missions_completed--; Campaign.missions[Campaign.next_mission].completed = 0; + if (Campaign.num_variables > 0) { + vm_free( Campaign.variables ); + } + + Campaign.num_variables = Campaign.redalert_num_variables; + + // copy backed up variables over + if (Campaign.redalert_num_variables > 0) { + Assert( Campaign.redalert_variables ); + + Campaign.variables = (sexp_variable *) vm_malloc(Campaign.num_variables * sizeof(sexp_variable)); + memcpy(Campaign.variables, Campaign.redalert_variables, Campaign.redalert_num_variables * sizeof(sexp_variable)); + } + Pilot.save_savefile(); // reset the player stats to be the stats from this level @@ -983,10 +998,10 @@ /** * Store mission's goals and events in Campaign struct */ -void mission_campaign_store_goals_and_events_and_variables() +void mission_campaign_store_goals_and_events() { char *name; - int cur, i, j; + int cur, i; cmission *mission; cur = Campaign.current_mission; @@ -1058,7 +1073,16 @@ } else Int3(); } +} +void mission_campaign_store_variables() +{ + int cur, i, j; + cmission *mission; + + cur = Campaign.current_mission; + mission = &Campaign.missions[cur]; + // Goober5000 - handle campaign-persistent variables ------------------------------------- if (mission->variables != NULL) { vm_free( mission->variables ); @@ -1113,9 +1137,18 @@ sexp_variable *n_variables = (sexp_variable *) vm_malloc(total_variables * sizeof(sexp_variable)); Assert( n_variables ); + if (Campaign.redalert_num_variables > 0) { + vm_free( Campaign.redalert_variables ); + } + + Campaign.redalert_num_variables = Campaign.num_variables; + // copy existing variables over if (Campaign.num_variables > 0) { Assert( Campaign.variables ); + + Campaign.redalert_variables = (sexp_variable *) vm_malloc(Campaign.num_variables * sizeof(sexp_variable)); + memcpy(Campaign.redalert_variables, Campaign.variables, Campaign.num_variables * sizeof(sexp_variable)); memcpy(n_variables, Campaign.variables, Campaign.num_variables * sizeof(sexp_variable)); variable_count = Campaign.num_variables; @@ -1161,6 +1194,12 @@ // -------------------------------------------------------------------------- } +void mission_campaign_store_goals_and_events_and_variables() +{ + mission_campaign_store_goals_and_events(); + mission_campaign_store_variables(); +} + /** * Called when the player's mission is over. It updates the internal store of goals * and their status then saves the state of the campaign in the campaign file. Index: code/mission/missioncampaign.h =================================================================== --- code/mission/missioncampaign.h (revision 10758) +++ code/mission/missioncampaign.h (working copy) @@ -126,6 +126,8 @@ cmission missions[MAX_CAMPAIGN_MISSIONS]; // decription of the missions int num_variables; // number of variables this campaign had - Goober5000 sexp_variable *variables; // malloced array of sexp_variables (of num_variables size) containing campaign-persistent variables - Goober5000 + int redalert_num_variables; // These two variables hold the previous state of the above for restoration + sexp_variable *redalert_variables; // if you replay the previous mission in a Red Alert scenario. -MageKing17 campaign() : desc(NULL), num_missions(0), variables(NULL) @@ -242,6 +244,12 @@ int mission_load_up_campaign( player *p = NULL ); // stores mission goals and events in Campaign struct +void mission_campaign_store_goals_and_events(); + +// stores campaign-persistent variables +void mission_campaign_store_variables(); + +// does both of the above void mission_campaign_store_goals_and_events_and_variables(); // evaluates next mission and possible loop mission Index: code/missionui/missiondebrief.cpp =================================================================== --- code/missionui/missiondebrief.cpp (revision 10758) +++ code/missionui/missiondebrief.cpp (working copy) @@ -1352,6 +1352,8 @@ // mission that isn't in a campaign. if ( Game_mode & GM_CAMPAIGN_MODE ) { + mission_campaign_store_variables(); + // check for possible mission loop // check for (1) mission loop available, (2) don't have to repeat last mission if(!(Game_mode & GM_MULTIPLAYER)){ @@ -2032,7 +2034,7 @@ if ( (Game_mode & GM_CAMPAIGN_MODE) && ( !MULTIPLAYER_CLIENT ) ) { // MUST store goals and events first - may be used to evaluate next mission // store goals and events - mission_campaign_store_goals_and_events_and_variables(); + mission_campaign_store_goals_and_events(); // evaluate next mission mission_campaign_eval_next_mission(); Index: code/pilotfile/csg.cpp =================================================================== --- code/pilotfile/csg.cpp (revision 10758) +++ code/pilotfile/csg.cpp (working copy) @@ -1029,6 +1029,29 @@ cfread_string_len(Campaign.variables[idx].variable_name, TOKEN_LENGTH, cfp); } } + + if (csg_ver < 4) { // CSG files before version 4 don't have a Red Alert set of CPVs to load, so just copy the regular set. + Campaign.redalert_num_variables = Campaign.num_variables; + Campaign.redalert_variables = (sexp_variable *) vm_malloc( Campaign.redalert_num_variables * sizeof(sexp_variable) ); + Verify( Campaign.redalert_variables != NULL); + + memcpy( Campaign.redalert_variables, Campaign.variables, Campaign.num_variables * sizeof(sexp_variable)); + } else { + Campaign.redalert_num_variables = cfread_int(cfp); + + if (Campaign.redalert_num_variables > 0) { + Campaign.redalert_variables = (sexp_variable *) vm_malloc( Campaign.redalert_num_variables * sizeof(sexp_variable) ); + Verify( Campaign.redalert_variables != NULL ); + + memset( Campaign.redalert_variables, 0, Campaign.redalert_num_variables * sizeof(sexp_variable) ); + + for (idx = 0; idx < Campaign.redalert_num_variables; idx++) { + Campaign.redalert_variables[idx].type = cfread_int(cfp); + cfread_string_len(Campaign.redalert_variables[idx].text, TOKEN_LENGTH, cfp); + cfread_string_len(Campaign.redalert_variables[idx].variable_name, TOKEN_LENGTH, cfp); + } + } + } } void pilotfile::csg_write_variables() @@ -1045,6 +1068,14 @@ cfwrite_string_len(Campaign.variables[idx].variable_name, cfp); } + cfwrite_int(Campaign.redalert_num_variables, cfp); + + for (idx = 0; idx < Campaign.redalert_num_variables; idx++) { + cfwrite_int(Campaign.redalert_variables[idx].type, cfp); + cfwrite_string_len(Campaign.redalert_variables[idx].text, cfp); + cfwrite_string_len(Campaign.redalert_variables[idx].variable_name, cfp); + } + endSection(); } Index: code/pilotfile/pilotfile.h =================================================================== --- code/pilotfile/pilotfile.h (revision 10758) +++ code/pilotfile/pilotfile.h (working copy) @@ -23,7 +23,8 @@ // 1 - re-add recent missions // 2 - separate single/multi squad name & pic // 3 - remove separate detail settings for campaigns -static const ubyte CSG_VERSION = 3; +// 4 - add CPV rollback for Red Alert missions +static const ubyte CSG_VERSION = 4; class pilotfile { |
|
New patch that fixes Red Alert by adding a "backup" set of CPVs that gets restored when you replay the previous mission. Tested and seems to be working. |
|
New patch to catch a couple cases where Campaign.variables get freed (and Campaign.redalert_variables should too). |
|
cpvars.patch (8,191 bytes)
Index: code/mission/missioncampaign.cpp =================================================================== --- code/mission/missioncampaign.cpp (revision 10778) +++ code/mission/missioncampaign.cpp (working copy) @@ -928,10 +928,25 @@ return 0; Campaign.current_mission = Campaign.prev_mission; + Campaign.prev_mission = -1; Campaign.next_mission = Campaign.current_mission; Campaign.num_missions_completed--; Campaign.missions[Campaign.next_mission].completed = 0; + if (Campaign.num_variables > 0) { + vm_free( Campaign.variables ); + } + + Campaign.num_variables = Campaign.redalert_num_variables; + + // copy backed up variables over + if (Campaign.redalert_num_variables > 0) { + Assert( Campaign.redalert_variables ); + + Campaign.variables = (sexp_variable *) vm_malloc(Campaign.num_variables * sizeof(sexp_variable)); + memcpy(Campaign.variables, Campaign.redalert_variables, Campaign.redalert_num_variables * sizeof(sexp_variable)); + } + Pilot.save_savefile(); // reset the player stats to be the stats from this level @@ -983,10 +998,10 @@ /** * Store mission's goals and events in Campaign struct */ -void mission_campaign_store_goals_and_events_and_variables() +void mission_campaign_store_goals_and_events() { char *name; - int cur, i, j; + int cur, i; cmission *mission; cur = Campaign.current_mission; @@ -1058,7 +1073,16 @@ } else Int3(); } +} +void mission_campaign_store_variables() +{ + int cur, i, j; + cmission *mission; + + cur = Campaign.current_mission; + mission = &Campaign.missions[cur]; + // Goober5000 - handle campaign-persistent variables ------------------------------------- if (mission->variables != NULL) { vm_free( mission->variables ); @@ -1113,9 +1137,18 @@ sexp_variable *n_variables = (sexp_variable *) vm_malloc(total_variables * sizeof(sexp_variable)); Assert( n_variables ); + if (Campaign.redalert_num_variables > 0) { + vm_free( Campaign.redalert_variables ); + } + + Campaign.redalert_num_variables = Campaign.num_variables; + // copy existing variables over if (Campaign.num_variables > 0) { Assert( Campaign.variables ); + + Campaign.redalert_variables = (sexp_variable *) vm_malloc(Campaign.num_variables * sizeof(sexp_variable)); + memcpy(Campaign.redalert_variables, Campaign.variables, Campaign.num_variables * sizeof(sexp_variable)); memcpy(n_variables, Campaign.variables, Campaign.num_variables * sizeof(sexp_variable)); variable_count = Campaign.num_variables; @@ -1161,6 +1194,12 @@ // -------------------------------------------------------------------------- } +void mission_campaign_store_goals_and_events_and_variables() +{ + mission_campaign_store_goals_and_events(); + mission_campaign_store_variables(); +} + /** * Called when the player's mission is over. It updates the internal store of goals * and their status then saves the state of the campaign in the campaign file. @@ -1350,6 +1389,11 @@ vm_free(Campaign.variables); Campaign.variables = NULL; } + Campaign.redalert_num_variables = 0; + if (Campaign.redalert_variables != NULL) { + vm_free(Campaign.redalert_variables); + Campaign.redalert_variables = NULL; + } } /** Index: code/mission/missioncampaign.h =================================================================== --- code/mission/missioncampaign.h (revision 10778) +++ code/mission/missioncampaign.h (working copy) @@ -126,6 +126,8 @@ cmission missions[MAX_CAMPAIGN_MISSIONS]; // decription of the missions int num_variables; // number of variables this campaign had - Goober5000 sexp_variable *variables; // malloced array of sexp_variables (of num_variables size) containing campaign-persistent variables - Goober5000 + int redalert_num_variables; // These two variables hold the previous state of the above for restoration + sexp_variable *redalert_variables; // if you replay the previous mission in a Red Alert scenario. -MageKing17 campaign() : desc(NULL), num_missions(0), variables(NULL) @@ -242,6 +244,12 @@ int mission_load_up_campaign( player *p = NULL ); // stores mission goals and events in Campaign struct +void mission_campaign_store_goals_and_events(); + +// stores campaign-persistent variables +void mission_campaign_store_variables(); + +// does both of the above void mission_campaign_store_goals_and_events_and_variables(); // evaluates next mission and possible loop mission Index: code/missionui/missiondebrief.cpp =================================================================== --- code/missionui/missiondebrief.cpp (revision 10778) +++ code/missionui/missiondebrief.cpp (working copy) @@ -1352,6 +1352,8 @@ // mission that isn't in a campaign. if ( Game_mode & GM_CAMPAIGN_MODE ) { + mission_campaign_store_variables(); + // check for possible mission loop // check for (1) mission loop available, (2) don't have to repeat last mission if(!(Game_mode & GM_MULTIPLAYER)){ @@ -2032,7 +2034,7 @@ if ( (Game_mode & GM_CAMPAIGN_MODE) && ( !MULTIPLAYER_CLIENT ) ) { // MUST store goals and events first - may be used to evaluate next mission // store goals and events - mission_campaign_store_goals_and_events_and_variables(); + mission_campaign_store_goals_and_events(); // evaluate next mission mission_campaign_eval_next_mission(); Index: code/pilotfile/csg.cpp =================================================================== --- code/pilotfile/csg.cpp (revision 10778) +++ code/pilotfile/csg.cpp (working copy) @@ -1029,6 +1029,29 @@ cfread_string_len(Campaign.variables[idx].variable_name, TOKEN_LENGTH, cfp); } } + + if (csg_ver < 4) { // CSG files before version 4 don't have a Red Alert set of CPVs to load, so just copy the regular set. + Campaign.redalert_num_variables = Campaign.num_variables; + Campaign.redalert_variables = (sexp_variable *) vm_malloc( Campaign.redalert_num_variables * sizeof(sexp_variable) ); + Verify( Campaign.redalert_variables != NULL); + + memcpy( Campaign.redalert_variables, Campaign.variables, Campaign.num_variables * sizeof(sexp_variable)); + } else { + Campaign.redalert_num_variables = cfread_int(cfp); + + if (Campaign.redalert_num_variables > 0) { + Campaign.redalert_variables = (sexp_variable *) vm_malloc( Campaign.redalert_num_variables * sizeof(sexp_variable) ); + Verify( Campaign.redalert_variables != NULL ); + + memset( Campaign.redalert_variables, 0, Campaign.redalert_num_variables * sizeof(sexp_variable) ); + + for (idx = 0; idx < Campaign.redalert_num_variables; idx++) { + Campaign.redalert_variables[idx].type = cfread_int(cfp); + cfread_string_len(Campaign.redalert_variables[idx].text, TOKEN_LENGTH, cfp); + cfread_string_len(Campaign.redalert_variables[idx].variable_name, TOKEN_LENGTH, cfp); + } + } + } } void pilotfile::csg_write_variables() @@ -1045,6 +1068,14 @@ cfwrite_string_len(Campaign.variables[idx].variable_name, cfp); } + cfwrite_int(Campaign.redalert_num_variables, cfp); + + for (idx = 0; idx < Campaign.redalert_num_variables; idx++) { + cfwrite_int(Campaign.redalert_variables[idx].type, cfp); + cfwrite_string_len(Campaign.redalert_variables[idx].text, cfp); + cfwrite_string_len(Campaign.redalert_variables[idx].variable_name, cfp); + } + endSection(); } @@ -1256,6 +1287,12 @@ Campaign.variables = NULL; } + if (Campaign.redalert_variables) { + Campaign.redalert_num_variables = 0; + vm_free(Campaign.redalert_variables); + Campaign.redalert_num_variables = NULL; + } + // clear out mission stuff for (idx = 0; idx < MAX_CAMPAIGN_MISSIONS; idx++) { missionp = &Campaign.missions[idx]; Index: code/pilotfile/pilotfile.h =================================================================== --- code/pilotfile/pilotfile.h (revision 10778) +++ code/pilotfile/pilotfile.h (working copy) @@ -23,7 +23,8 @@ // 1 - re-add recent missions // 2 - separate single/multi squad name & pic // 3 - remove separate detail settings for campaigns -static const ubyte CSG_VERSION = 3; +// 4 - add CPV rollback for Red Alert missions +static const ubyte CSG_VERSION = 4; class pilotfile { |
|
Fix committed to trunk@10786. |
|
Just reopening to say that I've reviewed & tested the patch, it looks good and works as advertised. |
fs2open: trunk r10786 2014-06-08 22:44 Ported: N/A Details Diff |
Fix mantis 3051 (From MageKing17) Only store CPVs on accepting mission results Add copy of CPVs to missioncampaign and pilot .csg so redalert missions can rollback the CPVs if required |
Affected Issues 0003051 |
|
mod - /trunk/fs2_open/code/mission/missioncampaign.cpp | Diff File | ||
mod - /trunk/fs2_open/code/mission/missioncampaign.h | Diff File | ||
mod - /trunk/fs2_open/code/missionui/missiondebrief.cpp | Diff File | ||
mod - /trunk/fs2_open/code/pilotfile/csg.cpp | Diff File | ||
mod - /trunk/fs2_open/code/pilotfile/pilotfile.h | Diff File |
Date Modified | Username | Field | Change |
---|---|---|---|
2014-05-29 22:25 | Axem | New Issue | |
2014-05-29 23:44 | niffiwan | Assigned To | => niffiwan |
2014-05-29 23:44 | niffiwan | Status | new => assigned |
2014-05-30 04:00 | Goober5000 | Priority | normal => high |
2014-05-30 04:00 | Goober5000 | Target Version | => 3.7.2 |
2014-06-05 09:44 | MageKing17 | File Added: cpvars.patch | |
2014-06-05 09:45 | MageKing17 | Note Added: 0015799 | |
2014-06-05 10:08 | MageKing17 | Note Added: 0015800 | |
2014-06-05 10:09 | MageKing17 | File Added: cpvars2.patch | |
2014-06-06 09:15 | niffiwan | File Added: CPVTestMKII.7z | |
2014-06-06 09:17 | niffiwan | Note Added: 0015809 | |
2014-06-06 09:37 | niffiwan | Note Edited: 0015809 | |
2014-06-06 17:55 | MageKing17 | Note Added: 0015810 | |
2014-06-06 19:04 | MageKing17 | Note Added: 0015811 | |
2014-06-07 02:04 | MageKing17 | File Added: cpvars3.patch | |
2014-06-07 02:05 | MageKing17 | Note Added: 0015812 | |
2014-06-09 01:21 | niffiwan | Assigned To | niffiwan => MageKing17 |
2014-06-09 01:41 | MageKing17 | Status | assigned => code review |
2014-06-09 01:41 | MageKing17 | File Deleted: cpvars.patch | |
2014-06-09 02:51 | MageKing17 | File Added: cpvars.patch | |
2014-06-09 02:52 | MageKing17 | Note Added: 0015817 | |
2014-06-09 03:24 | MageKing17 | File Deleted: cpvars.patch | |
2014-06-09 03:24 | MageKing17 | File Added: cpvars.patch | |
2014-06-09 03:26 | niffiwan | Changeset attached | => fs2open trunk r10786 |
2014-06-09 03:26 | niffiwan | Note Added: 0015818 | |
2014-06-09 03:26 | niffiwan | Status | code review => resolved |
2014-06-09 03:26 | niffiwan | Resolution | open => fixed |
2014-06-09 03:27 | niffiwan | Note Added: 0015819 | |
2014-06-09 03:27 | niffiwan | Status | resolved => feedback |
2014-06-09 03:27 | niffiwan | Resolution | fixed => reopened |
2014-06-09 03:28 | niffiwan | Status | feedback => resolved |
2014-06-09 03:28 | niffiwan | Fixed in Version | => 3.7.2 |
2014-06-09 03:28 | niffiwan | Resolution | reopened => fixed |