Fix bot/nation player ID collisions causing missing players 🔧

[FloPinguin]

Description:

BotSpawner used the same PRNG seed (simpleHash(gameID)) as createGameRunner, causing bot IDs to collide with nation IDs. When a bot's SpawnExecution found a nation with the same ID via hasPlayer(), it silently reused that nation instead of creating a new player - resulting in far fewer players than configured (e.g. ~670 instead of 800 with 400 bots + 400 nations) with no console warnings.

Offsets the BotSpawner seed by +2 to avoid the shared PRNG sequence (matching the +1 pattern already used by Executor).

Please complete the following:

• I have added screenshots for all UI updates
• I process any text displayed to the user through translateText() and I've added it to the en.json file
• I have added relevant tests to the test directory
• I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced

Please put your Discord username so you can be contacted if a bug or regression is found:

FloPinguin

[coderabbitai]

Walkthrough

A PRNG seed adjustment in BotSpawner offsets the hash-based seed by 2 to prevent ID collisions between bot identities and nation/human identities generated from the same sequence.

Changes

Cohort / File(s)	Summary
PRNG Seed Offset src/core/execution/BotSpawner.ts	Modified PRNG seed initialization from simpleHash(gameID) to simpleHash(gameID) + 2 to ensure bot ID generation does not collide with IDs produced by the same sequence for nations and humans.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

Two points of offset, so simple, so small,
Yet chaos prevented before it could fall,
Where bots claim their names with ID collisions no more,
A seed shifted gently—just add a bit more. 🌱

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main bug fix: preventing bot/nation player ID collisions that caused missing players. It directly relates to the primary change in the changeset.
Description check	✅ Passed	The description is directly related to the changeset, providing clear context about the bug (ID collisions between bots and nations), the impact (missing players), and the solution (offsetting the seed by +2).
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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 and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

src/core/execution/BotSpawner.ts (1)

16-18: Good fix for the ID collision bug.

The seed offset of +2 correctly separates BotSpawner's PRNG sequence from GameRunner (+0) and ExecutionManager (+1). The comment is clear and explains the "why" well.

One small note for future maintenance: the offset scheme is now spread across three files with no central list. If another component needs unique IDs later, a developer might accidentally pick a used offset. Consider adding a brief comment here (or in a shared location) listing all current offsets:

// Seed offsets:
// +0: GameRunner (human/nation IDs)
// +1: ExecutionManager
// +2: BotSpawner

This is optional — the current fix is correct and solves the immediate problem.

,

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/core/execution/BotSpawner.ts` around lines 16 - 18, Add a brief comment
near BotSpawner's PRNG seed initialization to document the seed offset scheme so
future authors don't reuse offsets; reference the existing line where BotSpawner
sets this.random = new PseudoRandom(simpleHash(gameID) + 2) and add a short list
noting "+0: GameRunner (human/nation IDs), +1: ExecutionManager, +2: BotSpawner"
(or place that same comment in a shared location referenced by GameRunner and
ExecutionManager) so offsets are centrally visible and maintained.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/core/execution/BotSpawner.ts`:
- Around line 16-18: Add a brief comment near BotSpawner's PRNG seed
initialization to document the seed offset scheme so future authors don't reuse
offsets; reference the existing line where BotSpawner sets this.random = new
PseudoRandom(simpleHash(gameID) + 2) and add a short list noting "+0: GameRunner
(human/nation IDs), +1: ExecutionManager, +2: BotSpawner" (or place that same
comment in a shared location referenced by GameRunner and ExecutionManager) so
offsets are centrally visible and maintained.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d4dc6568-a7be-4a92-918d-7a1639aae8e9

📥 Commits

Reviewing files that changed from the base of the PR and between e137fca and dddf8af.

📒 Files selected for processing (1)

• src/core/execution/BotSpawner.ts

[evanpelle]

thanks!