Skip to content
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
125 changes: 125 additions & 0 deletions map-generator/assets/maps/taiwanstrait/info.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
{
"name": "Taiwan Strait",
"nations": [
{
"coordinates": [835, 35],
"name": "Fuzhou",
"flag": "cn"
},
{
"coordinates": [777, 226],
"name": "Putian",
"flag": "cn"
},
{
"coordinates": [686, 330],
"name": "Quanzhou",
"flag": "cn"
},
{
"coordinates": [459, 116],
"name": "Chinese Military Base",
"flag": "cn"
},
{
"coordinates": [540, 465],
"name": "Xiamen",
"flag": "cn"
},
{
"coordinates": [435, 482],
"name": "Zhangzhou",
"flag": "cn"
},
{
"coordinates": [279, 412],
"name": "Longyan",
"flag": "cn"
},
{
"coordinates": [197, 743],
"name": "Shantou",
"flag": "cn"
},
{
"coordinates": [112, 793],
"name": "Jieyang",
"flag": "cn"
},
{
"coordinates": [125, 700],
"name": "Chaozhou",
"flag": "cn"
},
{
"coordinates": [14, 215],
"name": "Giant Pandas",
"flag": "cn"
},
{
"coordinates": [29, 503],
"name": "Meizhou",
"flag": "cn"
},
{
"coordinates": [612, 478],
"name": "Kinmen Island",
"flag": "tw"
},
{
"coordinates": [912, 718],
"name": "Penghu",
"flag": "tw"
},
{
"coordinates": [1134, 990],
"name": "Kaohsiung",
"flag": "tw"
},
{
"coordinates": [1085, 884],
"name": "Tainan",
"flag": "tw"
},
{
"coordinates": [1289, 931],
"name": "Taitung",
"flag": "tw"
},
{
"coordinates": [1378, 681],
"name": "Hualien",
"flag": "tw"
},
{
"coordinates": [1121, 768],
"name": "Chiayi",
"flag": "tw"
},
{
"coordinates": [1117, 618],
"name": "Taichung City",
"flag": "tw"
},
{
"coordinates": [1476, 320],
"name": "Keelung",
"flag": "tw"
},
{
"coordinates": [1388, 347],
"name": "Taipei",
"flag": "tw"
},
{
"coordinates": [1295, 350],
"name": "Taoyuan",
"flag": "tw"
},
{
"coordinates": [1247, 415],
"name": "Hsinchu",
"flag": "tw"
}
]
}
Comment on lines +1 to +125
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

i already credited OpenTopography

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧩 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.

1 change: 1 addition & 0 deletions map-generator/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ var maps = []struct {
{Name: "antarctica"},
{Name: "archipelagosea"},
{Name: "bajacalifornia"},
{Name: "taiwanstrait"},
{Name: "big_plains", IsTest: true},
{Name: "half_land_half_ocean", IsTest: true},
{Name: "ocean_and_land", IsTest: true},
Expand Down
3 changes: 2 additions & 1 deletion resources/lang/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,8 @@
"beringsea": "Bering Sea",
"antarctica": "Antarctica",
"archipelagosea": "Archipelago Sea",
"bajacalifornia": "Baja California"
"bajacalifornia": "Baja California",
"taiwanstrait": "Taiwan Strait"
},
"map_categories": {
"featured": "Featured",
Expand Down
140 changes: 140 additions & 0 deletions resources/maps/taiwanstrait/manifest.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
{
"map": {
"height": 1184,
"num_land_tiles": 748967,
"width": 1560
},
"map16x": {
"height": 296,
"num_land_tiles": 45756,
"width": 390
},
"map4x": {
"height": 592,
"num_land_tiles": 185553,
"width": 780
},
"name": "Taiwan Strait",
"nations": [
{
"coordinates": [835, 35],
"flag": "cn",
"name": "Fuzhou"
},
{
"coordinates": [777, 226],
"flag": "cn",
"name": "Putian"
},
{
"coordinates": [686, 330],
"flag": "cn",
"name": "Quanzhou"
},
{
"coordinates": [459, 116],
"flag": "cn",
"name": "Chinese Military Base"
},
{
"coordinates": [540, 465],
"flag": "cn",
"name": "Xiamen"
},
{
"coordinates": [435, 482],
"flag": "cn",
"name": "Zhangzhou"
},
{
"coordinates": [279, 412],
"flag": "cn",
"name": "Longyan"
},
{
"coordinates": [197, 743],
"flag": "cn",
"name": "Shantou"
},
{
"coordinates": [112, 793],
"flag": "cn",
"name": "Jieyang"
},
{
"coordinates": [125, 700],
"flag": "cn",
"name": "Chaozhou"
},
{
"coordinates": [14, 215],
"flag": "cn",
"name": "Giant Pandas"
},
{
"coordinates": [29, 503],
"flag": "cn",
"name": "Meizhou"
},
{
"coordinates": [612, 478],
"flag": "tw",
"name": "Kinmen Island"
},
{
"coordinates": [912, 718],
"flag": "tw",
"name": "Penghu"
},
{
"coordinates": [1134, 990],
"flag": "tw",
"name": "Kaohsiung"
},
{
"coordinates": [1085, 884],
"flag": "tw",
"name": "Tainan"
},
{
"coordinates": [1289, 931],
"flag": "tw",
"name": "Taitung"
},
{
"coordinates": [1378, 681],
"flag": "tw",
"name": "Hualien"
},
{
"coordinates": [1121, 768],
"flag": "tw",
"name": "Chiayi"
},
{
"coordinates": [1117, 618],
"flag": "tw",
"name": "Taichung City"
},
{
"coordinates": [1476, 320],
"flag": "tw",
"name": "Keelung"
},
{
"coordinates": [1388, 347],
"flag": "tw",
"name": "Taipei"
},
{
"coordinates": [1295, 350],
"flag": "tw",
"name": "Taoyuan"
},
{
"coordinates": [1247, 415],
"flag": "tw",
"name": "Hsinchu"
}
]
}
1 change: 1 addition & 0 deletions resources/maps/taiwanstrait/map.bin

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions resources/maps/taiwanstrait/map16x.bin

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions resources/maps/taiwanstrait/map4x.bin

Large diffs are not rendered by default.

Binary file added resources/maps/taiwanstrait/thumbnail.webp
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 2 additions & 0 deletions src/core/game/Game.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ export enum GameMapType {
ArchipelagoSea = "ArchipelagoSea",
BajaCalifornia = "Baja California",
MiddleEast = "Middle East",
TaiwanStrait = "Taiwan Strait",
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

dismiss. New maps in src/core/Game.ts do not need Tests, as no other map is specifically written in Tests

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧩 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.

}

export type GameMapName = keyof typeof GameMapType;
Expand Down Expand Up @@ -232,6 +233,7 @@ export const mapCategories: Record<string, GameMapType[]> = {
GameMapType.ArchipelagoSea,
GameMapType.BajaCalifornia,
GameMapType.MiddleEast,
GameMapType.TaiwanStrait,
],
fantasy: [
GameMapType.Pangaea,
Expand Down
1 change: 1 addition & 0 deletions src/server/MapPlaylist.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ const frequency: Partial<Record<GameMapName, number>> = {
ArchipelagoSea: 3,
BajaCalifornia: 4,
MiddleEast: 8,
TaiwanStrait: 4,
};

const TEAM_WEIGHTS: { config: TeamCountConfig; weight: number }[] = [
Expand Down