Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement save backups and better invalid json detection #749

Merged

Conversation

Archez
Copy link
Contributor

@Archez Archez commented Jul 21, 2024

This PR enables the builtin save backup system to work properly.

There were two main pieces missing on our end to make this happen:

First, when a new cycle save is persisted to file, the game writes the save data twice to the sram buffer and uses double the "page" size of a regular new cycle save. This translates into sram flashing both the main save and the backup save in one operation.

I've added a check within our save manager write method to check if the "page" size is this "special" value, and if so, perform a second save operation to the matching backup slot.

The second piece is that the game computes checksum values on saves and performs multiple check points. The file select menu does the following:

  • If a checksum fails, the save is discarded
  • If the main save and the backup save have mismatching values, the backup save is updated to match the main save

We were hard coding the checksum calculation to 1 which meant that backup saves where not created/updated. Additionally, in some of the variables that the game compares with the checksum, a value of 1 is used to indicated no backup save exists.

I've reverted the checksum calculation back to vanilla so that all this logic can run properly again. Because the checksum can be invalidated if someone modifies their save file externally, or a save migration changes the save, I've opted to always recompute the checksum upon parsing the save json. This way the game never thinks the save is "invalid". I think this is a worthy compromise to not make editing save files "painful".


This PR also expands the json parsing/typing try/catches to handle a few move edge cases with invalid json files.

Build Artifacts

@@ -690,10 +690,6 @@ void Sram_IncrementDay(void) {
}

u16 Sram_CalcChecksum(void* data, size_t count) {
// #region 2S2H [Port] I'm not really sure what this is doing or how to port it, for now always return the same
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we’re okay using the vanilla calculation? Originally I thought we might have needed to modify it cause it looked like pointer math, but now I see it’s seems to just be a sum of all bytes casted to u8 or something

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup. And the size passed in accounts for our ShipSaveInfo struct additions too.

I don't see an issue with using it. Ultimately, with this PR recomputing the value on save load, the checksum is only used for deciding if a new backup should be made.

@Archez Archez merged commit 542feb0 into HarbourMasters:develop Aug 18, 2024
5 checks passed
mckinlee pushed a commit to mckinlee/2ship2harkinian that referenced this pull request Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants