Conversation
|
|
WalkthroughAdds a new "Cyprus" map: new map assets and manifest, registers it in the map generator and server playlist, updates localization and client map descriptions, and replaces six older map entries across enums and default configs. Changes
Sequence Diagram(s)(omitted — changes are data and registration updates without new multi-component control flow) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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: 0
🧹 Nitpick comments (1)
resources/maps/cyprus/manifest.json (1)
100-100: Add trailing newline for consistency.The file is missing a trailing newline at the end. While not critical, most JSON files in projects include one for POSIX compliance and editor compatibility.
📝 Add trailing newline
Add a blank line after the closing brace on line 100.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
map-generator/assets/maps/cyprus/image.pngis excluded by!**/*.pngresources/maps/cyprus/map.binis excluded by!**/*.binresources/maps/cyprus/map16x.binis excluded by!**/*.binresources/maps/cyprus/map4x.binis excluded by!**/*.bin
📒 Files selected for processing (9)
map-generator/assets/maps/cyprus/info.jsonmap-generator/main.goresources/lang/en.jsonresources/maps/cyprus/manifest.jsonresources/maps/cyprus/thumbnail.webpsrc/client/components/Maps.tssrc/core/configuration/DefaultConfig.tssrc/core/game/Game.tssrc/server/MapPlaylist.ts
🧰 Additional context used
🧠 Learnings (6)
📚 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-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-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-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
📚 Learning: 2025-10-27T08:59:47.620Z
Learnt from: TheGiraffe3
Repo: openfrontio/OpenFrontIO PR: 2306
File: src/core/game/Game.ts:141-141
Timestamp: 2025-10-27T08:59:47.620Z
Learning: In OpenFrontIO, gamemode-specific map variants that have altered geography from their real-world counterparts should be categorized as "fantasy" rather than "regional", even if based on real locations. Example: BaikalNukeWars is in "fantasy" because it has different islands than real Baikal.
Applied to files:
src/core/game/Game.ts
📚 Learning: 2025-08-27T08:12:19.610Z
Learnt from: mokizzz
Repo: openfrontio/OpenFrontIO PR: 1940
File: resources/lang/en.json:763-766
Timestamp: 2025-08-27T08:12:19.610Z
Learning: In OpenFrontIO, some country entries in src/client/data/countries.json may have similar names but different codes (e.g., "Empire of Japan" vs "Empire of Japan1"). Each unique code requires its own translation key in resources/lang/en.json after normalization. Always verify against countries.json before suggesting removal of translation keys.
Applied to files:
resources/lang/en.json
🔇 Additional comments (10)
src/core/game/Game.ts (2)
110-110: LGTM! Cyprus enum entry added correctly.The new map type follows the established pattern and uses a clear, descriptive name.
149-149: LGTM! Cyprus correctly categorized as regional.Cyprus represents a real geographic location and should be in the "regional" category rather than "fantasy". This aligns with the project's categorization standards.
Based on learnings: gamemode variants with altered geography belong in "fantasy", while real-world locations belong in "regional".
src/core/configuration/DefaultConfig.ts (1)
88-88: LGTM! Player count configuration is appropriate.The values [50, 40, 30] are consistent with other regional maps of similar geographic scope (e.g., Iceland uses the same configuration).
src/client/components/Maps.ts (1)
47-47: LGTM! Map description entry added correctly.The key-value pair follows the established pattern and maintains type safety with the GameMapType enum.
resources/lang/en.json (1)
248-249: LGTM! Translation entry added correctly.The lowercase key format and translation value follow the established pattern for map names.
map-generator/main.go (1)
34-34: LGTM! Map registry entry added correctly.The entry is alphabetically ordered between "britannia" and "deglaciatedantarctica" and follows the established pattern. All required source assets and generated outputs are in place:
- Source: image.png and info.json exist with proper configuration
- Outputs: manifest.json, map binaries (map.bin, map4x.bin, map16x.bin), and thumbnail.webp are all generated
src/server/MapPlaylist.ts (1)
33-33: LGTM! Cyprus frequency is appropriate.The frequency value of 4 is consistent with other regional maps of similar size (Australia, FalklandIslands, etc.), and the alphabetical ordering is maintained correctly.
resources/maps/cyprus/manifest.json (2)
2-16: LGTM! Map dimension scaling looks correct.The dimensions scale appropriately across resolutions (1x, 4x, 16x), and the land tile counts show expected variance due to resampling at different resolutions.
18-99: Nations data matches info.json correctly.The nations array in manifest.json is identical to map-generator/assets/maps/cyprus/info.json. This ensures both files stay in sync. If both are maintained by hand, consider making one file the main source and generating the other automatically to avoid future differences.
map-generator/assets/maps/cyprus/info.json (1)
1-85: Coordinates are valid and within map bounds.All nation coordinates fall within the expected map dimensions (width: 2048, height: 1253). The x-range is 144–2019 and y-range is 131–1051, so no action is needed here.
|
Hiya, putting into draft as there are conflicts |
|
I am closing this draft PR now in preperation for v30 sprint. if this is still active please reopen |
5 participants
Description:
Adds a new Cyprus map featuring the island of Cyprus and surrounding coastal regions of Turkey and Syria. Three way Baikal/Nukewars for team games.
Changes
Added Cyprus map files (map.bin, map4x.bin, map16x.bin, manifest.json)
Added map-generator source assets (image.png, info.json)
Updated GameMapType enum with Cyprus
Added to regional map category
Added playlist frequency and player count configuration
Added language translation
Nations
Cyprus:
Nicosia
Paphos
Limassol
Northern Cyprus
Turkey:
Mersin
Hatay
Antalya
Syria:
Lattakia
Tartus
Hamah
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
TSProphet