Hard Light Productions Forums

Modding, Mission Design, and Coding => FS2 Open Coding - The Source Code Project (SCP) => Topic started by: AdmiralRalwood on March 08, 2017, 03:03:51 am

Title: Persistent variables
Post by: AdmiralRalwood on March 08, 2017, 03:03:51 am
As a result of another thread (http://www.hard-light.net/forums/index.php?topic=93300.msg1844199#msg1844199), I became aware that player-persistent variables have been broken since Antipodes 8 (the pilot code overhaul that was the major new feature in FSO 3.7.0). Namely, they're actually player-persistent now, instead of resetting when the campaign does.

"Wait, what?" you might ask. "Player-persistent variables being player-persistent is a bug?"

Yes. Yes it is.

When player-persistent variables were first introduced (all the way back in 2003), it wasn't really possible to play more than one campaign simultaneously. Whenever you changed campaigns, PPVs would be explicitly reset. This reset was removed at the start of Antipodes 8 back in 2010, but without any attempt made to keep the old behavior in some way. Presumably simply never resetting PPVs was considered preferable to them resetting if you just switched to a different campaign and then switched back, but the proper solution would have been to move them to the .csg file instead of leaving them in their current state.

Alas, they were left in their current state, and have been so for years (3.7.0 Final (http://www.hard-light.net/forums/index.php?topic=85435.0) was released in August of 2013).

No documentation appears to have been updated to account for the code change; the wiki still claims (http://hard-light.net/wiki/index.php/Variable#Four_Types_of_Variables) player-persistent variables get reset with the campaign. Why haven't I corrected the wiki? Well, because the way the wiki describes it is definitely how it's supposed to work, and how FREDers have been using them since they were introduced. The difference that's supposed to be between the two variable types is when they're saved: CPVs are saved at the end of the mission (assuming you accept the outcome), while PPVs are updated as they change (so restarting a mission after it sets a PPV will leave the PPV set to the new value).

Now, I'm currently thinking that the correct fix here is to move so-called "player-persistent variables" into the campaign savefile, even though that would require bumping the pilotfile version to accommodate this change, because as they're intended to be used, they're definitely still supposed to be tied to an individual campaign.

On the other hand, I'm sure designers would like variables that could be accessed between campaigns, so at least one new type of variable needs to be added with that functionality (the current, actually-wrong behavior of player-persistent variables). However, renaming PPVs and then making a new type of variable and calling them PPVs is just asking for somebody to get confused and pick the wrong one. So ideally, we need all new names for every kind of persistent variable so that there's no confusion about what they actually do and what they should actually be used for.

Karajorma suggested that 4 types of variables are needed, with the two axes being campaign-/player-specific and saved-on-accept/saved-on-change. Thinking about it as I write this post, perhaps there might even be a way to incorporate wookieejedi's request for a kind of variable that is persistent even through techroom runs; 8 kinds of variables?

Regardless, here's my initial suggestion for the new naming scheme:

If "techroom-usable" variables were a separate type (rather than a flag that could be set for one of the other kinds of variables, for example), I have no idea what they'd be called.

Thoughts? Comments? Completely-off-the-wall ideas? Anyone? Bueller?
Title: Re: Persistent variables
Post by: karajorma on March 08, 2017, 04:41:57 am
Given that the only difference between PPVs and CPVs as designed is whether or not they reset on a failed mission it strikes me that a better option is to leave things at only two types of persistent variables but then have a flag decide whether to write it to the player or campaign file. An advantage of this approach is that if someone thinks of another type of persistent variable we wouldn't need to come up with two new names for the player and campaign file versions.

Another major advantage of that would be that it would safely define a system for removing variables from the player file. If you had something like this

Mission 1 - Variable is Progressive and Eternal
Mission 2 - Variable is Progressive and Eternal
Mission 3 - Variable is Progressive

In mission 1 and 2 the value of the variable is written to the player file when the mission is successfully completed. But in Mission 3 the game detects that the Eternal flag has been switched off so the value would be written to the campaign file and the variable in the player file would be deleted on mission completion.
Title: Re: Persistent variables
Post by: 0rph3u5 on May 13, 2017, 05:08:53 am
Question:

Can this be paired with a convinence function to import variables form one mission to another in FRED?
Title: Re: Persistent variables
Post by: xenocartographer on May 13, 2017, 07:02:42 am
Going off karajorma's suggestion here... I think we could use a system of flags like this:

Persistent: Must be set for the variable to be persisted at all; without this, only the shadow flag has meaning.
Persist regardless of outcome: Writes the variable even if the mission was a failure.
Visible between campaigns: Whether to write the variable to the pilot file (true) or campaign file (false).
Visible in simulator:  Whether the tech room simulator can see the value of this variable.
Persist from simulator: Whether the simulator can modify this variable. Must be set along with the previous flag.
Allow shadowing: If not set, the game issues a warning if this variable has the same name, but different flags or type, as another variable in the same campaign.

All flags are false by default.
Title: Re: Persistent variables
Post by: The E on May 13, 2017, 07:42:27 am
Allow shadowing: If not set, the game issues a warning if this variable has the same name, but different flags or type, as another variable in the same campaign.

This makes little sense to me. Allowing variables to be shadowed leads to complications that are best avoided entirely.
Title: Re: Persistent variables
Post by: FrikgFeek on May 13, 2017, 07:48:56 am
Wait... so if this changes that means you won't have to manually delete savefiles when switching to a different version of a campaign with the same name? Because right now if you play campaign A beta v.091 and then switch to campaign A beta v.096 your pilot save file becomes invalid and you either need to delete it or create a new pilot.


Player-specific variables saved on accepting mission outcome are kills and ranks, right? Why not just call them statistics variables or achievement variables?
Title: Re: Persistent variables
Post by: xenocartographer on May 13, 2017, 08:03:54 am
Allow shadowing: If not set, the game issues a warning if this variable has the same name, but different flags or type, as another variable in the same campaign.

This makes little sense to me. Allowing variables to be shadowed leads to complications that are best avoided entirely.

I can imagine cases where someone would want different values for the "visible in simulator" or "persist regardless of outcome" flags.

FrikgFeek, we're talking about SEXP variables, not killcounts - stuff like your treatment of the Gefs in Nothing is True (though that's probably done through is-previous-event-true, but whatever).
Title: Re: Persistent variables
Post by: FrikgFeek on May 13, 2017, 08:13:35 am
Wouldn't that be the first case? Campaign persistent? I guess I just don't understand what the 4th case would be then. Saved on accepting mission outcome but not campaign persistent? What's player-specific even supposed to mean compared to player-persistent?
Is it possible to have campaign persistent variables that aren't player-specific?
Title: Re: Persistent variables
Post by: xenocartographer on May 13, 2017, 10:04:57 am
I'm... I'm confused. Could you try and give me some context for your question? I'm not sure we're talking about the same things here.
Title: Re: Persistent variables
Post by: FrikgFeek on May 13, 2017, 10:24:25 am
Quote
"No idea what to call player-specific variables saved on accepting mission outcome; please suggest something."


How is this 4th case different from the 1st case?
Quote
"Campaign-persistent variables" would become "Progression variables" (tied to the campaign, saved when accepting the mission outcome).

They're both saved on mission outcome and presumably campaign specific? Unless the 4th case is 'eternal' but saved on mission outcome? I'm not even sure what kinds of SEXP variables would be used like that. That's what I'm confused about. That's why I (wrongly) assumed those would be kills and ranks since they're player specific, saved on mission outcome, and are not campaign specific.


I'm also confused by MageKing's terminology, what's player-specific supposed to mean compared to player-persistent? How are those 2 even different? I know what a specific variable is in math terminology but I don't understand how it would apply here since you aren't solving any equations.
Title: Re: Persistent variables
Post by: karajorma on May 13, 2017, 10:43:19 am
The difference between the 1st and 4th is that the fourth one would persist even after you finish the campaign.

That means you could for instance make a variable called First_Playthrough and then have the campaign set up differently if you replay it. If you only have the first kind the second playthrough of the campaign would begin exactly the same as the first because all the variables would reset to their default values.
Title: Re: Persistent variables
Post by: AdmiralRalwood on May 14, 2017, 08:58:21 am
That's why I (wrongly) assumed those would be kills and ranks since they're player specific, saved on mission outcome, and are not campaign specific.
But they're not SEXP variables, so irrelevant to this conversation.

I'm also confused by MageKing's terminology, what's player-specific supposed to mean compared to player-persistent? How are those 2 even different? I know what a specific variable is in math terminology but I don't understand how it would apply here since you aren't solving any equations.
"Player-persistent" is the current name for what is actually still supposed to be a campaign variable. "Player-specific" is meant to be taken literally: a variable that is specific to the player (as opposed to being tied to the campaign).
Title: Re: Persistent variables
Post by: FrikgFeek on May 14, 2017, 10:49:34 am
Maybe call them 'stashed' variables? Like RPG magic stashes that stay with your character no matter where they go. Though that might just add confusion for people not familiar with RPG terminology...
Title: Re: Persistent variables
Post by: xenocartographer on May 14, 2017, 04:45:49 pm
This kind of terminology issue is why I suggested moving to a system of flags (http://www.hard-light.net/forums/index.php?topic=93306.msg1848151#msg1848151) instead.
Title: Re: Persistent variables
Post by: 0rph3u5 on July 30, 2017, 03:05:30 am
... how far a long is this now?

here is why I am asking:
Spoiler:
I frist became aware of this during a discussion how to hide a secret mission; right now I am considering to cut that mission from the initial release but to leave the system to access it in place hoping that a player-persistent variable can bridge the gap between to releases. (I always wanted that replaying the campaign to the requisiste for access and possibly functionality of the mission).

If the variable system is changed sooner rather than later, this is all for naught of course
Title: Re: Persistent variables
Post by: AdmiralRalwood on July 31, 2017, 02:20:02 am
... how far a long is this now?
No work done, and we're still in a feature freeze so I have no real desire to start working on something this complicated right now.
Title: Re: Persistent variables
Post by: karajorma on February 18, 2018, 09:14:43 pm
:bump:

I'm thinking of starting work on persistence for SEXP Containers. That means I need a decision to have been made on this. Personally I still like the idea of two names and a flag for whether it is eternal or not. We could also add a flag for techroom use if required.

In terms of backwards compatibility, I'd suggest that anything currently in the player file becomes an eternal variable. As soon as the next mission is played, the code should notice that the eternal flag is missing and remove the variable from the player file (writing it into the campaign file instead). This does mean that any campaign which is counting on the bug being present would break but given that this can easily be fixed (simply re-release the campaign file with the eternal flag ticked) and given that knossos makes it easy to distribute the fix, I think that's the best solution.
Title: Re: Persistent variables
Post by: xenocartographer on February 20, 2018, 09:13:27 am
How many mods rely on the bug, do you estimate?
Title: Re: Persistent variables
Post by: AdmiralRalwood on February 23, 2018, 05:57:26 am
I'd estimate very few; possibly even none, although that's always a dangerous guess regardless of circumstances.
Title: Re: Persistent variables
Post by: Goober5000 on February 23, 2018, 05:58:13 am
I'm thinking of starting work on persistence for SEXP Containers. That means I need a decision to have been made on this. Personally I still like the idea of two names and a flag for whether it is eternal or not. We could also add a flag for techroom use if required.

In terms of backwards compatibility, I'd suggest that anything currently in the player file becomes an eternal variable. As soon as the next mission is played, the code should notice that the eternal flag is missing and remove the variable from the player file (writing it into the campaign file instead). This does mean that any campaign which is counting on the bug being present would break but given that this can easily be fixed (simply re-release the campaign file with the eternal flag ticked) and given that knossos makes it easy to distribute the fix, I think that's the best solution.

There are campaigns that are counting on the non-buggy behavior -- I know since I've released one or two -- so the bug ought to be fixed.

I like the two-axis, four-type concept, and I personally can't think of a better design, so I say go ahead and implement it.  I'm not sold on the terminology though.  AdmiralRalwood's explanation was succinct and clear, but "progressive", "eternal", and so forth don't intuitively match to each use case IMO.

How about global vs. campaign-local for one axis, and checkpointed vs. non-checkpointed for the other axis?
Title: Re: Persistent variables
Post by: karajorma on February 23, 2018, 03:53:21 pm
Well I've actually pretty much coded the whole thing (including FRED) now. There's an issue with the old PPV type not updating if the player quits but updating if the player restarts that I need to take care of but apart from that I'm close to done. What I did was say that any player-persistent variable already in the campaign file will become an eternal but if the mission file doesn't have eternal set for the variable it gets set so that it saves to the campaign file from then on.


Until the pull request is in, I don't have an issue with changing the names. In FRED I went with calling the old types Save On Mission Complete and Save On Mission Close and then adding tool tips with an explanation. I prefer that to checkpointed which will make the FREDder think of in-mission checkpoints. Similarly, to be honest I don't think global is any more clear to non-coder than eternal and it might actually send a coder in the wrong direction (thinking that it can be accessed at any time in some way). The tool tip for eternals says "This type of variable is saved to the player file. So it can be referred to by other campaigns" which makes it pretty clear what eternals are for. The other thing I like about Eternal is that it does suggest that once they are set, they're there forever (and given that I haven't yet coded a method to remove eternals from the player file the only way to remove them is to edit the player file by hand) which should hopefully remind FREDders to be a little careful with them.
Title: Re: Persistent variables
Post by: karajorma on February 23, 2018, 10:58:49 pm
Save On Mission Close is proving to be a problem. The old Player-Persistent variables were written to the Player which meant that if you quit or restart a mission they would be written to the player file. Non-eternals are written to the campaign file and this isn't written to on a quit or restart. So that means I need to write to the campaign file if either of those two things happen, presumably in game_level_close(). Unfortunately I have no idea how to do this safely so I need some advice. I suspect just calling Pilot.save_savefile() is going to cause all kinds of bugs.

Here's the changes so far https://github.com/Karajorma/fs2open.github.com/tree/Feature/EternalVariables
Title: Re: Persistent variables
Post by: Goober5000 on March 07, 2018, 05:38:53 pm
The campaign file is not intended to be updated if the player quits the mission without actually making progress in the campaign.  Variables managed by the player are updated without regard to which campaign is being played or whether progress is being made, but variables managed by the campaign are only updated when the campaign advances.

Is this use case (variable is managed by the campaign but updated regardless of whether the campaign makes progress) actually necessary?  Because none of the other campaign-managed data (events, goals, score, kills, etc.) works that way.
Title: Re: Persistent variables
Post by: karajorma on March 07, 2018, 05:51:11 pm
The campaign file is not intended to be updated if the player quits the mission without actually making progress in the campaign.  Variables managed by the player are updated without regard to which campaign is being played or whether progress is being made, but variables managed by the campaign are only updated when the campaign advances.

Is this use case (variable is managed by the campaign but updated regardless of whether the campaign makes progress) actually necessary?  Because none of the other campaign-managed data (events, goals, score, kills, etc.) works that way.

The campaign file is also updated at the start of the mission, so it isn't as if the campaign file is only written to on mission progress. I didn't test for events and goals but none of the other data is updated if you add a write to the campaign file on mission quit / restart.

As for if it's necessary, yes it is. Persistent variables really should be written to the campaign file by default. They belong to a certain campaign and should only really be accessible by that campaign. The only reason for them being in the player file is if they are meant to be accessed from a second campaign (In which case they should be named appropriately). Otherwise you run the risk of calling a variable something like Money and having two campaigns trying to use it.
Title: Re: Persistent variables
Post by: chief1983 on March 08, 2018, 11:23:39 am
What if they were name-spaced by campaign within the player file?
Title: Re: Persistent variables
Post by: xenocartographer on March 08, 2018, 02:18:03 pm
That'd be a breaking change to any campaign that uses them.
Title: Re: Persistent variables
Post by: chief1983 on March 08, 2018, 02:26:26 pm
I just mean on the storage-side of things.  It wouldn't be compatible with campaign saves that already exist but it would work with existing campaigns themselves I think.  I assume that there is already some difference between player persistent vars and campaign persistent vars or this wouldn't be a discussion.
Title: Re: Persistent variables
Post by: xenocartographer on March 08, 2018, 02:46:28 pm
That's what I meant: breaking existing saves should probably be avoided if possible.
Title: Re: Persistent variables
Post by: chief1983 on March 08, 2018, 02:48:43 pm
I guess I don't understand how anything previously discussed would not have the same problem.  Any change to how the vars are stored, such as where they're stored, would break old saves wouldn't it?
Title: Re: Persistent variables
Post by: karajorma on March 08, 2018, 05:36:23 pm
What if they were name-spaced by campaign within the player file?

1) I've already written the code to have them in the campaign file.
2) If writing to the campaign file does cause bugs that are otherwise unavoidable, I don't mind exploring that as a workaround. However I don't see why it's an improvement to have data that has no business being in the player file in the first place namespaced in the player file just to avoid writing to the campaign file. The game already writes to the campaign file when the mission starts so I don't see why it's so heinous to write to it whenever the mission ends, regardless of progress, as long as the data it writes doesn't change stats, events completed, etc (Which as far as I'm aware, it doesn't).

I guess I don't understand how anything previously discussed would not have the same problem.  Any change to how the vars are stored, such as where they're stored, would break old saves wouldn't it?

The code I've written doesn't break existing saves. It loads them perfectly well. However you probably won't be able to load new saves into old versions of the engine. But given that we're moving over to JSON for savefiles at some point soon which has the same problem, I don't consider that to be a deal breaker.