fix(ai): prevent defense post placement failures from blocking bot economic construction - #4924
Conversation
…onomic construction
WalkthroughDefense-post placement no longer blocks normal structure handling when construction fails. A test verifies that ChangesNation structure handling
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
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 |
github-actions
Bot
added
the
small-fix
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@tests/NationStructureBehavior.test.ts`:
- Around line 524-545: The test should exercise handleStructures() through
setup() using a real map from tests/testdata/maps/ and real game/player
instances. Configure the scenario so tryBuildDefensePost() fails, remove the
doHandleStructures() mock, and assert that normal handling creates a real
structure while preserving the intended defense-post failure conditions.
🪄 Autofix
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 Plus
Run ID: e6e3f5ae-4e57-4796-9f7f-31e85cd7c8a9
📒 Files selected for processing (2)
src/core/execution/nation/NationStructureBehavior.tstests/NationStructureBehavior.test.ts
💤 Files with no reviewable changes (1)
- src/core/execution/nation/NationStructureBehavior.ts
| const game = { | ||
| config: () => ({ | ||
| gameConfig: () => ({ difficulty: Difficulty.Hard }), | ||
| isUnitDisabled: () => false, | ||
| startingGold: () => 0n, | ||
| }), | ||
| unitInfo: () => ({ cost: () => 0n }), | ||
| ticks: () => 100, | ||
| }; | ||
| const player = { | ||
| gold: () => 0n, | ||
| troops: () => 1000, | ||
| incomingAttacks: () => [{ troops: () => 1000, sourceTile: () => null, attacker: () => ({ id: () => "a" }) }], | ||
| }; | ||
| const behavior = makeBehavior(game, player); | ||
| (behavior as any).placementsCount = 1; | ||
| vi.spyOn(behavior as any, "tryBuildDefensePost").mockReturnValue(false); | ||
| const doHandleSpy = vi.spyOn(behavior as any, "doHandleStructures").mockReturnValue(true); | ||
|
|
||
| const result = behavior.handleStructures(); | ||
| expect(result).toBe(true); | ||
| expect(doHandleSpy).toHaveBeenCalled(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 6 '\bsetup\s*\(' tests --glob '*.ts'
rg -n -C 8 '\b(handleStructures|tryBuildDefensePost|doHandleStructures)\b' \
tests/NationStructureBehavior.test.ts \
src/core/execution/nation/NationStructureBehavior.tsRepository: openfrontio/OpenFrontIO
Length of output: 50380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file snippets =="
sed -n '500,560p' tests/NationStructureBehavior.test.ts | nl -ba -v500
sed -n '1,260p' src/core/execution/nation/NationStructureBehavior.ts | nl -ba -v1
echo "== direct tests for mockReturnValue around target =="
rg -n -C 4 'vi\.spyOn\(behavior as any, "doHandleStructures"\)\.mockReturnValue\(true\)|doHandleStructures' tests/NationStructureBehavior.test.ts src/core/execution/nation/NationStructureBehavior.tsRepository: openfrontio/OpenFrontIO
Length of output: 221
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file snippets =="
sed -n '1,60p' src/core/execution/nation/NationStructureBehavior.ts
sed -n '500,560p' tests/NationStructureBehavior.test.ts
echo "== relevant symbols =="
rg -n -C 6 'handleStructures|tryBuildDefensePost|doHandleStructures' \
tests/NationStructureBehavior.test.ts \
src/core/execution/nation/NationStructureBehavior.ts
echo "== imports around test =="
sed -n '1,50p' tests/NationStructureBehavior.test.tsRepository: openfrontio/OpenFrontIO
Length of output: 23565
Run handleStructures() through map-backed simulation.
handleStructures() delegates normal construction to doHandleStructures(), but this test mocks that method to return true after a fake defense-post failure. Use setup() from tests/util/Setup.ts with a map from tests/testdata/maps/, use a real player and game instance, let defense-post placement fail for the scenario, then assert that normal structure handling creates a real structure without mocking doHandleStructures().
🤖 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 `@tests/NationStructureBehavior.test.ts` around lines 524 - 545, The test
should exercise handleStructures() through setup() using a real map from
tests/testdata/maps/ and real game/player instances. Configure the scenario so
tryBuildDefensePost() fails, remove the doHandleStructures() mock, and assert
that normal handling creates a real structure while preserving the intended
defense-post failure conditions.
Sources: Coding guidelines, Learnings
github-project-automation
Bot
moved this from Triage
to Development
in OpenFront Release Management
1 participant
PR 5:
fix(ai): prevent defense post placement failures from blocking bot economic constructionDescription:
In
NationStructureBehavior.ts, when a bot nation is under land attack (defensePostNeeded() === true),handleStructures()attempts to build a defense post viatryBuildDefensePost().Previously, if
tryBuildDefensePost()failed (e.g. because the nation could not afford a defense post or no valid tile was found near the front), the method executedif (this.defensePostNeeded()) return false;. This early exit blocked all subsequent economic structure building (cities, factories, ports, SAM launchers) for as long as the attack persisted, completely paralyzing the bot's economy during sustained sieges.This PR removes the early return on defense post placement failure so that when a defense post cannot be placed, the bot AI gracefully falls through to handle regular structure construction and upgrades instead of stalling.
Please complete the following:
tests/NationStructureBehavior.test.ts)Please put your Discord username so you can be contacted if a bug or regression is found:
barfires