Fix anonymize-names desync: seed cluster-recalc offset from id() not name()

[evanpelle]

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:

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

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8ec52a0f-0e53-44f9-9edc-37fd54132868

📥 Commits

Reviewing files that changed from the base of the PR and between 1e4eaca and 1c4d5c1.

📒 Files selected for processing (1)

• src/core/execution/PlayerExecution.ts

💤 Files with no reviewable changes (1)

• src/core/execution/PlayerExecution.ts

---

Walkthrough

In PlayerExecution.init, the seed for the lastCalc phase offset changes from simpleHash(this.player.name()) to simpleHash(this.player.id()), keeping cluster-removal scheduling consistent across clients.

Changes

Cluster-recalc determinism fix

Layer / File(s)	Summary
Use stable id for lastCalc offset src/core/execution/PlayerExecution.ts	init() now uses player.id() instead of player.name() as the hash input for the lastCalc tick offset.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Suggested reviewers

• Celant

Poem

A name can change, a mask may hide,
But id() stays the same inside.
The clusters now sync, tick by tick,
One small hash swap did the trick! 🔑

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly matches the change: it describes fixing anonymize-names desync by seeding the offset from id() instead of name().
Description check	✅ Passed	The description is directly about the same desync fix and explains the root cause, fix, and scope.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}