Re-Enable HumansVsNations 🎉 #2689
Conversation
WalkthroughThis change refines the "Humans vs Nations" game mode by updating lobby labels, adjusting player slot distribution to 50/50 between humans and nations, implementing effective nation count logic for display, adding nation creation with dynamic name generation, and enabling optional spawn cell initialization for nations. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
resources/lang/en.jsonsrc/client/HostLobbyModal.tssrc/client/PublicLobby.tssrc/core/GameRunner.tssrc/core/configuration/DefaultConfig.tssrc/core/execution/NationExecution.tssrc/core/game/Game.tssrc/server/MapPlaylist.ts
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: Saphereye
Repo: openfrontio/OpenFrontIO PR: 2233
File: src/client/HostLobbyModal.ts:891-891
Timestamp: 2025-10-21T20:06:04.823Z
Learning: For the HumansVsNations game mode in `src/client/HostLobbyModal.ts` and related files, the implementation strategy is to generate all nations and adjust their strength for balancing, rather than limiting lobby size based on the number of available nations on the map.
Learnt from: Foorack
Repo: openfrontio/OpenFrontIO PR: 2141
File: src/client/ClientGameRunner.ts:228-234
Timestamp: 2025-10-08T17:14:49.369Z
Learning: For the window close confirmation feature in `ClientGameRunner.ts`, the troop count requirement (>10,000 troops) from issue #2137 was intentionally removed because it was arbitrary and troop count can be reported as low despite having significant land. The confirmation now triggers for any alive player regardless of troop count.
📚 Learning: 2025-10-21T20:06:04.823Z
Learnt from: Saphereye
Repo: openfrontio/OpenFrontIO PR: 2233
File: src/client/HostLobbyModal.ts:891-891
Timestamp: 2025-10-21T20:06:04.823Z
Learning: For the HumansVsNations game mode in `src/client/HostLobbyModal.ts` and related files, the implementation strategy is to generate all nations and adjust their strength for balancing, rather than limiting lobby size based on the number of available nations on the map.
Applied to files:
src/core/configuration/DefaultConfig.tssrc/server/MapPlaylist.tssrc/client/HostLobbyModal.tssrc/core/execution/NationExecution.tsresources/lang/en.jsonsrc/core/GameRunner.tssrc/client/PublicLobby.tssrc/core/game/Game.ts
📚 Learning: 2025-10-20T11:02:16.969Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:57-57
Timestamp: 2025-10-20T11:02:16.969Z
Learning: In src/core/execution/FakeHumanExecution.ts, the correct MIRV victory denial thresholds are VICTORY_DENIAL_TEAM_THRESHOLD = 0.8 (80% for team games) and VICTORY_DENIAL_INDIVIDUAL_THRESHOLD = 0.65 (65% for individual players), not 0.85 and 0.7 as might be mentioned in some documentation.
Applied to files:
src/core/configuration/DefaultConfig.tssrc/server/MapPlaylist.tssrc/client/PublicLobby.ts
📚 Learning: 2025-06-01T04:56:16.870Z
Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 992
File: src/core/configuration/DefaultConfig.ts:0-0
Timestamp: 2025-06-01T04:56:16.870Z
Learning: In the OpenFrontIO codebase, the number of player teams (`numPlayerTeams`) will never be higher than the minimum team size, so the modulo operation in `lobbyMaxPlayers()` will not result in zero players.
Applied to files:
src/core/configuration/DefaultConfig.tssrc/client/PublicLobby.ts
📚 Learning: 2025-10-26T15:37:07.732Z
Learnt from: GlacialDrift
Repo: openfrontio/OpenFrontIO PR: 2298
File: src/client/graphics/layers/TerritoryLayer.ts:200-210
Timestamp: 2025-10-26T15:37:07.732Z
Learning: In GameImpl.ts lines 124-139, team assignment logic varies by number of teams: when numPlayerTeams < 8, teams are assigned ColoredTeams values (Red, Blue, Yellow, Green, Purple, Orange, Teal); when numPlayerTeams >= 8, teams are assigned generic string identifiers like "Team 1", "Team 2", etc., which are not members of ColoredTeams.
Applied to files:
src/core/configuration/DefaultConfig.tssrc/server/MapPlaylist.tssrc/client/HostLobbyModal.tssrc/core/GameRunner.tssrc/client/PublicLobby.ts
📚 Learning: 2025-10-20T20:15:28.858Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:51-51
Timestamp: 2025-10-20T20:15:28.858Z
Learning: In src/core/execution/FakeHumanExecution.ts, game balance constants like MIRV_COOLDOWN_TICKS, MIRV_HESITATION_ODDS, VICTORY_DENIAL_TEAM_THRESHOLD, VICTORY_DENIAL_INDIVIDUAL_THRESHOLD, and STEAMROLL_CITY_GAP_MULTIPLIER are experimental tuning parameters subject to frequent change during balance testing. Do not flag changes to these values as issues or compare them against previous values.
Applied to files:
src/core/configuration/DefaultConfig.tssrc/server/MapPlaylist.tssrc/core/execution/NationExecution.tssrc/core/GameRunner.tssrc/client/PublicLobby.ts
📚 Learning: 2025-10-08T17:14:49.369Z
Learnt from: Foorack
Repo: openfrontio/OpenFrontIO PR: 2141
File: src/client/ClientGameRunner.ts:228-234
Timestamp: 2025-10-08T17:14:49.369Z
Learning: For the window close confirmation feature in `ClientGameRunner.ts`, the troop count requirement (>10,000 troops) from issue #2137 was intentionally removed because it was arbitrary and troop count can be reported as low despite having significant land. The confirmation now triggers for any alive player regardless of troop count.
Applied to files:
src/core/configuration/DefaultConfig.tssrc/client/HostLobbyModal.tssrc/client/PublicLobby.ts
📚 Learning: 2025-06-09T02:20:43.637Z
Learnt from: VariableVince
Repo: openfrontio/OpenFrontIO PR: 1110
File: src/client/Main.ts:293-295
Timestamp: 2025-06-09T02:20:43.637Z
Learning: In src/client/Main.ts, during game start in the handleJoinLobby callback, UI elements are hidden using direct DOM manipulation with classList.add("hidden") for consistency. This includes modals, buttons, and error divs. The codebase follows this pattern rather than using component APIs for hiding elements during game transitions.
Applied to files:
src/client/HostLobbyModal.tssrc/client/PublicLobby.ts
📚 Learning: 2025-08-23T07:48:19.060Z
Learnt from: ElMelchizedek
Repo: openfrontio/OpenFrontIO PR: 1876
File: src/core/execution/FakeHumanExecution.ts:470-473
Timestamp: 2025-08-23T07:48:19.060Z
Learning: In FakeHumanExecution.ts DefensePost placement logic, returning -Infinity from structureSpawnTileValue when no sampled border tiles neighbor enemies is intentional. The logic samples up to 50 border tiles as a heuristic - if none are adjacent to enemies, it assumes DefensePost placement is unnecessary and aborts the entire placement attempt rather than continuing to evaluate individual tiles.
Applied to files:
src/core/execution/NationExecution.ts
📚 Learning: 2025-11-26T20:49:29.140Z
Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2519
File: src/core/game/GameView.ts:516-525
Timestamp: 2025-11-26T20:49:29.140Z
Learning: In GameView.ts, when usesSharedTileState is true (SAB mode), packedTileUpdates contains unpacked tile references as BigInt(tileRef) only, because all tile state lives in the shared Uint16Array. In non-SAB mode, packedTileUpdates contains packed TileUpdate bigints in the format (tileRef << 16n | state), which must be decoded via updateTile(tu). Therefore, Number(tu) is correct in SAB mode and shifting right by 16 bits would be wrong.
Applied to files:
src/core/GameRunner.ts
📚 Learning: 2025-06-10T09:56:44.473Z
Learnt from: Ble4Ch
Repo: openfrontio/OpenFrontIO PR: 1063
File: src/core/configuration/PastelThemeDark.ts:53-53
Timestamp: 2025-06-10T09:56:44.473Z
Learning: In ColorAllocator class in src/core/configuration/Colors.ts, the correct method names are assignColor(id: string): Colord for general color assignment and assignTeamColor(team: Team): Colord for team-specific colors. There are no assignPlayerColor() or assignBotColor() methods.
Applied to files:
src/client/PublicLobby.ts
📚 Learning: 2025-08-12T00:31:50.144Z
Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 1752
File: src/core/game/Game.ts:750-752
Timestamp: 2025-08-12T00:31:50.144Z
Learning: In the OpenFrontIO codebase, changes to the PlayerInteraction interface (like adding canDonateGold and canDonateTroops flags) do not require corresponding updates to src/core/Schemas.ts or server serialization code.
Applied to files:
src/core/game/Game.ts
🧬 Code graph analysis (4)
src/server/MapPlaylist.ts (2)
src/core/game/Game.ts (1)
HumansVsNations(56-56)src/core/configuration/DefaultConfig.ts (1)
playerTeams(311-313)
src/client/HostLobbyModal.ts (1)
src/core/game/Game.ts (1)
HumansVsNations(56-56)
src/core/execution/NationExecution.ts (1)
src/core/execution/SpawnExecution.ts (1)
SpawnExecution(10-123)
src/client/PublicLobby.ts (3)
src/core/game/Game.ts (1)
HumansVsNations(56-56)src/client/LangSelector.ts (1)
translateText(258-278)src/client/Utils.ts (1)
translateText(92-147)
🔇 Additional comments (13)
src/core/configuration/DefaultConfig.ts (1)
209-212: LGTM!The 50/50 slot split is clear and well-commented. Using
Math.floorkeeps the result as an integer, which is correct for player counts.src/core/execution/NationExecution.ts (1)
258-259: LGTM!Good defensive check. This guard ensures the function isn't called in an invalid state after the earlier early-return path handles undefined spawn cells.
src/core/game/Game.ts (1)
311-316: LGTM!Making
spawnCelloptional (Cell | undefined) supports dynamically created nations that spawn at random locations. This aligns with the new handling inNationExecution.src/core/GameRunner.ts (2)
92-157: LGTM!Clean implementation of nation creation logic. The function:
- Returns empty array when nations are disabled.
- Uses manifest nations directly for non-HVN modes.
- Balances nation count to match humans in HVN mode.
- Shuffles manifest nations for variety when selecting a subset.
- Creates additional nations with
undefinedspawnCell when manifest has too few.The Fisher-Yates shuffle at lines 134-139 is correctly implemented.
159-166: LGTM!Simple name generator using existing bot name parts. Works well for dynamically created nations.
src/server/MapPlaylist.ts (2)
76-77: LGTM!Adding
HumansVsNationsto the team configuration set enables this mode in public lobbies.
98-99: LGTM!Setting
Difficulty.Hardfor HumansVsNations aligns with the PR notes about giving nations a slight advantage to compensate for humans being able to donate troops while nations cannot.resources/lang/en.json (1)
274-275: LGTM!Good additions:
- Consistent capitalization for "vs".
- The detailed label template
{num} Humans vs {num} Nationssupports the new per-team display in public lobbies.src/client/HostLobbyModal.ts (2)
875-889: LGTM!The helper method is well-documented and handles all three cases correctly:
- Nations disabled → 0
- HumansVsNations mode → matches human player count
- Otherwise → uses manifest nation count
This ensures the lobby UI displays accurate information.
552-565: LGTM!Using
getEffectiveNationCount()in both the header display and theLobbyTeamViewdata keeps the UI consistent with the actual game behavior.src/client/PublicLobby.ts (3)
219-220: LGTM!For HumansVsNations,
teamSizeequalsmaxPlayersbecause in HVN mode,maxPlayersalready represents the per-team human count (after the 50/50 split inDefaultConfig.lobbyMaxPlayers). This value is then used for the detailed label.
243-249: LGTM!When HumansVsNations mode is active and
teamSizeis available, the method returns the detailed label (e.g., "5 Humans vs 5 Nations"). This provides clear information about the matchup to players.
263-265: LGTM!Returning
nullfor HumansVsNations avoids duplicate information since the mode label already shows the detailed breakdown.
…ctly (Lol it somehow worked without that)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/core/GameRunner.ts (1)
127-137: Use existingshuffleArraymethod from PseudoRandom.The shuffle logic here duplicates the
shuffleArraymethod already available inPseudoRandomclass (seesrc/core/PseudoRandom.tslines 66-72). Using the existing method keeps code clean and avoids duplication.Proposed fix
// If we have enough manifest nations, use a subset if (manifestNations.length >= targetNationCount) { - // Shuffle manifest nations to add variety - const shuffled = [...manifestNations]; - for (let i = shuffled.length - 1; i > 0; i--) { - const j = random.nextInt(0, i + 1); - const temp = shuffled[i]; - shuffled[i] = shuffled[j]; - shuffled[j] = temp; - } + const shuffled = random.shuffleArray(manifestNations); return shuffled.slice(0, targetNationCount).map(toNation); }src/core/execution/utils/NationNames.ts (1)
203-220: Consider usingrandElementfor cleaner array selection.
PseudoRandomhas arandElementmethod that handles random array selection. This makes the code slightly more readable.Proposed refactor
export function generateNationName(random: PseudoRandom): string { - const template = NAME_TEMPLATES[random.nextInt(0, NAME_TEMPLATES.length)]; - const noun = NOUNS[random.nextInt(0, NOUNS.length)]; + const template = random.randElement(NAME_TEMPLATES); + const noun = random.randElement(NOUNS); const result: string[] = [];
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/core/GameRunner.tssrc/core/execution/utils/NationNames.ts
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: Saphereye
Repo: openfrontio/OpenFrontIO PR: 2233
File: src/client/HostLobbyModal.ts:891-891
Timestamp: 2025-10-21T20:06:04.823Z
Learning: For the HumansVsNations game mode in `src/client/HostLobbyModal.ts` and related files, the implementation strategy is to generate all nations and adjust their strength for balancing, rather than limiting lobby size based on the number of available nations on the map.
Learnt from: Foorack
Repo: openfrontio/OpenFrontIO PR: 2141
File: src/client/ClientGameRunner.ts:228-234
Timestamp: 2025-10-08T17:14:49.369Z
Learning: For the window close confirmation feature in `ClientGameRunner.ts`, the troop count requirement (>10,000 troops) from issue #2137 was intentionally removed because it was arbitrary and troop count can be reported as low despite having significant land. The confirmation now triggers for any alive player regardless of troop count.
📚 Learning: 2025-10-21T20:06:04.823Z
Learnt from: Saphereye
Repo: openfrontio/OpenFrontIO PR: 2233
File: src/client/HostLobbyModal.ts:891-891
Timestamp: 2025-10-21T20:06:04.823Z
Learning: For the HumansVsNations game mode in `src/client/HostLobbyModal.ts` and related files, the implementation strategy is to generate all nations and adjust their strength for balancing, rather than limiting lobby size based on the number of available nations on the map.
Applied to files:
src/core/execution/utils/NationNames.tssrc/core/GameRunner.ts
📚 Learning: 2025-10-20T20:15:28.858Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:51-51
Timestamp: 2025-10-20T20:15:28.858Z
Learning: In src/core/execution/FakeHumanExecution.ts, game balance constants like MIRV_COOLDOWN_TICKS, MIRV_HESITATION_ODDS, VICTORY_DENIAL_TEAM_THRESHOLD, VICTORY_DENIAL_INDIVIDUAL_THRESHOLD, and STEAMROLL_CITY_GAP_MULTIPLIER are experimental tuning parameters subject to frequent change during balance testing. Do not flag changes to these values as issues or compare them against previous values.
Applied to files:
src/core/GameRunner.ts
📚 Learning: 2025-11-26T20:49:29.140Z
Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2519
File: src/core/game/GameView.ts:516-525
Timestamp: 2025-11-26T20:49:29.140Z
Learning: In GameView.ts, when usesSharedTileState is true (SAB mode), packedTileUpdates contains unpacked tile references as BigInt(tileRef) only, because all tile state lives in the shared Uint16Array. In non-SAB mode, packedTileUpdates contains packed TileUpdate bigints in the format (tileRef << 16n | state), which must be decoded via updateTile(tu). Therefore, Number(tu) is correct in SAB mode and shifting right by 16 bits would be wrong.
Applied to files:
src/core/GameRunner.ts
📚 Learning: 2025-10-26T15:37:07.732Z
Learnt from: GlacialDrift
Repo: openfrontio/OpenFrontIO PR: 2298
File: src/client/graphics/layers/TerritoryLayer.ts:200-210
Timestamp: 2025-10-26T15:37:07.732Z
Learning: In GameImpl.ts lines 124-139, team assignment logic varies by number of teams: when numPlayerTeams < 8, teams are assigned ColoredTeams values (Red, Blue, Yellow, Green, Purple, Orange, Teal); when numPlayerTeams >= 8, teams are assigned generic string identifiers like "Team 1", "Team 2", etc., which are not members of ColoredTeams.
Applied to files:
src/core/GameRunner.ts
🧬 Code graph analysis (1)
src/core/execution/utils/NationNames.ts (1)
src/core/PseudoRandom.ts (1)
PseudoRandom(3-76)
🔇 Additional comments (4)
src/core/GameRunner.ts (2)
89-126: LGTM! Clean function structure with clear logic.Good use of early returns for disabled nations and non-HVN modes. The documentation comment clearly explains the purpose. The helper
toNationkeeps the code readable.
143-151: The undefinedspawnCellis properly handled by design. During the spawn phase, nations created without a predefined spawn cell are delegated toSpawnExecution, which assigns them random spawn locations. The code includes an explicit check with a comment: "Place nations without a spawn cell (Dynamically created for HumansVsNations) randomly by SpawnExecution." Once placed, subsequent operations use the assigned cell safely.src/core/execution/utils/NationNames.ts (2)
3-6: Good use of Symbols for type-safe placeholder tokens.Using Symbols with a typed union for
NameTemplateis a clean, idiomatic TypeScript approach. This avoids magic strings and provides compile-time safety.
8-125: LGTM! Good variety of templates.The template collection provides good variety for generated nation names. The mix of prefix-only, suffix-only, and combined patterns ensures interesting combinations.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/core/execution/utils/NationNames.ts (1)
209-211: Optional: The "o" rule works now but isn't universal.The current rule adds "es" to all words ending in "o", which works for "Tomato" and "Potato" in the NOUNS list. However, if the list expands to include words like "Piano" or "Photo", those would incorrectly become "Pianoes" and "Photoes" instead of "Pianos" and "Photos".
Since the current nouns are fine, this is just a heads-up if the list grows in the future. You could add a specific check for common "-o" plurals or keep a special-case list.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/core/execution/utils/NationNames.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Saphereye
Repo: openfrontio/OpenFrontIO PR: 2233
File: src/client/HostLobbyModal.ts:891-891
Timestamp: 2025-10-21T20:06:04.823Z
Learning: For the HumansVsNations game mode in `src/client/HostLobbyModal.ts` and related files, the implementation strategy is to generate all nations and adjust their strength for balancing, rather than limiting lobby size based on the number of available nations on the map.
Learnt from: Foorack
Repo: openfrontio/OpenFrontIO PR: 2141
File: src/client/ClientGameRunner.ts:228-234
Timestamp: 2025-10-08T17:14:49.369Z
Learning: For the window close confirmation feature in `ClientGameRunner.ts`, the troop count requirement (>10,000 troops) from issue #2137 was intentionally removed because it was arbitrary and troop count can be reported as low despite having significant land. The confirmation now triggers for any alive player regardless of troop count.
📚 Learning: 2025-10-21T20:06:04.823Z
Learnt from: Saphereye
Repo: openfrontio/OpenFrontIO PR: 2233
File: src/client/HostLobbyModal.ts:891-891
Timestamp: 2025-10-21T20:06:04.823Z
Learning: For the HumansVsNations game mode in `src/client/HostLobbyModal.ts` and related files, the implementation strategy is to generate all nations and adjust their strength for balancing, rather than limiting lobby size based on the number of available nations on the map.
Applied to files:
src/core/execution/utils/NationNames.ts
🧬 Code graph analysis (1)
src/core/execution/utils/NationNames.ts (1)
src/core/PseudoRandom.ts (1)
PseudoRandom(3-76)
🔇 Additional comments (2)
src/core/execution/utils/NationNames.ts (2)
196-213: Pluralization logic looks solid.The past review concerns about "Puppy", "Jelly", and "Sandwich" have been addressed. The current logic correctly handles:
- Words ending in s/ch/sh/x/z → add "es"
- Words ending in consonant + "y" → change to "ies"
- Words ending in "o" → add "es"
All nouns in the NOUNS list should pluralize correctly with these rules.
1-232: Clean, type-safe nation name generator.The module is well-structured:
- Uses typed unions and symbols instead of class hierarchies
- Clean, idiomatic TypeScript with no unnecessary libraries
- PseudoRandom integration is correct
- Template expansion logic handles all cases properly
The generated nation names should be fun and varied for the HumansVsNations mode!
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/core/execution/utils/NationNames.ts (1)
199-216: LGTM! Past pluralization issues resolved.The pluralization logic now correctly handles the cases flagged in the previous review:
- "Puppy" → "Puppies" ✓ (line 209-210)
- "Jelly" → "Jellies" ✓ (line 209-210)
- "Sandwich" → "Sandwiches" ✓ (line 200-207)
- "Potato" → "Potatoes" ✓ (line 212-214)
The implementation covers the main English pluralization rules appropriate for this game context.
Optional: Add defensive check for edge case
If very short nouns (length < 2) are added in the future, line 209 could access
noun[noun.length - 2]asundefined. Consider adding a length check, though all current nouns are safe:function pluralize(noun: string): string { + if (noun.length < 2) { + return `${noun}s`; + } if ( noun.endsWith("s") ||
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/core/execution/utils/NationNames.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Saphereye
Repo: openfrontio/OpenFrontIO PR: 2233
File: src/client/HostLobbyModal.ts:891-891
Timestamp: 2025-10-21T20:06:04.823Z
Learning: For the HumansVsNations game mode in `src/client/HostLobbyModal.ts` and related files, the implementation strategy is to generate all nations and adjust their strength for balancing, rather than limiting lobby size based on the number of available nations on the map.
Learnt from: Foorack
Repo: openfrontio/OpenFrontIO PR: 2141
File: src/client/ClientGameRunner.ts:228-234
Timestamp: 2025-10-08T17:14:49.369Z
Learning: For the window close confirmation feature in `ClientGameRunner.ts`, the troop count requirement (>10,000 troops) from issue #2137 was intentionally removed because it was arbitrary and troop count can be reported as low despite having significant land. The confirmation now triggers for any alive player regardless of troop count.
📚 Learning: 2025-10-21T20:06:04.823Z
Learnt from: Saphereye
Repo: openfrontio/OpenFrontIO PR: 2233
File: src/client/HostLobbyModal.ts:891-891
Timestamp: 2025-10-21T20:06:04.823Z
Learning: For the HumansVsNations game mode in `src/client/HostLobbyModal.ts` and related files, the implementation strategy is to generate all nations and adjust their strength for balancing, rather than limiting lobby size based on the number of available nations on the map.
Applied to files:
src/core/execution/utils/NationNames.ts
🧬 Code graph analysis (1)
src/core/execution/utils/NationNames.ts (1)
src/core/PseudoRandom.ts (1)
PseudoRandom(3-76)
🔇 Additional comments (6)
src/core/execution/utils/NationNames.ts (6)
1-2: LGTM!Clean import structure. The
PseudoRandomdependency provides seeded randomization needed for deterministic nation name generation.
3-6: LGTM!Good use of symbols for unique template placeholders. The
NameTemplatetype is a clean typed union following TypeScript best practices.
8-125: LGTM!Creative and diverse template collection. The mix of humorous and traditional nation-style patterns will generate entertaining names for the game mode.
127-194: LGTM!Good variety of nouns for name generation. All entries are properly capitalized and suitable for the humorous tone of the game mode.
196-197: LGTM!The
O_TO_OESset correctly handles irregular plurals for "Potato" and "Tomato" as mentioned in the commit message. This addresses the irregular pluralization issue.
218-235: LGTM!The name generation logic is clean and straightforward:
- Selects random template and noun via
PseudoRandom- Substitutes placeholders with singular/plural forms
- Returns space-joined result
The implementation is type-safe and aligns with the clean TypeScript style requested.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
resources/lang/en.jsonsrc/client/HostLobbyModal.tssrc/client/PublicLobby.tssrc/core/GameRunner.tssrc/core/configuration/DefaultConfig.tssrc/core/execution/NationExecution.tssrc/core/game/Game.tssrc/server/MapPlaylist.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- src/server/MapPlaylist.ts
- resources/lang/en.json
- src/client/HostLobbyModal.ts
- src/core/configuration/DefaultConfig.ts
- src/core/execution/NationExecution.ts
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: Saphereye
Repo: openfrontio/OpenFrontIO PR: 2233
File: src/client/HostLobbyModal.ts:891-891
Timestamp: 2025-10-21T20:06:04.823Z
Learning: For the HumansVsNations game mode in `src/client/HostLobbyModal.ts` and related files, the implementation strategy is to generate all nations and adjust their strength for balancing, rather than limiting lobby size based on the number of available nations on the map.
Learnt from: Foorack
Repo: openfrontio/OpenFrontIO PR: 2141
File: src/client/ClientGameRunner.ts:228-234
Timestamp: 2025-10-08T17:14:49.369Z
Learning: For the window close confirmation feature in `ClientGameRunner.ts`, the troop count requirement (>10,000 troops) from issue #2137 was intentionally removed because it was arbitrary and troop count can be reported as low despite having significant land. The confirmation now triggers for any alive player regardless of troop count.
📚 Learning: 2025-10-21T20:06:04.823Z
Learnt from: Saphereye
Repo: openfrontio/OpenFrontIO PR: 2233
File: src/client/HostLobbyModal.ts:891-891
Timestamp: 2025-10-21T20:06:04.823Z
Learning: For the HumansVsNations game mode in `src/client/HostLobbyModal.ts` and related files, the implementation strategy is to generate all nations and adjust their strength for balancing, rather than limiting lobby size based on the number of available nations on the map.
Applied to files:
src/core/GameRunner.tssrc/client/PublicLobby.tssrc/core/game/Game.ts
📚 Learning: 2025-10-20T20:15:28.858Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:51-51
Timestamp: 2025-10-20T20:15:28.858Z
Learning: In src/core/execution/FakeHumanExecution.ts, game balance constants like MIRV_COOLDOWN_TICKS, MIRV_HESITATION_ODDS, VICTORY_DENIAL_TEAM_THRESHOLD, VICTORY_DENIAL_INDIVIDUAL_THRESHOLD, and STEAMROLL_CITY_GAP_MULTIPLIER are experimental tuning parameters subject to frequent change during balance testing. Do not flag changes to these values as issues or compare them against previous values.
Applied to files:
src/core/GameRunner.tssrc/client/PublicLobby.ts
📚 Learning: 2025-11-26T20:49:29.140Z
Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2519
File: src/core/game/GameView.ts:516-525
Timestamp: 2025-11-26T20:49:29.140Z
Learning: In GameView.ts, when usesSharedTileState is true (SAB mode), packedTileUpdates contains unpacked tile references as BigInt(tileRef) only, because all tile state lives in the shared Uint16Array. In non-SAB mode, packedTileUpdates contains packed TileUpdate bigints in the format (tileRef << 16n | state), which must be decoded via updateTile(tu). Therefore, Number(tu) is correct in SAB mode and shifting right by 16 bits would be wrong.
Applied to files:
src/core/GameRunner.ts
📚 Learning: 2025-10-26T15:37:07.732Z
Learnt from: GlacialDrift
Repo: openfrontio/OpenFrontIO PR: 2298
File: src/client/graphics/layers/TerritoryLayer.ts:200-210
Timestamp: 2025-10-26T15:37:07.732Z
Learning: In GameImpl.ts lines 124-139, team assignment logic varies by number of teams: when numPlayerTeams < 8, teams are assigned ColoredTeams values (Red, Blue, Yellow, Green, Purple, Orange, Teal); when numPlayerTeams >= 8, teams are assigned generic string identifiers like "Team 1", "Team 2", etc., which are not members of ColoredTeams.
Applied to files:
src/core/GameRunner.tssrc/client/PublicLobby.ts
📚 Learning: 2025-06-09T02:20:43.637Z
Learnt from: VariableVince
Repo: openfrontio/OpenFrontIO PR: 1110
File: src/client/Main.ts:293-295
Timestamp: 2025-06-09T02:20:43.637Z
Learning: In src/client/Main.ts, during game start in the handleJoinLobby callback, UI elements are hidden using direct DOM manipulation with classList.add("hidden") for consistency. This includes modals, buttons, and error divs. The codebase follows this pattern rather than using component APIs for hiding elements during game transitions.
Applied to files:
src/client/PublicLobby.ts
📚 Learning: 2025-10-20T11:02:16.969Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:57-57
Timestamp: 2025-10-20T11:02:16.969Z
Learning: In src/core/execution/FakeHumanExecution.ts, the correct MIRV victory denial thresholds are VICTORY_DENIAL_TEAM_THRESHOLD = 0.8 (80% for team games) and VICTORY_DENIAL_INDIVIDUAL_THRESHOLD = 0.65 (65% for individual players), not 0.85 and 0.7 as might be mentioned in some documentation.
Applied to files:
src/client/PublicLobby.ts
📚 Learning: 2025-06-01T04:56:16.870Z
Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 992
File: src/core/configuration/DefaultConfig.ts:0-0
Timestamp: 2025-06-01T04:56:16.870Z
Learning: In the OpenFrontIO codebase, the number of player teams (`numPlayerTeams`) will never be higher than the minimum team size, so the modulo operation in `lobbyMaxPlayers()` will not result in zero players.
Applied to files:
src/client/PublicLobby.ts
🧬 Code graph analysis (1)
src/client/PublicLobby.ts (3)
src/core/game/Game.ts (1)
HumansVsNations(56-56)src/client/LangSelector.ts (1)
translateText(258-278)src/client/Utils.ts (1)
translateText(92-147)
🔇 Additional comments (7)
src/core/game/Game.ts (1)
313-318: Approve: Clean API change to support dynamic nation spawning.Making
spawnCelloptional enables creating nations without predefined coordinates, which is needed when a map has fewer nations than human players in HumansVsNations mode. The downstreamNationExecution.tshandles theundefinedcase by dynamically finding a spawn location.src/core/GameRunner.ts (4)
1-40: LGTM: Clean imports for nation creation support.The new imports are well-organized:
generateNationNamefor dynamic names, game type enums for mode detection, andManifestNationalias to distinguish from the domainNationclass.
90-155: Approve: Well-structured nation balancing logic.The
createNationsForGamefunction cleanly handles all scenarios:
- Disabled nations → empty array
- Non-HumansVsNations → use all manifest nations
- HumansVsNations with enough manifest nations → shuffle and pick subset
- HumansVsNations with insufficient manifest nations → fill with generated names
The Fisher-Yates shuffle is correctly implemented, and dynamically created nations use
undefinedspawnCell to let the spawn system pick locations. This aligns with the learned strategy of generating nations and adjusting for balance rather than limiting lobby size.
66-71: LGTM: Clean integration of nation creation helper.The call site passes the right arguments and keeps the
createGameRunnerfunction focused on orchestration rather than nation balancing details.
132-137: No issues found —PseudoRandom.nextIntbounds are correct.The
nextInt(min, max)implementation treatsmaxas exclusive, producing[min, max). For the Fisher-Yates shuffle callingnextInt(0, i + 1), this correctly generates values in[0, i]inclusive, as required. The codebase even includes a verifiedshuffleArraymethod inPseudoRandom.tsusing the exact same pattern without issues.src/client/PublicLobby.ts (2)
289-291: LGTM!Correctly returns
nullfor HumansVsNations since the detailed label is already included ingetModeLabel. This prevents duplicate or conflicting detail labels in the UI.
269-275: Translation key is correctly added to en.json.The key
public_lobby.teams_hvn_detailedexists inen.jsonwith the correct value. Non-English locale files will be updated in a separate translation PR per project workflow.
github-project-automation
bot
moved this from Peer Review
to Development
in OpenFront Release Management
src/core/GameRunner.ts
Outdated
| * Creates the nations array for a game, handling HumansVsNations mode specially. | ||
| * In HumansVsNations mode, the number of nations matches the number of human players to ensure fair gameplay. | ||
| */ | ||
| function createNationsForGame( |
There was a problem hiding this comment.
Can this function be moved to a different file? Maybe rename NationNames => NationUtils, and move this function there?
There was a problem hiding this comment.
Moved it into a NationUtils.ts 👍
src/core/GameRunner.ts
Outdated
| const shuffled = [...manifestNations]; | ||
| for (let i = shuffled.length - 1; i > 0; i--) { | ||
| const j = random.nextInt(0, i + 1); | ||
| const temp = shuffled[i]; | ||
| shuffled[i] = shuffled[j]; | ||
| shuffled[j] = temp; | ||
| } | ||
| return shuffled.slice(0, targetNationCount).map(toNation); |
There was a problem hiding this comment.
use random.shuffleArray instead
src/core/GameRunner.ts
Outdated
| numHumans: number, | ||
| random: PseudoRandom, | ||
| ): Nation[] { | ||
| // If nations are disabled, return empty array |
… NationNames utility
…on and readability
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/core/game/NationUtils.ts (1)
18-72: Excellent implementation of nation balancing for HVN mode.The logic correctly handles all scenarios:
- Disables nations when configured
- Balances nation count to match humans in HVN mode (1 for singleplayer, numHumans otherwise)
- Shuffles manifest nations for variety when selecting a subset
- Generates additional nations with dynamic spawn locations when needed
The use of
undefinedfor spawn cells on dynamically generated nations (line 65) is intentional and aligns with the PR objectives.Optional: Add input validation for robustness
Consider adding a validation check for
numHumansto prevent potential issues if a negative value is passed:export function createNationsForGame( gameStart: GameStartInfo, manifestNations: ManifestNation[], numHumans: number, random: PseudoRandom, ): Nation[] { + if (numHumans < 0) { + throw new Error(`numHumans must be non-negative, got ${numHumans}`); + } + if (gameStart.config.disableNations) { return []; }This is purely defensive programming and may not be necessary if upstream validation is already in place.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/core/GameRunner.tssrc/core/game/NationUtils.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/core/GameRunner.ts
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: Saphereye
Repo: openfrontio/OpenFrontIO PR: 2233
File: src/client/HostLobbyModal.ts:891-891
Timestamp: 2025-10-21T20:06:04.823Z
Learning: For the HumansVsNations game mode in `src/client/HostLobbyModal.ts` and related files, the implementation strategy is to generate all nations and adjust their strength for balancing, rather than limiting lobby size based on the number of available nations on the map.
Learnt from: Foorack
Repo: openfrontio/OpenFrontIO PR: 2141
File: src/client/ClientGameRunner.ts:228-234
Timestamp: 2025-10-08T17:14:49.369Z
Learning: For the window close confirmation feature in `ClientGameRunner.ts`, the troop count requirement (>10,000 troops) from issue #2137 was intentionally removed because it was arbitrary and troop count can be reported as low despite having significant land. The confirmation now triggers for any alive player regardless of troop count.
Learnt from: FloPinguin
Repo: openfrontio/OpenFrontIO PR: 2689
File: src/client/PublicLobby.ts:245-245
Timestamp: 2025-12-26T22:21:15.466Z
Learning: In public lobbies with HumansVsNations mode in src/client/PublicLobby.ts, maxPlayers represents only human player slots (already halved in DefaultConfig.ts). The nation NPCs are added automatically server-side and don't count toward maxPlayers. Therefore, getTeamSize correctly returns maxPlayers directly for HumansVsNations to display the proper team size (e.g., maxPlayers=5 yields "5 Humans vs 5 Nations").
📚 Learning: 2025-10-21T20:06:04.823Z
Learnt from: Saphereye
Repo: openfrontio/OpenFrontIO PR: 2233
File: src/client/HostLobbyModal.ts:891-891
Timestamp: 2025-10-21T20:06:04.823Z
Learning: For the HumansVsNations game mode in `src/client/HostLobbyModal.ts` and related files, the implementation strategy is to generate all nations and adjust their strength for balancing, rather than limiting lobby size based on the number of available nations on the map.
Applied to files:
src/core/game/NationUtils.ts
📚 Learning: 2025-12-26T22:21:15.466Z
Learnt from: FloPinguin
Repo: openfrontio/OpenFrontIO PR: 2689
File: src/client/PublicLobby.ts:245-245
Timestamp: 2025-12-26T22:21:15.466Z
Learning: In public lobbies with HumansVsNations mode in src/client/PublicLobby.ts, maxPlayers represents only human player slots (already halved in DefaultConfig.ts). The nation NPCs are added automatically server-side and don't count toward maxPlayers. Therefore, getTeamSize correctly returns maxPlayers directly for HumansVsNations to display the proper team size (e.g., maxPlayers=5 yields "5 Humans vs 5 Nations").
Applied to files:
src/core/game/NationUtils.ts
🔇 Additional comments (2)
src/core/game/NationUtils.ts (2)
74-284: Creative and well-structured name generation system.The template-based approach with symbol expansion (
NOUN,PLURAL_NOUN) is clean and maintainable. The variety of templates (196) and nouns (67) ensures diverse nation names across games.
289-306: Pluralization logic is correct for all current nouns.The implementation properly handles:
- Sibilants (s, ch, sh, x, z) → +es
- Consonant + y → ies (e.g., "Puppy" → "Puppies")
- Irregular o-endings (Potato, Tomato) → +es
- Default → +s
As noted in the past review comments, the earlier pluralization issues have been resolved.
github-project-automation
bot
moved this from Development
to Final Review
in OpenFront Release Management
github-project-automation
bot
moved this from Final Review
to Complete
in OpenFront Release Management
3 participants
Description:
HumansVsNations is back!
The original PR had an issue: only the nations listed in the map’s
manifest.jsonwere being spawned, which resulted in completely unbalanced games.What did I change with this PR?
NationNames.ts. The name generator was taken from the closed PR Fix profanity filter + new username generator #2245 (idea from @VariableVince).These changes apply to private lobbies and singleplayer as well. In singleplayer, you now simply play a 1-vs-1 against a nation.
For public lobbies, we use 50% of the regular team-game player count. The remaining 50% are nations.
We are also using the Hard difficulty for HumansVsNations.
At the moment, it’s important that nations cheat a little because humans can donate troops, whereas nations cannot, at least not yet. In the future, we may make that work.
We might need to adjust the difficulty or do fine-tuning depending on the humans’ win rate in production. Ideally, we want a ~50% win rate; otherwise, the mode may become boring. Over time, humans will likely develop strategies that nations can’t counter, in which case we’ll need to improve the nation AI.
Here is a screenshot showing that the number of nations now matches the number of humans in the private lobby UI:
The
PuplicLobbydisplay was a bit bugged for HumansVsNations:So I fixed it to look like this;
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
FloPinguin