Nations rarely sent boats, now they do! #2161
Conversation
WalkthroughPrefers immediate-border enemies for attack/alliance decisions, adds nearest-island enemy fallback, and updates boat targeting to try high-interest tiles first; changes include signature updates (BotBehavior.selectEnemy, randomBoatTarget) and guards to avoid targeting much larger players. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant FE as FakeHumanExecution
participant BB as BotBehavior
participant MAP as Map
rect rgba(230,245,255,0.6)
note over FE: compute borderingEnemies from borderTiles
FE->>MAP: query borderTiles
FE->>FE: build borderingEnemies
alt borderingEnemies non-empty
FE->>BB: selectEnemy(borderingEnemies)
BB-->>FE: targetPlayer
else no borderingEnemies
FE->>BB: selectNearestIslandEnemy()
BB-->>FE: targetPlayer or null
end
end
rect rgba(245,235,245,0.6)
note over FE: Boat dispatch — high-interest-first
FE->>FE: sendBoatRandomly(highInterestOnly=true)
alt high-interest target found
FE->>MAP: dispatch boat -> high-interest tile
else no high-interest
FE->>FE: sendBoatRandomly(highInterestOnly=false)
opt normal target found
FE->>MAP: dispatch boat -> normal tile
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas needing extra attention:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
This pull request is stale because it has been open for 14 days with no activity. If you want to keep this pull request open, add a comment or update the branch. |
github-actions
bot
added
the
Stale
|
@FloPinguin could you resolve conflicts? |
TheGiraffe3
removed
the
Stale
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/core/execution/utils/BotBehavior.ts (1)
201-269: Border candidates: dedupe and filter before picking.borderingEnemies can include duplicates (same player via many tiles) and friendly/untargetable players. This biases selection and wastes cycles.
Refine selection to unique, legal candidates before choosing weakest/random.
Apply this minimal change:
- // Select the weakest player - if (this.enemy === null && borderingEnemies.length > 0) { - this.setNewEnemy(borderingEnemies[0]); - } + // Select the weakest viable border enemy + if (this.enemy === null && borderingEnemies.length > 0) { + const unique = Array.from(new Set(borderingEnemies)); + const legal = unique.filter( + (p) => !this.player.isFriendly(p) && this.player.canTarget(p), + ); + if (legal.length > 0) this.setNewEnemy(legal[0]); + } - // Select a random player - if (this.enemy === null && borderingEnemies.length > 0) { - this.setNewEnemy(this.random.randElement(borderingEnemies)); - } + // Select a random viable border enemy + if (this.enemy === null && borderingEnemies.length > 0) { + const unique = Array.from(new Set(borderingEnemies)); + const legal = unique.filter( + (p) => !this.player.isFriendly(p) && this.player.canTarget(p), + ); + if (legal.length > 0) this.setNewEnemy(this.random.randElement(legal)); + }src/core/execution/FakeHumanExecution.ts (3)
209-221: Dedupe owners before sorting; keep behavior identical but fairer and faster.enemyborder can map many tiles to the same player. Dedupe before filtering/sorting to avoid tile-weighting the RNG and redundant work.
Apply:
- const borderPlayers = enemyborder.map((t) => - this.mg.playerBySmallID(this.mg.ownerID(t)), - ); + const borderPlayers = Array.from( + new Set( + enemyborder.map((t) => this.mg.playerBySmallID(this.mg.ownerID(t))), + ), + ); if (borderPlayers.some((o) => !o.isPlayer())) { this.behavior.sendAttack(this.mg.terraNullius()); return; } - borderingEnemies = borderPlayers - .filter((o) => o.isPlayer()) - .sort((a, b) => a.troops() - b.troops()); + borderingEnemies = Array.from( + new Set(borderPlayers.filter((o): o is Player => o.isPlayer())), + ).sort((a, b) => a.troops() - b.troops());Optional: if you want to attempt both expansion and enemy logic when both exist, do not early-return after TerraNullius; instead prioritize expansion this tick with a chance gate.
591-611: Floor boat troop amount; avoid fractional troops.Current code sends this.player.troops() / 5 which is a float. Safest to send an integer.
Apply:
- this.mg.addExecution( + const sendTroops = Math.floor(this.player.troops() / 5); + if (sendTroops < 1) return; + this.mg.addExecution( new TransportShipExecution( this.player, this.mg.owner(dst).id(), dst, - this.player.troops() / 5, + sendTroops, null, ), );
640-679: Small readability tweaks in randomBoatTarget; behavior unchanged.Tighten early continues and make intent clear; keep guards that avoid huge targets.
Apply:
- const owner = this.mg.owner(randTile); - if (owner === this.player) { - continue; - } - // Don't spam boats into players that are more than twice as large as us - if (owner.isPlayer() && owner.troops() > this.player.troops() * 2) { - continue; - } + const owner = this.mg.owner(randTile); + if (owner === this.player) continue; + // Avoid overwhelming enemies (>2x our troops) + if (owner.isPlayer() && owner.troops() > this.player.troops() * 2) + continue; // High-interest targeting: prioritize unowned tiles or tiles owned by bots if (highInterestOnly) { if (!owner.isPlayer() || owner.type() === PlayerType.Bot) { return randTile; } } else { // Normal targeting: return unowned tiles or tiles owned by non-friendly players if (!owner.isPlayer() || !owner.isFriendly(this.player)) { return randTile; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/core/execution/FakeHumanExecution.ts(4 hunks)src/core/execution/utils/BotBehavior.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-23T07:48:19.060Z
Learnt from: ElMelchizedek
PR: openfrontio/OpenFrontIO#1876
File: src/core/execution/FakeHumanExecution.ts:470-473
Timestamp: 2025-08-23T07:48:19.060Z
Learning: In FakeHumanExecution.ts DefensePost placement logic, returning -Infinity from structureSpawnTileValue when no sampled border tiles neighbor enemies is intentional. The logic samples up to 50 border tiles as a heuristic - if none are adjacent to enemies, it assumes DefensePost placement is unnecessary and aborts the entire placement attempt rather than continuing to evaluate individual tiles.
Applied to files:
src/core/execution/FakeHumanExecution.ts
📚 Learning: 2025-10-18T17:54:01.311Z
Learnt from: sambokai
PR: openfrontio/OpenFrontIO#2225
File: src/core/execution/FakeHumanExecution.ts:172-173
Timestamp: 2025-10-18T17:54:01.311Z
Learning: In src/core/execution/FakeHumanExecution.ts, MIRVs and ground attacks should not be mutually exclusive. The considerMIRV() method should not short-circuit maybeAttack() - both actions can occur in the same tick.
Applied to files:
src/core/execution/FakeHumanExecution.ts
🧬 Code graph analysis (2)
src/core/execution/utils/BotBehavior.ts (3)
src/core/game/Game.ts (1)
Player(523-657)src/core/game/GameImpl.ts (1)
allPlayers(406-408)src/core/Util.ts (1)
calculateBoundingBoxCenter(146-155)
src/core/execution/FakeHumanExecution.ts (4)
src/core/game/Game.ts (1)
Player(523-657)src/core/execution/alliance/AllianceRequestExecution.ts (1)
AllianceRequestExecution(9-72)src/core/game/GameMap.ts (1)
TileRef(3-3)src/core/game/GameImpl.ts (1)
owner(165-167)
…vent crashes; filter candidates with territory.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/core/execution/utils/BotBehavior.ts (1)
271-322: Good implementation with guards against edge cases.The method correctly guards against empty borders (lines 272-273) and filters out dead players or those without territory (lines 277-278). The distance-based selection logic is sound.
Optional: Consider using
randElementfor cleaner nearest/second-nearest selection.At lines 311-316, the current approach using
chance(2)works but could be more uniform. Apply this diff for cleaner code:- // Select the nearest or second-nearest enemy (So our boat doesn't always run into the same warship, if there is one) - let selectedEnemy: Player | null; - if (sortedPlayers.length > 1 && this.random.chance(2)) { - selectedEnemy = sortedPlayers[1].player; - } else { - selectedEnemy = sortedPlayers[0].player; - } + // Select the nearest or second-nearest enemy (So our boat doesn't always run into the same warship, if there is one) + const top = sortedPlayers.slice(0, Math.min(2, sortedPlayers.length)); + const selectedEnemy = this.random.randElement(top).player;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/core/execution/utils/BotBehavior.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/core/execution/utils/BotBehavior.ts (2)
src/core/game/Game.ts (1)
Player(523-657)src/core/Util.ts (1)
calculateBoundingBoxCenter(146-155)
🔇 Additional comments (3)
src/core/execution/utils/BotBehavior.ts (3)
12-12: LGTM!The import of
calculateBoundingBoxCenteris necessary for the new island enemy selection logic.
252-264: LGTM!The logic correctly tries weakest bordering enemy first, then random bordering enemy, then falls back to island-based targeting when isolated. This addresses the PR objective of nations sending more boats.
201-201: Parameter rename verified and consistent.All call sites of
selectEnemyhave been updated correctly. The method now receivesborderingEnemiesas the parameter, and the single call site inFakeHumanExecution.ts(line 234) passes the correct filtered list. The rename successfully clarifies the method's intent without breaking the implementation.
|
@TheGiraffe3 I resolved the conflict, all checks passed now |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/core/execution/utils/BotBehavior.ts (1)
272-330: Consider simplifying the random selection logic.The implementation correctly guards against empty borders and leverages precomputed bounding boxes. The distance-based targeting makes sense for cross-island attacks.
The random selection logic (lines 319-324) works but could be cleaner. Additionally, the null check on line 326 is redundant since
sortedPlayers[0]always exists when we reach that point.Consider this refactor for cleaner uniform randomness:
- // Select the nearest or second-nearest enemy (So our boat doesn't always run into the same warship, if there is one) - let selectedEnemy: Player | null; - if (sortedPlayers.length > 1 && this.random.chance(2)) { - selectedEnemy = sortedPlayers[1].player; - } else { - selectedEnemy = sortedPlayers[0].player; - } - - if (selectedEnemy !== null) { - this.setNewEnemy(selectedEnemy); - } + // Select the nearest or second-nearest enemy (So our boat doesn't always run into the same warship, if there is one) + const top = sortedPlayers.slice(0, Math.min(2, sortedPlayers.length)); + const selectedEnemy = this.random.randElement(top).player; + this.setNewEnemy(selectedEnemy);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/core/execution/utils/BotBehavior.ts(4 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-08-23T07:48:19.060Z
Learnt from: ElMelchizedek
PR: openfrontio/OpenFrontIO#1876
File: src/core/execution/FakeHumanExecution.ts:470-473
Timestamp: 2025-08-23T07:48:19.060Z
Learning: In FakeHumanExecution.ts DefensePost placement logic, returning -Infinity from structureSpawnTileValue when no sampled border tiles neighbor enemies is intentional. The logic samples up to 50 border tiles as a heuristic - if none are adjacent to enemies, it assumes DefensePost placement is unnecessary and aborts the entire placement attempt rather than continuing to evaluate individual tiles.
Applied to files:
src/core/execution/utils/BotBehavior.ts
📚 Learning: 2025-10-27T09:47:26.395Z
Learnt from: sambokai
PR: openfrontio/OpenFrontIO#2225
File: src/core/execution/FakeHumanExecution.ts:770-795
Timestamp: 2025-10-27T09:47:26.395Z
Learning: In src/core/execution/FakeHumanExecution.ts, the selectSteamrollStopTarget() method intentionally allows MIRV targeting when secondHighest city count is 0 (e.g., nuclear endgame scenarios where structures are destroyed). This is valid game design—"if you can afford it, you're good to go"—and should not be flagged as requiring a guard condition.
Applied to files:
src/core/execution/utils/BotBehavior.ts
📚 Learning: 2025-10-08T17:14:49.369Z
Learnt from: Foorack
PR: openfrontio/OpenFrontIO#2141
File: src/client/ClientGameRunner.ts:228-234
Timestamp: 2025-10-08T17:14:49.369Z
Learning: For the window close confirmation feature in `ClientGameRunner.ts`, the troop count requirement (>10,000 troops) from issue #2137 was intentionally removed because it was arbitrary and troop count can be reported as low despite having significant land. The confirmation now triggers for any alive player regardless of troop count.
Applied to files:
src/core/execution/utils/BotBehavior.ts
📚 Learning: 2025-10-18T17:54:01.311Z
Learnt from: sambokai
PR: openfrontio/OpenFrontIO#2225
File: src/core/execution/FakeHumanExecution.ts:172-173
Timestamp: 2025-10-18T17:54:01.311Z
Learning: In src/core/execution/FakeHumanExecution.ts, MIRVs and ground attacks should not be mutually exclusive. The considerMIRV() method should not short-circuit maybeAttack() - both actions can occur in the same tick.
Applied to files:
src/core/execution/utils/BotBehavior.ts
📚 Learning: 2025-10-20T20:15:28.858Z
Learnt from: sambokai
PR: openfrontio/OpenFrontIO#2225
File: src/core/execution/FakeHumanExecution.ts:51-51
Timestamp: 2025-10-20T20:15:28.858Z
Learning: In src/core/execution/FakeHumanExecution.ts, game balance constants like MIRV_COOLDOWN_TICKS, MIRV_HESITATION_ODDS, VICTORY_DENIAL_TEAM_THRESHOLD, VICTORY_DENIAL_INDIVIDUAL_THRESHOLD, and STEAMROLL_CITY_GAP_MULTIPLIER are experimental tuning parameters subject to frequent change during balance testing. Do not flag changes to these values as issues or compare them against previous values.
Applied to files:
src/core/execution/utils/BotBehavior.ts
🧬 Code graph analysis (1)
src/core/execution/utils/BotBehavior.ts (2)
src/core/game/Game.ts (2)
Player(521-657)Cell(309-331)src/core/Util.ts (1)
calculateBoundingBoxCenter(146-155)
🔇 Additional comments (3)
src/core/execution/utils/BotBehavior.ts (3)
3-3: LGTM: Imports support the new island-targeting logic.The
CellandcalculateBoundingBoxCenterimports are appropriate for computing centers and distances in the newselectNearestIslandEnemymethod.Also applies to: 13-13
202-270: LGTM: Parameter rename clarifies intent, island fallback improves targeting.The rename from
enemiestoborderingEnemiesmakes the method's expectations explicit. The fallback toselectNearestIslandEnemy()when no bordering enemies exist directly addresses the PR objective of improving boat-sending behavior for island-isolated nations.
332-338: LGTM: Helper avoids recomputation when bounding box is precomputed.The
boundingBoxCenterhelper complementscalculateBoundingBoxCenterfrom Util by working directly with precomputed bounding boxes. This avoids redundant tile iteration whenlargestClusterBoundingBoxis available.
3 participants
Description:
Nations rarely sent random boats. Now they are sending twice as many.
It feels right now, not too many and not too few random boats.
To make sure that small island nations with, for example, 10k troops don't repeatedly spam boats into an nation with 1.5M troops (that makes no sense), they no longer send boats to opponents which have more than twice the amount of troops.
8 Boats activeonly 2 Boats activemuch more boatsonly 4 boats activeThere was a bug in the random boat sending.
It did not check if the target is the player himself.
That caused console warnings and a reduced amount of boat-sending.
The Hiding-Strategy on small islands on the impossible difficulty is now harder!
Because the random-boat-sending-method preferred large landmasses instead of small islands (it randomly selects a valid tile), human-players could easily hide on them, play a warship-infestation-strat and nearly NEVER get boat-attacked by nations.
I implemented that the random boat functionality now searches for bots and untaken tiles before searching for nations / humans. That way, they will try to take some of these tiny islands before the human-player does. Also its cool to see nations playing on the entire available land-tiles instead of just the bigger landmasses.
Fixes #1916 (Please check this issue for screenshots and more info)
only 5 islands not taken!19 islands not taken!all islands taken!8 islands not taken!Nations now boat-attack other nations more often!
If there are two nations on two very large landmasses, which are divided by water, they nearly NEVER attacked each other.
Most attack-functionality relies on the enemies sharing a border. If they don't have one, the only possible attack-mechanism is the random-boat-sending. But for very large landmasses (=> very large number of coastal tiles) it can take a long, long time before it randomly selects an enemy-tile. With the bug I described above ("did not check if the target is the player himself") it took even longer. And on the world-map, the nations have to go over iceland, this is also very unlikely.
So I implemented the method selectNearestIslandEnemy, which specifically doesn't cares about borders. It makes sure that a nation always has someone to attack. This method only gets used as a fallback, and only if a nation has no borders with anybody.
Fixes #1916 (Please check this issue for screenshots and more info)
China won and is in the process of killing the last enemyUK finally won. There are still 14 players on the map...Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
FloPinguin