-
Notifications
You must be signed in to change notification settings - Fork 777
Remove hardcoded numPlayersConfig, calculate it based on the maps land tiles 🔧 #2874
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughPlayer-count and map-eligibility logic was moved from ServerConfig/DefaultConfig into MapPlaylist and made asynchronous via map land-tile lookups. gameConfig() now returns a Promise; callers were updated. MapDescription and related per-map constants were removed; a new MapLandTiles utility was added. Changes
Sequence Diagram(s)sequenceDiagram
participant Worker as Worker/Master
participant Playlist as MapPlaylist
participant LandTiles as MapLandTiles
participant Loader as FetchGameMapLoader
Worker->>Playlist: await gameConfig()
Playlist->>Playlist: lobbyMaxPlayers(map, mode, teams, isCompact)
Playlist->>LandTiles: await getMapLandTiles(map)
LandTiles->>Loader: getMapLoader() / load(map)
Loader-->>LandTiles: Promise<manifest>
LandTiles->>Loader: await manifest()
Loader-->>LandTiles: manifest (num_land_tiles)
LandTiles-->>Playlist: land tile count
Playlist->>Playlist: calculateMapPlayerCounts(landTiles) / apply team adjustments
Playlist-->>Worker: Promise<GameConfig> (resolved)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @src/core/configuration/DefaultConfig.ts:
- Around line 1-3: DefaultConfig currently imports getMapLandTiles from server
code, creating a hard core->server dependency; remove the direct import and
instead accept a map-tile provider as an injected dependency (e.g., a
fetchMapTiles or getMapLandTiles function parameter on the DefaultConfig
constructor or init function) or lazily load it only when needed; update call
sites to pass the server implementation (or swap in a core implementation if you
decide to move the logic into src/core/), and ensure DefaultConfig has a safe
no-op/default provider for environments without a server (tests/clients) so core
remains independent.
In @src/server/MapLandTiles.ts:
- Around line 13-14: The top-of-file comment incorrectly refers to
BinaryLoaderGameMapLoader; update the comment to mention the actual loader in
use, FetchGameMapLoader, so it reads something like "FetchGameMapLoader already
caches maps, so no need for additional caching here." Locate the header comment
in MapLandTiles (around the function/class handling land tile counts) and
replace the wrong class name with FetchGameMapLoader to keep documentation
accurate.
- Around line 7-11: getMapLoader currently hardcodes
"http://localhost:3000/maps"; change it to read the base URL from an environment
variable (e.g. process.env.GAME_MAPS_URL or MAPS_SERVICE_URL) and pass that into
the FetchGameMapLoader constructor (fall back to the existing localhost value
only for local dev if desired). Update the getMapLoader function so it uses the
env var when creating new FetchGameMapLoader and keep the mapLoader caching
logic intact.
In @tests/util/TestServerConfig.ts:
- Around line 52-54: The lobbyMaxPlayers stub's signature doesn't match the
ServerConfig interface; update the async method lobbyMaxPlayers(map:
GameMapType) to accept the full parameter list async lobbyMaxPlayers(map:
GameMapType, mode: GameMode, numPlayerTeams: TeamCountConfig, isCompactMap:
boolean): Promise<number>, add the missing imports for GameMode and
TeamCountConfig, and either implement the logic or keep the body throwing an
Error("Method not implemented.") so the type matches the interface.
🧹 Nitpick comments (1)
src/server/MapLandTiles.ts (1)
21-24: Silent fallback may mask production issues.Returning
1_000_000on error hides failures. The fallback is reasonable for resilience, but logging alone may not surface issues quickly in production.Consider whether this should throw in certain environments (e.g., during startup/preload) or emit a metric/alert when the fallback is used.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/core/configuration/Config.tssrc/core/configuration/DefaultConfig.tssrc/server/MapLandTiles.tssrc/server/MapPlaylist.tssrc/server/Master.tssrc/server/Worker.tstests/util/TestServerConfig.ts
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
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-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.tssrc/server/MapPlaylist.tstests/util/TestServerConfig.tssrc/core/configuration/DefaultConfig.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/configuration/Config.tssrc/server/Master.tstests/util/TestServerConfig.tssrc/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.tssrc/server/MapPlaylist.tstests/util/TestServerConfig.tssrc/core/configuration/DefaultConfig.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.tssrc/core/configuration/DefaultConfig.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/server/MapPlaylist.tstests/util/TestServerConfig.tssrc/core/configuration/DefaultConfig.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/DefaultConfig.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.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.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/configuration/DefaultConfig.ts
🧬 Code graph analysis (3)
src/server/MapLandTiles.ts (3)
src/core/game/GameMapLoader.ts (1)
GameMapLoader(4-6)src/core/game/FetchGameMapLoader.ts (1)
FetchGameMapLoader(4-73)src/core/game/GameImpl.ts (1)
map(206-208)
src/server/MapPlaylist.ts (1)
src/core/Schemas.ts (1)
GameConfig(90-90)
src/core/configuration/DefaultConfig.ts (1)
src/server/MapLandTiles.ts (1)
getMapLandTiles(15-25)
🔇 Additional comments (10)
src/server/Master.ts (1)
310-310: LGTM!The
awaitis correct here sincegameConfig()now returns aPromise<GameConfig>. The surroundingtry/catchblock handles any rejection properly.src/server/Worker.ts (1)
499-501: LGTM!The
awaitis properly placed, and mutating the resolvedgameConfigobject before passing tocreateGameworks as expected.src/core/configuration/Config.ts (2)
34-39: Interface signature update looks good.The return type
Promise<number>makes sense for dynamic calculations based on map data.
62-62: LGTM!Return type
Promise<boolean>aligns with the async pattern for map-based computations.src/server/MapPlaylist.ts (3)
91-91: LGTM!Method signature correctly updated to async, returning
Promise<GameConfig>.
110-115: Await in conditional is correct.Using
awaitinside theifcondition works fine here. The logic remains clear: disable compact maps for team games when the map does not support it.
122-127: LGTM!The
awaitforlobbyMaxPlayersis properly placed, and all required arguments are passed through.tests/util/TestServerConfig.ts (1)
85-87: LGTM!The async signature matches the updated
ServerConfiginterface forsupportsCompactMapForTeams.src/core/configuration/DefaultConfig.ts (2)
131-166: Async conversion looks correct.The method now awaits
getMapLandTilesand usescalculateMapPlayerCountsfor dynamic player counts. The logic for team modes, compact maps, and random tier selection is preserved.
209-215: Async conversion forsupportsCompactMapForTeamsis consistent.The method correctly awaits
getMapLandTilesand uses the third element (smallest count) fromcalculateMapPlayerCountsto determine support. The threshold of 50 is reasonable.
Duwibi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice change. This part of mapmaking finally seems balanced!
There was a problem hiding this 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 (2)
src/server/MapLandTiles.ts (2)
15-25: Consider adding type safety for manifest access.The
manifest.map.num_land_tilespath assumes a specific shape fromresponse.json()(which returnsany). If the manifest structure ever changes or the property is missing, this silently falls back to 1M players worth of tiles.For robustness, validate the manifest shape or add a typed interface.
♻️ Suggested type guard
+interface MapManifest { + map: { + num_land_tiles: number; + }; +} + +function isValidManifest(obj: unknown): obj is MapManifest { + return ( + typeof obj === "object" && + obj !== null && + "map" in obj && + typeof (obj as MapManifest).map?.num_land_tiles === "number" + ); +} + export async function getMapLandTiles(map: GameMapType): Promise<number> { try { const loader = getMapLoader(); const mapData = loader.getMapData(map); const manifest = await mapData.manifest(); + if (!isValidManifest(manifest)) { + console.warn(`Invalid manifest structure for ${map}, using fallback`); + return 1_000_000; + } return manifest.map.num_land_tiles; } catch (error) { console.error(`Failed to load manifest for ${map}:`, error); return 1_000_000; // Default fallback } }
21-24: Consider tracking repeated fallback usage.The fallback silently returns 1M tiles when manifest loading fails. In production, repeated failures could indicate network issues or misconfiguration that might go unnoticed with only
console.error.If the project has metrics or structured logging, consider incrementing a counter on fallback so operations can alert on elevated failure rates.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
map-generator/README.mdsrc/client/components/Maps.tssrc/core/configuration/DefaultConfig.tssrc/server/MapLandTiles.tstests/util/TestServerConfig.ts
💤 Files with no reviewable changes (1)
- src/client/components/Maps.ts
✅ Files skipped from review due to trivial changes (1)
- map-generator/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- src/core/configuration/DefaultConfig.ts
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
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-07-12T06:35:10.457Z
Learnt from: Aotumuri
Repo: openfrontio/OpenFrontIO PR: 1357
File: resources/lang/ja.json:0-0
Timestamp: 2025-07-12T06:35:10.457Z
Learning: In OpenFrontIO project, "giantworldmap" is the correct localization key name for the giant world map, used consistently across all language files and TypeScript code. Do not suggest renaming this key.
Applied to files:
src/server/MapLandTiles.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/server/MapLandTiles.ts
📚 Learning: 2026-01-12T21:37:01.156Z
Learnt from: FloPinguin
Repo: openfrontio/OpenFrontIO PR: 2874
File: src/server/MapLandTiles.ts:7-11
Timestamp: 2026-01-12T21:37:01.156Z
Learning: In this repository's OpenFrontIO deployment, inter-service HTTP calls to the master should target http://localhost:3000 (master at port 3000) as the canonical address. Apply this as the standard for all server-side TypeScript code that communicates with the master. Avoid hardcoding non-master URLs; centralize the master address (e.g., via config or env) when possible, and ensure internal service communication uses localhost:3000 in this architecture.
Applied to files:
src/server/MapLandTiles.tstests/util/TestServerConfig.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:
tests/util/TestServerConfig.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:
tests/util/TestServerConfig.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:
tests/util/TestServerConfig.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:
tests/util/TestServerConfig.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:
tests/util/TestServerConfig.ts
🧬 Code graph analysis (1)
src/server/MapLandTiles.ts (3)
src/core/game/GameMapLoader.ts (1)
GameMapLoader(4-6)src/core/game/FetchGameMapLoader.ts (1)
FetchGameMapLoader(4-73)src/core/game/GameImpl.ts (1)
map(206-208)
🔇 Additional comments (2)
tests/util/TestServerConfig.ts (1)
52-54: LGTM!The async signatures and removal of the
mapparameter match the updatedServerConfiginterface. Stub implementations throw as expected for test doubles.Also applies to: 85-87
src/server/MapLandTiles.ts (1)
5-11: LGTM on singleton loader setup.Lazy initialization with
??=is clean. Per learnings,localhost:3000is the correct internal master address for this deployment.
There was a problem hiding this 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 (2)
src/core/configuration/MapLandTiles.ts (1)
15-24: Add type guard for manifest data to avoid silent failures.The manifest is fetched as untyped JSON. If
manifest.mapornum_land_tilesis missing or malformed, you could getundefinedorNaNflowing into calculations downstream.Consider adding a simple check:
♻️ Suggested improvement
export async function getMapLandTiles(map: GameMapType): Promise<number> { try { const loader = getMapLoader(); const mapData = loader.getMapData(map); const manifest = await mapData.manifest(); - return manifest.map.num_land_tiles; + const tiles = manifest?.map?.num_land_tiles; + if (typeof tiles !== "number" || !Number.isFinite(tiles)) { + console.warn(`Invalid num_land_tiles for ${map}, using fallback`); + return 1_000_000; + } + return tiles; } catch (error) { console.error(`Failed to load manifest for ${map}:`, error); return 1_000_000; // Default fallback } }src/core/configuration/DefaultConfig.ts (1)
209-215: Consider caching or combining calls to reduce duplicate fetches.Both
lobbyMaxPlayersandsupportsCompactMapForTeamscallgetMapLandTiles(map)independently. If both methods are called for the same map in sequence (e.g., during lobby setup), this results in two async lookups.
FetchGameMapLoadercaches map data internally, so this is not a correctness issue, but you could extract a shared helper or have callers pass pre-fetched tile counts if performance matters.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/core/configuration/DefaultConfig.tssrc/core/configuration/MapLandTiles.ts
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
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-07-12T06:35:10.457Z
Learnt from: Aotumuri
Repo: openfrontio/OpenFrontIO PR: 1357
File: resources/lang/ja.json:0-0
Timestamp: 2025-07-12T06:35:10.457Z
Learning: In OpenFrontIO project, "giantworldmap" is the correct localization key name for the giant world map, used consistently across all language files and TypeScript code. Do not suggest renaming this key.
Applied to files:
src/core/configuration/MapLandTiles.ts
📚 Learning: 2026-01-12T21:37:01.156Z
Learnt from: FloPinguin
Repo: openfrontio/OpenFrontIO PR: 2874
File: src/server/MapLandTiles.ts:7-11
Timestamp: 2026-01-12T21:37:01.156Z
Learning: In this repository's OpenFrontIO deployment, inter-service HTTP calls to the master should target http://localhost:3000 (master at port 3000) as the canonical address. Apply this as the standard for all server-side TypeScript code that communicates with the master. Avoid hardcoding non-master URLs; centralize the master address (e.g., via config or env) when possible, and ensure internal service communication uses localhost:3000 in this architecture.
Applied to files:
src/core/configuration/MapLandTiles.tssrc/core/configuration/DefaultConfig.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/MapLandTiles.tssrc/core/configuration/DefaultConfig.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/DefaultConfig.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/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/DefaultConfig.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/configuration/DefaultConfig.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/DefaultConfig.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/configuration/DefaultConfig.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.ts
📚 Learning: 2025-12-13T14:58:29.645Z
Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 2607
File: src/core/execution/PlayerExecution.ts:271-295
Timestamp: 2025-12-13T14:58:29.645Z
Learning: In src/core/execution/PlayerExecution.ts surroundedBySamePlayer(), the `as Player` cast on `mg.playerBySmallID(scan.enemyId)` is intentional. Since scan.enemyId comes from ownerID() on an owned tile and playerBySmallID() only returns Player or undefined, the cast expresses a known invariant. The maintainers prefer loud failures (runtime errors) over silent masking (early returns with guards) for corrupted game state scenarios at trusted call sites.
Applied to files:
src/core/configuration/DefaultConfig.ts
📚 Learning: 2025-06-22T05:48:19.241Z
Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 786
File: src/client/TerritoryPatternsModal.ts:337-338
Timestamp: 2025-06-22T05:48:19.241Z
Learning: In src/client/TerritoryPatternsModal.ts, the bit shifting operators (<<) used in coordinate calculations with decoder.getScale() are intentional and should not be changed to multiplication. The user scottanderson confirmed this is functioning as intended.
Applied to files:
src/core/configuration/DefaultConfig.ts
📚 Learning: 2025-05-31T18:15:03.445Z
Learnt from: 1brucben
Repo: openfrontio/OpenFrontIO PR: 977
File: src/core/execution/AttackExecution.ts:123-125
Timestamp: 2025-05-31T18:15:03.445Z
Learning: The removeTroops function in PlayerImpl.ts already prevents negative troop counts by using minInt(this._troops, toInt(troops)) to ensure it never removes more troops than available.
Applied to files:
src/core/configuration/DefaultConfig.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.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/configuration/DefaultConfig.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.ts
🧬 Code graph analysis (2)
src/core/configuration/MapLandTiles.ts (2)
src/core/game/GameMapLoader.ts (1)
GameMapLoader(4-6)src/core/game/FetchGameMapLoader.ts (1)
FetchGameMapLoader(4-73)
src/core/configuration/DefaultConfig.ts (1)
src/core/configuration/MapLandTiles.ts (1)
getMapLandTiles(15-25)
🔇 Additional comments (3)
src/core/configuration/MapLandTiles.ts (1)
7-11: LGTM on loader singleton and localhost URL.The singleton pattern avoids repeated instantiation. Based on learnings,
http://localhost:3000/mapsis the correct canonical address for inter-service calls to the master in this architecture.src/core/configuration/DefaultConfig.ts (2)
131-166: LGTM on async lobbyMaxPlayers implementation.The logic is clear:
- Fetch land tiles once, calculate tiers, apply mode/team adjustments.
- The random distribution (30%/30%/40%) adds variety to lobby sizes.
- Team mode 1.5x multiplier and compact map 25% reduction are well-bounded.
175-187: Consider edge case for very small maps.When
limitedBaseis 5 (the minimum):
- 75% of 5 →
roundToNearest5(3.75)= 5- 50% of 5 →
roundToNearest5(2.5)= 5All three tiers become identical (5). This is probably fine since tiny maps should have uniform small player counts, but worth confirming this is the intended behavior.
| return 60 * 1000; | ||
| } | ||
|
|
||
| lobbyMaxPlayers( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we move this logic from Config to MapPlaylist? Since i think that's the only place it's used anyways
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok did that 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be moved to server/ because that's the only place it's used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did that 👍
Previously it was a problem because in DefaultConfig we would have imported server stuff, while DefaultConfig is core.
There was a problem hiding this 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 (4)
src/server/MapLandTiles.ts (2)
15-24: Consider caching the manifest result to avoid repeated fetches.
FetchGameMapLoader.getMapData()caches theMapDataobject, but callingmanifest()creates a new HTTP request each time. InMapPlaylist.ts,getMapLandTilesis called twice pergameConfig()invocation (once insupportsCompactMapForTeams, once inlobbyMaxPlayers), resulting in duplicate network calls for the same map.♻️ Proposed fix: add manifest result caching
+const manifestCache = new Map<GameMapType, number>(); + export async function getMapLandTiles(map: GameMapType): Promise<number> { + const cached = manifestCache.get(map); + if (cached !== undefined) { + return cached; + } + try { const loader = getMapLoader(); const mapData = loader.getMapData(map); const manifest = await mapData.manifest(); - return manifest.map.num_land_tiles; + const landTiles = manifest.map.num_land_tiles; + manifestCache.set(map, landTiles); + return landTiles; } catch (error) { console.error(`Failed to load manifest for ${map}:`, error); return 1_000_000; // Default fallback } }
21-23: Fallback value silently hides failures.Returning 1,000,000 on error produces ~50 players (reasonable), but
console.errormay get lost in logs. Consider using the structuredloggerfrom./Loggerfor consistency with the rest of the server code.♻️ Proposed fix
+import { logger } from "./Logger"; + +const log = logger.child({}); + // ... } catch (error) { - console.error(`Failed to load manifest for ${map}:`, error); + log.error({ error, map }, `Failed to load manifest for map`); return 1_000_000; // Default fallback }src/server/MapPlaylist.ts (2)
169-175: Duplicate async calls to getMapLandTiles for the same map.Both
supportsCompactMapForTeams(line 172) andlobbyMaxPlayers(line 183) fetch land tiles independently. If manifest caching isn't added inMapLandTiles.ts, consider passing the land tiles as a parameter:♻️ Alternative: pass landTiles to avoid duplicate fetches
- private async supportsCompactMapForTeams(map: GameMapType): Promise<boolean> { - const landTiles = await getMapLandTiles(map); + private supportsCompactMapForTeams(landTiles: number): boolean { const [, , smallest] = this.calculateMapPlayerCounts(landTiles); return smallest >= 50; } private async lobbyMaxPlayers( map: GameMapType, mode: GameMode, numPlayerTeams: TeamCountConfig | undefined, isCompactMap?: boolean, ): Promise<number> { const landTiles = await getMapLandTiles(map); + // Pass landTiles to supportsCompactMapForTeams if needed upstream // ... }Then in
gameConfig():+ const landTiles = await getMapLandTiles(map); + if ( mode === GameMode.Team && - !(await this.supportsCompactMapForTeams(map)) + !this.supportsCompactMapForTeams(landTiles) ) { isCompact = false; }Also applies to: 177-184
226-227: Consider extracting magic numbers as named constants.Values like
1_000_000,50,125, and5define the player count formula. Naming them improves readability:♻️ Proposed constants
const TILES_PER_PLAYER_UNIT = 1_000_000; const PLAYERS_PER_UNIT = 50; const MAX_PLAYERS = 125; const MIN_PLAYERS = 5; // In calculateMapPlayerCounts: const base = roundToNearest5((landTiles / TILES_PER_PLAYER_UNIT) * PLAYERS_PER_UNIT); const limitedBase = Math.min(Math.max(base, MIN_PLAYERS), MAX_PLAYERS);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/core/configuration/Config.tssrc/core/configuration/DefaultConfig.tssrc/server/MapLandTiles.tssrc/server/MapPlaylist.ts
💤 Files with no reviewable changes (2)
- src/core/configuration/Config.ts
- src/core/configuration/DefaultConfig.ts
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
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: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").
📚 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/server/MapLandTiles.ts
📚 Learning: 2025-07-12T06:35:10.457Z
Learnt from: Aotumuri
Repo: openfrontio/OpenFrontIO PR: 1357
File: resources/lang/ja.json:0-0
Timestamp: 2025-07-12T06:35:10.457Z
Learning: In OpenFrontIO project, "giantworldmap" is the correct localization key name for the giant world map, used consistently across all language files and TypeScript code. Do not suggest renaming this key.
Applied to files:
src/server/MapLandTiles.ts
📚 Learning: 2026-01-12T21:37:01.156Z
Learnt from: FloPinguin
Repo: openfrontio/OpenFrontIO PR: 2874
File: src/server/MapLandTiles.ts:7-11
Timestamp: 2026-01-12T21:37:01.156Z
Learning: In this repository's OpenFrontIO deployment, inter-service HTTP calls to the master should target http://localhost:3000 (master at port 3000) as the canonical address. Apply this as the standard for all server-side TypeScript code that communicates with the master. Avoid hardcoding non-master URLs; centralize the master address (e.g., via config or env) when possible, and ensure internal service communication uses localhost:3000 in this architecture.
Applied to files:
src/server/MapLandTiles.tssrc/server/MapPlaylist.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/server/MapLandTiles.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/server/MapPlaylist.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/server/MapPlaylist.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/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/server/MapPlaylist.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/server/MapPlaylist.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/server/MapPlaylist.ts
🧬 Code graph analysis (2)
src/server/MapLandTiles.ts (2)
src/core/game/GameMapLoader.ts (1)
GameMapLoader(4-6)src/core/game/FetchGameMapLoader.ts (1)
FetchGameMapLoader(4-73)
src/server/MapPlaylist.ts (4)
src/core/Schemas.ts (2)
GameConfig(90-90)TeamCountConfig(164-164)src/core/configuration/DefaultConfig.ts (1)
playerTeams(212-214)src/core/game/Game.ts (5)
PublicGameModifiers(206-209)Duos(55-55)Trios(56-56)Quads(57-57)HumansVsNations(58-58)src/server/MapLandTiles.ts (1)
getMapLandTiles(15-25)
🔇 Additional comments (7)
src/server/MapLandTiles.ts (1)
1-11: Singleton pattern and imports look good.Clean setup using nullish coalescing assignment for lazy initialization. The hardcoded
http://localhost:3000/mapsaligns with the project's architecture for inter-service calls to master. Based on learnings, this is the canonical address.src/server/MapPlaylist.ts (6)
90-141: Async gameConfig looks well-structured.The flow is clear: get map, determine team count, apply modifiers, check compact support, then build config. Good use of
satisfies GameConfigfor type safety.
107-114: Verify compact map logic with the awaited check.The comment on lines 107-108 explains the intent well. The async check
supportsCompactMapForTeams(map)correctly guards compact mode for team games.
158-167: Simple helpers, clean implementation.
getTeamCountandgetRandomPublicGameModifiersare straightforward. The 10%/5% probabilities for random spawn and compact mode are reasonable.
185-191: Random tier selection and compact reduction look correct.The weighted distribution (30% large, 30% medium, 40% small) and 75% reduction for compact maps (keeping 25%) are clear.
Math.max(3, ...)ensures at least 3 players for compact games.
193-211: Team divisibility adjustments are correct.Each team type correctly adjusts player count to be divisible by the team size. The
HumansVsNationscase halving the count aligns with the retrieved learning that nation NPCs are added server-side.
221-233: Edge case: 75% and 50% tiers could round to same value for small maps.When
limitedBaseis 5 (the minimum):
- 75% →
5 * 0.75 = 3.75→ rounds to 5- 50% →
5 * 0.5 = 2.5→ rounds to 5All three tiers would be
[5, 5, 5]. This is likely fine since small maps need consistent sizing, but worth noting.Is this expected behavior for very small maps, or should the lower tiers have explicit minimums (e.g., 75% tier min 4, 50% tier min 3)?
evanpelle
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
…d tiles 🔧 (#2874) ## Description: The calculation is based on: 50 players per 1_000_000 land tiles, limited at 125 players because of performance Second number is 75% of that, third one 50% That way, the player counts are staying mostly the same Look at the "Dynamic Config" column, these are the new player counts: (The 125 players limit is missing in that column, only relevant for the twolakes map) <img width="930" height="1033" alt="Screenshot_2026-01-12_152758" src="https://github.com/user-attachments/assets/e1791740-e263-47b3-8b27-4f9aa358d381" /> <img width="926" height="324" alt="Screenshot_2026-01-12_152814" src="https://github.com/user-attachments/assets/78d6789b-374f-4f8b-b50f-f6f08395572b" /> This PR also removes `MapDescription` from `Maps.ts` because its unused. And this PR updates the map-generator `README.md` to reflect the changes ## Please complete the following: - [X] I have added screenshots for all UI updates - [X] I process any text displayed to the user through translateText() and I've added it to the en.json file - [X] I have added relevant tests to the test directory - [X] 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
Description:
The calculation is based on: 50 players per 1_000_000 land tiles, limited at 125 players because of performance
Second number is 75% of that, third one 50%
That way, the player counts are staying mostly the same
Look at the "Dynamic Config" column, these are the new player counts:
(The 125 players limit is missing in that column, only relevant for the twolakes map)
This PR also removes
MapDescriptionfromMaps.tsbecause its unused.And this PR updates the map-generator
README.mdto reflect the changesPlease complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
FloPinguin