Skip to content

Conversation

@FloPinguin
Copy link
Contributor

Description:

Added a public game modifier system. It causes that

5% of public games are played on the compact version of the map
10% of public games have "Random Spawn" activated

Percentages can easily get changed via DefaultConfig.
We can also easily add more modifiers.
Modifiers can stack, so in rare cases you will play on a compact map with random spawn 😄
More variety!

"Compact Map" modifier implementation

  • With the "Compact Map" modifier the lobby max player count gets reduced to 25% and only 25% of the regular bots and only 25% of the regular nations will spawn (because the map has only 25% of its regular size)
  • In private lobbies and singleplayer the nation reduction happens too (When "Compact Map" is enabled).

Restrictions

  • Duos/Trios/Quads team modes do not get Random Spawn (defeats the purpose)
  • Maps with smallest player count < 50 do not get Compact Map in team games (not enough players after the reduction to 25%). I have calculated all the possible max player counts.

How it looks like

Random Spawn modifier:

Screenshot 2026-01-06 194959

Very rare case: Two modifiers at the same time and only 10 max players have been chosen from [GameMapType.FaroeIslands]: [20, 15, 10]. Because of the 75% reduction in player count only 3 players are allowed (3 is the minimum). I think its funny that you can play a 1v1v1 in rare occasions 😄

Screenshot 2026-01-06 194938

Funny side-effect

Team games with random spawn. That will be interesting. No more "Who is better in donating troops to the frontline". Instead you have to heavily coordinate with your teammates.

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

@FloPinguin FloPinguin requested a review from a team as a code owner January 6, 2026 20:34
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 6, 2026

Warning

Rate limit exceeded

@FloPinguin has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 8 minutes and 5 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 9f97e2f and ee26e65.

📒 Files selected for processing (2)
  • src/core/configuration/DefaultConfig.ts
  • src/server/MapPlaylist.ts

Walkthrough

Adds public game modifiers (compact map, random spawn) across server, schema, nation-creation, and client layers: config can pick random modifiers, MapPlaylist propagates them into GameConfig, nation counts adjust for compact maps, schema persists modifiers, and lobby UIs display modifier badges.

Changes

Cohort / File(s) Summary
Core types & interface
src/core/game/Game.ts
Added PublicGameModifiers interface (isCompact, isRandomSpawn).
Schema
src/core/Schemas.ts
Added optional publicGameModifiers: z.object({ isCompact: z.boolean(), isRandomSpawn: z.boolean() }).optional() to GameConfigSchema.
Server config interface & default implementation
src/core/configuration/Config.ts, src/core/configuration/DefaultConfig.ts
Extended ServerConfig with getRandomPublicGameModifiers() and supportsCompactMapForTeams(map); added isCompactMap?: boolean param to lobbyMaxPlayers(...); default impl returns random modifiers and reduces capacity ~75% for compact maps.
Map playlist / game config generation
src/server/MapPlaylist.ts
Requests random modifiers from config, filters them by team/map rules, sets isCompact/isRandomSpawn, passes isCompact into lobbyMaxPlayers, sets compact map size/bot count, and includes publicGameModifiers in produced GameConfig.
Nation creation / sizing
src/core/game/NationCreation.ts, src/client/HostLobbyModal.ts
Exported getCompactMapNationCount(manifestNationCount, isCompactMap) and used it to compute effective nation counts (25% min 1) for compact maps; HostLobbyModal uses this helper when appropriate.
Client lobby UI
src/client/PublicLobby.ts
Added private getModifierLabels(publicGameModifiers) and renders translated modifier badges in the lobby header.
Localization
resources/lang/en.json
Added public_game_modifier group with random_spawn and compact_map entries.
Tests / Test config
tests/util/TestServerConfig.ts
Added stub implementations for getRandomPublicGameModifiers() and supportsCompactMapForTeams() to satisfy interface changes.

Sequence Diagram(s)

sequenceDiagram
  participant Client as Client (PublicLobby / HostLobbyModal)
  participant Server as MapPlaylist
  participant Config as ServerConfig / DefaultConfig
  participant Nation as NationCreation
  participant Schema as GameConfig Schema

  Note over Server,Config: Server asks config for random modifiers
  Server->>Config: getRandomPublicGameModifiers()
  Config-->>Server: { isCompact?, isRandomSpawn? }

  Server->>Server: filter modifiers (team rules, map support)
  alt Compact enabled
    Server->>Server: isCompact = true
    Server->>Server: pass isCompact to lobbyMaxPlayers(...)
    Server->>Server: set map size = Compact, bots = 100
  else Compact disabled
    Server->>Server: isCompact = false
  end

  Server->>Schema: persist GameConfig (includes publicGameModifiers)
  Schema-->>Server: persisted

  Server->>Client: send GameConfig (publicGameModifiers, isCompact)
  Client->>Client: getModifierLabels(publicGameModifiers)
  Client->>Nation: getCompactMapNationCount(manifestCount, isCompact)
  Nation-->>Client: effective nation count
  Client-->>User: render lobby with modifier badges and nation count
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

Feature, Gameplay, Feature - Frontend, Feature - Simulation, Translation

Suggested reviewers

  • evanpelle
  • scottanderson

Poem

🎲 Two small modifiers enter the fray,
Compact maps shrink nations, spawns may stray,
Servers choose chances, configs mark the tag,
Clients show badges, lobbies hum and wag,
A tiny tweak, a livelier play.

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: adding a public game modifier system. While the emoji and casual tone are present, the core message directly reflects the primary feature introduced across all modified files.
Description check ✅ Passed The description is comprehensive and directly related to the changeset. It explains the feature (5% compact maps, 10% random spawn), implementation details, restrictions, UI updates, localization, and testing—all matching the file changes shown in the summary.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/client/PublicLobby.ts (1)

308-323: Consider simplifying modifier label mapping.

The if-else chain works for two modifiers but won't scale well as more modifiers are added. A more maintainable approach would convert the enum value to the localization key dynamically:

🔎 Refactor for better maintainability
 private getModifierLabels(
   publicGameModifiers: PublicGameModifier[] | undefined,
 ): string[] {
   if (!publicGameModifiers || publicGameModifiers.length === 0) {
     return [];
   }
-  const labels: string[] = [];
-  for (const modifier of publicGameModifiers) {
-    if (modifier === PublicGameModifier.RandomSpawn) {
-      labels.push(translateText("public_game_modifier.random_spawn"));
-    } else if (modifier === PublicGameModifier.CompactMap) {
-      labels.push(translateText("public_game_modifier.compact_map"));
-    }
-  }
-  return labels;
+  return publicGameModifiers.map(modifier => {
+    // Convert "RandomSpawn" -> "random_spawn", "CompactMap" -> "compact_map"
+    const key = modifier.replace(/([A-Z])/g, '_$1').toLowerCase().substring(1);
+    return translateText(`public_game_modifier.${key}`);
+  });
 }

This eliminates the need to add new conditions when modifiers are added, as long as the enum value matches the localization key pattern.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 24716f8 and a94e3d2.

📒 Files selected for processing (9)
  • resources/lang/en.json
  • src/client/HostLobbyModal.ts
  • src/client/PublicLobby.ts
  • src/core/Schemas.ts
  • src/core/configuration/Config.ts
  • src/core/configuration/DefaultConfig.ts
  • src/core/game/Game.ts
  • src/core/game/NationCreation.ts
  • src/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.
📚 Learning: 2025-06-02T14:27:37.609Z
Learnt from: andrewNiziolek
Repo: openfrontio/OpenFrontIO PR: 1007
File: resources/lang/de.json:115-115
Timestamp: 2025-06-02T14:27:37.609Z
Learning: For OpenFrontIO project: When localization keys are renamed in language JSON files, the maintainers separate technical changes from translation content updates. They wait for community translators to update the actual translation values rather than attempting to translate in the same PR. This allows technical changes to proceed while ensuring accurate translations from native speakers.

Applied to files:

  • resources/lang/en.json
📚 Learning: 2025-08-16T10:52:08.292Z
Learnt from: TheGiraffe3
Repo: openfrontio/OpenFrontIO PR: 884
File: resources/lang/en.json:456-461
Timestamp: 2025-08-16T10:52:08.292Z
Learning: In OpenFrontIO, translation files in resources/lang/*.json (except en.json) should not be updated in regular PRs. Only dedicated translation PRs titled "mls" and made by Aotumori should update non-English locale files. Regular PRs should only update en.json when adding or modifying translation keys.

Applied to files:

  • resources/lang/en.json
📚 Learning: 2025-08-19T11:00:55.422Z
Learnt from: TheGiraffe3
Repo: openfrontio/OpenFrontIO PR: 1864
File: resources/maps/arabianpeninsula/manifest.json:13-170
Timestamp: 2025-08-19T11:00:55.422Z
Learning: In OpenFrontIO, nation names in map manifests are displayed directly in the UI without translation. They do not need to be added to resources/lang/en.json or processed through translateText(). This is the established pattern across all existing maps including Europe, World, Asia, Africa, and others.

Applied to files:

  • resources/lang/en.json
  • src/core/game/NationCreation.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/Schemas.ts
  • src/core/configuration/Config.ts
  • src/core/configuration/DefaultConfig.ts
  • src/server/MapPlaylist.ts
📚 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/Schemas.ts
  • src/core/configuration/Config.ts
  • src/core/game/NationCreation.ts
  • src/core/configuration/DefaultConfig.ts
  • src/server/MapPlaylist.ts
  • src/client/HostLobbyModal.ts
  • src/client/PublicLobby.ts
📚 Learning: 2025-12-26T22:21:21.904Z
Learnt from: FloPinguin
Repo: openfrontio/OpenFrontIO PR: 2689
File: src/client/PublicLobby.ts:245-245
Timestamp: 2025-12-26T22:21:21.904Z
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/configuration/Config.ts
  • src/core/game/NationCreation.ts
  • src/core/configuration/DefaultConfig.ts
  • src/server/MapPlaylist.ts
  • src/client/HostLobbyModal.ts
📚 Learning: 2025-11-12T23:11:34.445Z
Learnt from: MaxHT0x
Repo: openfrontio/OpenFrontIO PR: 2262
File: src/core/configuration/DefaultConfig.ts:806-832
Timestamp: 2025-11-12T23:11:34.445Z
Learning: In src/core/configuration/DefaultConfig.ts, JSDoc documentation for configuration methods should not be added inline, as it was requested that documentation be placed elsewhere in the codebase.

Applied to files:

  • src/core/configuration/Config.ts
  • src/core/configuration/DefaultConfig.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/Config.ts
  • src/core/game/NationCreation.ts
  • src/core/configuration/DefaultConfig.ts
  • src/server/MapPlaylist.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/Config.ts
  • src/core/configuration/DefaultConfig.ts
  • src/server/MapPlaylist.ts
📚 Learning: 2026-01-02T18:11:06.832Z
Learnt from: ryanbarlow97
Repo: openfrontio/OpenFrontIO PR: 2740
File: src/client/HostLobbyModal.ts:821-821
Timestamp: 2026-01-02T18:11:06.832Z
Learning: In src/client/HostLobbyModal.ts, the ?s=xxxxx URL suffix used in lobby URLs is purely for cache-busting platform previews (e.g., Discord, WhatsApp, x.com) and is not used by the join logic. The join flow ignores the suffix value, so regenerating it via updateUrlWithSuffix() on configuration changes will not break existing shared URLs; it only prompts platforms to refresh preview metadata. Treat the suffix as non-functional for join behavior and ensure any related changes preserve that invariant.

Applied to files:

  • src/client/HostLobbyModal.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
🧬 Code graph analysis (5)
src/core/configuration/Config.ts (1)
src/core/game/GameImpl.ts (1)
  • map (199-201)
src/core/game/NationCreation.ts (1)
src/core/game/Game.ts (1)
  • HumansVsNations (56-56)
src/server/MapPlaylist.ts (2)
src/core/configuration/DefaultConfig.ts (1)
  • isRandomSpawn (339-341)
src/core/game/Game.ts (4)
  • Duos (53-53)
  • Trios (54-54)
  • Quads (55-55)
  • HumansVsNations (56-56)
src/client/HostLobbyModal.ts (1)
src/core/game/NationCreation.ts (1)
  • getCompactMapNationCount (87-98)
src/client/PublicLobby.ts (2)
src/client/LangSelector.ts (1)
  • translateText (223-243)
src/client/Utils.ts (1)
  • translateText (92-147)
🪛 GitHub Actions: 🧪 CI
src/core/Schemas.ts

[warning] 1-1: Module "src/core/Schemas.ts" tried to import "resources/countries.json" with no attributes, but it was already imported elsewhere with "type": "json" attributes. Please ensure that import attributes for the same module are always consistent.

🔇 Additional comments (25)
src/client/HostLobbyModal.ts (2)

17-17: LGTM: Import added for compact map nation count calculation.

The import of getCompactMapNationCount correctly supports the new compact map modifier functionality.


951-961: LGTM: Compact map nation count logic properly implemented.

The method now correctly delegates to getCompactMapNationCount for non-HumansVsNations team modes, which will return 25% of nations when compact map is enabled. The comment clearly explains the behavior, and the HumansVsNations mode continues to use its own strategy (all nations with adjusted strength) as intended.

src/core/game/Game.ts (1)

189-192: LGTM: Public game modifier enum added.

The PublicGameModifier enum is cleanly defined with two modifiers (RandomSpawn, CompactMap). The string enum pattern is consistent with other enums in the codebase.

resources/lang/en.json (1)

365-368: LGTM: Localization entries added correctly.

The new public_game_modifier localization block provides English translations for the Random Spawn and Compact Map modifiers. Following the project's translation workflow, only en.json is updated in this PR.

src/client/PublicLobby.ts (1)

9-9: LGTM: Modifier labels integrated into lobby UI.

The import, label computation, and rendering logic correctly display public game modifiers in the lobby interface. The modifier labels appear alongside the mode label using consistent styling.

Also applies to: 117-119, 168-176

src/core/Schemas.ts (1)

18-18: No action required. The code uses the correct Zod 4 syntax: z.enum(PublicGameModifier) is the proper way to validate TypeScript enums in Zod v4. The deprecated z.nativeEnum() has been replaced by the overloaded z.enum() function in Zod v4.

src/core/configuration/Config.ts (3)

10-10: LGTM!

The PublicGameModifier import is correctly added to support the new interface methods.


34-39: LGTM!

The optional isCompactMap parameter is well-designed and maintains backward compatibility with existing callers.


62-63: LGTM!

The new methods are well-designed:

  • getRandomPublicGameModifiers() returns an array supporting zero, one, or multiple stacked modifiers as documented in the PR objectives.
  • supportsCompactMapForTeams() provides clear validation logic for compact map eligibility.
src/core/game/NationCreation.ts (5)

5-5: LGTM!

The GameMapSize import is correctly added to support compact map detection.


18-18: LGTM!

The updated comment clearly documents the compact map behavior (25% of nations).


36-48: LGTM!

The compact map nation reduction logic is well-implemented:

  • Correctly excludes HumansVsNations mode (which uses its own balancing strategy).
  • Shuffles nations for variety before selecting the 25% subset.
  • Uses the helper function for consistent calculation.

Based on learnings, HumansVsNations mode generates all nations and adjusts their strength for balancing, so excluding it from compact map reduction is correct.


51-52: LGTM!

Correctly returns effectiveNations which contains either the full set or the compact map reduced set based on the game configuration.


86-98: LGTM!

The getCompactMapNationCount helper is well-designed:

  • Handles the edge case of zero nations gracefully.
  • Correctly implements 25% reduction with a minimum of 1 nation for compact maps.
  • Exported for reuse in other modules (e.g., HostLobbyModal as noted in the AI summary).
src/server/MapPlaylist.ts (7)

11-11: LGTM!

The PublicGameModifier import is correctly added to support the modifier system.


96-119: LGTM!

The modifier selection and filtering logic is well-implemented:

  • Correctly excludes RandomSpawn from Duos/Trios/Quads team modes (as documented in PR objectives).
  • Correctly excludes CompactMap from team games on small maps using the supportsCompactMapForTeams validation.
  • Derives boolean flags for clear usage downstream.

The implementation matches the PR requirements precisely.


126-126: LGTM!

The lobbyMaxPlayers call correctly passes the isCompactMap flag to apply the 75% player reduction when compact map is active.


128-128: LGTM!

The gameMapSize is correctly set based on the compact map modifier, which is then used by NationCreation.ts to apply nation count reduction.


129-130: LGTM!

The conditional publicGameModifiers field cleanly handles the case where no modifiers are active by using undefined instead of an empty array.


137-137: LGTM!

The randomSpawn field now correctly reflects whether the RandomSpawn modifier is active, replacing the previous static configuration.


141-141: LGTM!

The bot count adjustment correctly implements the 25% reduction for compact maps (100 bots vs 400 regular), matching the PR objectives.

src/core/configuration/DefaultConfig.ts (4)

15-15: LGTM!

The PublicGameModifier import is correctly added to support the modifier implementation.


175-209: LGTM!

The lobbyMaxPlayers implementation correctly applies the 75% reduction for compact maps:

  • Multiplies by 0.25 to get 25% of the original player count.
  • Enforces a minimum of 3 players to ensure viable games.
  • Applied after base calculation but before team alignment adjustments.

This matches the PR objective: "lobby max player count is reduced to 25%".


227-238: LGTM!

The getRandomPublicGameModifiers implementation correctly implements the modifier probabilities:

  • 10% chance for RandomSpawn (matching PR objectives).
  • 5% chance for CompactMap (matching PR objectives).
  • Independent selection allows stacking with ~0.5% probability, supporting the "rare stacked-modifier case" mentioned in the PR description.

240-244: LGTM!

The supportsCompactMapForTeams implementation correctly enforces the minimum map size for compact team games:

  • Uses the smallest player count configuration (third element of the tuple).
  • Applies the threshold of 50 players as documented in the PR objectives.
  • Prevents compact map application on team games where the reduced player count would be too small.

The threshold ensures that even after 75% reduction, team games remain viable.

coderabbitai[bot]
coderabbitai bot previously approved these changes Jan 6, 2026
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/client/PublicLobby.ts (1)

308-322: Method works correctly for current modifiers.

The PascalCase to snake_case conversion logic handles the current modifiers (RandomSpawn, CompactMap) correctly and gracefully falls back to returning the key if a translation is missing.

The regex conversion assumes simple PascalCase without consecutive capitals (e.g., XMLParserx_m_l_parser). Consider adding a comment documenting this assumption, or using an explicit mapping if more complex modifier names are added in the future.

📝 Optional: Add a comment documenting the naming assumption
  private getModifierLabels(
    publicGameModifiers: PublicGameModifier[] | undefined,
  ): string[] {
    if (!publicGameModifiers || publicGameModifiers.length === 0) {
      return [];
    }
    return publicGameModifiers.map((modifier) => {
-     // Convert PascalCase to snake_case for localization key
+     // Convert simple PascalCase to snake_case for localization key
+     // Assumes enum names like "RandomSpawn" or "CompactMap" without consecutive capitals
      const key = modifier
        .replace(/([A-Z])/g, "_$1")
        .toLowerCase()
        .slice(1);
      return translateText(`public_game_modifier.${key}`);
    });
  }
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a94e3d2 and 48249ff.

📒 Files selected for processing (1)
  • src/client/PublicLobby.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.
📚 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-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/client/PublicLobby.ts
🧬 Code graph analysis (1)
src/client/PublicLobby.ts (2)
src/client/LangSelector.ts (1)
  • translateText (223-243)
src/client/Utils.ts (1)
  • translateText (92-147)
🔇 Additional comments (2)
src/client/PublicLobby.ts (2)

9-9: LGTM!

The import is necessary for the new modifier display functionality.


117-119: Clean integration with existing UI patterns.

The modifier labels are computed and rendered consistently with the existing mode label styling. The .map() approach handles empty arrays gracefully.

Also applies to: 168-176

coderabbitai[bot]
coderabbitai bot previously approved these changes Jan 6, 2026
coderabbitai[bot]
coderabbitai bot previously approved these changes Jan 6, 2026
@VariableVince
Copy link
Contributor

VariableVince commented Jan 6, 2026

  1. How would the buttons fit, UI-wise, with longer non-English translations? Three buttons fit in English but probably not as nicely in other languages, especially with long map names.

  2. As @scamiv said in the Discord, browser fingerprint is accurate for detecting mobile users vs checking user agent. So I'm still thinking Compact Map might be something we only offer to mobile/tablet players (for performance, screen size and disadvantage against desktop players with hotkeys and scripts etc).

Mobile players could then also have their own stats, ranks and leaderboards in the future. While having an opt-in setting in which they can choose to compete against desktop players.

When we would start offering Compact Map in public lobbies to all players independent of their platform, we take away the exclusivity that Compact Map can offer. In other words it can be harder to offer it exclusively to mobile players in the future if desktop users get used to it being offered to them too.

So I'm a bit on the fence on offerting Compact Map just for fun and variety. With possibility of making it a mobile mode in the future. And knowing we have other nice game options to offer for variance too.

coderabbitai[bot]
coderabbitai bot previously approved these changes Jan 6, 2026
@FloPinguin
Copy link
Contributor Author

  1. How would the buttons fit, UI-wise, with longer non-English translations? Three buttons fit in English but probably not as nicely in other languages, especially with long map names.
Screenshot 2026-01-06 223431

Added a little change so it also looks ok in the edge case of very long and multiple labels

  1. As @scamiv said in the Discord, browser fingerprint is accurate for detecting mobile users vs checking user agent. So I'm still thinking Compact Map might be something we only offer to mobile/tablet players (for performance, screen size and disadvantage against desktop players with hotkeys and scripts etc).

Ah I didn't notice in the discord that you want "Compact Map" 100% exclusive. 🤔
I don't know... With 5% chance it's already quite rare and we could turn it down even more so that its like an easteregg 😄

@VariableVince
Copy link
Contributor

VariableVince commented Jan 6, 2026

  1. How would the buttons fit, UI-wise, with longer non-English translations? Three buttons fit in English but probably not as nicely in other languages, especially with long map names.
Screenshot 2026-01-06 223431 Added a little change so it also looks ok in the edge case of very long and multiple labels
  1. As @scamiv said in the Discord, browser fingerprint is accurate for detecting mobile users vs checking user agent. So I'm still thinking Compact Map might be something we only offer to mobile/tablet players (for performance, screen size and disadvantage against desktop players with hotkeys and scripts etc).

Ah I didn't notice in the discord that you want "Compact Map" 100% exclusive. 🤔 I don't know... With 5% chance it's already quite rare and we could turn it down even more so that its like an easteregg 😄

Thanks!

Yeah Compact Map would still be offered for Private lobbies but for Public I'm pro leaving it exclusive for future use for mobile players for the mentioned reasons (if performance becomes less of an issue, there's still screen size and not having hotkeys etc). Even with only 5% chance of Compact Map offered to desktop and mobile players alike, I'm not sure if it's the wisest decision. Just my 2c and Evan might think differently.

In place of Compact Map we could use other options like the pre-set Game Length or PVP Immunity? Or Starting Gold as in (closed stale) PR #2461? Because the idea of adding more variety like this is great of course

@FloPinguin
Copy link
Contributor Author

FloPinguin commented Jan 6, 2026

Thanks!

Yeah Compact Map would still be offered for Private lobbies but for Public I'm pro leaving it exclusive for future use for mobile players for the mentioned reasons (if performance becomes less of an issue, there's still screen size and not having hotkeys etc). Even with only 5% chance of Compact Map offered to desktop and mobile players alike, I'm not sure if it's the wisest decision. Just my 2c and Evan might think differently.

In place of Compact Map we could use other options like the pre-set Game Length or PVP Immunity? Or Starting Gold as in (closed stale) PR #2461? Because the idea of adding more variety like this is great of course

I personally would have lots of fun playing public games on tiny maps on desktop (reduces game length by a lot)
Maybe we should let evan decide 😄

I also thought about pre-set Game Length and PVP Immunity 🤔 maybe even disable some buildings or MIRVs 😄
But its better to put it in another PR I think

);
}

// Small maps (3rd player count < 50) don't get compact map in team games
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does 3rd player count mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the comment to say:
// Maps with smallest player count < 50 don't support compact map in team games
// The smallest player count is the 3rd number in numPlayersConfig

return false;
}

getRandomPublicGameModifiers(): PublicGameModifier[] {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of a list i think just do:

getRandomPublicGameModifiers(): {isCompact: boolean; isRandomSpawn: boolean}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, turned it into an interface

# Conflicts:
#	src/client/PublicLobby.ts
coderabbitai[bot]
coderabbitai bot previously approved these changes Jan 6, 2026
coderabbitai[bot]
coderabbitai bot previously approved these changes Jan 7, 2026
@evanpelle evanpelle added this to the v29 milestone Jan 7, 2026
@evanpelle evanpelle merged commit ebcb654 into openfrontio:main Jan 7, 2026
7 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants