Skip to content

Save optimization - #2152

Open
Cvolton wants to merge 12 commits into
mainfrom
save-optimization
Open

Save optimization#2152
Cvolton wants to merge 12 commits into
mainfrom
save-optimization

Conversation

@Cvolton

@Cvolton Cvolton commented Aug 6, 2026

Copy link
Copy Markdown
Member

Draft for now, since I see some space for improvements, but already creating the PR to get a second pair of eyes on this so I can see if this approach is worth pursuing.

This PR supersedes #1952. It adds "dirty" and "taken" flag tracking to saved and settings jsons and avoids saving them. As mentioned before, the motivation is primarily Android saving performance, though this leads to measurable improvements on Windows as well.

The idea is simple - do not resave settings and saved jsons if they haven't changed. To achieve this it essentially tracks 3 states - clean, dirty, taken. Clean is simple - do not save. Dirty means that it should be saved the next time the game saves but it doesn't need to be saved again afterwards. Taken means that a ref to the container may be stored somewhere, so it should be resaved every time until game exit. This logic is applied to both settings and saved values. Dirty flag is also set if the mod contains settings the user does not have or if the settings file is corrupted (didn't successfully load) for some reason.

In addition to this major change, the PR contains the following minor changes:

  • Mod settings are now saved when the user clicks "Apply" in the mod settings popup (to save time on game exit)
  • Saved values are now loaded even if settings fail to load

Stuff I'd like to figure out before marking this prod ready

  • Using RAII to track container borrow lifespans to not require a permanent taken flag (might not be doable without V6 though?)
  • probs just that, i also need to write doc comments and maybe reorder the members i added

thanks for reading my big yap

@Cvolton

Cvolton commented Aug 7, 2026

Copy link
Copy Markdown
Member Author

upon further thought, I decided that using RAII to manage lifetimes here would likely not work as intended, even assuming a breaking change. A wrapper struct could be made, however for proper json interfacing we'd still need to keep the json struct extractable, which fundamentally breaks the contract given by the potential wrapper struct.

For the same reason I've decided to make getSaveContainerTemp private, since there is also no way to guarantee that the caller won't just keep holding onto the container forever. The Const version is fine to stay public from this perspective IMO, since the const return type alone shows that it should be read only and if the caller const_casts it away, the worst thing that can happen is that their changes won't be saved.

2 extremely minor semantical questions remain

  • is it better to express the dirty/taken difference using two bools (current version) or an enum?
  • the markDirty/unmarkDirty func pair might be better as markDirty(bool)?

With that said I think I'm happy enough with this for now that I can mark this as ready for review.

@Cvolton
Cvolton marked this pull request as ready for review August 7, 2026 08:08
@Fleeym

Fleeym commented Aug 7, 2026

Copy link
Copy Markdown
Member

I'd say an enum works a bit better for dirty / taken, since the 2 bools exclude each other (it doesn't make sense for both to be true at the same time). Since it's not exposed in the API it should be fine the way it is right now too.

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