fix ige snapshot sharing state with the live handler - #7
Open
dynamite885 wants to merge 1 commit into
Open
Conversation
snapshot() returned the player entries by reference, so a snapshot kept changing along with the live handler: receive() decrements item.amount on the very objects the snapshot holds. Restoring one left less cancellation available than it should, so more garbage landed. fromSnapshot() had the mirror problem, inserting the given objects straight into the live map.
|
@dynamite885 is attempting to deploy a commit to the Haelp Team on Vercel. A member of the Team first needs to authorize it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
IGEHandler#snapshot()returns the player entries by reference, so a snapshot keeps changing along with the live handler.Object.fromEntriesbuilds a new object, but thePlayerDatavalues are the same references the map holds.receive()then mutates them in place:So after taking a snapshot and continuing to play, the snapshot's outgoing amounts have been decremented too. Restoring it leaves less cancellation available than there should be, and more garbage lands than the replay actually had.
fromSnapshot()has the mirror problem — it inserts the given objects straight into the live map, so a snapshot that is restored more than once gets corrupted by the run in between.How I ran into it
I'm building a replay analysis tool on the engine. It keeps periodic snapshots so playback can be rewound, and it snapshots/restores around a short forward scan to find the next lock frame, for piece-by-piece stepping.
Stepping piece by piece and seeking directly to the same frame produced different boards. On one 1v1 replay, at one piece the pending garbage was 16 when stepped into but 11 when sought directly. Stepping back and forward again gave 11, because going backwards re-derives from an earlier snapshot instead of continuing from the corrupted one.
Stated generally: any two navigation paths to the same frame should end in the same engine state. With the current snapshot they don't.
The change
Deep copies on both sides, using the existing
deepCopyrather thanstructuredClone, given the note indeepCopyabout it being slow.I kept this to
ige.ts. While tracking it down I also noticedTetromino#snapshot()returnskeysandfallingRotationsby reference, andEngine#snapshot()passesmultiplayer?.targetsthrough uncopied. Those look like the same class of issue, but I haven't confirmed they cause visible problems, so I left them alone — happy to follow up separately.Let me know if you'd prefer a different shape for this.