crit fix: indian subcontinent map crash

[blontd6]

Resolves #4401

Description:

Fixes a critical browser tab crash ("Aw, Snap! Something went wrong") when loading the game on the new Indian Subcontinent map (or any map with >= 253 water components) in Solo Mode.

Technical Cause:

1. When a map contains >= 253 disconnected water components, the array mapping tiles to component IDs is dynamically promoted from a Uint8Array to a Uint16Array.
2. This promotion upgrades the land sentinel LAND_MARKER from 0xff (255) to LAND_MARKER_WIDE (0xffff / 65535).
3. The BFS local search filter in AStarWaterHierarchical had a hardcoded sentinel check: (t: TileRef) => this.graph.getComponentId(t) !== LAND_MARKER (evaluating against 255).
4. On promoted maps, land tiles (65535) matched this check as water. The local BFS then traversed the entire landmass of the map, resulting in CPU exhaustion and memory/stack overflows that crashed the rendering process.

Solution:

Changed the hardcoded sentinel check to query the map's terrain directly via this.map.isWater(t). This makes the check immune to any component ID promotions or sentinel representation upgrades. Verified that the existing water pathfinding test suite passes successfully.

Please complete the following:

• I have added screenshots for all UI updates (N/A)
• I process any text displayed to the user through translateText() and I've added it to the en.json file (N/A)
• I have added relevant tests to the test directory (Existing tests in tests/core/pathfinding/PathFinding.Water.test.ts cover water pathfinding behavior and run successfully)

Please put your Discord username so you can be contacted if a bug or regression is found:

blontd6

[coderabbitai]

Walkthrough

The PR changes water path selection to use map.isWater(t), adds a giant world water-route test, and switches GameMap coordinate lookup tables to typed arrays.

Changes

Water pathfinding and map lookup

Layer / File(s)	Summary
Typed coordinate lookup tables src/core/game/GameMap.ts	GameMapImpl declares refToX and refToY as Uint16Array and yToRef as Int32Array, and allocates those typed arrays in the constructor.
Update water tile filter src/core/pathfinding/algorithms/AStar.WaterHierarchical.ts	findNearestNode now filters tiles with map.isWater(t) instead of checking graph.getComponentId(t) !== LAND_MARKER, and the unused LAND_MARKER import is removed.
Add giant world route test tests/core/pathfinding/PathFinding.Water.test.ts	The test setup now creates giantWorldGame, and a new suite checks that a water path is found on the giant world map with matching start and end tiles.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• openfrontio/OpenFrontIO#4214: Updates water and lake classification used by isWater, which affects the same pathfinding filter changed here.
• openfrontio/OpenFrontIO#4429: Changes connected-component land and water marker handling related to the sentinel logic removed from the path filter here.

Suggested reviewers: evanpelle, FloPinguin

Poem

A water path found a new clue,
Typed tables hum in shades of blue.
One giant map, one careful test,
And routes now sail a little best. 🌊

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main fix: the Indian Subcontinent map crash.
Description check	✅ Passed	The description matches the change by explaining the map crash and the intended fix.
Linked Issues check	✅ Passed	The code changes target the reported crash and add regression coverage for #4401.
Out of Scope Changes check	✅ Passed	All modified files are tied to the crash fix or supporting regression tests, with no clear unrelated changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

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 `@src/core/pathfinding/algorithms/AStar.WaterHierarchical.ts`:
- Line 316: Add a regression test for the 253+ water-component crash in
AStar.WaterHierarchical by exercising the ConnectedComponents sentinel-width
promotion path and verifying findNearestNode/BFS completes without crossing
land. Use the AStar.WaterHierarchical and ConnectedComponents symbols to locate
the relevant logic, and add a targeted map fixture or mocked map setup that
forces more than 252 water components so the component array upgrades from
Uint8Array to Uint16Array. Ensure the test asserts termination and correct
water-only traversal, since this change touches src/core/ and must be covered by
tests.

🪄 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: 346ef8d2-f1b7-47fb-b7f6-ce90f7c1c293

📥 Commits

Reviewing files that changed from the base of the PR and between 1d5a6ae and eceeb60.

📒 Files selected for processing (1)

• src/core/pathfinding/algorithms/AStar.WaterHierarchical.ts

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Add a regression test for the 253+ water-component crash scenario.

No test changes are included with this fix. The repository guideline requires that all changes to src/core/ include tests, and this change fixes a severe production crash (browser tab crash on maps like Indian Subcontinent), so a targeted regression test would guard against reintroducing the sentinel-width bug (e.g., a map fixture forcing ConnectedComponents to promote from Uint8Array to Uint16Array, then asserting findNearestNode/BFS terminates without traversing land).

Do you want me to draft a test case that constructs a map with 253+ water components to exercise this path?

As per path instructions, "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/pathfinding/algorithms/AStar.WaterHierarchical.ts` at line 316, Add
a regression test for the 253+ water-component crash in AStar.WaterHierarchical
by exercising the ConnectedComponents sentinel-width promotion path and
verifying findNearestNode/BFS completes without crossing land. Use the
AStar.WaterHierarchical and ConnectedComponents symbols to locate the relevant
logic, and add a targeted map fixture or mocked map setup that forces more than
252 water components so the component array upgrades from Uint8Array to
Uint16Array. Ensure the test asserts termination and correct water-only
traversal, since this change touches src/core/ and must be covered by tests.

Source: Path instructions

[evanpelle]

I don't think this fixes the issue, the indian subcontinent only has 15 water components

[coderabbitai]

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 `@src/core/game/GameMap.ts`:
- Around line 112-121: The Uint16Array-backed coordinate tables in GameMap
constructor logic assume x/y always stay within 65535, but this is only
documented and not enforced. Add an explicit invariant check in the GameMap
initialization path (where refToX and refToY are created) to fail fast if map
dimensions or coordinate ranges exceed the Uint16 limit, using the existing
GameMap symbols to locate the code. Also add or update a unit test around this
boundary to verify the guard is enforced and the assumption is locked in.

🪄 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: 3b3d1e2b-1f7a-4e8e-ad86-da1aa8ceff64

📥 Commits

Reviewing files that changed from the base of the PR and between 609276b and 33e347f.

📒 Files selected for processing (1)

• src/core/game/GameMap.ts

[blontd6]

GameMapImpl allocated refToX and refToY as plain Array<number>, costing ~8 bytes/element in V8. On Indian Subcontinent (2000×2220 = 4.44M tiles), two instances at startup consumed ~142 MB just for these LUTs, OOM-killing the tab.
Switched to Uint16Array (coords fit in 0–65535) and Int32Array for yToRef. Saves ~67 MB. tsc --noEmit clean.
This should fully fix it as my original diagnosis did fix a bad code practice (the wrong fix though!). Looking at it though it is ranked 12th in map size. Of course giant world map is first and second being hawaii which is odd. I'm researching this right now...
this change is inherently meaningful if it doesn't break anything. My tests did not show anything breaking but im still trying to figure out why Indian Subcontinent is the specific one that broke

TLDR
This fixes a genuine memory inefficiency. Whether it fully resolves the reported crash or additional factors (high nation count, new custom flags) are also contributing is worth verifying after merge.