bugfix: Nations rarely launch nukes #1860
Conversation
|
|
WalkthroughDelegates enemy selection to BotBehavior.selectEnemy(enemies), adds difficulty- and relationship-aware attack gating, forces retaliation to largest attacker, removes early return after alliance requests so tick processing continues, narrows trigger/reserve/expand ratios, and exposes EMOJI_HECKLE. Changes
Sequence Diagram(s)sequenceDiagram
actor Tick as GameTick
participant F as FakeHumanExecution
participant B as BotBehavior
participant G as GameEngine
Tick->>F: runTick()
F->>F: maybeSendAllianceRequest()
F->>F: maybeForgetOldEnemies() / maybeAssistAllies()
F->>B: selectEnemy(enemies)
B-->>F: Player | null
alt enemy found
F->>F: maybeSendEmoji() / maybeSendNuke()
alt sharesBorder
F->>G: issueAttack(target)
else
F->>G: issueBoatAttack(target)
end
else no enemy
F->>G: no attack
end
sequenceDiagram
participant Att as IncomingAttacks
participant B as BotBehavior
Att->>B: report attackers
B->>B: largestAttacker = max(attackers)
B->>B: setNewEnemy(largestAttacker, true)
B-->>Att: enemy updated (forced)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Assessment against linked issues
Out-of-scope changes
Possibly related PRs
Suggested labels
Suggested reviewers
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
scottanderson
added
Balance Tweak
scottanderson
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/core/execution/utils/BotBehavior.ts (1)
175-231: EnsureselectEnemysafely handles empty enemy listsThe final fallback in
selectEnemydoesthis.random.randElement(enemies)without checkingenemies.length, which can throw if the array is empty. Add a guard so that an empty array immediately returnsnull.• In src/core/execution/utils/BotBehavior.ts, at the top of
selectEnemy(enemies: Player[]):selectEnemy(enemies: Player[]): Player | null { + if (enemies.length === 0) return null; if (this.enemy === null) { // …existing logic… } }
🧹 Nitpick comments (6)
src/core/execution/FakeHumanExecution.ts (2)
186-197: Optionally dedupe enemies before sorting to avoid overweighting duplicatesMultiple border tiles owned by the same player will bias selection toward that player (intentional weighting or not). If you want a fair “weakest neighbor” choice, dedupe by player ID before sorting.
Apply this diff to deduplicate players by smallID and then sort by troops:
- const borderPlayers = enemyborder.map((t) => - this.mg.playerBySmallID(this.mg.ownerID(t)), - ); + const enemiesByID = new Map<number, Player>(); + for (const t of enemyborder) { + const owner = this.mg.playerBySmallID(this.mg.ownerID(t)); + if (owner.isPlayer()) { + enemiesByID.set(owner.smallID(), owner); + } + } + const borderPlayers = Array.from(enemiesByID.values()); @@ - const enemies = borderPlayers - .filter((o) => o.isPlayer()) - .sort((a, b) => a.troops() - b.troops()); + const enemies = borderPlayers.sort((a, b) => a.troops() - b.troops());
206-217: Avoid coupling BotBehavior to caller’s sort orderBotBehavior.selectEnemy currently assumes the input is sorted when it picks enemies[0] as the weakest. This is brittle if future callers pass an unsorted list. Consider letting BotBehavior compute the weakest internally (see my comment and diff on BotBehavior.ts lines 223-230).
src/core/execution/utils/BotBehavior.ts (4)
75-91: Remove unreachable null-check; simplify shouldAttackplayer is a required constructor dependency and not nullable. The null guard adds noise and an impossible branch.
Apply this diff:
- private shouldAttack(other: Player): boolean { - if (this.player === null) throw new Error("not initialized"); + private shouldAttack(other: Player): boolean {
175-231: Make selectEnemy robust to input order and empty listsTwo improvements:
- Don’t assume callers pass a sorted enemies list; compute the weakest inside BotBehavior.
- Guard against an empty enemies array to avoid randElement() on [] if a future caller passes no options.
Apply this diff:
selectEnemy(enemies: Player[]): Player | null { + // Defensive: avoid picking from an empty list + if (enemies.length === 0) { + return this.enemySanityCheck(); + } @@ - if (this.enemy === null && enemies.length > 0) { - this.setNewEnemy(enemies[0]); - } + if (this.enemy === null) { + // Pick the actual weakest regardless of incoming order + const weakest = enemies.reduce( + (min, p) => (p.troops() < min.troops() ? p : min), + enemies[0], + ); + this.setNewEnemy(weakest); + } @@ - if (this.enemy === null) { - this.setNewEnemy(this.random.randElement(enemies)); - } + if (this.enemy === null && enemies.length > 0) { + this.setNewEnemy(this.random.randElement(enemies)); + }
17-31: Initialize enemyUpdated to avoid NaN math and strict-init warningsenemyUpdated is used in forgetOldEnemies but isn’t initialized. While harmless at runtime (NaN comparison => false), it can trip strictPropertyInitialization. Initialize to 0 for clarity.
Outside this hunk, set a default:
private enemyUpdated: Tick = 0;
64-67: Nit: name this helper sendEmoji for clarityemoji(player, emoji) reads oddly and shadows the parameter name. sendEmoji(player, emojiIndex) is clearer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/core/execution/FakeHumanExecution.ts(1 hunks)src/core/execution/utils/BotBehavior.ts(6 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/core/execution/utils/BotBehavior.ts (1)
src/core/game/Game.ts (1)
Player(498-632)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Deploy to openfront.dev
🔇 Additional comments (4)
src/core/execution/FakeHumanExecution.ts (1)
206-214: Good delegation: enemy selection now belongs in BotBehaviorPassing the precomputed border-enemies into BotBehavior keeps this class focused on execution. Nice move toward single responsibility.
src/core/execution/utils/BotBehavior.ts (3)
69-73: Enemy assignment gate looks solidsetNewEnemy(newEnemy, force) now respects shouldAttack unless forced (e.g., retaliation). Clear and composable.
93-110: Difficulty-aware “don’t attack humans” reads wellOnly discouraging attacks on Humans (non-traitor) for Easy/Medium keeps behavior approachable on lower difficulties. Looks good.
139-140: Retaliation override is correctForcing largest attacker as the new enemy is a good, simple rule that avoids being stuck on a passive target.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
src/core/execution/utils/BotBehavior.ts (3)
69-71: Be explicit about when to bypass attack-gating; consider forcing in ally-assist paths.setNewEnemy now blocks updates unless shouldAttack(newEnemy) passes. This is good safety, but it can silently neuter ally assistance (assistAllies calls setNewEnemy without force). If we intend “assist means commit,” pass force = true from assistAllies or make assist logic probabilistic explicitly.
Outside this hunk, a minimal change in assistAllies would be:
- this.setNewEnemy(target); + this.setNewEnemy(target, true);
75-91: Remove impossible null check; keep the method lean.this.player is a readonly constructor param and cannot be null here. Drop the guard to avoid noise and ease testing.
- if (this.player === null) throw new Error("not initialized");
176-232: Retaliate first, then heuristics; also guard random pick on empty input.Two points:
- If a low-density bot neighbor exists, we may pick it before retaliating. The PR/summary says “forced retaliation to largest attacker,” but the current order retaliates only if no bot was chosen. Recommend checking incoming attacks first.
- When falling back to “random enemy,” add a length check to avoid calling randElement([]) if callers ever pass an empty list.
Proposed changes within selectEnemy:
selectEnemy(enemies: Player[]): Player | null { if (this.enemy === null) { // Save up troops until we reach the trigger ratio if (!this.hasSufficientTroops()) return null; + // Retaliate against incoming attacks first + if (this.enemy === null) { + this.checkIncomingAttacks(); + } + // Prefer neighboring bots const bots = this.player .neighbors() .filter( (n): n is Player => n.isPlayer() && n.type() === PlayerType.Bot, ); if (bots.length > 0) { const density = (p: Player) => p.troops() / p.numTilesOwned(); let lowestDensityBot: Player | undefined; let lowestDensity = Infinity; for (const bot of bots) { const currentDensity = density(bot); if (currentDensity < lowestDensity) { lowestDensity = currentDensity; lowestDensityBot = bot; } } if (lowestDensityBot !== undefined) { this.setNewEnemy(lowestDensityBot); } } - // Retaliate against incoming attacks - if (this.enemy === null) { - // Only after clearing bots - this.checkIncomingAttacks(); - } + // (No-op: retaliation already checked above) // Select the most hated player if (this.enemy === null && this.random.chance(2)) { // 50% chance const mostHated = this.player.allRelationsSorted()[0]; if ( mostHated !== undefined && mostHated.relation === Relation.Hostile ) { this.setNewEnemy(mostHated.player); } } // Select the weakest player if (this.enemy === null && enemies.length > 0) { this.setNewEnemy(enemies[0]); } // Select a random player - if (this.enemy === null) { - this.setNewEnemy(this.random.randElement(enemies)); - } + if (this.enemy === null && enemies.length > 0) { + this.setNewEnemy(this.random.randElement(enemies)); + } }Follow-up: if the “weakest” policy assumes enemies is pre-sorted, either document that contract on the method or sort locally to avoid hidden coupling.
src/core/execution/FakeHumanExecution.ts (4)
200-203: Deduplicate border players before sorting to avoid skewed randomness and wasted work.borderPlayers may contain many duplicates (multiple tiles of the same owner). Dedup before sorting so “weakest” truly picks the weakest unique neighbor and the random fallback isn’t biased.
- const enemies = borderPlayers - .filter((o) => o.isPlayer()) - .sort((a, b) => a.troops() - b.troops()); + const unique = new Map<PlayerID, Player>(); + for (const o of borderPlayers) { + if (o.isPlayer()) unique.set(o.id(), o); + } + const enemies = Array.from(unique.values()) + .sort((a, b) => a.troops() - b.troops());
225-260: Remove duplicated, now-unused attack heuristics; rely on BotBehavior as single source of truth.These shouldAttack/shouldDiscourageAttack methods duplicate BotBehavior logic and appear unused after delegating selection to BotBehavior. Keeping both invites drift.
Apply:
- private shouldAttack(other: Player): boolean { - if (this.player === null) throw new Error("Not initialized"); - if (this.player.isOnSameTeam(other)) { - return false; - } - if (this.player.isFriendly(other)) { - if (this.shouldDiscourageAttack(other)) { - return this.random.chance(200); - } - return this.random.chance(50); - } else { - if (this.shouldDiscourageAttack(other)) { - return this.random.chance(4); - } - return true; - } - } - - private shouldDiscourageAttack(other: Player) { - if (other.isTraitor()) { - return false; - } - if (this.mg === undefined) throw new Error("Not initialized"); - const { difficulty } = this.mg.config().gameConfig(); - if ( - difficulty === Difficulty.Hard || - difficulty === Difficulty.Impossible - ) { - return false; - } - if (other.type() !== PlayerType.Human) { - return false; - } - // Only discourage attacks on Humans who are not traitors on easy or medium difficulty. - return true; - }Optional: after removal, clean up any now-unused imports (e.g., Difficulty).
204-211: No callers rely on an early return after creating an alliance requestI scanned all uses of
createAllianceRequest(in tests and execution code) and found no code that depends on an early exit here. It’s safe to continue attacks or assists in the same tick after diplomacy.If you’d like the bot to sometimes pause after sending a request—making its behavior less aggressively diplomatic—you can add a small random “pause” right after
createAllianceRequest:src/core/execution/FakeHumanExecution.ts @@ lines 204-211 if (this.player.canSendAllianceRequest(toAlly)) { this.player.createAllianceRequest(toAlly); + // 10% chance to pause after diplomacy + if (this.random.chance(10)) { + return; // skip further actions this tick + } }Adjust the pause percentage to suit your desired bot style.
278-326: Allow maybeSendNuke to choose HydrogenBomb when availableIn
FakeHumanExecution.tsmaybeSendNuke, we currently hard-codeUnitType.AtomBomb. The codebase already definesUnitType.HydrogenBombin many places, so we can offer the AI both bomb types and prefer H-bombs when affordable. This requires treating the bomb type as a small typed union and updating the gold, build, and send checks accordingly.• File:
src/core/execution/FakeHumanExecution.ts(lines 278–326)
– Introduce aBombType = UnitType.AtomBomb | UnitType.HydrogenBombunion
– Build an array of affordable bombs in priority order:
ts const bombTypes: BombType[] = [UnitType.HydrogenBomb, UnitType.AtomBomb] .filter(t => this.player.gold() >= this.cost(t));
– Replace the initial cost check (this.player.gold() < this.cost(UnitType.AtomBomb)) withbombTypes.length === 0
– Inside the tile loop, swap thecanBuildcheck to:
ts const bombType = bombTypes.find(t => this.player.canBuild(t, tile)); if (!bombType) continue;
– Callthis.sendNuke(bestTile, bombType)(adding a bombType parameter or overload as needed)Example diff sketch:
- if (… || this.player.gold() < this.cost(UnitType.AtomBomb) || …) { + type BombType = UnitType.AtomBomb | UnitType.HydrogenBomb; + const bombTypes: BombType[] = + [UnitType.HydrogenBomb, UnitType.AtomBomb] + .filter(t => this.player.gold() >= this.cost(t)); + if (… || bombTypes.length === 0 || …) { return; } … - if (!this.player.canBuild(UnitType.AtomBomb, tile)) continue; + const bombType = bombTypes.find(t => this.player.canBuild(t, tile)); + if (!bombType) continue; … - this.sendNuke(bestTile); + this.sendNuke(bestTile, bombType);This keeps the API simple, uses a typed union, and lets the AI save up for thermonuclear bombs under heavy threat.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/core/execution/FakeHumanExecution.ts(1 hunks)src/core/execution/utils/BotBehavior.ts(6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-23T08:03:44.892Z
Learnt from: ElMelchizedek
PR: openfrontio/OpenFrontIO#1876
File: src/core/execution/FakeHumanExecution.ts:547-592
Timestamp: 2025-08-23T08:03:44.892Z
Learning: In OpenFrontIO, FakeHumanExecution is used for AI-controlled nations that simulate human behavior, which is distinct from PlayerType.Bot. FakeHumanExecution players are not PlayerType.Bot - they represent more sophisticated AI that mimics human nation behavior, while PlayerType.Bot represents basic AI players.
Applied to files:
src/core/execution/FakeHumanExecution.ts
🧬 Code graph analysis (1)
src/core/execution/utils/BotBehavior.ts (1)
src/core/game/Game.ts (1)
Player(498-632)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Deploy to openfront.dev
🔇 Additional comments (4)
src/core/execution/utils/BotBehavior.ts (3)
93-109: LGTM: Difficulty-aware discouragement is clear and simple.Logic matches the comment: discourage attacks only on non-traitor Humans on Easy/Medium. Clean and idiomatic.
139-142: LGTM: Retaliation now overrides gating.Forcing the largest attacker as the new enemy is a sensible change.
176-176: All selectEnemy calls updated to require an enemies list
I searched the codebase and confirmed there are no stray no-arg calls to selectEnemy—every call now passes the enemies: Player[] parameter. Ready to merge.src/core/execution/FakeHumanExecution.ts (1)
214-214: LGTM: Migrated to the new BotBehavior.selectEnemy(enemies) API.This aligns the responsibility (selection logic) with BotBehavior. Nice composition.
scottanderson
added
the
Release Blocker
scottanderson
force-pushed
the
refactor-nations
branch
from
d05a871 to
f7d8150
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/core/execution/utils/BotBehavior.ts (1)
204-267: Protect against empty enemies array before selection
- Add an early guard in
selectEnemyto returnnullifenemiesis empty. This preventsrandElementfrom being called on an empty array.- No change is needed for the “most hated” logic:
allRelationsSorted()uses.sort((a, b) => a.relation - b.relation), so the lowest numericrelation(i.e. most hostile) is at index 0.src/core/execution/utils/BotBehavior.ts selectEnemy(enemies: Player[]): Player | null { + if (enemies.length === 0) return null; if (this.enemy === null) { @@ // Select a random player - if (this.enemy === null) { - this.setNewEnemy(this.random.randElement(enemies)); - } + if (this.enemy === null) { + this.setNewEnemy(this.random.randElement(enemies)); + } } // Sanity check, don't attack our allies or teammates return this.enemySanityCheck(); }src/core/execution/FakeHumanExecution.ts (3)
223-259: Dead code: duplicate shouldAttack/shouldDiscourageAttackThese methods are no longer used (moved into BotBehavior). Remove to reduce confusion and unused imports (Difficulty).
- private shouldAttack(other: Player): boolean { - if (this.player === null) throw new Error("Not initialized"); - if (this.player.isOnSameTeam(other)) { - return false; - } - if (this.player.isFriendly(other)) { - if (this.shouldDiscourageAttack(other)) { - return this.random.chance(200); - } - return this.random.chance(50); - } else { - if (this.shouldDiscourageAttack(other)) { - return this.random.chance(4); - } - return true; - } - } - - private shouldDiscourageAttack(other: Player) { - if (other.isTraitor()) { - return false; - } - if (this.mg === undefined) throw new Error("Not initialized"); - const { difficulty } = this.mg.config().gameConfig(); - if ( - difficulty === Difficulty.Hard || - difficulty === Difficulty.Impossible - ) { - return false; - } - if (other.type() !== PlayerType.Human) { - return false; - } - // Only discourage attacks on Humans who are not traitors on easy or medium difficulty. - return true; - }Also remove unused imports made only for these methods.
- Difficulty,
306-314: Nuke candidate filter is too strict vs comment; prevents many launchesComment says “at least 15 tiles,” but code requires the entire radius to be owned by the target. This likely blocks nukes and conflicts with the PR goal.
- outer: for (const tile of new Set(allTiles)) { + for (const tile of new Set(allTiles)) { if (tile === null) continue; - for (const t of this.mg.bfs(tile, manhattanDistFN(tile, 15))) { - // Make sure we nuke at least 15 tiles in border - if (this.mg.owner(t) !== other) { - continue outer; - } - } + // Require at least 15 tiles in radius to be owned by target (not 100%) + let covered = 0; + for (const t of this.mg.bfs(tile, manhattanDistFN(tile, 15))) { + if (this.mg.owner(t) === other) covered++; + if (covered >= 15) break; + } + if (covered < 15) continue;This should materially increase nuke frequency as intended.
321-323: Implement HydrogenBomb support in FakeHumanExecution.tsI confirmed that
UnitType.HydrogenBombis defined throughout the codebase (e.g. in StatsSchemas.ts, Game.ts, NukeExecution.ts, etc.), so runtime detection will work as intended. Please apply these refactors to add an optional H-bomb path under high threat, while preserving AtomBomb as the default:• File:
src/core/execution/FakeHumanExecution.ts
– Around lines 321–323 (inmaybeSendNuke):
diff if (bestTile !== null) { - this.sendNuke(bestTile); + const nukeType = this.pickNukeType(other); + this.sendNuke(bestTile, nukeType); }
– Around lines 338–346 (thesendNukemethod):
diff - private sendNuke(tile: TileRef) { + private sendNuke(tile: TileRef, type: UnitType = UnitType.AtomBomb) { if (this.mg === undefined) throw new Error("Not initialized"); if (this.player === null) throw new Error("Not initialized"); const tick = this.mg.ticks(); this.lastNukeSent.push([tick, tile]); this.mg.addExecution( - new NukeExecution(UnitType.AtomBomb, this.player, tile), + new NukeExecution(type, this.player, tile), ); }• Add these helpers (e.g. right below
maybeSendNuke) to choose H-bomb only when available, affordable, and threat is large:private pickNukeType(target: Player): UnitType { // Only if H-bomb exists in this build const hasHB = (UnitType as any).HydrogenBomb !== undefined; if (!hasHB || !this.mg || !this.player) return UnitType.AtomBomb; const hCost = this.mg.unitInfo((UnitType as any).HydrogenBomb).cost(this.player); const canAfford = this.player.gold() >= hCost; const largeThreat = this.isUnderLargeAttack(target); return canAfford && largeThreat ? (UnitType as any).HydrogenBomb : UnitType.AtomBomb; } private isUnderLargeAttack(target: Player): boolean { if (!this.player || !this.mg) return false; const maxTroops = this.mg.config().maxTroops(this.player); const heavyTarget = target.troops() > 0.75 * maxTroops; const incoming = this.player .incomingAttacks() .filter(a => a.attacker() === target) .reduce((sum, a) => sum + a.troops(), 0); const heavyIncoming = incoming > 0.5 * maxTroops; return heavyTarget || heavyIncoming; }These changes close the feature gap by saving for and optionally firing HydrogenBomb under major threat, without affecting existing AtomBomb behavior.
🧹 Nitpick comments (2)
src/core/execution/utils/BotBehavior.ts (2)
97-113: Remove impossible null check on this.playerthis.player is a required ctor param and cannot be null; the guard is dead code and can confuse readers.
- private shouldAttack(other: Player): boolean { - if (this.player === null) throw new Error("not initialized"); + private shouldAttack(other: Player): boolean {
269-305: Two paths for enemy choice divergeselectRandomEnemy duplicates some gating logic. Consider consolidating selection paths to a single function to keep behavior consistent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/core/execution/FakeHumanExecution.ts(4 hunks)src/core/execution/utils/BotBehavior.ts(9 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-05-31T18:15:03.445Z
Learnt from: 1brucben
PR: openfrontio/OpenFrontIO#977
File: src/core/execution/AttackExecution.ts:123-125
Timestamp: 2025-05-31T18:15:03.445Z
Learning: The removeTroops function in PlayerImpl.ts already prevents negative troop counts by using minInt(this._troops, toInt(troops)) to ensure it never removes more troops than available.
Applied to files:
src/core/execution/utils/BotBehavior.ts
🧬 Code graph analysis (2)
src/core/execution/FakeHumanExecution.ts (1)
src/core/execution/utils/BotBehavior.ts (1)
EMOJI_HECKLE(36-39)
src/core/execution/utils/BotBehavior.ts (3)
src/core/Util.ts (1)
flattenedEmojiTable(297-297)src/core/game/Game.ts (1)
Player(498-632)src/core/configuration/DefaultConfig.ts (1)
maxTroops(731-760)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Deploy to openfront.dev
🔇 Additional comments (9)
src/core/execution/utils/BotBehavior.ts (5)
91-95: Force flag is a clear improvementsetNewEnemy(newEnemy, force) correctly bypasses shouldAttack when forced (e.g., retaliation). Good composition.
115-131: Difficulty-aware “don’t bully humans” rule looks goodThe logic matches the intent: only discourage attacks on non-traitor humans on Easy/Medium. Clear and simple.
145-155: Troop ratio gates are a nice control knobReserve vs trigger ratios improve pacing and should make nukes/attacks feel less erratic.
157-170: Largest incoming attack forces enemy swap — good callThis aligns with “retaliate to the biggest threat” and uses the new force flag correctly.
179-201: Please invert the relation delta when assisting alliesAssisting an ally currently lowers your relation by 20, which makes the player less friendly despite helping. To keep rewards consistent with other “help” actions (e.g. donating troops/gold uses
+50), consider changing the delta to positive:- this.player.updateRelation(ally, -20); + this.player.updateRelation(ally, 20);If the negative delta was intentional (to discourage over‐dependence), please confirm the design choice and consider a smaller penalty value.
Affected location:
src/core/execution/utils/BotBehavior.tsat line 196src/core/execution/FakeHumanExecution.ts (4)
1-1: Centralized heckle emoji — niceImporting EMOJI_HECKLE from BotBehavior removes duplication and keeps emoji curation in one place.
142-156: Behavior injection is cleanConstructing BotBehavior with the pacing ratios is simple and testable. Good composition over extending Execution.
260-274: Emoji cadence and sourcing look goodRate-limiting via lastEmojiSent and using EMOJI_HECKLE is clean and readable.
210-213: No remaining zero-arg selectEnemy() calls
Verified with a project-wide search—there are no instances ofselectEnemy()without arguments insrc. All callsites now use the newselectEnemy(enemies)signature.
github-project-automation
bot
moved this from Development
to Complete
in OpenFront Release Management
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/core/execution/utils/BotBehavior.ts (1)
17-39: Harden emoji mapping: prevent -1 IDs; variation selector risk (“🤦♂️”, “🕊️”).indexOf can return -1 for glyphs missing from flattenedEmojiTable (common with ZWJ/VS). Filter out invalid indices to avoid emitting bad EmojiExecution payloads. Also prefer a helper for all lists.
Apply:
-const emojiId = (e: typeof flattenedEmojiTable[number]) => flattenedEmojiTable.indexOf(e); +const emojiId = (e: string) => flattenedEmojiTable.indexOf(e); +const mapEmojiIds = (list: readonly string[]) => + list.map(emojiId).filter((i) => i >= 0); -const EMOJI_ASSIST_ACCEPT = ([ - "👍", - "⛵", - "🤝", - "🎯", -] as const).map(emojiId); +const EMOJI_ASSIST_ACCEPT = mapEmojiIds(["👍", "⛵", "🤝", "🎯"]); -const EMOJI_RELATION_TOO_LOW = ([ - "🥱", - "🤦♂️", -] as const).map(emojiId); +const EMOJI_RELATION_TOO_LOW = mapEmojiIds(["🥱", "🤦"]); -const EMOJI_TARGET_ME = ([ - "🥺", - "💀", -] as const).map(emojiId); +const EMOJI_TARGET_ME = mapEmojiIds(["🥺", "💀"]); -const EMOJI_TARGET_ALLY = ([ - "🕊️", - "👎", -] as const).map(emojiId); +const EMOJI_TARGET_ALLY = mapEmojiIds(["🕊", "👎"]); -export const EMOJI_HECKLE = ([ - "🤡", - "😡", -] as const).map(emojiId); +export const EMOJI_HECKLE = mapEmojiIds(["🤡", "😡"]);
🧹 Nitpick comments (2)
src/core/execution/utils/BotBehavior.ts (2)
97-113: Remove unreachable null-check.this.player is readonly via constructor; cannot be null.
Apply:
- private shouldAttack(other: Player): boolean { - if (this.player === null) throw new Error("not initialized"); + private shouldAttack(other: Player): boolean {
254-257: Don’t assume enemies[0] is weakest. Compute it or assert ordering.If caller doesn’t pre-sort, this picks arbitrary “first” and degrades behavior.
Apply:
- if (this.enemy === null && enemies.length > 0) { - this.setNewEnemy(enemies[0]); - } + if (this.enemy === null && enemies.length > 0) { + const weakest = enemies.reduce( + (min, p) => (p.troops() < min.troops() ? p : min), + enemies[0], + ); + this.setNewEnemy(weakest); + }If upstream guarantees ordering, document it in selectEnemy(enemies) JSDoc.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/core/execution/utils/BotBehavior.ts(8 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-05-31T18:15:03.445Z
Learnt from: 1brucben
PR: openfrontio/OpenFrontIO#977
File: src/core/execution/AttackExecution.ts:123-125
Timestamp: 2025-05-31T18:15:03.445Z
Learning: The removeTroops function in PlayerImpl.ts already prevents negative troop counts by using minInt(this._troops, toInt(troops)) to ensure it never removes more troops than available.
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 (1)
Player(502-636)src/core/configuration/DefaultConfig.ts (1)
maxTroops(735-764)
🔇 Additional comments (9)
src/core/execution/utils/BotBehavior.ts (9)
3-3: Import is correct and used.Difficulty is used in shouldDiscourageAttack; keep.
115-131: Discourage logic reads clean and matches difficulty intent.Traitors and non-humans are never discouraged; Easy/Medium humans are. Good.
145-150: Reserve threshold check is fine.Uses maxTroops from config; ratio math looks safe.
151-155: Trigger threshold check is fine.Consistent with reserve logic.
168-169: Good: retaliation ignores attack gate.Force flag ensures we can always switch to the biggest attacker. Matches design.
204-211: Attack gating order may slow early aggression; confirm intent vs. PR goal.You now require reserveRatio and usually triggerRatio before picking any enemy. Given narrower ratios elsewhere, this is probably fine, but please double‑check claimed “more nukes” effect in playtests.
Would you like a script to scan recent replays for average ticks-to-first-attack before/after this change?
259-262: Good: guarded random pick fixes empty-array hazard.Prevents randElement on empty list.
314-330: Attack send logic is sound.Respects team, uses reserve/expand ratios, and avoids small dust attacks.
243-245: No changes needed for probability comment
Thechance(odds: number)method returns true with probability 1/odds (i.e.,Math.floor(this.rng() * 2) === 0yields 50% for odds = 2), so the “50% chance” comment is correct.
| private setNewEnemy(newEnemy: Player | null, force = false) { | ||
| if (newEnemy !== null && !force && !this.shouldAttack(newEnemy)) return; | ||
| this.enemy = newEnemy; | ||
| this.enemyUpdated = this.game.ticks(); | ||
| } | ||
|
|
There was a problem hiding this comment.
Assist path can silently no-op due to shouldAttack gate; force enemy when assisting and fix relation sign.
assistAllies calls setNewEnemy without force; if shouldAttack returns false (e.g., same team), we “assist” but never set an enemy. Also, helping an ally should not penalize relations.
Apply:
- this.player.updateRelation(ally, -20);
- this.setNewEnemy(target);
+ this.player.updateRelation(ally, +20);
+ this.setNewEnemy(target, true);Also applies to: 196-199
🤖 Prompt for AI Agents
In src/core/execution/utils/BotBehavior.ts around lines 91-96 (and also apply
same change to lines ~196-199), setNewEnemy can silently no-op because it checks
shouldAttack and returns early; update callers that are performing assistance to
pass force = true so the enemy is always set when assisting, and adjust the
relation update logic so helping an ally does not penalize relations (flip the
sign/operation so assistance increases or leaves relations unchanged instead of
decrementing). Specifically, change assistAllies (and the equivalent block at
196-199) to call setNewEnemy(newEnemy, true) and modify the relation adjustment
to use the correct sign/operation for assistance.
## Description: In #1860, some unused code was left behind. Remove the unused code. ## Please complete the following: - [x] I have added screenshots for all UI updates - [x] I process any text displayed to the user through translateText() and I've added it to the en.json file - [x] I have added relevant tests to the test directory - [x] I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced
## Description: Simplify nation enemy selection to make nations more likely to launch nukes. Partially fixes #1855 by addressing a v24 regression in nation behavior. ## Please complete the following: - [x] I have added screenshots for all UI updates - [x] I process any text displayed to the user through translateText() and I've added it to the en.json file - [x] I have added relevant tests to the test directory - [x] I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced
## Description: In #1860, some unused code was left behind. Remove the unused code. ## Please complete the following: - [x] I have added screenshots for all UI updates - [x] I process any text displayed to the user through translateText() and I've added it to the en.json file - [x] I have added relevant tests to the test directory - [x] I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced
## Description: Simplify nation enemy selection to make nations more likely to launch nukes. Partially fixes openfrontio#1855 by addressing a v24 regression in nation behavior. ## Please complete the following: - [x] I have added screenshots for all UI updates - [x] I process any text displayed to the user through translateText() and I've added it to the en.json file - [x] I have added relevant tests to the test directory - [x] I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced
## Description: In openfrontio#1860, some unused code was left behind. Remove the unused code. ## Please complete the following: - [x] I have added screenshots for all UI updates - [x] I process any text displayed to the user through translateText() and I've added it to the en.json file - [x] I have added relevant tests to the test directory - [x] I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced
## Description: Simplify nation enemy selection to make nations more likely to launch nukes. Partially fixes openfrontio#1855 by addressing a v24 regression in nation behavior. ## Please complete the following: - [x] I have added screenshots for all UI updates - [x] I process any text displayed to the user through translateText() and I've added it to the en.json file - [x] I have added relevant tests to the test directory - [x] I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced
## Description: In openfrontio#1860, some unused code was left behind. Remove the unused code. ## Please complete the following: - [x] I have added screenshots for all UI updates - [x] I process any text displayed to the user through translateText() and I've added it to the en.json file - [x] I have added relevant tests to the test directory - [x] I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced
## Description: Simplify nation enemy selection to make nations more likely to launch nukes. Partially fixes #1855 by addressing a v24 regression in nation behavior. ## Please complete the following: - [x] I have added screenshots for all UI updates - [x] I process any text displayed to the user through translateText() and I've added it to the en.json file - [x] I have added relevant tests to the test directory - [x] I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced
## Description: In #1860, some unused code was left behind. Remove the unused code. ## Please complete the following: - [x] I have added screenshots for all UI updates - [x] I process any text displayed to the user through translateText() and I've added it to the en.json file - [x] I have added relevant tests to the test directory - [x] I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced
## Description: Simplify nation enemy selection to make nations more likely to launch nukes. Partially fixes #1855 by addressing a v24 regression in nation behavior. ## Please complete the following: - [x] I have added screenshots for all UI updates - [x] I process any text displayed to the user through translateText() and I've added it to the en.json file - [x] I have added relevant tests to the test directory - [x] I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced
## Description: In #1860, some unused code was left behind. Remove the unused code. ## Please complete the following: - [x] I have added screenshots for all UI updates - [x] I process any text displayed to the user through translateText() and I've added it to the en.json file - [x] I have added relevant tests to the test directory - [x] I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced
## Description: Simplify nation enemy selection to make nations more likely to launch nukes. Partially fixes openfrontio#1855 by addressing a v24 regression in nation behavior. ## Please complete the following: - [x] I have added screenshots for all UI updates - [x] I process any text displayed to the user through translateText() and I've added it to the en.json file - [x] I have added relevant tests to the test directory - [x] I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced
## Description: In openfrontio#1860, some unused code was left behind. Remove the unused code. ## Please complete the following: - [x] I have added screenshots for all UI updates - [x] I process any text displayed to the user through translateText() and I've added it to the en.json file - [x] I have added relevant tests to the test directory - [x] I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced
Description:
Simplify nation enemy selection to make nations more likely to launch nukes.
Partially fixes #1855 by addressing a v24 regression in nation behavior.
Please complete the following: