Skip to content

Prevent AI from placing ports on small lakes 🚢#4429

Open
FloPinguin wants to merge 4 commits into
mainfrom
fix/ai-port-on-tiny-lakes
Open

Prevent AI from placing ports on small lakes 🚢#4429
FloPinguin wants to merge 4 commits into
mainfrom
fix/ai-port-on-tiny-lakes

Conversation

@FloPinguin

Copy link
Copy Markdown
Contributor

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:

  • ConnectedComponents now tracks component sizes during its existing flood-fill (zero extra cost - counts tiles as they're visited)
  • AbstractGraph, WaterManager, and the Game interface expose getWaterComponentSize(tile) so callers can query the size of any water body
  • NationStructureBehavior.randCoastalTileArray() filters out non-ocean water components below MIN_PORT_WATER_COMPONENT_SIZE (3000 minimap tiles, ~12000 full-map tiles)
  • Ocean tiles bypass the check entirely since they're always large enough

Please complete the following:

  • I have added screenshots for all UI updates
  • I process any text displayed to the user through translateText() and I've added it to the en.json file
  • I have added relevant tests to the test directory

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

FloPinguin

@coderabbitai

coderabbitai Bot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c6233565-84a8-4d94-a586-406169e20533

📥 Commits

Reviewing files that changed from the base of the PR and between 751c0ff and 07c3c67.

📒 Files selected for processing (2)
  • src/core/pathfinding/algorithms/ConnectedComponents.ts
  • tests/core/pathfinding/WaterComponents.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • tests/core/pathfinding/WaterComponents.test.ts
  • src/core/pathfinding/algorithms/ConnectedComponents.ts

Walkthrough

Tracks water component sizes, exposes the size through game APIs, and uses it to block port placement near small water bodies.

Changes

Water Component Size for Port Filtering

Layer / File(s) Summary
Component size tracking in ConnectedComponents
src/core/pathfinding/algorithms/ConnectedComponents.ts, tests/core/pathfinding/WaterComponents.test.ts
Adds per-component size storage, updates land marker handling for the widened buffer, accumulates span sizes during flood fill, and exposes getComponentSize(). Tests cover valid and invalid component sizes.
Water size API through graph and game
src/core/pathfinding/algorithms/AbstractGraph.ts, src/core/game/WaterManager.ts, src/core/game/Game.ts, src/core/game/GameImpl.ts
Adds getComponentSize() on AbstractGraph, getWaterComponentSize() on WaterManager, and forwards the new method through Game and GameImpl.
Port placement size check
src/core/execution/nation/NationStructureBehavior.ts
Adds a minimum water component size constant and updates randCoastalTileArray to skip coastal tiles near small water bodies on non-Easy difficulties.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🌊 Small bays now get a careful count,
Each water shape must measure up.
A tiny port can’t claim the shore,
If the sea is small, it’s shown the door.
The flood fill hums, the tiles add up,
And coastal picks avoid the cup.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the main change: blocking AI port placement on small lakes.
Description check ✅ Passed The description accurately summarizes the water-component size check and related API changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

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

@coderabbitai coderabbitai Bot left a comment

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.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 6b95a23 and 518cd3f.

📒 Files selected for processing (7)
  • src/core/execution/nation/NationStructureBehavior.ts
  • src/core/game/Game.ts
  • src/core/game/GameImpl.ts
  • src/core/game/WaterManager.ts
  • src/core/pathfinding/algorithms/AbstractGraph.ts
  • src/core/pathfinding/algorithms/ConnectedComponents.ts
  • tests/core/pathfinding/WaterComponents.test.ts

Comment thread src/core/execution/nation/NationStructureBehavior.ts
Comment thread src/core/pathfinding/algorithms/ConnectedComponents.ts
@github-project-automation github-project-automation Bot moved this from Triage to Development in OpenFront Release Management Jun 27, 2026

@coderabbitai coderabbitai Bot left a comment

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.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 518cd3f and ddea7f9.

📒 Files selected for processing (2)
  • src/core/execution/nation/NationStructureBehavior.ts
  • src/core/pathfinding/algorithms/ConnectedComponents.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/core/execution/nation/NationStructureBehavior.ts

Comment thread src/core/pathfinding/algorithms/ConnectedComponents.ts
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Development

Development

Successfully merging this pull request may close these issues.

1 participant