From 560713e42a7b5962de2dad5d797b737879608a6b Mon Sep 17 00:00:00 2001 From: Matthew Date: Sat, 25 Jul 2026 08:24:48 -0700 Subject: [PATCH] Fix determinism & entity-loss bugs in engine state reconstruction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- .../src/Game/Entity/BlastPowerupEntity.ts | 7 ++-- .../src/Game/Entity/FreezePowerupEntity.ts | 7 ++-- .../World/createWorld/createWorldFromState.ts | 34 ++++++++++++++++++- .../createWorld/reconstructEntity.test.ts | 34 +++++++++++++++++++ .../World/createWorld/reconstructEntity.ts | 16 ++++++--- .../Game/Training/evaluateNextState.test.ts | 12 +++++++ 6 files changed, 98 insertions(+), 12 deletions(-) diff --git a/engine/bomberland-engine/src/Game/Entity/BlastPowerupEntity.ts b/engine/bomberland-engine/src/Game/Entity/BlastPowerupEntity.ts index abac3101..62ad235d 100644 --- a/engine/bomberland-engine/src/Game/Entity/BlastPowerupEntity.ts +++ b/engine/bomberland-engine/src/Game/Entity/BlastPowerupEntity.ts @@ -6,18 +6,19 @@ import { Environment } from "../../Environment"; import { IConfig } from "../../Config/IConfig"; export class BlastPowerupEntity extends AbstractEntity { - public constructor(config: IConfig, cellNumber: number, mapWidth: number, currentTick: number) { + public constructor(config: IConfig, cellNumber: number, mapWidth: number, currentTick: number, createdOverride?: number) { const engineTelemetry = new CoderOneApi(Environment.Environment, config, true, Environment.Build); const telemetry = new Telemetry(engineTelemetry, config.IsTelemetryEnabled); + const createdTick = createdOverride ?? currentTick; const initialValues: IInitialEntityValues = { cellNumber, mapWidth, type: EntityType.BlastPowerup, - created: currentTick, + created: createdTick, hp: 1, unitId: undefined, agentId: undefined, - expires: currentTick + config.BlastPowerupDurationTicks, + expires: createdTick + config.BlastPowerupDurationTicks, blastDiameter: undefined, }; super(telemetry, initialValues); diff --git a/engine/bomberland-engine/src/Game/Entity/FreezePowerupEntity.ts b/engine/bomberland-engine/src/Game/Entity/FreezePowerupEntity.ts index 26b59cc6..308a8011 100644 --- a/engine/bomberland-engine/src/Game/Entity/FreezePowerupEntity.ts +++ b/engine/bomberland-engine/src/Game/Entity/FreezePowerupEntity.ts @@ -6,18 +6,19 @@ import { Environment } from "../../Environment"; import { IConfig } from "../../Config/IConfig"; export class FreezePowerupEntity extends AbstractEntity { - public constructor(config: IConfig, cellNumber: number, mapWidth: number, currentTick: number) { + public constructor(config: IConfig, cellNumber: number, mapWidth: number, currentTick: number, createdOverride?: number) { const engineTelemetry = new CoderOneApi(Environment.Environment, config, true, Environment.Build); const telemetry = new Telemetry(engineTelemetry, config.IsTelemetryEnabled); + const createdTick = createdOverride ?? currentTick; const initialValues: IInitialEntityValues = { cellNumber, mapWidth, type: EntityType.FreezePowerup, - created: currentTick, + created: createdTick, hp: 1, unitId: undefined, agentId: undefined, - expires: currentTick + config.BlastPowerupDurationTicks, + expires: createdTick + config.FreezePowerupDurationTicks, blastDiameter: undefined, }; super(telemetry, initialValues); diff --git a/engine/bomberland-engine/src/Game/Entity/World/createWorld/createWorldFromState.ts b/engine/bomberland-engine/src/Game/Entity/World/createWorld/createWorldFromState.ts index e66d1ca0..8b6b88c4 100644 --- a/engine/bomberland-engine/src/Game/Entity/World/createWorld/createWorldFromState.ts +++ b/engine/bomberland-engine/src/Game/Entity/World/createWorld/createWorldFromState.ts @@ -13,6 +13,38 @@ import { EntityTracker } from "./../EntityTracker"; import { UnitTracker } from "./../UnitTracker"; import { reconstructEntity } from "./reconstructEntity"; +/** + * Derives a deterministic PRNG seed purely from the input state so that reconstructing the + * same state (e.g. via evaluateNextState) reproduces the same game PRNG stream. Uses an + * FNV-1a style rolling hash over the serialisable tick + layout. + * + * The seed intentionally does NOT depend on config.PrngSeed: getConfig() randomises PrngSeed + * on every call (see getConfig.ts), so mixing it in would reintroduce the non-determinism + * this fix removes. Deriving from state alone guarantees identical (state, actions) inputs + * yield identical output — the property the forward model relies on. + * + * NOTE: this does not reproduce the *original* game's stream position — that would require + * serialising the PRNG state into IGameState (a wire-schema change, out of scope). + */ +const deriveDeterministicSeed = (tick: number, worldState: IWorldState): number => { + let hash = (2166136261 ^ (tick | 0)) >>> 0; + const mix = (value: number) => { + hash = Math.imul(hash ^ (value | 0), 16777619) >>> 0; + }; + worldState.units.forEach((unit) => { + const [x, y] = unit.coordinates; + mix(x); + mix(y); + mix(unit.hp); + }); + worldState.entities.forEach((entity) => { + mix(entity.x); + mix(entity.y); + mix(entity.hp ?? 0); + }); + return hash >>> 0; +}; + const reconstructUnitTracker = (config: IConfig, worldState: IWorldState, prngGame: PRNG): UnitTracker => { const unitTracker = new UnitTracker(prngGame); worldState.units.forEach((unit) => { @@ -63,7 +95,7 @@ export const generateWorldFromState = ( gameTicker: GameTicker ): World => { const config = getConfig({ MapHeight: height, MapWidth: width }); - const randomSeed = Math.floor(Math.random() * Math.random() * (10 ^ 6)); + const randomSeed = deriveDeterministicSeed(gameTicker.CurrentTick, worldState); const mersenneTwisterGame = new MersenneTwister(randomSeed); const prngGame: PRNG = () => mersenneTwisterGame.random(); const cellReserver = new CellReserver(width * height, prngGame, width); diff --git a/engine/bomberland-engine/src/Game/Entity/World/createWorld/reconstructEntity.test.ts b/engine/bomberland-engine/src/Game/Entity/World/createWorld/reconstructEntity.test.ts index 1416eee8..86718f01 100644 --- a/engine/bomberland-engine/src/Game/Entity/World/createWorld/reconstructEntity.test.ts +++ b/engine/bomberland-engine/src/Game/Entity/World/createWorld/reconstructEntity.test.ts @@ -14,4 +14,38 @@ describe("reconstructEntity", () => { const expected = entity; expect(result).toStrictEqual(expected); }); + + test(`it preserves a FreezePowerup's created and derives expires from FreezePowerupDurationTicks (not BlastPowerupDurationTicks)`, () => { + const created = 100; + // Distinct durations so a regression to BlastPowerupDurationTicks would be visible. + const config = getConfig({ MapHeight: 6, MapWidth: 6, FreezePowerupDurationTicks: 40, BlastPowerupDurationTicks: 999 }); + const entity: IEntity = { created, x: 4, y: 2, type: EntityType.FreezePowerup, expires: created + 40, hp: 1 }; + const cellNumber = getCellNumberFromCoordinates([entity.x, entity.y], 6); + const gameTicker = new GameTicker(150, 300); // current tick deliberately != created + const result = reconstructEntity(entity, 6, mock6x6GameState, cellNumber, gameTicker, config).ToJSON(); + expect(result.created).toBe(created); + expect(result.expires).toBe(created + config.FreezePowerupDurationTicks); + }); + + test(`it preserves a BlastPowerup's created and expires on reload`, () => { + const created = 100; + const config = getConfig({ MapHeight: 6, MapWidth: 6, BlastPowerupDurationTicks: 40 }); + const entity: IEntity = { created, x: 4, y: 2, type: EntityType.BlastPowerup, expires: created + 40, hp: 1 }; + const cellNumber = getCellNumberFromCoordinates([entity.x, entity.y], 6); + const gameTicker = new GameTicker(150, 300); + const result = reconstructEntity(entity, 6, mock6x6GameState, cellNumber, gameTicker, config).ToJSON(); + expect(result.created).toBe(created); + expect(result.expires).toBe(created + config.BlastPowerupDurationTicks); + }); + + test(`it reconstructs a bomb with its serialised blast_diameter, not the owning unit's current diameter`, () => { + // Agent "a" units in mock6x6GameState carry blast_diameter 3; the serialised 5 must win. + const bomb: IEntity = { created: 50, x: 2, y: 2, type: EntityType.Bomb, unit_id: "c", agent_id: "a", blast_diameter: 5, expires: 80 }; + const cellNumber = getCellNumberFromCoordinates([bomb.x, bomb.y], 6); + const gameTicker = new GameTicker(60, 300); + const config = getConfig({ MapHeight: 6, MapWidth: 6 }); + const result = reconstructEntity(bomb, 6, mock6x6GameState, cellNumber, gameTicker, config).ToJSON(); + expect(result.blast_diameter).toBe(5); + expect(result.created).toBe(50); + }); }); diff --git a/engine/bomberland-engine/src/Game/Entity/World/createWorld/reconstructEntity.ts b/engine/bomberland-engine/src/Game/Entity/World/createWorld/reconstructEntity.ts index 50caf6ae..053d8395 100644 --- a/engine/bomberland-engine/src/Game/Entity/World/createWorld/reconstructEntity.ts +++ b/engine/bomberland-engine/src/Game/Entity/World/createWorld/reconstructEntity.ts @@ -28,21 +28,27 @@ export const reconstructEntity = ( } else if (type === EntityType.Ammo) { return new AmmoEntity(config, cellNumber, tick, entity.created); } else if (type === EntityType.FreezePowerup) { - return new FreezePowerupEntity(config, cellNumber, width, tick); + return new FreezePowerupEntity(config, cellNumber, width, tick, entity.created); } else if (type === EntityType.Blast) { return new BlastEntity(config, cellNumber, unit_id, agent_id, entity.expires, tick, entity.created); } else if (type === EntityType.BlastPowerup) { - return new BlastPowerupEntity(config, cellNumber, width, tick); + return new BlastPowerupEntity(config, cellNumber, width, tick, entity.created); } else if (type === EntityType.Bomb) { const unit = worldState.units.find((unit) => { return unit.agent_id === agent_id; }); - if (unit !== undefined) { - return new BombEntity(config, cellNumber, width, unit_id, agent_id, tick, unit?.blast_diameter, entity.created); + // Prefer the bomb's own serialised blast diameter (the radius it was thrown with); + // fall back to the owning unit's current diameter only when the bomb didn't carry one. + const blastDiameter = entity.blast_diameter ?? unit?.blast_diameter; + + if (blastDiameter !== undefined) { + return new BombEntity(config, cellNumber, width, unit_id, agent_id, tick, blastDiameter, entity.created); } - throw new Error("Agent cannot be undefined since bombs must have an owner"); + throw new Error( + `Cannot reconstruct bomb at cell ${cellNumber}: no serialised blast_diameter and no owning unit for agent ${agent_id}` + ); } else { throw new Error(`Unhandled entity type ${type} when reconstructing entity`); } diff --git a/engine/bomberland-engine/src/Game/Training/evaluateNextState.test.ts b/engine/bomberland-engine/src/Game/Training/evaluateNextState.test.ts index 0e6f157d..e3b8c058 100644 --- a/engine/bomberland-engine/src/Game/Training/evaluateNextState.test.ts +++ b/engine/bomberland-engine/src/Game/Training/evaluateNextState.test.ts @@ -261,4 +261,16 @@ describe("evaluateNextState", () => { expect(result2).toStrictEqual(expectedState2); }); + + test(`it is deterministic: identical (state, actions) inputs reproduce an identical next_state`, async () => { + // Force a spawn every tick while keeping several empty cells so BOTH the spawn location + // and type depend on the game PRNG. This exercises the state-reconstruction seeding path + // (createWorldFromState) that previously used Math.random() and was non-reproducible. + process.env["ENTITY_SPAWN_PROBABILITY_PER_TICK"] = "1"; + const runA = await evaluateNextState(telemetry, mock4x4GameState, []); + const runB = await evaluateNextState(telemetry, mock4x4GameState, []); + const spawned = runA.tick_result.events.some((event) => event.type === GameEventType.EntitySpawned); + expect(spawned).toBe(true); + expect(runB).toStrictEqual(runA); + }); });