Open Cherve3 opened 1 week ago
Couple of concerns and a "how do we plan" question. But broadly this looks super solid and I think it's just tweaks away from merging.
The concerns:
I think you might be missing some commits on this PR
When I try to run this without an autoload_scene set on Driver it fails bc of the signal/handler parameter mismatch.
When I do have an autoload_scene it fails because level_name gets set in level_setup which doesn't happen until
load_level
is being called after this which requires level_nameprint("Using level path: " + LEVEL_FILE_PATH + target_level_name + ".tscn") load_level = load(LEVEL_FILE_PATH + target_level_name + ".tscn")
output of that snippet is
Target Level Name: LevelBase Using level path: res://ZZ_Scratch/GreyboxingTools/LevelBase.tscn Loading non-persisting level
Nice catches, I don't think I am missing anything regarding this but it is possible because I stashed some work before I did the PR. I was a bit confused on how that first level gets called so I didn't try to mess with it too much. Just made sure the persistence operates. I felt it didn't need too much attention for this becuase I assume a lot of this will be changing. My thoughts were we probably won't have a button that immediately loads the first level for a new game there will some story or cutscene in between maybe even menus that will change how this operates.
How do we...
What is the plan for loading and saving games. Right now this looks like it works pretty seamlessly with a single save file. How do you imagine us structuring things to manage 5 save slots or whatever and selecting one that we want to restore.
Or updating the game state as we move around but not overwriting the last time that the player chose to record progress?
I mentioned I was trying to get the inventory saving but I put that on hold since I am leaning towards that being saved as a json file but I have to look it over and see if that will work well along with learning hands on how saving json works. I shifted to focusing on saving and loading the levels. I have saving working for levels and the preparations in the SerializationManager
to also save inventory and the player. I was going to try and get the loading functioning next.
It maybe overkill but I figured out how to package everything into a zip file and then rename it to TGO.sav . To accommodate multiple saves we can scan the folder for any .sav files and be able to load them. There is a couple things I am unsure about.
we will probably want to access some data of the save file like a name, image of the screen the save happened, etc. I was thinking maybe creating a json or txt file with this information that would be created during the save process.
There needs to be a way to know what level the player saved on last to make sure that is the level that is loaded. This could also be included in the txt file mentioned in the first point but I am still thinking about if there is a better way.
The new work of this is on this branch I didn't know if I should wait to clear this pr out and work on a new branch when this gets merged or I continue working as normal. I could commit what I have so far while including your other changes mentioned.
I think you might be missing some commits on this PR ...
...I don't think I am missing anything regarding this but it is possible because I stashed some work before I did the PR. I was a bit confused on how that first level gets called so I didn't try to mess with it too much. Just made sure the persistence operates.
Eh, the reason I thought there might be missing commits was because when I ran it I couldn't get any levels to load due to the signal / request_debug_load
parameter thing causing errors. I'm willing to bet what actually happened is that you saw the linting error, fixed that, and didn't think to recheck loadability since it wasn't really changing any code functionality.
I felt it didn't need too much attention for this becuase I assume a lot of this will be changing. My thoughts were we probably won't have a button that immediately loads the first level for a new game there will some story or cutscene in between maybe even menus that will change how this operates.
Broadly that's true but we also can't break the ability to load directly into levels while designers are working / until we get the menu system in place. I think as built rn the only level it can load is BadLevelA since that's hard coded as FIRST_LEVEL_NAME and the callback is discarding the path that's coming into request_debug_load.
We need to ensure that, at least, the autoload_scene path continues to work which (I think?) is busted at the moment as described in the concerns section
How do we...
What is the plan for loading and saving games...
I mentioned I was trying to get the inventory saving but I put that on hold since I am leaning towards that being saved as a json file but I have to look it over and see if that will work well along with learning hands on how saving json works. I shifted to focusing on saving and loading the levels. I have saving working for levels and the preparations in the
SerializationManager
to also save inventory and the player. I was going to try and get the loading functioning next.It maybe overkill but ...
Haha :sickos:. Yes I'm extremely on board with all your thoughts about how to aggregate everything into a single save file etc. And also that we'll probably need some non-compressed metadata. I don't have a solution off hand but I agree that's where we should eventually end up (In a lot of languages you can mount a zip file as a readable file system so we might be able to pull an impl from C# stdlib)
That said my question was more along the lines of:
so by my understanding let's say you have a saved game. When you start the game and enter a level it'll see that there is a non-default version of the level to use and grab it.
You then leave the level it will persist any changes you've made back to disk.
Uh oh, you realize that you've fucked up and pushed a block where you didn't mean to causing a soft lock let's quit the game and reload...
But because we only have one fixed path where saves go that broken level state is now persisted to disk even though the player didn't want to save.
This is a very specific example but the same holds true if we want to have multiple save slots -- because we hard code the path where levels go and everything hangs off that we start getting collisions when we have multiple versions of a level we need to track at different states.
❗ And to be clear we don't need to solve this now -- but we should have a plan to do so so that we know that this solution has a path to evolve into a more general one that meets all the needs we'll eventually have. (I'm pretty sure I see one but I don't want to bias your take -- the last time I had an idea about saving state and let you run free yours was way better / easier to deal with)
Adding the persistence to any levels that are visited while playing. Currently the levels we have are in the
res://ZZ_Scratch/GreyboxingTools/
folder. There is a TODO inDriver.gd
to replace a const for the official place for levels in the future. I touched the BadLevelA and B to for a change on the portals to a new level. Instead of using the levels location just the level name is required. I figured this would be easier since levels would most likely have distinct names and be easier to remember when we have the official levels. Left some unused functions in SerializationManager I think i will be using them still but they will be removed if I don't later on.In Driver.gd I was getting an error in Rider for previously line 23 which is line 28 now, Returns a type [Object] which do not match function's [Driver]. It didn't break anything but it was annoying to see all the time for me and adding the
as Driver
at the end is all it needed to stop yelling at me.The saved levels currently will always be accessible in the user folder here: C:\Users\[username]\AppData\Roaming\Godot\app_userdata\TGO\save Godot shorthand is
user://save/
This is the default location up to the TGO directory. Anything after is created inSerializationManager.gd