Fix parallel bot boat attacks of nations 🐛

[FloPinguin]

Description:

When a nation attacked multiple bots via boat attacks in parallel, each boat attack computed its troop allocation independently using player.troops() / 5 without subtracting botAttackTroopsSent. The cumulative troop commitment could exceed the nation's actual troop count, and when the queued AttackExecutions ran init(), they drained the nation to zero.

Planetary Realignment found this bug by accident, here Russia has only 39 troops:

The land attack path already handled this correctly. The bug in sendBoatAttack was introduced by #3786, which made nations see and attack enemies across rivers via boats, and changed attackBots() from .neighbors() to .nearby().

So the bug was on prod for the entirety of v31.

This fix extracts the shared attack troop calculation (reserve, bot-aware allocation, troopSendCap, isAttackTooWeak, emoji) into a new calculateAttackTroops method, with a callback for the non-bot troop default (land: player.troops() - targetTroops, boat: player.troops() / 5). Bot targets in both paths now go through the same reserve-aware calculation.

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]

Walkthrough

A new private helper calculateAttackTroops(target, nonBotTroops) is added to AiAttackBehavior, centralizing the neighbor troop threshold cap and "attack too weak" null-return logic. sendLandAttack and sendBoatAttack both delegate their troop-sizing decisions to this helper, removing duplicated inline checks.

Changes

Attack Troop Calculation Centralization

Layer / File(s)	Summary
calculateAttackTroops helper signature src/core/execution/utils/AiAttackBehavior.ts	Declares the new private method with a nonBotTroops callback parameter, returning number | null.
Helper body and sendLandAttack delegation src/core/execution/utils/AiAttackBehavior.ts	Implements bot/non-bot branching, neighbor-cap clamping, and attack-too-weak null return inside the helper; rewrites sendLandAttack to call it and early-return false on null.
sendBoatAttack delegation src/core/execution/utils/AiAttackBehavior.ts	Passes this.player.troops() / 5 as the non-bot baseline to calculateAttackTroops, aborting on null and removing the previous standalone branching and local checks.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• openfrontio/OpenFrontIO#4278: Modifies the same AiAttackBehavior.ts land/boat attack troop computation, directly overlapping with the neighbor-cap and retaliation gating logic this PR centralizes.
• openfrontio/OpenFrontIO#4239: Introduced the "attack too weak (<20%)" and neighbor-cap logic in sendLandAttack/sendBoatAttack that this PR reorganizes into calculateAttackTroops.
• openfrontio/OpenFrontIO#4265: Changes troopSendCap/isAttackTooWeak exemptions for team games, the same checks this PR unifies into the new helper.

Suggested reviewers

• evanpelle

Poem

A helper arrives, small and neat,
calculateAttackTroops takes a seat.
Land and boat both call the same gate,
No more copy-paste, no duplicate fate.
One check for weak, one cap for all — ⚔️
Clean TypeScript wins, one function to rule them all!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main bug fix: parallel bot boat attacks causing incorrect troop allocation, which matches the core changeset.
Description check	✅ Passed	The description is directly related to the changeset, explaining the bug, its root cause, and how the fix implements a unified troop calculation method.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed due to a network error.

---

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 and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/core/execution/utils/AiAttackBehavior.ts (1)

914-961: ⚠️ Potential issue | 🟠 Major

Add a regression test for the parallel bot boat attacks bug.

Per coding guidelines, all changes to src/core/ must include tests. While existing tests cover troop cap and weak attack logic, there is no test for the specific bug fixed in this change: parallel boat attacks against bots should not drain troops to zero. Add a test that verifies botAttackTroopsSent is properly tracked across multiple concurrent bot attacks to prevent troops from being exhausted.

🤖 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/execution/utils/AiAttackBehavior.ts` around lines 914 - 961, Add a
regression test for the parallel bot boat attacks bug in the AiAttackBehavior
class. The test should verify that when calculateAttackTroops is called multiple
times in parallel against bot targets, the botAttackTroopsSent counter is
properly tracked and deducted from available troops, ensuring that troops are
not drained to zero across multiple concurrent attacks. The test should simulate
at least two concurrent attack scenarios and assert that the remaining available
troops are correctly calculated after accounting for all sent troops.

Source: Coding guidelines

🧹 Nitpick comments (1)

src/core/execution/utils/AiAttackBehavior.ts (1)

140-153: 💤 Low value

Consider: attackWithRandomBoat uses different troop calculation.

This method still calculates troops with this.player.troops() / 5 and the troopSendCap() directly, bypassing calculateAttackTroops. Since attackWithRandomBoat is a single-shot action (not parallel bot attacks), this may be fine. But if consistency is desired, this path could also delegate to the helper.

Not blocking since the PR's bug (parallel bot boat attacks) happens through sendBoatAttack, which is now fixed.

🤖 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/execution/utils/AiAttackBehavior.ts` around lines 140 - 153,
Consider refactoring the `attackWithRandomBoat` method to use the
`calculateAttackTroops` helper method for consistency with other attack methods
like `sendBoatAttack`. Currently, the troop calculation is done inline using
`Math.min(this.player.troops() / 5, this.troopSendCap())`, but other attack
methods delegate this logic to `calculateAttackTroops`. Replace the inline
calculation with a call to `calculateAttackTroops` to maintain consistent troop
calculation across all attack methods in the class.

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

Outside diff comments:
In `@src/core/execution/utils/AiAttackBehavior.ts`:
- Around line 914-961: Add a regression test for the parallel bot boat attacks
bug in the AiAttackBehavior class. The test should verify that when
calculateAttackTroops is called multiple times in parallel against bot targets,
the botAttackTroopsSent counter is properly tracked and deducted from available
troops, ensuring that troops are not drained to zero across multiple concurrent
attacks. The test should simulate at least two concurrent attack scenarios and
assert that the remaining available troops are correctly calculated after
accounting for all sent troops.

---

Nitpick comments:
In `@src/core/execution/utils/AiAttackBehavior.ts`:
- Around line 140-153: Consider refactoring the `attackWithRandomBoat` method to
use the `calculateAttackTroops` helper method for consistency with other attack
methods like `sendBoatAttack`. Currently, the troop calculation is done inline
using `Math.min(this.player.troops() / 5, this.troopSendCap())`, but other
attack methods delegate this logic to `calculateAttackTroops`. Replace the
inline calculation with a call to `calculateAttackTroops` to maintain consistent
troop calculation across all attack methods in the class.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0c4ee478-0b9e-438b-8da7-78844cbb2cba

📥 Commits

Reviewing files that changed from the base of the PR and between 5be72db and 8a240c9.

📒 Files selected for processing (1)

• src/core/execution/utils/AiAttackBehavior.ts