mirror of
https://github.com/openfrontio/OpenFrontIO.git
synced 2026-06-30 13:32:12 +00:00
Fix anonymize-names desync: seed cluster-recalc offset from id() not name() (#4426)
## Description Fixes a hard desync that hit anonymous-names lobbies (e.g. the OFM tournament): in an anonymized FFA, roughly half the clients desynced. ### Root cause `PlayerExecution.init()` seeded the per-player phase offset that schedules `removeClusters()` from the player's **username**: ```ts this.lastCalc = ticks + (simpleHash(this.player.name()) % this.ticksPerClusterCalc); ``` `removeClusters()` is not display-only — it both sets `largestClusterBoundingBox` (read by `AiAttackBehavior` for targeting) **and removes disconnected/surrounded territory, mutating tile ownership**. The anonymize-names option (#4318) sends each client a *different* username for the same player (`anonName(target + viewer)`). So `simpleHash(name()) % 20` differs per client → `removeClusters()` runs on different ticks per client → `numTilesOwned()` diverges → the every-10-tick state hash (`simpleHash(id) * (troops + numTilesOwned) + units`) diverges → **desync**. **Why only half the lobby:** clients granted real-name reveal (host / admins / casters via `nameReveals` / `nameRevealPublicIds`) all see real names, compute identical offsets, and stay in sync with each other and the server record. Every non-granted (anonymized) client sees a unique random name per player and diverges. Hence the clean split. This line has been latent and harmless since 2024 (`f01949f0`) because `name()` used to be identical on every client; #4318 was the first feature to feed a per-client value into the core. The PR comment in `GameServer.ts` even states the assumption — *"username … neither of which the simulation reads"* — which turned out to be false. ### Fix Seed the offset from `id()` instead of `name()`. Player `id()` is assigned from a `gameID`-seeded PRNG by spawn order, so it is identical on every client while still spreading cluster recalculation across players (the line's original load-balancing intent). One-line sim change; no schema/wire change. ### Verification / scope - Audited every `name()`/`username` read in `src/core/`: this was the **only** state-affecting one (all others are `console.log` / error strings / display-only update fields). So this closes the whole desync class. - Confirmed player order, ids, config, `clanTag`/`friends` and cosmetics are all client-identical under anonymize-names — `username` was the sole per-client field reaching the sim. - Swept the other recent core commits (impassable terrain, rail network, nations AI, inline sfc32 PRNG, troop/economy changes) for independent determinism regressions — none found. ### Follow-up `name()` should be removed from the core `Player` surface entirely so a per-client username can never re-enter the sim again (the remaining reads are logging + the display payload the renderer needs). Tracked separately. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -41,7 +41,7 @@ export class PlayerExecution implements Execution {
|
||||
this.map = mg.map();
|
||||
this.config = mg.config();
|
||||
this.lastCalc =
|
||||
ticks + (simpleHash(this.player.name()) % this.ticksPerClusterCalc);
|
||||
ticks + (simpleHash(this.player.id()) % this.ticksPerClusterCalc);
|
||||
}
|
||||
|
||||
tick(ticks: number) {
|
||||
|
||||
Reference in New Issue
Block a user