Skip to content

fix persistent variables and containers #7192

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Goober5000 wants to merge 2 commits into
base: master
Choose a base branch
from
+176 −301
Open

fix persistent variables and containers #7192

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
98ecb98
fix persistent variables and containers
Goober5000 Jan 18, 2026
3fb7ed5
check for name conflict between red-alert and persistent campaign var…
Goober5000 Jan 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
258 changes: 87 additions & 171 deletions code/mission/missioncampaign.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -928,15 +928,11 @@ void mission_campaign_eval_next_mission()
*/
void mission_campaign_store_goals_and_events()
{
int cur;
cmission *mission_obj;

if (!(Game_mode & GM_CAMPAIGN_MODE) || (Campaign.current_mission < 0))
return;

cur = Campaign.current_mission;

mission_obj = &Campaign.missions[cur];
int cur = Campaign.current_mission;
auto mission_obj = &Campaign.missions[cur];

// first we must save the status of the current missions goals in the campaign mission structure.
// After that, we can determine which mission is tagged as the next mission. Finally, we
Expand Down Expand Up @@ -992,66 +988,54 @@ void mission_campaign_store_goals_and_events()

void mission_campaign_store_variables(int persistence_type, bool store_red_alert)
{
int cur, i, j;
cmission *mission_obj;

if (!(Game_mode & GM_CAMPAIGN_MODE) || (Campaign.current_mission < 0))
return;

cur = Campaign.current_mission;
mission_obj = &Campaign.missions[cur];
int cur = Campaign.current_mission;
auto mission_obj = &Campaign.missions[cur];

// handle variables that are saved on mission victory -------------------------------------
mission_obj->variables.clear();

int num_mission_variables = sexp_campaign_file_variable_count();
int num_sexp_variables = sexp_variable_count();
for (int i = 0; i < num_sexp_variables; i++) {
if (!(Sexp_variables[i].type & persistence_type)) {
continue;
}

if (num_mission_variables > 0) {

if (store_red_alert) {
for (auto& current_rav : Campaign.red_alert_variables) {
Campaign.persistent_variables.push_back(current_rav);
// player-persistent (aka "eternal")
if (Sexp_variables[i].type & SEXP_VARIABLE_SAVE_TO_PLAYER_FILE) {
// see if we already have a variable with this name
int j = find_item_with_string(Player->variables, &sexp_variable::variable_name, Sexp_variables[i].variable_name);
if (j >= 0) {
Player->variables[j].type = Sexp_variables[i].type;
strcpy_s(Player->variables[j].text, Sexp_variables[i].text);
}
// new variable
else {
Player->variables.push_back(Sexp_variables[i]);
}
}

for (i = 0; i < sexp_variable_count(); i++) {
if (!(Sexp_variables[i].type & SEXP_VARIABLE_SAVE_TO_PLAYER_FILE)) {
if (Sexp_variables[i].type & persistence_type) {
bool add_it = true;

// see if we already have a variable with this name
for (j = 0; j < (int)Campaign.persistent_variables.size(); j++) {
if (!(stricmp(Sexp_variables[i].variable_name, Campaign.persistent_variables[j].variable_name))) {
add_it = false;
Campaign.persistent_variables[j].type = Sexp_variables[i].type;
strcpy_s(Campaign.persistent_variables[j].text, Sexp_variables[i].text);
break;
}
}

// new variable
if (add_it) {
Campaign.persistent_variables.push_back(Sexp_variables[i]);
}
}
// campaign-persistent
else {
// see if we already have a variable with this name
int j = find_item_with_string(Campaign.persistent_variables, &sexp_variable::variable_name, Sexp_variables[i].variable_name);
if (j >= 0) {
Campaign.persistent_variables[j].type = Sexp_variables[i].type;
strcpy_s(Campaign.persistent_variables[j].text, Sexp_variables[i].text);
}
// we might need to save some eternal variables
else if ((persistence_type & SEXP_VARIABLE_SAVE_ON_MISSION_PROGRESS) && (Sexp_variables[i].type & persistence_type) && (Sexp_variables[i].type & SEXP_VARIABLE_SAVE_TO_PLAYER_FILE)) {
bool add_it = true;

for (j = 0; j < (int)Player->variables.size(); j++) {
if (!(stricmp(Sexp_variables[i].variable_name, Player->variables[j].variable_name))) {
Player->variables[j] = Sexp_variables[i];

add_it = false;
break;
}
}
// new variable
else {
Campaign.persistent_variables.push_back(Sexp_variables[i]);
}
}
}

// if not found then add new entry
if (add_it) {
Player->variables.push_back(Sexp_variables[i]);
}
if (store_red_alert) {
Copy link
Member

@jg18 jg18 Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering why we now save red alert variables at the end, instead of at the start?

for (const auto& current_rav : Campaign.red_alert_variables) {
if (find_item_with_string(Campaign.persistent_variables, &sexp_variable::variable_name, current_rav.variable_name) < 0) {
Campaign.persistent_variables.push_back(current_rav);
} else {
Warning(LOCATION, "A red alert variable has the same name as a persistent variable!");
}
}
}
Expand All @@ -1063,58 +1047,68 @@ void mission_campaign_store_containers(ContainerType persistence_type, bool stor
if (!(Game_mode & GM_CAMPAIGN_MODE) || (Campaign.current_mission < 0))
return;

if (!sexp_container_has_persistent_non_eternal_containers()) {
// nothing to do
return;
}

if (store_red_alert) {
for (const auto& current_con : Campaign.red_alert_containers) {
Campaign.persistent_containers.emplace_back(current_con);
for (const auto &container : get_all_sexp_containers()) {
if (none(container.type & persistence_type)) {
continue;
}
}

for (const auto &container : get_all_sexp_containers()) {
if (!container.is_eternal()) {
if (any(container.type & persistence_type)) {
// see if we already have a container with this name
auto cpc_it = std::find_if(Campaign.persistent_containers.begin(),
Campaign.persistent_containers.end(),
[container](const sexp_container &cpc) {
return cpc.name_matches(container);
});

if (cpc_it != Campaign.persistent_containers.end()) {
*cpc_it = container;
} else {
// new container
Campaign.persistent_containers.emplace_back(container);
}
}
} else if (any(persistence_type & ContainerType::SAVE_ON_MISSION_PROGRESS) &&
any(container.type & persistence_type) && container.is_eternal()) {
// we might need to save some eternal player-persistent containers
// player-persistent (aka "eternal")
Copy link
Member

@jg18 jg18 Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional but strongly encouraged: I think it's worth documenting in code somewhere the 5 types of persistence and what "campaign-persistent"/"player-persistent" originally meant, to avoid potential future confusion, but up to you.

if (container.is_eternal()) {
// see if we already have a container with this name
auto ppc_it = std::find_if(Player->containers.begin(),
Player->containers.end(),
[container](const sexp_container &ppc) {
[&container](const sexp_container& ppc) {
Copy link
Member

@jg18 jg18 Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch about the &container. I wasn't aware of that syntax.

return ppc.name_matches(container);
});

if (ppc_it != Player->containers.end()) {
*ppc_it = container;
} else {
// new player-persistent container
Player->containers.emplace_back(container);
Player->containers.push_back(container);
}
}
// campaign-persistent
else {
// see if we already have a container with this name
auto cpc_it = std::find_if(Campaign.persistent_containers.begin(),
Campaign.persistent_containers.end(),
[&container](const sexp_container& cpc) {
return cpc.name_matches(container);
});

if (cpc_it != Campaign.persistent_containers.end()) {
*cpc_it = container;
} else {
// new container
Campaign.persistent_containers.push_back(container);
}
}
}

if (store_red_alert) {
for (const auto& container : Campaign.red_alert_containers) {
// see if we already have a container with this name
auto cpc_it = std::find_if(Campaign.persistent_containers.begin(),
Campaign.persistent_containers.end(),
[&container](const sexp_container& cpc) {
return cpc.name_matches(container);
});

if (cpc_it == Campaign.persistent_containers.end()) {
Campaign.persistent_containers.push_back(container);
} else {
Warning(LOCATION, "A red alert container has the same name as a persistent container!");
}
}
}
}

void mission_campaign_store_goals_and_events_and_variables()
void mission_campaign_store_goals_and_events_and_variables(bool store_red_alert_data)
{
mission_campaign_store_goals_and_events();
mission_campaign_store_variables(SEXP_VARIABLE_SAVE_ON_MISSION_PROGRESS);
mission_campaign_store_containers(ContainerType::SAVE_ON_MISSION_PROGRESS);
mission_campaign_store_variables(SEXP_VARIABLE_SAVE_ON_MISSION_PROGRESS, store_red_alert_data);
mission_campaign_store_containers(ContainerType::SAVE_ON_MISSION_PROGRESS, store_red_alert_data);
}

/**
Expand Down Expand Up @@ -1146,9 +1140,6 @@ void mission_campaign_mission_over(bool do_next_mission)
Campaign.weapons_allowed[Granted_weapons[i]] = 1;
}

// Goober5000 - player-persistent variables are handled when the mission is
// over, not necessarily when the mission is accepted

// update campaign.mission stats (used to allow backout inRedAlert)
// .. but we don't do this if we are inside of the prev/current loop hack
if ( Campaign.prev_mission != Campaign.current_mission ) {
Expand Down Expand Up @@ -1682,6 +1673,9 @@ void mission_campaign_end_init()

void mission_campaign_end_do()
{
mission_campaign_store_variables(SEXP_VARIABLE_SAVE_ON_MISSION_PROGRESS, false);
mission_campaign_store_containers(ContainerType::SAVE_ON_MISSION_PROGRESS, false);
Copy link
Member

@jg18 jg18 Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused on the reason for this change. Were we failing to save "save on mission progress" persistent data?


// close out the mission
event_music_level_close();
mission_goal_fail_incomplete();
Expand Down Expand Up @@ -1728,7 +1722,7 @@ void mission_campaign_skip_to_next()
mission_goal_mark_events_complete();

// store
mission_campaign_store_goals_and_events_and_variables();
mission_campaign_store_goals_and_events_and_variables(false);

// now set the next mission
mission_campaign_eval_next_mission();
Expand Down Expand Up @@ -1845,84 +1839,6 @@ bool mission_campaign_jump_to_mission(const char* filename, bool no_skip)
}
}

// Goober5000
void mission_campaign_save_on_close_variables()
{
int i;

// make sure we are actually playing a single-player campaign
if (!(Game_mode & GM_CAMPAIGN_MODE) || (Campaign.type != CAMPAIGN_TYPE_SINGLE) || (Campaign.current_mission < 0))
return;
Copy link
Member

@jg18 jg18 Jan 30, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This "save on close"-specific check is now getting lost, which makes me nervous about potentially introducing regressions.

From a quick look, seems like the difference is that "save on close" applies to single-player campaigns only.

Should "save on close" variables even be supported for multi campaigns?


// now save variables
for (i = 0; i < sexp_variable_count(); i++) {
// we only want the on mission close type. On campaign progress type are dealt with elsewhere
if ( !(Sexp_variables[i].type & SEXP_VARIABLE_SAVE_ON_MISSION_CLOSE) ) {
continue;
}

bool found = false;

// deal with eternals
if ((Sexp_variables[i].type & SEXP_VARIABLE_SAVE_TO_PLAYER_FILE)) {
// check if variable already exists and updated it
for (auto& current_variable : Player->variables) {
if (!(stricmp(Sexp_variables[i].variable_name, current_variable.variable_name))) {
current_variable = Sexp_variables[i];

found = true;
break;
}
}

// if not found then add new entry
if (!found) {
Player->variables.push_back(Sexp_variables[i]);
}
}

}

// store any non-eternal on mission close variables
mission_campaign_store_variables(SEXP_VARIABLE_SAVE_ON_MISSION_CLOSE, false);
}

// jg18 - adapted from mission_campaign_save_on_close_variables()
void mission_campaign_save_on_close_containers()
{
// make sure we are actually playing a single-player campaign
if (!(Game_mode & GM_CAMPAIGN_MODE) || (Campaign.type != CAMPAIGN_TYPE_SINGLE) || (Campaign.current_mission < 0))
return;

// now save containers
for (const auto &container : get_all_sexp_containers()) {
// we only want the on mission close type. On campaign progress type are dealt with elsewhere
if (none(container.type & ContainerType::SAVE_ON_MISSION_CLOSE)) {
continue;
}

// deal with eternals
if (container.is_eternal()) {
// check if container already exists and update it
auto ppc_it = std::find_if(Player->containers.begin(),
Player->containers.end(),
[container](const sexp_container &ppc) {
return ppc.name_matches(container);
});

if (ppc_it != Player->containers.end()) {
*ppc_it = container;
} else {
// if not found then add new entry
Player->containers.emplace_back(container);
}
}
}

// store any non-eternal on mission close containers
mission_campaign_store_containers(ContainerType::SAVE_ON_MISSION_CLOSE, false);
}

void mission_campaign_load_failure_popup()
{
if (Campaign_load_failure == 0) {
Expand Down
12 changes: 3 additions & 9 deletions code/mission/missioncampaign.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,13 +226,13 @@ int mission_load_up_campaign(bool fall_back_from_current = false);
void mission_campaign_store_goals_and_events();

// stores variables which will be saved only on mission progression
void mission_campaign_store_variables(int persistence_type, bool store_red_alert = true);
void mission_campaign_store_variables(int persistence_type, bool store_red_alert);

// stores containers which will be saved only on mission progression
void mission_campaign_store_containers(ContainerType persistence_type, bool store_red_alert = true);
void mission_campaign_store_containers(ContainerType persistence_type, bool store_red_alert);
Copy link
Member

@jg18 jg18 Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call on removing default values. Too much potential for hiding bugs, as we've learned here.


// does all three of the above
void mission_campaign_store_goals_and_events_and_variables();
void mission_campaign_store_goals_and_events_and_variables(bool store_red_alert_data);

// evaluates next mission and possible loop mission
void mission_campaign_eval_next_mission();
Expand All @@ -254,12 +254,6 @@ void mission_campaign_end_init();
void mission_campaign_end_close();
void mission_campaign_end_do();

// save eternal variables
extern void mission_campaign_save_on_close_variables();

// save eternal containers
extern void mission_campaign_save_on_close_containers();

extern void mission_campaign_load_failure_popup();

SCP_string mission_campaign_get_name(const char* filename);
Expand Down
Loading