Prevent AI from placing ports on small lakes 🚢#4429
Conversation
|
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 as they are similar to previous changes (2)
WalkthroughTracks water component sizes, exposes the size through game APIs, and uses it to block port placement near small water bodies. ChangesWater Component Size for Port Filtering
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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.
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 `@src/core/execution/nation/NationStructureBehavior.ts`:
- Around line 81-87: The port-water cutoff in NationStructureBehavior is using
the wrong size unit, since Game.getWaterComponentSize already returns full-map
tiles while MIN_PORT_WATER_COMPONENT_SIZE is still set as if it were minimap
tiles. Update the threshold to match the unit used by the water-component check
in the port-placement logic, either by raising MIN_PORT_WATER_COMPONENT_SIZE to
the full-map equivalent or by comparing against the raw minimap component size
consistently. Keep the fix localized to the constants/logic around
NationStructureBehavior and the port-filtering path that consumes
Game.getWaterComponentSize.
In `@src/core/pathfinding/algorithms/ConnectedComponents.ts`:
- Around line 177-179: The LAND_MARKER sentinel still collides with real
component id 255 after initialize() upgrades the storage to Uint16Array, so
getComponentId() and getComponentSize() can misread a valid water component as
land. Update ConnectedComponents so the land sentinel is moved to a value
outside the assigned component range after the upgrade (for example a 16-bit max
sentinel) or otherwise ensure component ids never reach 255, and make the
related logic in initialize(), getComponentId(), and getComponentSize() use the
same reserved marker consistently.
🪄 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: e4f1ad4e-e52a-4732-839e-f939340c18b0
📒 Files selected for processing (7)
src/core/execution/nation/NationStructureBehavior.tssrc/core/game/Game.tssrc/core/game/GameImpl.tssrc/core/game/WaterManager.tssrc/core/pathfinding/algorithms/AbstractGraph.tssrc/core/pathfinding/algorithms/ConnectedComponents.tstests/core/pathfinding/WaterComponents.test.ts
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 `@src/core/pathfinding/algorithms/ConnectedComponents.ts`:
- Around line 6-7: `LAND_MARKER_WIDE` is reserved and must not be assigned to a
real component id in `ConnectedComponents`. Update the `nextId` allocation path
and any `Uint16Array` promotion logic so the code either widens storage before
`0xffff` is reached or skips that value entirely, and make sure the identifiers
used by the connected-components labeling flow never collide with
`LAND_MARKER`/`LAND_MARKER_WIDE`.
🪄 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: a5fd9310-4b9f-4295-8b23-89c9ce177aad
📒 Files selected for processing (2)
src/core/execution/nation/NationStructureBehavior.tssrc/core/pathfinding/algorithms/ConnectedComponents.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/core/execution/nation/NationStructureBehavior.ts
…AND_MARKER collision
Description:
AI nations were placing ports on small decorative ponds scattered across maps (Missisipi for example), wasting structure slots on strategically useless water bodies. This fix adds a water component size check to the port placement logic so the AI skips lakes that are too small for meaningful port use. We already had a check for available trade partners, but trading in small lakes is usually stupid.
How it works:
ConnectedComponentsnow tracks component sizes during its existing flood-fill (zero extra cost - counts tiles as they're visited)AbstractGraph,WaterManager, and theGameinterface exposegetWaterComponentSize(tile)so callers can query the size of any water bodyNationStructureBehavior.randCoastalTileArray()filters out non-ocean water components belowMIN_PORT_WATER_COMPONENT_SIZE(3000 minimap tiles, ~12000 full-map tiles)Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
FloPinguin