Add a function to recover from old versions of saves #5706
Conversation
|
It would be absolutely fantastic to have this in master before next "stable" release (in less than a month now!). My thoughts based on what I saw in the video: (italics indicate suggested change)
In what way will this collide with @JonBooth78's PR #5666? Just checking so he knows if there's something he should do before we merge that PR. Great work! |
|
@impaktor Thanks! For questions:
The fact is that in a sense, all saves are supported, there is no minimum version, it’s just that the older the save, the more errors there will be when restoring. And although the "readers" are written based on version 89, perhaps older versions will also be restored somehow. I have a save of version 86 (October 11, 2020), "only" cargo and equipment failed to be restored from it.
placed in TODO, there might be more similar comments |
I guess this comes down to semantics in the end. Either way, I think calling version 90 "Current version" or "Minimum version" is preferable over "Actual version", as I don't think the use of the word "Actual" is optimal here. |
|
I don't think this will make much difference to my change in #5666 - if it does I can account for that as I'll need to rebase it anyway now as things have moved on. |
|
@JonBooth78 Merged locally #5666 to this branch, with minor conflicts, but not difficult.
That is, the structure of the parameter changes, and the reader from version 89 needs to be modified, and in the future a new reader from version 90 will be required. A bit annoying, but not fatal. |
|
Hmm, OK, I'll need to look at that then; since the new version was written to read with backwards compatibility I'd hoped it would 'just work'... Moving forwards then, after this change to give us backward compatibility on old save versions will we need to write multiple code paths? |
|
Note: we will be savebumping to v90 as part of this release. I'd ideally like to merge this PR "last" after all other major feature PRs to keep the amount of rewriting required to a minimum. |
|
@JonBooth78 Yes, I saw you provided backwards compatibility, but unfortunately, backwards compatibility won't work because as soon as the save version is increased, the save simply won't load, at least that's how it works now. |
If I understood the question correctly, "inside the game code", you do not need to maintain any backward compatibility, and your code can be quite straightforward. But it is required to maintain the correct operation of changed "starting parameter" in the start menu, including editing in the "Custom Game" mode and restoring from a save. Here is a list of starting parameters to make it more clear what I’m talking about: |
|
I thought it would be useful to be able to also restore saves from the current version, for testing purposes or if an incompatible change occurs between releases. Therefore, I added this thing - if you hold Ctrl, the button forcibly changes to "Recover". |
Gliese852
force-pushed
the
restore-saves
branch
from
f50a50e
to
d2bbfb1
Compare
|
Rebased and removed conflicts. |
|
I now see what you've done here - very nice. I was playing around with this trying to load some old save games that were made unloadable during this dev cycle. Turns out the flight log for the stations isn't getting loaded properly with graceful degradation as at least one of the stations appears to be on an invalid index planet. (I was looking to see if I could help integrate this with the updated flightlog code...) Attached: |
|
@JonBooth78 Thank you! I'll do something about it. |
|
Fixed the problem, but only in the recovery logic and the start menu - inside the game it will crash when going to the station log, since it does not know how to show "outdated" system paths. Haven’t touched the GUI in the game yet to avoid unnecessary conflicts with #5666. |
There was a problem hiding this comment.
I'm noticing a general pattern that reader functions do two things: they define a path in the savefile to unpickle data from, and then they convert the unpickled saved data into a "live" object.
I wonder if it'd be worthwhile to evolve this pattern to treat those as two separate processes, such that there is one function concerned only with loading unpickled data from the current save version, and there is another (versioned) function concerned with finding the data from the save game and unpickling it into the format currently expected by the game.
In a lot of cases, readers seem to boil down to a simple "passthrough" function which loads data from a (set of) specific key(s), unpickles it, and optionally transforms it before assigning to the parameter value. Some of this functionality could be made implicit, I think.
This is probably not a topic for this iteration of the save game restoration however, but I did want to record the idea for potential future improvements.
For more on-topic discourse, I've left several review comments and change requests where there are things that I'd like to see addressed before merge. As this PR has a (somewhat) low bug surface and extremely high value for the upcoming release I'm willing to merge it even after we enter feature freeze if they can't be addressed before then, but "the sooner the better" is the motto of the remaining 8 days before release. 😄
Apologies that this has taken so long to review - I've not had mental bandwidth available to deal with the review process for most of this week. In general, I have high confidence in the quality of code being written here, and since it's mostly touching code you've written yourself I don't have to worry about it not matching the existing style of (this particular corner of) the codebase. 😄
| if (filename == "--pretty") { | ||
| indent = 2; | ||
| shift = 1; | ||
| } |
There was a problem hiding this comment.
Note: I prefer to run dumped savegames through https://jsonviewer.stack.hu/, but this is a good secondary option if your text editor is happy with highlighting 1MB+ JSON files.
There was a problem hiding this comment.
Fortunately, I don't experience any problems with large files (in particular json) in vim. And all my bindings with me!
Gliese852
force-pushed
the
restore-saves
branch
from
badc2e4
to
cfe6f32
Compare
|
Made a rebase to resolve the conflict, minimally adapted to the new flightlog, it works, but there are 2 problems. Firstly, the entries in the game are grouped by type in violation of chronology. Secondly, the log inside the game cannot show a "non-existent" system/station path, an error occurs. |
|
Fixed the above 2 problems and it is now everything is working. |
Gliese852
changed the title
|
I think I finished everything I wanted. |
Accorging to reference https://www.lua.org/manual/5.2/manual.html#5 When a function in the auxiliary library uses less than five slots, it does not check the stack size. This means that we should reserve 4 additional slots if we plan to call a function from an auxiliary library.
This function has not been used yet, but will soon be useful for restoring saves. But loading the array was strange. When loading an array, it numbers indices starting from 0, and also adds the number of array elements under the key 'n'. Remove the additional field 'n' and make indexing from 1.
When a StarSystem is deleted, we will reset all pointers to it in system bodies, and this way we can determine that this system body is orphaned. This will allow us to reliably distinguish such bodies and not regenerate the system if it already exists. Which, for example, in the SystemBody.parent attribute will allow us not to access the global galaxy if we are sure that the StarSystem is not deleted.
If the 'updateLayout' function is called when setting/changing the size of the window, so that all tabs can pre-calculate the required dimensions of the elements, 'updateParams' is designed to update dependent elements in accordance with (updated) GameParam.value. The function is guaranteed to run inside the ImGui frame, but we do not know any dimensions. Neither one we want to run every frame, but the second one might be more expensive and doesn't seem to need to be called if we're just resizing. Although window resizing is not currently supported, this may change in the future.
We will simply show an additional tab in the start menu, no editing is expected. To format the string from the SystemPath, access to the galaxy generator is required, this will be done using the Location tab, and also adding the GetStarSystem method to LuaGalaxy. If, for example, when restoring the flight log it turns out that such a station no longer exists, we restore this as a system-only path or even as a sector-only path. The flight log gui inside the game could not handle this and crashed with an error, fixed.
Make the Crew.value a plain array of Character, since the wage can be stored in the contract subtable. Make the Crew.list element similar to PlayerChar in order to be treated more uniformly when rendering a flat list of the player and crew.
The numerical value of the rating turns out to be the number of kills, so renamed player.rating to player.kills. And added a drag to display/change the number of kills itself, interacting with the rating combobox. It turned out that before at startup these two parameters were not passed to the game at all, fixed it.
Remove lock setters, let it be a completely passive property. Also add showKills property to PlayerKills to hide it from the new player.
By "standard" we mean time +1200 years from the current. This will allow to simply indicate "standartness" in the value, without additional flags.
- scroll content if it's too big - reuse code in boxes with different buttons - use pionillium.body font - determine button size depending on font
Inside the handler, this modal could be closed and removed along with its children from the stack, while other modals could be created. And the next modal on the stack may no longer be a child of this modal, but a child of another modal that has not yet been opened.
Although this is not used when the game starts, and is only shown in the summary, it seems worth making this a parameter - can get rid of the bad "global" variable, and it is very easy to set the description when restoring the game.
With the exception of launching with custom start (debugging mode) - so as not to spoil a possible custom start configured earlier. In this case, the option is loaded only the very first time it is opened - as the default value. Also immediately set the combo item "Custom start".
If the saveload menu decides that it cannot load a save (most likely due to a version mismatch), the Load button will be replaced with Recover and you can try to restore the game. In fact, this is the start of a new game, but we are trying to read the parameters from the save. After a recovery attempt, the same dialog for starting a new game opens, but without selecting a start option. If an error occurs when reading a parameter from a save, or after restoring the parameter is in an invalid state, it is unlocked so that the player can manually specify it. The exception is the flight log - it is always locked. Version 89 is currently being successfully restored (2023-02-03). For older versions, it is planned to add "reader" versions in the future. Added the ability to create a "versioned" function; when it is launched, dispatching occurs depending on the specified save version. You can also force recovery by holding Ctrl in the boot menu. This allows to restore a save with the same version as the current one. It seems like this might be useful if changes are made between versions that cause saves to be incompatible. The input for recovery is not a raw document converted from json, but a preprocessed one, namely all Lua objects are unpickled, and all references are resolved.
If you create a ship in a rotating frame corresponding to a body, in the very next (time) frame it will be thrown into a non-rotating frame, and the ship's speed will be changed, which will lead to a deviation of the orbit from circular. Also use body data for the orbital radius and not the system body, so as not to accidentally fall under the terrain. For example, this sometimes happened on Phobos.
Since the previous version of the flight log was divided into 3 sections, and now everything is in one, items need to be sorted after recovery. This functionality was already present in the flight log module, made it public and reused.
Since previously, when starting a new game and placing a ship in a station, the onShipDocked event was emitted, the player was charged a docking fee. What was removed here was a temporary solution to the problem, the event is no longer emitted and workaround is no longer needed.
Gliese852
force-pushed
the
restore-saves
branch
from
dff8e9c
to
4bff75a
Compare
|
This is a clean copy. |
|
Finally, peace and quiet in my inbox!
Nice work folks, that's a nifty feature 🙂
…On Wed, 31 Jan 2024, 23:09 Webster Sheets, ***@***.***> wrote:
Merged #5706 <#5706> into
master.
—
Reply to this email directly, view it on GitHub
<#5706 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEQ4NPUOI26XSZ4FANWKQ3YRK6LDAVCNFSM6AAAAABBNSXL7CVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJRGY3DCOBYGY4DMNA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
recording.webm
If the saveload menu decides that it cannot load a save (most likely due to a version mismatch), the
Loadbutton will be replaced withRecoverand you can try to restore the game.In fact, this is the start of a new game, but we are trying to read the parameters from the save. After a recovery attempt, the same dialog for starting a new game opens, but without selecting a start option.
If an error occurs when reading a parameter from a save, or after restoring the parameter is in an invalid state, it is unlocked so that the player can manually specify it. The exception is the flight log - it is always locked.
Version 89 is currently being successfully restored (2023-02-03). For older versions, it is planned to add "reader" versions in the future. Added the ability to create a "versioned" function; when it is launched, dispatching occurs depending on the specified save version.
Also, now when you launch a new game window in debug mode (with ctrl), it immediately switches to "Custom start". In this case, there are no changes to the parameters after the previous launch of the new game window. That is, if you previously ran a recovery, you can open a new game window in debug mode and there will be the same options, but unlocked.