Load older game saves into updated game: Attempt 2

[Crimson Wizard]

I'd like to revisit a problem of loading older saves into an updated AGS game.

For the reference, the old thread about my previous working prototype:
https://www.adventuregamestudio.co.uk/forums/engine-development/load-older-game-saves-prototype-solution/
And the more recent conversation on the issue:
https://www.adventuregamestudio.co.uk/forums/engine-development/save-system-overhaul/

The problem with saves: in case someone does not know or have forgotten

AGS "saves" are in fact "savestates", not exactly equivalent, but close to a plain game memory dump.
When the savestate is restored, engine tries to fill the read data into the game objects. Dynamically created objects don't have any issues, but objects created at design-time (we may call these "static" objects) must match in numbers and order. If number of order of these objects is different, restoration of this save will fail (if only order is different, then engine may proceed, but result will be erroneous). This refers to script data as well (global script variables).
The potential save breaking reasons and existing workarounds are explained in full detail in this article in the manual:
https://adventuregamestudio.github.io/ags-manual/GameSavesCompatibility.html

When you look at this problem, at the first glance the solution could be to use unique object identifiers (like "script names") to match read data with existing objects. But this is complicated by the following major reasons:
1. Not all "objects" have a script name or any kind of unique id.
2. Not all "objects" have their names written into a compiled game, some exist only at design time and during compilation (best example: names of script variables).
3. Strange as it is, but in many places script names are not obligatory in AGS, and may be missing.
(Also, even when present, they are not written into a save game, although that's easier to fix on its own.)

There are other major issues with the existing save system, such as the fact that it stores literally everything, even something that you won't normally expect to be saved and restored in a game. This fact makes me believe that in the long run it may not be worth to "fix" existing save system, but rather worth developing a different one, based on a new principles.

Unfortunately, I do not feel like having an energy and enthusiasm for such task today. So I leave that for other times or other developers.

But at the same time I would like to try to provide a minimal possible solution that would let users patch their published games without fear of breaking older saves, at least for the time being.
There are still workarounds for that (mentioned in the page I linked above), but these still require predicting this issue, and making preparations.

The requirements for this "minimal solution", in my opinion, are:

1. User should be able to apply this solution when upgrading an older game, published before this solution was implemented.
2. This solution should be safe to add to existing game, and removed from game, that is - toggled on and off, without affecting the game itself.
3. User should at least be able to add more items of each type, while letting older saves still load. I think it's okay to not support item removal, as unnecessary items may be detached from the use (hidden etc).
4. With this solution, user must have an opportunity to script reaction to a "old" save, and tell whether to load it or not; and be able to update the game state after load is completed as necessary.
5. If an older save is loaded and has less items of certain type in it, the remaining items must reset to their initial state, as they are at design time. If requirement 4 is implemented, then user will be able to adjust these items after load, as necessary.
6. For the requirement 4, there has to be a guaranteed way to deduce which "game version" the save was written by. It's likely that script variables cannot be used here, as the preliminary save check has to be done prior to loading any modifiable game data such as script variables. Also because of requirements 1. and 2.


There are two things that I've been thinking about adding to AGS for this purpose. These may be reviewed, implemented and used in parallel or individually (separate from each other).

1. Minimal way of telling which parts of game data to write into save or load from a save.

In the past I noticed that there are things that likely should not be a part of a game's save, but are netherless written into a save state. This may cause problems when patching a game, but also may unnecessarily increase save's size.

One good example are GUI. In most games GUI are used just as menus, so something that lies "outside" of the game's progress state.
Classic problem here, an author wants to add a new translation, and translations are selected in-game by pressing a button. New translation means new button, but that new button will break the old saves. That is stupid, especially if we note that GUI don't have to be in saves.

Views - rarely have a meaning to be saved, but since they are saved "just in case" (because some of the frame properties may be modified at runtime), this makes them contributing the older save breakage.

Another example is dynamically created assets (dynamic sprites). These take much memory, but may be recreated knowing game situation from script variables right after a save is restored, for example. So there may be a reason to not put them into saves.

My thought is to have a simple way to tell the engine which types of data to put into a save and load from existing save. Thankfully, our current save format is divided into "components", which makes it technically easier to accomplish. What is needed here is to figure out a script API. Either this should be a new function that sets "save configuration" for the further use, or an optional argument to existing SaveGameSlot / RestoreGameSlot functions. Such argument may be either a set of bit flags (numeric constants combined in one integer value), or a formatted string.

Such option might solve this problem for many of the use cases already.


2. Allow to read saves with less contents than the current game

This is the original solution that I tried to implement a number years ago. It might go roughly like this:

1. While the engine reads the save, and finds out that save has less saved objects (or less script data, etc) than the current game, it runs a script callback. In this script callback user's script must be used to pass back the resolution whether to keep loading or cancel with error.
If save has more data than the game, or if there's no callback found, or if callback returned negative answer, then the restoration is cancelled.
This way the presence of a callback in script serves a safety check, that the user expected their games to load older saves with less data.

2. If restoration is allowed, engine restarts the game, loading its initial set of data, and then restores this save. This is done so to ensure that all data which is not present in the game is reset to initial state, for safety.

There are few nuances to think through here. One is a timing of executing that script callback. It may be done immediately as a problem is detected, but if done after partially restoring a save, the whole game may be in an unstable state. So running script could not be reliable. Another option is to only find if such callback is present in the script, restore the save, but run callback after the save got restored. Since it's still technically possible to read it. This way it may be possible to also check contents of a script data in that callback.

Another nuance is the game versioning. If you read the old thread you may notice that there I implemented a whole new struct containing numbers of various game objects for the user to check. Today I doubt this is a good idea, as it may be not very convenient or reliable, so I'd rather go with a more explicit "game version".
I don't know if we should rely on user having a variable that tells this game's version. We might also add a game property for this, and write it both into a game data and saves.

[Crimson Wizard]

I maybe will try to make a dirty version of this soon.

I've been thinking about the script callback accompanying save/restore system, and a conclusion is that in the current engine no callback may be run during the save/restore process:
* if run during save, this callback may modify data or trigger some action, resulting in inconsistent data.
* if run during restore, the script data is not loaded, and so is dynamic objects pool (where managed objects are stored) so any random operation may lead to errors.

This means that a script callback may only be run before and after save and restore process.



If we think about a callback that checks for the loaded save and possibly adjusting game data, - that one should definitely be run after a save was already loaded. If callback decides that the game should not proceed with this restored save, this means a bit of wasted time, but I suppose this is not a big deal.
More important is the question what to do after that. Because if the restored save is to be discarded, game appears in a unreliable state, and cannot run; cannot even display a menu, etc, since in AGS there's no distinction between "menu objects" and "gameplay objects". This in turn means that it should either:
* quit immediately, with some error message (provided by developer?);
* do RestartGame, if restart slot was created;
* load a reserved save slot as per developer's instruction.



Now, if we think about an optional callback that lets to read or write custom data into save... Since callbacks cannot be run during save, this means that the callback for writing custom data must be called right before saving begins, and write not to the file, but to a temporary memory buffer, which later will be written as a extra component into save file. Writing to a memory buffer may also be performed using File API; that will only require implementing a "memory file" mode for the script File object.
And callback for reading this custom data must be called after save was loaded, where reading will be done from a memory buffer (previously filled during save restore).
This custom data reading callback may in fact be merged with the "save check and update game" into one callback that lets do both. Or not...



To summarize, we may have something like (following is a draft):

Code: ags

void on_save_custom_data(File* savefile)
{
}

and

Code: ags

bool on_restore_custom_data(File* savefile)
{
    return true; // if everything is fine and game continues to run
    return false; // if we decided that save is not compatible
}

and

Code: ags

bool on_restored_save_validate()
{
    return true; // if everything is fine and game continues to run
    return false; // if we decided that save is not compatible

    // can we order to restart game / restore another save here on failure?
}

[Crimson Wizard]

Alright, so, I am trying to write a more explicit plan, and break the solution into several big steps. These steps may be implemented in sequence, or parallel, some of these is optional, and they probably loosely depend on each other.

1. Support reading saves with different counts of items and data (latter is regarding scripts), where user has the final decision in a script callback, based on certain information that engine passes there.

2. Support identifying object entries in saves by script names, as opposed to the array index. This might help in case object order changes, after deleting something in the middle. The idea is that engine writes object's script name along its data when making a save, and when reading back engine looks up for the matching object in the game. Since AGS currently does not enforce object's script name (it's virtually not obligatory, you can create a object that does not have any name at all), these that don't have names may be identified by a generated "name" like "#ID" (where ID is a numeric index).
Such approach still won't on things that don't have unique named IDs, and on script data.
(Potentially, this may be done in the latest AGS 4, where script compiler gathers script's table of contents, for "watch variables" feature.)

3. Support callbacks that let write/read custom user data in saves. This needs to be thought through bit more, to clarify the use cases, because in regular circumstances engine would write script variables into saves, which are already containing custom data. So likely this feature may be useful would the full script data NOT saved (following some setting).

4. Support partial save states, that is - let user configure which data save and not. Of course this won't be a fine tuning, - as there's too many options there, - but a rough one, where you choose types of objects that are saved or not (e.g. GUI, script data, etc). Such feature will remove certain game data from save/restore operation completely. This will require users to reconfigure the game state after restoring a save, but on the other hand there will be no problems with these objects breaking saves.
I've been also thinking about maybe removing certain things from saves completely. Such as - Views and dynamic sprites, for instance. Views may be changed at runtime (frame images and sounds), but they also may be restored by a script after save is loaded, based on current game situation. Same refers to dynamic sprites. I think that it could have been a mistake to add these to saves. Views are annoying, as they contribute to breaking saves even if a single frame is added or removed. And dynamic sprites may increase the disk size of a save file tenfold.

[Crimson Wizard]

Planning this:

1. Support reading saves with different counts of items and data (latter is regarding scripts), where user has the final decision in a script callback, based on certain information that engine passes there.

The way I see this, the process of reading a save is going to be as folows:

1. Engine reads the save, and compares the entries in a save with the game data, for consistency.
2. If an inconsistency is found (e.g. number of entries or size of data is different),
   2.1. If save has more data than the engine, then this fact is marked, but reading continues,
        only the matching amount of data is applied to the game.
   2.2. If save has less data then the engine, then
       a) engine marks and remembers this fact;
       b) engine aborts reading save, and reloads the game in order to clear all its data to the initial state.
       c) after reloading the game, engine restarts reading same save once again, but keeps in memory the fact that it is being read after clean reset.
       d) if there's a marker of a clean reset, then the above restart does not happen, and reading continues.
3. Save is read, and game data reinitialized as necessary.
4. Engine tries to run a script callback, named something like "on_restored_save_validate" (this is just an example and may be changed).
   4.1. If such callback does not exist, and there's no indication of save being inconsistent, then the game is simply continued as normal.
   4.2. If such callback does not exist, and there's a recorded indication about save being inconsitent, then the game is aborted with error message.
   4.3. If such callback exists in script, then it is run.
5. The script callback receives a struct, containing information about the restored save:
   - a variable telling if save is inconsistent, and how (possibly using a set of flags);
   - a group of variables that tell counts of entries per object type:
     how many characters, gui, etc was read from this save.
   This information, as well as a read script data, may be used by game developer to decide what to do with this save.
   Developer's decision may be passed to the engine either with function's return value, or a settable field in the passed struct.
   The instruction can be:
   - continue the game,
   - abort with a error message,
   - restore a different save (something like a fallback to default state).

[Crimson Wizard]

I've made a draft of the solution described above,
may be found in this branch:
https://github.com/ivan-mogilko/ags-refactoring/tree/draft--upgradesaves

At first I wanted to let load both saves with less and more data, but implemented only loading saves with less for now. Skipping extra bits is simply annoying to code, so I'll leave this for the time when this functionality is fleshed out enough.

The problem of resetting un-restored data is solved very simply. It is strange how i did not figure this out back when I tried this for the first time. AGS already has an ability to "reload game"; this is used for example in RunAGSGame function. So, when engine detects that the save has less data, it cancels loading this save, reloads game, and then loads save again, now knowing that the game was "reset".
This way all data which was not loaded from a save gets reset to the default state.



From the scripting perspective, there's a new callback and a new struct:

Code: ags

enum RestoredSaveResult
{
  eRestoredSave_ClearData   = 0x01,
  eRestoredSave_MissingData = 0x08,
  eRestoredSave_ExtraData   = 0x10
};


managed struct RestoredSaveInfo
{
  bool         Cancel;
  readonly RestoredSaveResult Result;
  readonly int AudioClipTypeCount;
  readonly int CharacterCount;
  readonly int DialogCount;
  readonly int GUICount;
  readonly int InventoryItemCount;
  readonly int CursorCount;
  readonly int ViewCount;
  readonly int GlobalScriptDataSize;
  readonly int ScriptModuleCount;
  readonly int RoomScriptDataSize;

  import int[] GetGUIControlCounts();
  import int[] GetViewLoopCounts();
  import int[] GetViewFrameCounts();
  import int[] GetScriptDataSizes();

};

struct RestoredSaveInfo contains some info about how the restoration went in Result field, and then numbers of all the objects found in save, and sizes of script data (variables).

Notice the "Cancel" field, it's the only writeable field in this struct, and user may set it to true or false in a script callback. The value of this field is read back by the engine after the callback has finished. If it's set to "true", the engine will quit the game with error message stating that the save is mismatching the game.

The callback is optional and is defined as:

Code: ags

function validate_restored_save(RestoredSaveInfo* saveInfo)
{
  // check saveInfo and decide what to do
}

Here user may script a "compatible" or "incompatible" save detection, and set the saveInfo.Cancel field.

The Cancel field will default to false if no mismatches were found, and to true if there are mismatches. This is done in case this callback is not present in script: in that case the engine will always fail.


EDIT:

I might add that a user does not have to "Cancel" the save with default error message. It's possible to handle this in a custom way, let save to restore, and then display error in game in a more pretty way.

[Crimson Wizard]

OPENED A DRAFT PR
PR: https://github.com/adventuregamestudio/ags/pull/2489
Download: https://cirrus-ci.com/task/5522434697199616

UPDATED: 31th August

[Crimson Wizard]

I was under impression that this feature is very wanted by a number of people, but as I keep posting here for 3 months, no one replies.

I think this needs to be tested by users, and not only for functionality, but also for usability, because I do not have a full confidence in the chosen design.

How do I proceed from here?

[Crimson Wizard]

I posted this in PR recently, but I might repeat here for convenience.

While it's not quite possible to support auto loading old saves when the order of items changed, because unique ids are not enforced in AGS (and some objects dont have any), there are couple of things, for which it would make sense to make an effort and support their reordering. These are:
* script modules
* plugins

Sometimes a dev might want to add a extra feature to the game, and this feature is implemented by a script module or a plugin. Script Modules must be ordered in certain way in game because of their dependencies, and the order of plugins seems undefined.
Also, dev may want to remove or replace a plugin. Unlike other things in game, plugins is not something you like to keep when it's unused, as it's an extra dll coming along with the distributive.

Therefore, it would be nice to be able to make these two things independent of the order in saves.
In order to achieve that we'd need to ensure they are identified by a unique name.

Plugins already are, and their data is actually already written under their respective names in game saves since AGS 3.6.1. So it must be possible to remove plugins without breaking saves.
It's a shame that this is not how it was done from a start.

This leaves script modules.
Script modules also have name. Unfortunately it was never written into game saves. But it's possible to add starting with the next version.

However, everything comes at a price. And so if I do this, the downside will be that if you rename a script module, then it no longer will be restored from a older game save.
Supposedly this should not be a problem when patching the released game, as you don't normally rename something like scripts at that point.
But maybe someone can see potential troubles with that?

[eri0o]

Regarding script modules, they have a unique ID which both exists in the XML and is written in the SCM file, I don't remember if the editor properly uses it when importing a module.

In theory we could make - at least from now forward - the plugins to also have an unique ID. We just add this and then ask the plugin authors to write a GUID there. It's less clean only because there isn't the automation from the Editor but in theory possible.

Now for the actual feature. My feeling is this is just hard, I get all the things that can be done for the structures like characters and other adventure game stuff, but things like a variable in script, it feels though. My general feeling is it would be easier to add some facility to make it easier for people to override the existing savegame feature and use their own instead - like we can do pretty much with everything else.

I need to think about it but it would be basically: things to help with serialization and things to help with migration. So you have something to help serialize all the existing adventure objects (characters, inv items, ...) and some stuff to help the user write their own save/load stuff for script variables (maybe dictionary based?).

The way currently is that saves both script stuff and the adventure objects stuff seems tricky to be able to do the save upgrade right.

[Crimson Wizard]

Regarding script modules, they have a unique ID which both exists in the XML and is written in the SCM file, I don't remember if the editor properly uses it when importing a module.

But it's not saved to a compiled script nor game data though, engine does not know anything about this GUID.
EDIT: I had to double check, and, yes, unfortunately it does not. Could be useful if it did though.
EDIT2: umh, actually, it's not a GUID, it's an integer key, created by a Random function, meaning it's not exactly unique. Frankly, i cannot tell what is its purpose anyway.

In theory we could make - at least from now forward - the plugins to also have an unique ID. We just add this and then ask the plugin authors to write a GUID there. It's less clean only because there isn't the automation from the Editor but in theory possible.

That would be great if it worked, but in reality will be prone to dev's negligence.
Just as an illustration to how devs treat this kind of thing:
- When several people made their custom builds of AGS engine, they did not tag it as "custom", even though such field exists in the engine, and left version number unchanged. As a result, with these you cannot tell if the game you're running uses official engine or not.
- When "Clifftop Games" devs made custom variant of SpriteFont plugin, which works differently from original, they did not think to change its name (even like "spritefont2" or something), so now the engine has to guess which plugin to emulate based on each individual game's GUID.


Regarding everything else, I mentioned this in the beginning of this thread, and in PR, that what I currently do is meant as a short term and minimal solution for patching the games.
Not a big game update with new chapters and such, but something that would just let users to release a patch containing few more script variables, new controls on guis, a couple new inventory items to fill game logic's loophole, that sort of thing.
It's also meant as a last resort that a user could use if their game was already released and them forgot to plan save compatibility.

I doubt if that it will be just "easier" to have a custom save utilities, because it's not easy to develop and design such thing, and because it will require users to learn making their custom saves and migration process.
I DO believe that such system is doable, but I don't that it's easy. And that definitely will require more time to develop and flesh out, and document and write tutorials, and so forth.

Separating data between what is "game progress" and what is not will definitely be a key here though, that's absolutely true.

[eri0o]

Uhm, only small thing, about the engine, I don't think we save the special engine version in the engine itself, it's only in the JSON file at the root of the repository and goes to the Editor I think. I wanted to add a PR for this at some point but it slipped my mind and I forgot to add a note on this somewhere.

umh, actually, it's not a GUID, it's an integer key, created by a Random function

Well the field already exists, I think .NET has GUID methods, we could use this from 3.6.2 and forward.