[karajorma]

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.

[karajorma]

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

[Goober5000]

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.

[karajorma]

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.

[chief1983]

What if they were name-spaced by campaign within the player file?

[xenocartographer]

That'd be a breaking change to any campaign that uses them.

[chief1983]

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.

[xenocartographer]

That's what I meant: breaking existing saves should probably be avoided if possible.

[chief1983]

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?

[karajorma]

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.