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

[P1] Fix wrong local save being deleted when creating a new run #4598

Merged
merged 2 commits into from
Oct 8, 2024

Conversation

MokaStitcher
Copy link
Collaborator

What are the changes the user will see?

No more accidentally erased save when creating a new run offline
One less trigger for save rollbacks when playing online

Why am I making these changes?

Losing runs or progress for no good reason is not great
The problem happens when creating a new run in a slot that has already a save file.
The game tries to delete the local save file in that slot before making the new one, but instead it deletes the save in the slot that was last played after a save & quit, otherwise it defaults to the first slot
Offline the run is fully lost, online there could be progress rollback if the run wasn't synced up with the server.

Note that this bug is part of why online players get rollbacks on their saves, but it's not the only reason. For example save & quitting or refreshing the page just after creating a new run seem to always result in all local saves getting erased, causing rollbacks for non synced runs.
This PR does not address those other issues so rollbacks will still happen

What are the changes from a developer perspective?

deleteSession and tryClearSession now use the provided slotId parameter instead of scene.sessionSlotId

Note that for tryClearSession it does not change anything since it's only called after a run is finished (either lost or won), so the slotId and scene.sessionSlotId are the same in this case.
Which is also why my examples are all about creating new games

Screenshots/Videos

Before: offline - slot 1 getting deleted when creating a run in slot 3

before-offline.mp4

After: offline - no deleted save after creating a new run

_after_offline.mp4

Before: online - slot 1 getting rolled back when creating a run in slot 3

before_online.mp4

After: online - no rollback for slot 1 after creating a new run in slot 2

after_online.mp4

How to test the changes?

  • Oflline:
    • Make sure you have a run in the first slot and at least another slot
    • New game
    • Save in a non empty slot, but not the first one
    • Save & quit
    • All saves should still be here

Online:

  • Make sure you have a run in the first slot
  • Play a couple waves and refresh (not save & quit) so that the save is not synced with the server
  • New Game
  • Save in a non empty slot, but not the first one
  • Play at least one wave then refresh (save & quitting or refreshing before wave 2 would cause an unrelated rollback)
  • All saves should still be here, with no rollback on any slots

Checklist

  • I'm using beta as my base branch
  • There is no overlap with another PR? Partial overlap / very limited conflict with [Refactor] Pokerogue API client #4583
  • The PR is self-contained and cannot be split into smaller PRs?
  • Have I provided a clear explanation of the changes?
  • Have I considered writing automated tests for the issue?
  • If I have text, did I make it translatable and add a key in the English locale file(s)?
  • Have I tested the changes (manually)?
    • Are all unit tests still passing? (npm run test)
  • Are the changes visual?
    • Have I provided screenshots/videos of the changes?

@MokaStitcher MokaStitcher added the P1 Bug Major. Game crashing move/ability/interaction label Oct 6, 2024
@MokaStitcher MokaStitcher requested a review from a team as a code owner October 6, 2024 15:42
@Tempo-anon Tempo-anon merged commit 57a9678 into pagefaultgames:beta Oct 8, 2024
13 checks passed
@MokaStitcher MokaStitcher deleted the session-clear-fix branch November 1, 2024 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 Bug Major. Game crashing move/ability/interaction
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants