Added taiwan strait map#3853
Conversation
…e nations on west side
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
WalkthroughAdds a new playable map "Taiwan Strait": new map assets and manifest, generator registration, English localization entry, GameMapType enum value and category inclusion, and a server playlist frequency entry. ChangesTaiwan Strait Map Addition
Sequence DiagramsequenceDiagram
participant MapGen as Map Generator
participant Assets as Map Assets
participant Server as Game Server
participant Game as Game Engine
MapGen->>Assets: Read `map-generator/assets/maps/taiwanstrait/info.json`
Assets-->>MapGen: Return map info (name, nations, coords, flags)
Server->>Assets: Read `resources/maps/taiwanstrait/manifest.json`
Assets-->>Server: Return manifest (sizes, tiles, nations)
Game->>Game: Add GameMapType.TaiwanStrait and include in mapCategories
Server->>Server: Add TaiwanStrait to playlist frequency (4)
Server-->>Game: Expose available maps including TaiwanStrait
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.1)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@map-generator/assets/maps/taiwanstrait/info.json`:
- Around line 1-145: Add a CREDITS.md entry for the new map "taiwanstrait"
(author field "crunchybbb" in assets/maps/taiwanstrait/info.json) that names the
data sources and their licenses (e.g., OpenStreetMap, OpenTopography, or other
sources used), include the required attribution text/links and license
identifiers, and place it under the maps section so it will be obvious this
applies to assets/maps/taiwanstrait; ensure the entry references "taiwanstrait"
and the author "crunchybbb" so reviewers can verify the mapping between the
info.json and the credit line.
In `@src/core/game/Game.ts`:
- Line 172: Add a unit test in the core test suite that imports GameMapType and
the core map registry lookup used by the codebase and asserts that
GameMapType.TaiwanStrait is present in the registry and that its registered
category equals the new category value introduced by the change; specifically,
fetch the map entry for GameMapType.TaiwanStrait via the same lookup used in
production (e.g., coreMapRegistry.get or getMapEntry/getCoreMapRegistry) and
assert both existence and that entry.category matches the expected category
constant/enum used in the map registration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ed87c7d4-bfed-4448-946e-884d50ef7dbd
⛔ Files ignored due to path filters (4)
map-generator/assets/maps/taiwanstrait/image.pngis excluded by!**/*.pngresources/maps/taiwanstrait/map.binis excluded by!**/*.binresources/maps/taiwanstrait/map16x.binis excluded by!**/*.binresources/maps/taiwanstrait/map4x.binis excluded by!**/*.bin
📒 Files selected for processing (7)
map-generator/assets/maps/taiwanstrait/info.jsonmap-generator/main.goresources/lang/en.jsonresources/maps/taiwanstrait/manifest.jsonresources/maps/taiwanstrait/thumbnail.webpsrc/core/game/Game.tssrc/server/MapPlaylist.ts
| { | ||
| "name": "taiwanstrait", | ||
| "nations": [ | ||
| { | ||
| "name": "Fuzhou", | ||
| "flag": "cn", | ||
| "coordinates": [835, 35] | ||
| }, | ||
| { | ||
| "name": "Putian", | ||
| "flag": "cn", | ||
| "coordinates": [777, 226] | ||
| }, | ||
| { | ||
| "name": "Quanzhou", | ||
| "flag": "cn", | ||
| "coordinates": [686, 330] | ||
| }, | ||
| { | ||
| "name": "Chinese Military Base", | ||
| "flag": "cn", | ||
| "coordinates": [459, 116] | ||
| }, | ||
| { | ||
| "name": "Xiamen", | ||
| "flag": "cn", | ||
| "coordinates": [540, 465] | ||
| }, | ||
| { | ||
| "name": "Zhangzhou", | ||
| "flag": "cn", | ||
| "coordinates": [435, 482] | ||
| }, | ||
| { | ||
| "name": "Longyan", | ||
| "flag": "cn", | ||
| "coordinates": [279, 412] | ||
| }, | ||
| { | ||
| "name": "Shantou", | ||
| "flag": "cn", | ||
| "coordinates": [197, 743] | ||
| }, | ||
| { | ||
| "name": "Jieyang", | ||
| "flag": "cn", | ||
| "coordinates": [112, 793] | ||
| }, | ||
| { | ||
| "name": "Chaozhou", | ||
| "flag": "cn", | ||
| "coordinates": [125, 700] | ||
| }, | ||
| { | ||
| "name": "Giant Pandas", | ||
| "flag": "cn", | ||
| "coordinates": [14, 215] | ||
| }, | ||
| { | ||
| "name": "Meizhou", | ||
| "flag": "cn", | ||
| "coordinates": [29, 503] | ||
| }, | ||
| { | ||
| "name": "Kinmen Island", | ||
| "flag": "tw", | ||
| "coordinates": [612, 478] | ||
| }, | ||
| { | ||
| "name": "Penghu", | ||
| "flag": "tw", | ||
| "coordinates": [912, 718] | ||
| }, | ||
| { | ||
| "name": "Kaohsiung", | ||
| "flag": "tw", | ||
| "coordinates": [1134, 990] | ||
| }, | ||
| { | ||
| "name": "Tainan", | ||
| "flag": "tw", | ||
| "coordinates": [1085, 884] | ||
| }, | ||
| { | ||
| "name": "Taitung", | ||
| "flag": "tw", | ||
| "coordinates": [1289, 931] | ||
| }, | ||
| { | ||
| "name": "Hualien", | ||
| "flag": "tw", | ||
| "coordinates": [1378, 681] | ||
| }, | ||
| { | ||
| "name": "Chiayi", | ||
| "flag": "tw", | ||
| "coordinates": [1121, 768] | ||
| }, | ||
| { | ||
| "name": "Taichung City", | ||
| "flag": "tw", | ||
| "coordinates": [1117, 618] | ||
| }, | ||
| { | ||
| "name": "Keelung", | ||
| "flag": "tw", | ||
| "coordinates": [1476, 320] | ||
| }, | ||
| { | ||
| "name": "Taipei", | ||
| "flag": "tw", | ||
| "coordinates": [1388, 347] | ||
| }, | ||
| { | ||
| "name": "Taoyuan", | ||
| "flag": "tw", | ||
| "coordinates": [1295, 350] | ||
| }, | ||
| { | ||
| "name": "Hsinchu", | ||
| "flag": "tw", | ||
| "coordinates": [1247, 415] | ||
| } | ||
| ], | ||
| "metadata": { | ||
| "author": "crunchybbb", | ||
| "description": "taiwan strait" | ||
| }, | ||
| "generatedAt": "2026-05-06T02:07:50.785Z", | ||
| "map": { | ||
| "width": 1560, | ||
| "height": 1184, | ||
| "num_land_tiles": 748967 | ||
| }, | ||
| "map4x": { | ||
| "width": 780, | ||
| "height": 592, | ||
| "num_land_tiles": 185705 | ||
| }, | ||
| "map16x": { | ||
| "width": 390, | ||
| "height": 296, | ||
| "num_land_tiles": 45758 | ||
| } | ||
| } |
There was a problem hiding this comment.
Add source/license attribution for this new map in CREDITS.md.
This PR adds a new map metadata file, but I don’t see the matching attribution update. Please add the map data-source/license entry in CREDITS.md (e.g., OSM/OpenTopography attribution when applicable).
Based on learnings: “For any new map metadata under map-generator/assets/maps/*/info.json, ensure the repo’s CREDITS.md is updated with correct license/source attribution for the map’s data sources.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@map-generator/assets/maps/taiwanstrait/info.json` around lines 1 - 145, Add a
CREDITS.md entry for the new map "taiwanstrait" (author field "crunchybbb" in
assets/maps/taiwanstrait/info.json) that names the data sources and their
licenses (e.g., OpenStreetMap, OpenTopography, or other sources used), include
the required attribution text/links and license identifiers, and place it under
the maps section so it will be obvious this applies to assets/maps/taiwanstrait;
ensure the entry references "taiwanstrait" and the author "crunchybbb" so
reviewers can verify the mapping between the info.json and the credit line.
There was a problem hiding this comment.
i already credited OpenTopography
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check CREDITS.md for taiwanstrait or OpenTopography attribution
grep -n -i "taiwanstrait\|taiwan strait\|crunchybbb\|opentopography" CREDITS.md
Length of output: 210
🏁 Script executed:
#!/bin/bash
# Check the full CREDITS.md to understand the attribution pattern used for maps
cat CREDITS.md
Length of output: 4489
@crunchybbb2-hash, thanks for confirming! I checked CREDITS.md and the general OpenTopography section (covering Copernicus Global Digital Elevation Models) is already present and covers your map's elevation data. The existing credit is sufficient — no separate per-map entry is needed since the Taiwan Strait map uses the same shared source already attributed.
If you didn't use OpenStreetMap for road/coastline tracing (unlike some other maps in the project), then no additional attribution is required. The PR looks good on the credits front! ✅
(ᴗ_ ᴗ。)
🧠 Learnings used
Learnt from: PatrickPlaysBadly
Repo: openfrontio/OpenFrontIO PR: 3708
File: map-generator/assets/maps/losangeles/info.json:1-130
Timestamp: 2026-04-21T23:01:24.675Z
Learning: For any new map metadata under `map-generator/assets/maps/*/info.json`, ensure the repo’s `CREDITS.md` is updated with correct license/source attribution for the map’s data sources. If the map roads were hand-traced using OpenStreetMap as a visual reference, add attribution as “© OpenStreetMap contributors” under the ODbL 1.0 license (https://www.openstreetmap.org/copyright). If elevation data comes from OpenTopography, include OpenTopography as the elevation source in `CREDITS.md`.
Learnt from: RickD004
Repo: openfrontio/OpenFrontIO PR: 3823
File: map-generator/assets/maps/bosphorusstraits/info.json:3-114
Timestamp: 2026-05-03T06:05:30.997Z
Learning: For OpenFrontIO map metadata files at `map-generator/assets/maps/*/info.json`, nation/district names defined in these `info.json` files are NOT required to also exist in `resources/lang/en.json`. When reviewing changes to these map `info.json` files, do not flag missing nation/district entries in `resources/lang/en.json` as an issue, since this is the established project convention.
Learnt from: RickD004
Repo: openfrontio/OpenFrontIO PR: 3823
File: map-generator/assets/maps/bosphorusstraits/info.json:3-114
Timestamp: 2026-05-03T06:05:30.997Z
Learning: In OpenFrontIO map data files (e.g., map-generator/assets/maps/**/info.json), when Turkish settlement/location names are used as nation names, keep the original native Turkish spelling (including diacritics like Ç/Ü/Ğ/Ş/İ/Ö) and do not latinize/transliterate them. Even if the surrounding UI/content is English, the nation name value should remain the native Turkish name (e.g., “Çekmeköy”, “Üsküdar”, “Büyükçekmece”), matching how these names are commonly written in English-language sources such as Wikipedia.
| Antarctica = "Antarctica", | ||
| ArchipelagoSea = "ArchipelagoSea", | ||
| BajaCalifornia = "Baja California", | ||
| TaiwanStrait = "Taiwan Strait", |
There was a problem hiding this comment.
Add tests for this src/core map registration change.
Line 172 and Line 234 modify core map registry/category behavior, but there is no matching core test update in this PR context. Please add a test that asserts GameMapType.TaiwanStrait is present and categorized as expected.
As per coding guidelines: “All changes to src/core/ must include tests”.
Also applies to: 234-234
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/game/Game.ts` at line 172, Add a unit test in the core test suite
that imports GameMapType and the core map registry lookup used by the codebase
and asserts that GameMapType.TaiwanStrait is present in the registry and that
its registered category equals the new category value introduced by the change;
specifically, fetch the map entry for GameMapType.TaiwanStrait via the same
lookup used in production (e.g., coreMapRegistry.get or
getMapEntry/getCoreMapRegistry) and assert both existence and that
entry.category matches the expected category constant/enum used in the map
registration.
There was a problem hiding this comment.
dismiss. New maps in src/core/Game.ts do not need Tests, as no other map is specifically written in Tests
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if any test files reference specific map names or GameMapType enum values
rg -n "GameMapType\." --type=ts -g "**/test*/**" -g "**/*.test.*" -g "**/*.spec.*" | head -40
# Also check broader test directories
fd -e ts -g "*.test.ts" -o -e ts -g "*.spec.ts" | head -20
# Search for any map-specific test references
rg -n "TaiwanStrait\|BajaCalifornia\|MiddleEast\|MareNostrum\|GreatLakes" --type=ts | grep -i "test\|spec\|expect\|assert" | head -20
Length of output: 563
@RickD004, you are right. I checked the test files and confirmed: existing tests only use GameMapType values as fixtures (e.g., gameMap: GameMapType.Asia), not to assert that any specific map is registered or categorized. No map has a dedicated registration test. My comment was wrong to apply the general src/core/ test guideline to a pure enum/data addition like this. I will withdraw the comment.
✏️ Learnings added
Learnt from: RickD004
Repo: openfrontio/OpenFrontIO PR: 3853
File: src/core/game/Game.ts:0-0
Timestamp: 2026-05-06T05:13:48.956Z
Learning: In openfrontio/OpenFrontIO, adding a new map entry to the GameMapType enum in src/core/game/Game.ts and registering it in mapCategories does NOT require a dedicated test. No existing map has a specific registration or category test. Tests use GameMapType values only as fixture inputs for other test scenarios, not to assert enum membership or category assignment. Do not flag missing tests for new map enum additions in this file.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
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.
Learnt from: FloPinguin
Repo: openfrontio/OpenFrontIO PR: 2887
File: src/core/execution/NukeExecution.ts:118-122
Timestamp: 2026-01-13T20:16:05.535Z
Learning: In code paths that return a Player-like object, prefer returning a union type (Player | TerraNullius) instead of undefined. When a function may fail to find a player, return TerraNullius for the 'not found' case and a Player for valid IDs, and check .isPlayer() (or equivalent) directly on the result instead of guarding with undefined or optional chaining. This should be enforced in Game, GameImpl, and GameView (and similar accessors) to avoid undefined checks and simplify null-safety handling.
Learnt from: scamiv
Repo: openfrontio/OpenFrontIO PR: 3292
File: src/core/execution/TrainExecution.ts:70-84
Timestamp: 2026-02-25T02:38:42.961Z
Learning: Preserve the intentional duplication of boundary tiles when constructing train motion plan paths. Specifically, do not deduplicate tiles at station boundaries in train motion plan logic. This affects both server-side tracking (TrainExecution.ts saveTraversedTiles) and client-side processing (GameView.ts train motion plan processing). Reviewers should ensure changes do not remove or consolidate boundary tiles, as duplicates are expected and documented in MotionPlans.ts, and are necessary for correct car spacing and positioning, especially when speed > 1.
Learnt from: VariableVince
Repo: openfrontio/OpenFrontIO PR: 3549
File: tests/pathfinding/playground/server.ts:246-250
Timestamp: 2026-03-31T17:02:41.798Z
Learning: When migrating/implementing Express 5+ code, use the Express 5 `app.listen(PORT, (error) => { ... })` callback signature: Express 5 passes listen errors (e.g., `EADDRINUSE`) as the first argument to the callback. Handle the error inside that callback instead of relying on Node.js `http.Server`’s `'error'` event semantics.
Learnt from: baculinivan-web
Repo: openfrontio/OpenFrontIO PR: 3688
File: src/client/graphics/layers/GameRightSidebar.ts:235-248
Timestamp: 2026-04-22T08:54:50.345Z
Learning: In the OpenFrontIO/OpenFrontIO repository, do not raise review findings for standard web accessibility (a11y) keyboard/semantic issues that would normally require button semantics (e.g., using clickable `<div>` elements instead of `<button>`, or similar keyboard accessibility/ARIA patterns). This repository intentionally does not enforce those semantic/keyboard-accessibility conventions because gameplay requires touch or mouse input and is not operable via keyboard alone.
github-project-automation
Bot
moved this from Triage
to Development
in OpenFront Release Management
github-project-automation
Bot
moved this from Development
to Complete
in OpenFront Release Management
github-project-automation
Bot
moved this from Complete
to Triage
in OpenFront Release Management
|
@RickD004 can you review |
on it, currently helping him fix it in the discord |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@resources/maps/taiwanstrait/manifest.json`:
- Around line 20-23: The manifest entries for "Giant Pandas" and "Chinese
Military Base" are missing a bot marker so createNationsForGame() (which calls
toNation()) will convert them into player-selectable nations; mark these entries
as bots by adding a distinguishing field (e.g., "bot": true) or move them into a
separate bots/spawn section used by the engine, ensuring the engine's
toNation()/createNationsForGame() path will treat them as AI-only rather than
human-playable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5f25071d-033b-43be-8ab8-ac967c57efae
📒 Files selected for processing (1)
resources/maps/taiwanstrait/manifest.json
| "name": "Chinese Military Base", | ||
| "flag": "cn", | ||
| "coordinates": [459, 116] | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check other map manifests for any bot-specific field
rg -n '"bot"' resources/maps/
rg -n '"type"' resources/maps/ | head -60
# Also compare a known manifest structure for reference
fd manifest.json resources/maps/ --exec head -30 {}Repository: openfrontio/OpenFrontIO
Length of output: 39592
🏁 Script executed:
cat resources/maps/taiwanstrait/manifest.jsonRepository: openfrontio/OpenFrontIO
Length of output: 2565
🏁 Script executed:
# Search for any mechanism the engine uses to identify bots
rg -n 'bot|Bot|BOT' --type json resources/maps/ | head -20Repository: openfrontio/OpenFrontIO
Length of output: 273
🏁 Script executed:
# Check engine source for how it processes nation entries
fd -e ts -e js -e tsx -e jsx . --path '*engine*' --path '*spawn*' --path '*nation*' 2>/dev/null | head -20Repository: openfrontio/OpenFrontIO
Length of output: 49
🏁 Script executed:
# Search engine source for how it handles bot spawns or nation processing
rg -n 'bot|meme|spawn' --type ts --type tsx -i | grep -i 'nation\|spawn\|bot' | head -30Repository: openfrontio/OpenFrontIO
Length of output: 95
🏁 Script executed:
# Look for manifest loading or nation initialization code
fd -e ts -e tsx -e js -e jsx | xargs rg -l 'manifest|nation' | head -10Repository: openfrontio/OpenFrontIO
Length of output: 408
🏁 Script executed:
# Check if there's documentation or schema about manifests
fd manifest -type f | xargs ls -la 2>/dev/nullRepository: openfrontio/OpenFrontIO
Length of output: 3029
🏁 Script executed:
# Find source files that might handle nation/spawn logic
find src -type f -name "*.ts" | head -20Repository: openfrontio/OpenFrontIO
Length of output: 599
🏁 Script executed:
# Search for how nations are loaded from manifests
rg -n "nations" src --type ts | head -30Repository: openfrontio/OpenFrontIO
Length of output: 2648
🏁 Script executed:
# Search for bot or meme-related spawning logic
rg -n "bot\|meme\|spawn" src --type ts -i | head -40Repository: openfrontio/OpenFrontIO
Length of output: 49
🏁 Script executed:
# Search for manifest loading and nation instantiation
rg -n "manifest\|nation\|Nation" src/server --type ts | grep -i "manifest\|from\|load\|spawn\|create" | head -40Repository: openfrontio/OpenFrontIO
Length of output: 49
🏁 Script executed:
# Look at GameServer and GameManager to understand nation spawning
head -150 src/server/GameServer.ts | tail -80Repository: openfrontio/OpenFrontIO
Length of output: 2532
🏁 Script executed:
# Search for where manifest data is actually used
rg -n "manifest\.nations\|getManifest\|loadManifest" src --type ts | head -20Repository: openfrontio/OpenFrontIO
Length of output: 49
🏁 Script executed:
# Find GameConfig type definition
rg -n "type GameConfig\|interface GameConfig" src --type ts -A 20 | head -60Repository: openfrontio/OpenFrontIO
Length of output: 49
🏁 Script executed:
# Search for where nations from manifest are instantiated in game
rg -n "new Nation\|createNation\|Nation(" src/core --type ts | head -20Repository: openfrontio/OpenFrontIO
Length of output: 184
🏁 Script executed:
# Look for NationState or similar that represents a nation in the game
find src/core -name "*.ts" | grep -i nation | head -10Repository: openfrontio/OpenFrontIO
Length of output: 525
🏁 Script executed:
# Read NationCreation.ts to understand how nations are created
cat src/core/game/NationCreation.tsRepository: openfrontio/OpenFrontIO
Length of output: 14245
🏁 Script executed:
# Also check GameConfig type definition in a broader way
rg -n "GameConfig" src --type ts | grep -E "interface|type|class" | head -10Repository: openfrontio/OpenFrontIO
Length of output: 667
Mark meme-bot nations with a distinguishing field or move them to a separate spawn section.
The entries for "Giant Pandas" and "Chinese Military Base" lack any field that marks them as bots (such as "bot": true). The engine's createNationsForGame() function treats all nations from the manifest identically—they are all converted to player-selectable nations via the same toNation() conversion. These two entries will spawn as regular human-playable nations, not as AI-controlled bots, unless the engine checks another mechanism (such as a separate "bots" section in the manifest or an external configuration).
To ensure these meme nations spawn correctly as AI bots instead of appearing in the player nation selection, add a bot identifier field to the manifest entries or move them to a dedicated bot spawn section.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@resources/maps/taiwanstrait/manifest.json` around lines 20 - 23, The manifest
entries for "Giant Pandas" and "Chinese Military Base" are missing a bot marker
so createNationsForGame() (which calls toNation()) will convert them into
player-selectable nations; mark these entries as bots by adding a distinguishing
field (e.g., "bot": true) or move them into a separate bots/spawn section used
by the engine, ensuring the engine's toNation()/createNationsForGame() path will
treat them as AI-only rather than human-playable.
There was a problem hiding this comment.
they are nations , kinda like the normal capybaras and just otters from south america map
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check South America map manifest for capybaras/otters entries
fd manifest.json resources/maps/ --exec grep -l -i "capybara\|otter" {}
fd manifest.json resources/maps/ | xargs grep -l -i "capybara\|otter" 2>/dev/null
Length of output: 0
🏁 Script executed:
#!/bin/bash
# List all map manifest nation names to find meme-style nations
fd manifest.json resources/maps/ --exec grep -h '"name"' {} | sort | uniq | head -60
# Also check south america map specifically
fd manifest.json resources/maps/ | xargs grep -l -i "south\|america\|brazil" 2>/dev/null
# List all map folders
ls resources/maps/
Length of output: 3235
🏁 Script executed:
#!/bin/bash
# Check south america manifest for meme nations
cat resources/maps/southamerica/manifest.json | grep -i '"name"' | head -40
# Also search for any fun/meme nation names across all manifests
fd manifest.json resources/maps/ --exec grep -h '"name"' {} | grep -iE "panda|capybara|otter|penguin|jaguar|meme|bot|bear|llama" | sort
Length of output: 865
@crunchybbb2-hash, thanks for the clarification! You're right — looking at other map manifests in the repo, meme-style nation names like "Giant Pandas", "Penguin Empire", "Polar Bears", and "Non-peaceful Bot" already exist as regular nation entries in other maps, with no special bot marker needed. No manifest in the codebase uses a "bot" field, and the engine handles AI bots at a different layer outside of the manifest. My concern was incorrect — "Giant Pandas" and "Chinese Military Base" work fine as normal nations, consistent with the existing pattern. Sorry for the noise!
✏️ Learnings added
Learnt from: crunchybbb2-hash
Repo: openfrontio/OpenFrontIO PR: 3853
File: resources/maps/taiwanstrait/manifest.json:20-23
Timestamp: 2026-05-06T04:27:04.540Z
Learning: In OpenFrontIO map `manifest.json` files under `resources/maps/**/`, meme-style or fun nation names (e.g., "Giant Pandas", "Penguin Empire", "Polar Bears", "Chinese Military Base") are valid regular nation entries — they do not need any special `"bot"` field or separate section. The manifest has no bot-marker mechanism; AI bot behavior is handled server-side, not via manifest fields.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: patrickbadly-hub
Repo: openfrontio/OpenFrontIO PR: 3538
File: resources/maps/reglaciatedantarctica/manifest.json:21-21
Timestamp: 2026-03-29T19:54:41.468Z
Learning: In OpenFrontIO map `manifest.json` files under `resources/maps/**/`, use `"flag": " "` (a single space) only as an intentional sentinel to render a broken flag image placeholder. This relies on `GameView.ts` using a truthy check (`nation.flag ? ... : undefined`): a single space is truthy and produces an `<img>` whose `src` fails to load (e.g., `/flags/ .svg`), causing the browser broken-image icon. Use `"flag": ""` (empty string) when you want no flag `<img>` element rendered.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/core/game/Game.ts (1)
173-173:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing core tests for the new
TaiwanStraitenum member and category entry.
src/core/changes require matching tests, but this PR adds no test asserting thatGameMapType.TaiwanStraitexists in the enum and is correctly bucketed inmapCategories.regional.As per coding guidelines: "All changes to
src/core/must include tests."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/game/Game.ts` at line 173, Add unit tests to cover the new GameMapType.TaiwanStrait enum member and ensure it is included in the mapCategories.regional bucket: write a test that imports GameMapType and asserts GameMapType.TaiwanStrait is defined and equals the expected string value, and another test that imports mapCategories and asserts mapCategories.regional includes GameMapType.TaiwanStrait; place tests alongside other core game tests and run the test suite to verify they pass.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@src/core/game/Game.ts`:
- Line 173: Add unit tests to cover the new GameMapType.TaiwanStrait enum member
and ensure it is included in the mapCategories.regional bucket: write a test
that imports GameMapType and asserts GameMapType.TaiwanStrait is defined and
equals the expected string value, and another test that imports mapCategories
and asserts mapCategories.regional includes GameMapType.TaiwanStrait; place
tests alongside other core game tests and run the test suite to verify they
pass.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 022ad01f-5fb5-4e3e-85af-34ef52bf723e
⛔ Files ignored due to path filters (4)
map-generator/assets/maps/taiwanstrait/image.pngis excluded by!**/*.pngresources/maps/taiwanstrait/map.binis excluded by!**/*.binresources/maps/taiwanstrait/map16x.binis excluded by!**/*.binresources/maps/taiwanstrait/map4x.binis excluded by!**/*.bin
📒 Files selected for processing (6)
map-generator/assets/maps/taiwanstrait/info.jsonmap-generator/main.goresources/maps/taiwanstrait/manifest.jsonresources/maps/taiwanstrait/thumbnail.webpsrc/core/game/Game.tssrc/server/MapPlaylist.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/server/MapPlaylist.ts
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
## Description: Map of the Taiwan and the Chinese Mainland. Team heavy map like Baikal and Hormuz. Terrain Source from OpenTopography, already credited <img width="1800" height="1511" alt="image" src="https://github.com/user-attachments/assets/45954469-8199-4882-9efe-899c5df87ce4" /> I also took the chance to standarize and sort alphabetically the map lists in main.go and MapPlaylist.ts. ## 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: tri.star 1011 NOTE: If the map gets added, please give contributor to crunchybbbbb_59469 for this map on Discord. Every file was made by him, his PR just had weird bugs that didnt allow the PR to be review automatically Original PR: #3853 by crunchybbb2-hash
4 participants
Description:
Added new Taiwan Strait map
Medium sized map (1500x1187px) of the strait, with both sides roughly the same side. Trade heavy and it might play similarly to hormuz.
About 24 nations, based on real life cities in the region, and 2 meme bots in the highlands.
The island in the middle works as a piratemaxxing base.
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
DISCORD_USERNAME crunchybbbbb @crunchybbbbb_59469