Fix determinism & entity-loss bugs in engine state reconstruction - #110
Merged
Merged
Conversation
The state-reconstruction path (backing evaluateNextState, the forward-model API used for training/lookahead) carried several correctness bugs that made reconstructed ticks non-reproducible and dropped entity fields on reload. - Deterministic PRNG seed on state reload: replace `Math.floor(Math.random() * Math.random() * (10 ^ 6))` — which used the unseeded global RNG and where `10 ^ 6` is XOR (=12), not 1e6 — with a deterministic FNV-1a hash over tick + layout. Seed is derived from state alone (NOT config.PrngSeed, which getConfig() randomises per call), so identical (state, actions) inputs reproduce identical output. - Powerups no longer lose created/expires on reload: add createdOverride to FreezePowerupEntity/BlastPowerupEntity and thread entity.created through reconstructEntity. - FreezePowerup now expires off FreezePowerupDurationTicks, not BlastPowerupDurationTicks. - Bombs reconstruct with their serialised blast_diameter instead of an arbitrary owner unit's current diameter. Tests: add a determinism check (forced spawn with multiple empty cells so location and type depend on the PRNG) and powerup/bomb round-trip assertions. Known limitation: guarantees same input -> same output, but does not reproduce the original game's exact PRNG stream position (would require serialising PRNG state into IGameState — a wire-schema change, out of scope).
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
1 participant
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.
The state-reconstruction path (backing evaluateNextState, the forward-model API used for training/lookahead) carried several correctness bugs that made reconstructed ticks non-reproducible and dropped entity fields on reload.
Math.floor(Math.random() * Math.random() * (10 ^ 6))— which used the unseeded global RNG and where10 ^ 6is XOR (=12), not 1e6 — with a deterministic FNV-1a hash over tick + layout. Seed is derived from state alone (NOT config.PrngSeed, which getConfig() randomises per call), so identical (state, actions) inputs reproduce identical output.Tests: add a determinism check (forced spawn with multiple empty cells so location and type depend on the PRNG) and powerup/bomb round-trip assertions.
Known limitation: guarantees same input -> same output, but does not reproduce the original game's exact PRNG stream position (would require serialising PRNG state into IGameState — a wire-schema change, out of scope).