fungus icon indicating copy to clipboard operation
fungus copied to clipboard

Consolidate and Extend Save System

Open stevehalliwell opened this issue 5 years ago • 27 comments

Follows on and extends #782

Big Changes and Additions from the previous save system:

  • Saves now live in their own files.
  • Saves can be used as 'slot based'
  • More DataSavers added, flowcharts with partial block state, narrative log, global variables, and text variation.
  • SavePoints are now AutoSaves
  • SaveManager tracks AutoSaves and User saves separately
  • Concept of ProgressMarkers as distinct from Save Points
  • Rewind Fastforward removed (ATM)
  • FungusVariables now must state their ability to be serialised and implement to and from
  • Block and individual commands can prevent themselves from being saved
  • Introduced concept of Player Profiles each with their own folder of saves

Pending/Other:

  • [x] Prevent Saving and Loading
  • [x] Specialised Command data saving (E.G. Menu Shuffle). Generic key val store per save slot.
  • [x] Fungus Commands for interacting with save system (new auto save, new manual save, number of auto saves, save to slot number, reset/delete all)
  • [x] Custom data saver example
  • [x] Review all existing commands and systems for potential serialisable requirements
  • [x] Attempt to restore current Say Dialog, Menu Dialog, View, FungusPriority.
    • [x] Character portraits on stage.
  • [x] Player profile data
    • [x] Language
  • [x] Edit & Play Mode Tests
    • [x] Interactive AutoSave w/ portraits on stage, custom Say and Menu set
    • [x] Interactive SlotSave via menu
    • [x] Interactive SlotSave via custom calls from Fungus Commands
    • [x] Interactive SlotSave via custom calls from FungusLua
    • [x] Test 100s of save slots
  • [x] Re-evaluate Rewind/FastForward behaviour
    • It wil be removed, at least in this PR.
  • [x] Music & atmo load and save. Problem here being fungus presently has no direct objects that manage this. A Character stores all portraits so save and load can use that, but with music and atmo clips are just manually fed into the system.
    • Note: could store and re-run the most recent music and atmo commands or could house references to all possible clips and saving name of clip in save and finding by name in load.
  • [x] Improve fungus variable saving routine.
  • [x] Saving during logic flow requires more block data (required; PreviousActiveCommandIndex, call uses lastOnCompleteAction in block to continue. not required but recommended; JumpToCommandIndex)
    • Note, we don't have a mechanism of saving the lastOnCompleteAction and we're probably not going to. Doco needs to be clear about this, in some cases it won't matter as the command is going to execute and generate the same logic in the callback (an await), but user code may be doing more elaborate logic with them.
  • [x] Global vars need additional information so they can be created when they don't already exist.
  • [x] Collection state save restore?

stevehalliwell avatar Feb 17 '20 00:02 stevehalliwell

I apologize for not responding much sooner, Steve. For some reason, Github's systems never told me someone was trying to do a pull request from my save system ._. If there is anything I can do to help, please let me know. I see there is a branch conflict here

CG-Tespy avatar Mar 29 '20 17:03 CG-Tespy

@CG-Tespy You're all good. This PR is for monitoring on going work. I've pulled your original PR into a local within Fungus. I've already gone through and consolidated much of what your original PR was with the existing Fungus base. In doing so a number of changes were made to minimise impact to existing users along with a number of items to include in the save system which is still being worked on.

I've paused my work on this for the moment, as I want to get the existing new features and bug fixes in develop into a release, after which I'll merge in those changes in here and continue to chip away at it.

stevehalliwell avatar Mar 30 '20 02:03 stevehalliwell

Ah, understood 👍

CG-Tespy avatar Mar 31 '20 19:03 CG-Tespy

There is a big problem in the latest stable release of my save system that, some time ago, I was working to fix: the SaveSlot component. It handled displaying every particular field in the save slot, and having all that done in one script can make things hard to extend and maintain.

It would be great if you'd implement a more modular, more extendable and user-friendly solution. Something that makes it as simple as letting the user write a component to handle part of a save slot's UI, and then adding said component to said part.

I myself was working on such a solution until some time ago. It works like this...

There's a ModularSaveSlot component that manages the parts of a single save slot, mainly by just keeping a GameSaveData reference updated. What really does the work is the subcomponents it manages, each of which handle one part of the save slot's UI.

Examples of such subcomponents:

  • SaveSlotNumber: handles displaying the save slot's number
  • SaveSlotDescription: handles displaying the save data's description
  • SaveSlotDate: handles displaying the date the save data was created on

I myself have a partial implementation of this in the ModularRevamp branch of my save system's repo. Here are the directories of the scripts you should look at, if you want to better understand it:

  • SlotComponents
  • SlotComponents/BaseAlgorithms
  • SlotComponents/Interfaces
  • Experimental

These are relative to the [CGT] Fungus Slot-based Save System/Scripts directory. Also ignore the SaveDataTypes and Utils folders within the Experimental folder; I just moved stuff there for a reason I myself have forgotten ^^:

CG-Tespy avatar Jun 23 '20 03:06 CG-Tespy

Could I get some more details for: *Prevent Saving and Loading *Improve fungus variable saving routine.

I'm wondering if either of these changes would prove to be a solution for #846.

Ideally, I'd like variables to perform a Save Variable command right after Set Variable and after getting a return value from an Invoke Method and saving it to a variable, without adding said Save Variable command after every single time I do these things.

Additionally, I'd like variables to perform a Load Variable upon For, If, Else if etc flow commands, because currently I have to use Load Variable every time before using these flow commands.

The reason for these use cases is that if I don't use the Save Variable/Load Variable in these described scenarios, if a player quits the game, restarts it and resumes playing, the variable will not have been saved, therefore the Ifs/Else Ifs/etc will not go through properly.

For clarity's sake I will add the above information to #846 as well.

TheEmbracedOne avatar Oct 08 '20 16:10 TheEmbracedOne

@TheEmbracedOne Added to the other issue tool. The intent is that you won't need both to get what you are after. Hopefully be able to confirm that once 3.14 gets closer or at RC.

Assuming that it does, I'd like to reword the save and load commands to make their function more clear.

stevehalliwell avatar Oct 15 '20 20:10 stevehalliwell

@stevehalliwell I understand now, thanks for clarifying. Would the functionality of: with the improvements in the new save system you won't find the need to use both the save system and playerprefs to get the behaviour you are seeking. The save system alone will get it done. be using playerprefs of a save system unrelated to playerprefs? I'm asking because playerprefs are not cross-platform friendly (consoles), so if your intent is to utilise playerprefs in this system more, perhaps it's worth considering something else that's console-friendly and asynchronous.

TheEmbracedOne avatar Oct 15 '20 20:10 TheEmbracedOne

@TheEmbracedOne sorry for the confusion. The save system does not use playerprefs at all, it is the SaveVariable and LoadVariable commands that do and will continue to use the player prefs. Another reason why I'd like to make the descriptions of those commands clearer in 3.14 :)

stevehalliwell avatar Oct 17 '20 06:10 stevehalliwell

Thank you for further clarifying @stevehalliwell - so to just confirm, do the SaveVariable/LoadVariable (all Fungus variables essentially) currently use PlayerPrefs too?

What other parts of Fungus use PlayerPrefs?

TheEmbracedOne avatar Oct 17 '20 11:10 TheEmbracedOne

If this isn't going to use PlayerPrefs, then is this going to save to files on disk?

CG-Tespy avatar Oct 24 '20 21:10 CG-Tespy

I've been taking a look at the source more, and I like what I've seen thus far. There are several things I'd like to address, but I think it'd be best to do so one at a time.

One issue I see (which admittedly is my fault, as this here is based on my system) is how... restrictive the SaveSlotController script is. What if the user wants to add little pictures to the slots? Or if the game is chapter-based, have the chapter name in it? That would require a fair bit of hacking around the SaveSlotController, which is not good software design.

I created a more flexible architecture a while back that addresses this restrictiveness. How about I create a branch where I fix this, and create a PR to merge into this one?

Edit: Sorry, that was a bit of a silly question...

CG-Tespy avatar Nov 17 '20 19:11 CG-Tespy

@CG-Tespy Thanks, I've merged in your View PR. Looking forward to any further thoughts, suggestions, and/or changes.

stevehalliwell avatar Dec 08 '20 07:12 stevehalliwell

Glad to help! I'll definitely have more contributions to offer in the future ^^

On Tue, Dec 8, 2020 at 2:41 AM Steve Halliwell [email protected] wrote:

@CG-Tespy https://github.com/CG-Tespy Thanks, I've merged in your View PR. Looking forward to any further thoughts, suggestions, and/or changes.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/snozbot/fungus/pull/795#issuecomment-740443860, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFLBGF6MY4JO5KBJZU2NW3TSTXKC7ANCNFSM4KWILMBQ .

CG-Tespy avatar Dec 08 '20 10:12 CG-Tespy

Hiya,

Is there an ETA for when this will be merged? We've been looking at how to implement Steam Cloud Saves, and this PR looks to make this integration a lot easier.

@CG-Tespy nice work!

Cheers, BG

bytemech avatar Dec 13 '20 12:12 bytemech

Thanks! Also, I think the system is still pretty far from being in a good-enough state to be merged, given how much there is left to do and refine

CG-Tespy avatar Dec 15 '20 20:12 CG-Tespy

What's left to do @CG-Tespy? Checklist looks complete?

bytemech avatar Dec 20 '20 14:12 bytemech

@bytemech I can't give a full checklist, but I really do feel that there's a heck of a lot that should be done before this really makes it to the next major version of Fungus. Things that boil down to making things more flexible, programmer-friendly and cleanly-coded.

CG-Tespy avatar Dec 21 '20 02:12 CG-Tespy

Is the new preservation system complete? How long will it take?

zfh2773333333 avatar Apr 07 '21 07:04 zfh2773333333

I need a Public Variable Manager. It does not exist in the hierarchy. It is a separate asset in the Project. Flowchart in all scenarios can be set or saved in the Public Variable Manager Public variables in. In multiple scenarios, the set and save of the existing public variables are both troublesome.

zfh2773333333 avatar Apr 07 '21 08:04 zfh2773333333

I need a Public Variable Manager. It does not exist in the hierarchy. It is a separate asset in the Project.

Wait, in which branch is this? I can't seem to find it in this one.

CG-Tespy avatar Jul 10 '21 04:07 CG-Tespy

I think that saving GameObjects' positions should be listed as one of the more important features to implement. Such functionality is needed for various types of games, such as RPGs, Metroidvanias, and point-and-clicks.

Also, should I make more technical suggestions in this thread, or just more general feature stuff? I may think up ways to design the system better on a coding level, so yeah

CG-Tespy avatar Jul 10 '21 06:07 CG-Tespy

+1 for saving position.

I've tested this PR a while ago, and in my visual novel game there are lots of movements(tweening via Custom command, LeanTween), serious issues appear when I save the game, while the character is in the middle of tweening, when I load the game, the character is gone, like nowhere to be seen. as mentioned here https://github.com/snozbot/fungus/issues/962

Edit: If you somehow want to try to reproduce it, see in the git issue linked above(last comment) on how to do it

breadnone avatar Jul 10 '21 06:07 breadnone

in my visual novel game there are lots of movements(tweening via Custom command, LeanTween), serious issues appear when I save the game, while the character is in the middle of tweening, when I load the game, the character is gone, like nowhere to be seen. as mentioned here #962

@breadnone Your problem here is a bit more complex than simply saving positions; if a character is moving from 0 to 100 on the X axis and you quit out mid-tweening when its at only 60. will it load the character where it was, at 60? If so, will it remain at 60? Or should the system somehow "remember" that it was tweening and finish the tweening to 100? Because all of that is a lot more than just saving the positions of gameobjects; I think this would require a system where commands "report" to a central manager about their progress, and thus the system knows on load whether there were any unfinished commands... but I think it is way more work than what it's worth.

And if you mean the object to be at 0 on load, and perhaps attempt playing out the tweening again, there is an easier solution.

I have similar issues in my game, but not with the position of gameobjects. How I counter it, is by placing savepoints very carefully to combat this. To clarify my game uses autosave and we use a lot of savepoints so the game is technically constantly saving but only when we want it to, specifically to avoid this kind of problem!

If I had to work around your issue in my game, I'd do the following: savepoint right before the tweening savepoint right after the tweening

this means that if you quit out mid-tween, you're just gonna see the tween play out from scratch again. Would this work for you or am I missing the core of your issue?

TheEmbracedOne avatar Jul 10 '21 10:07 TheEmbracedOne

@stevehalliwell Could you outline the flow of the Save-Data-creation process in a public document? I've been having a bit of a hard time reverse-engineering things like the aforementioned process, so yeah

CG-Tespy avatar Jul 11 '21 03:07 CG-Tespy

I did some reverse-engineering, and I think I've more or less gotten the flow. Please let me know if I'm mistaken.

Loading

  1. Ask SaveManager to Load using a metadata
  2. SaveManager delegates the task to the SaveFileManager (SFM), passing it the meta
  3. SFM uses the meta to find the file on disk, extracting its contents to a string.
  4. SFM has its SaveHandler (SH) submodule decode that string into a proper SaveData object
  5. SFM loads the scene, asking the SH to load the SaveData when the scene has loaded
  6. On scene load, SH passes the job to the SaveHandlerUtils (SHU)
  7. SHU executes the PreDecode for each serializer in SH
  8. SHU has each serializer decode the appropriate data (which sadly also handles applying the state in said data to the game)
  9. SHU executes the PostDecode for each serializer in SH

Saving

  1. Ask SaveManager to Load using a name and desc
  2. SaveManager delegates the task to the SaveFileManager (SFM), passing it the name and desc
  3. SFM delegates the task to its SaveHandler (SH), passing the name and desc
  4. SH delegates the task to SaveHandlerUtils (SHU), passing it the name and desc
  5. SHU creates a fresh SaveData
  6. SHU has the serializers do their encoding, registering the results to the SaveData before returning it
  7. Control returns to the SFM
  8. SFM registers something in the save data: a string pair linking the name of the scene as well as the key identifying it as such
  9. SFM encodes the now-ready-for-disk SaveData to a json
  10. SFM writes that json to a file on disk

It also seems like the SaveHandler is being phased out, looking at the SetCurrentSaveGameHandler component's source.

CG-Tespy avatar Jul 11 '21 06:07 CG-Tespy

I think it'd be good if we can assign varying priority levels to the features planned. Perhaps we can also have public documents that show how things are planned to be architected, making it easier for potential contributors to reverse-engineer the system

CG-Tespy avatar Aug 22 '21 06:08 CG-Tespy

Figured I might as well put this here for future ref: I'd like to add compatibility with my third-party system for those who were using it, and would like to switch to the new official one with minimal hassle

CG-Tespy avatar Aug 30 '21 14:08 CG-Tespy