Skip to content

Conversation

@sambokai
Copy link
Contributor

@sambokai sambokai commented Oct 17, 2025

Description:

Summary

Implements intelligent MIRV usage for fakehuman players, enabling them to make strategic nuclear strikes based on game state analysis.

Changes

Core FakeHuman Strategy (FakeHumanExecution.ts)

  • MIRV Decision System: Added considerMIRV() method that evaluates game state and determines optimal MIRV usage
  • Three Strategic Targeting Modes:
    1. Counter-MIRV: Retaliatory strikes against players actively launching MIRVs at the fakehuman
    2. Victory Denial: Preemptive strikes against players approaching win conditions
      • Team threshold: n% of total land (configurable)
      • Individual threshold: n% of total land (configurable)
    3. Steamroll Prevention: Strikes against players with dominant city counts (n% ahead of next competitor)

FakeHuman Behavior Tuning

  • Cooldown System: n-minute cooldown between MIRV attempts (configurable)
  • Failure Rate: ~n% chance of cooldown trigger without launch (simulates human hesitation/resource management; configurable)
  • Territory Targeting: Centers MIRV strikes on enemy territory center-of-mass for maximum impact

Technical Improvements

  • Type Safety: Updated UnitParamsMap to include targetTile parameter for MIRV units
  • Execution Flow: Integrated MIRV consideration into fakehuman tick cycle outside of standard attack logic, due to its holistic strategic nature

Game Balance Impact

  • FakeHuman Threat Level: Increases late-game fakehuman competitiveness
  • Endgame Dynamics: Prevents runaway victories, extends game tension

Breaking Changes

None - purely additive feature

Related GitHub Issues:


Other

  • 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
  • I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced

Discord Username: samsammiliah

@CLAassistant
Copy link

CLAassistant commented Oct 17, 2025

CLA assistant check
All committers have signed the CLA.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 17, 2025

Walkthrough

Adds MIRV decision-making and cooldowns to FakeHumanExecution, passes an explicit targetTile when spawning MIRVs, adds a territory-center utility, updates MIRV unit typing, and introduces integration tests for FakeHuman MIRV retaliation and targeting.

Changes

Cohort / File(s) Change Summary
MIRV AI Decision Logic
src/core/execution/FakeHumanExecution.ts
Import MirvExecution; add MIRV constants and private state (lastMIRVSent, mirvTargetsCache); integrate considerMIRV() into tick flow; add inbound MIRV detection, city counting, and territory-center usage; implement target-selection strategies (counter‑MIRV, victory‑denial, steamroll‑stop); add MIRV pipeline methods (maybeSendMIRV, sendMIRV, cooldown and cleanup).
MIRV spawn initialization
src/core/execution/MIRVExecution.ts
When spawning the MIRV, pass targetTile: this.dst into buildUnit so the spawned unit carries an explicit numeric targetTile param.
Unit parameter typing
src/core/game/Game.ts
Change UnitParamsMap[UnitType.MIRV] from Record<string, never> to { targetTile?: number }, enabling optional numeric targetTile in MIRV params.
Territory helper
src/core/execution/Util.ts
Add `calculateTerritoryCenter(game: Game, target: Player): TileRef
Integration tests
tests/FakeHumanMIRV.test.ts
Add tests simulating attacker MIRV launches and asserting FakeHuman retaliation, victory‑denial, and steamroll‑stop targeting across seeded runs and team scenarios.

Sequence Diagram(s)

sequenceDiagram
    participant FH as FakeHumanExecution
    participant GS as GameState
    participant STR as MIRV Strategy
    participant MIR as MirvExecution

    rect rgb(245,250,255)
      FH->>GS: read snapshot + recent events
      GS-->>FH: inbound MIRV events, player states
      FH->>STR: considerMIRV(snapshot)
      rect rgb(255,250,240)
        Note right of STR: evaluate Counter‑MIRV / Victory‑Denial / Steamroll‑Stop
      end
      alt MIRV chosen
        STR-->>FH: plan(targetPlayer, targetTile)
        FH->>MIR: maybeSendMIRV(plan)
        MIR-->>GS: buildUnit({ targetTile }) → spawn/launch event
      else no MIRV
        FH-->>GS: continue other tick actions
      end
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Focus review on src/core/execution/FakeHumanExecution.ts (dense decision logic, multiple heuristics, cooldowns).
  • Verify targetTile typing and propagation in src/core/execution/MIRVExecution.ts and src/core/game/Game.ts.
  • Check calculateTerritoryCenter correctness and edge-case handling in src/core/execution/Util.ts.
  • Validate determinism and seeding in tests/FakeHumanMIRV.test.ts.

Possibly related PRs

Suggested labels

Feature - AI, Feature - Simulation

Suggested reviewers

  • scottanderson

Poem

A silo hums beneath the night,
The bot counts cities, weighs the fight.
A center found, a cold request,
Cooldowns wait, the plan is dressed. 🚀

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title "Feature: Enable FakeHumans ("Nation Bots") to Launch MIRVs Strategically" is directly and clearly related to the main objective of this changeset. The raw summary confirms that all modifications across five files center on implementing strategic MIRV capability for FakeHuman players: FakeHumanExecution.ts adds MIRV decision logic and targeting strategies, MIRVExecution.ts integrates targetTile parameter handling, Game.ts updates the UnitParamsMap type, Util.ts provides territory center calculation, and comprehensive tests validate the new behavior. The title is concise, uses clear language, avoids vague terminology, and effectively summarizes the primary change from a developer perspective.
Description Check ✅ Passed The pull request description is comprehensive and directly related to the changeset. It provides a clear summary of intelligent MIRV implementation for fakehuman players and details the three strategic targeting modes (counter-MIRV, victory denial, steamroll prevention), behavior tuning parameters, technical improvements to type safety and execution flow, and game balance impact. The description explains both the functional additions and their integration into the existing codebase, and it aligns closely with the modifications outlined in the raw summary. The description is neither vague nor generic; it conveys meaningful information about what the changeset accomplishes.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7e8c999 and c9c4e18.

📒 Files selected for processing (1)
  • src/core/game/Game.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/core/game/Game.ts

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (2)
src/core/execution/FakeHumanExecution.ts (2)

689-701: Sample target tiles instead of scanning all enemy tiles.

Large empires make this O(n). Use the existing arraySampler for a small random sample (e.g., 25).

   private maybeSendMIRV(enemy: Player): void {
     if (this.player === null) throw new Error("not initialized");

-    const enemyTiles = Array.from(enemy.tiles());
-    if (enemyTiles.length === 0) return;
-
-    for (const tile of enemyTiles) {
+    const enemyTiles = Array.from(enemy.tiles());
+    if (enemyTiles.length === 0) return;
+    // Try a random sample to find a legal MIRV target quickly.
+    for (const tile of this.arraySampler(enemyTiles, Math.min(25, enemyTiles.length))) {
       if (this.player.canBuild(UnitType.MIRV, tile)) {
         this.sendMIRV(tile);
         return;
       }
     }
   }

752-780: Victory‑denial logic: thresholds are clear; keep as constants.

The 80%/95% cutoffs are easy to tune later. Consider lifting to config for balance tweaks without code changes.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 141c431 and 992bfb1.

📒 Files selected for processing (1)
  • src/core/execution/FakeHumanExecution.ts (4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/core/execution/FakeHumanExecution.ts (2)
src/core/game/Game.ts (1)
  • Player (517-651)
src/core/execution/MIRVExecution.ts (2)
  • owner (179-181)
  • MirvExecution (16-190)
🔇 Additional comments (3)
src/core/execution/FakeHumanExecution.ts (3)

22-22: Import looks good.

Dependency and naming match MIRVExecution.ts.


782-800: Steamroll‑stop heuristic LGTM.

Structure gap check is simple and readable; good use of countStructures.


660-672: The review comment is incorrect. Unit.targetTile() exists and is properly defined.

The targetTile() method is defined in the Unit interface in src/core/game/Game.ts, implemented in src/core/game/UnitImpl.ts with return type TileRef | undefined, and wrapped in src/core/game/GameView.ts. The method signature shows it safely returns TileRef | undefined.

The original code at line 665 is type-safe and already handles the undefined case with the guard if (!dst) continue;. No type casting, optional chaining, or fallback heuristics are needed. The suggested diff introduces unnecessary complexity and a questionable fallback behavior (attacker.targets().includes(this.player)) that changes the method's semantics.

Likely an incorrect or invalid review comment.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (2)
src/core/execution/FakeHumanExecution.ts (2)

660-672: Simplify with .some() for better readability.

The manual loop can be replaced with a cleaner functional approach.

 private isInboundMIRVFrom(attacker: Player): boolean {
   if (this.player === null) throw new Error("not initialized");
-  // Detect MIRV units launched by attacker whose target tile belongs to us
-  const enemyMirvs = attacker.units(UnitType.MIRV);
-  for (const mirv of enemyMirvs) {
-    const dst = mirv.targetTile();
-    if (!dst) continue;
-    if (!this.mg.hasOwner(dst)) continue;
-    const owner = this.mg.owner(dst);
-    if (owner === this.player) return true;
-  }
-  return false;
+  return attacker.units(UnitType.MIRV).some((mirv) => {
+    const dst = mirv.targetTile();
+    return dst && this.mg.owner(dst) === this.player;
+  });
 }

674-687: Consider using .reduce() for cleaner aggregation.

The manual loop can be simplified.

 private countStructures(p: Player): number {
-  // Consider key structures that contribute to board control
   const importantTypes: UnitType[] = [
     UnitType.City,
     UnitType.Factory,
     UnitType.Port,
     UnitType.DefensePost,
     UnitType.SAMLauncher,
     UnitType.MissileSilo,
   ];
-  let total = 0;
-  for (const t of importantTypes) total += p.units(t).length;
-  return total;
+  return importantTypes.reduce((total, t) => total + p.units(t).length, 0);
 }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 992bfb1 and 0d34e05.

📒 Files selected for processing (1)
  • src/core/execution/FakeHumanExecution.ts (4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/core/execution/FakeHumanExecution.ts (3)
src/core/game/Game.ts (1)
  • Player (517-651)
src/core/execution/MIRVExecution.ts (2)
  • owner (179-181)
  • MirvExecution (16-190)
src/core/game/GameMap.ts (2)
  • TileRef (3-3)
  • x (122-124)
🔇 Additional comments (4)
src/core/execution/FakeHumanExecution.ts (4)

22-22: LGTM!

Import is clean and correctly scoped.


233-233: Good - addresses past review feedback.

Short-circuit prevents MIRV from stacking with nukes and other heavy actions in the same tick.


703-709: LGTM - addresses past review feedback.

Correctly prunes old nuke events before pushing MIRV entry to prevent unbounded memory growth.


798-798: Verify: Steamroll strategy never triggers in 1v1.

Line 798 returns null when there are fewer than 2 eligible opponents, meaning the Steamroll strategy won't fire in 1v1 games. Is this intentional? If yes, consider adding a comment explaining the design decision.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
src/core/execution/FakeHumanExecution.ts (1)

711-723: Add strategic targeting and avoid iterating all enemy tiles.

This method has two issues:

  1. Performance: Iterating through ALL enemy tiles (potentially thousands) to find the first buildable one is inefficient. If the enemy owns 5,000 tiles, this checks all 5,000 every time.

  2. Strategy: Unlike maybeSendNuke (lines 271-324), this picks a random tile without scoring strategic value. MIRVs are expensive and should target structures like Cities, Missile Silos, or Ports.

Consider this approach:

 private maybeSendMIRV(enemy: Player): void {
   if (this.player === null) throw new Error("not initialized");
 
-  const enemyTiles = Array.from(enemy.tiles());
-  if (enemyTiles.length === 0) return;
-
-  for (const tile of enemyTiles) {
-    if (this.player.canBuild(UnitType.MIRV, tile)) {
-      this.sendMIRV(tile);
-      return;
-    }
+  // Target structures first, then sample random tiles
+  const structures = enemy.units(
+    UnitType.City,
+    UnitType.MissileSilo,
+    UnitType.Port,
+    UnitType.DefensePost,
+    UnitType.SAMLauncher,
+  );
+  const structureTiles = structures.map((u) => u.tile());
+  const randomTiles = this.randTerritoryTileArray(10);
+  const allTiles = [...structureTiles, ...randomTiles];
+
+  let bestTile: TileRef | null = null;
+  let bestValue = 0;
+
+  for (const tile of new Set(allTiles)) {
+    if (!this.player.canBuild(UnitType.MIRV, tile)) continue;
+    const value = this.mirvTileScore(tile, structures);
+    if (value > bestValue) {
+      bestTile = tile;
+      bestValue = value;
+    }
+  }
+
+  if (bestTile) {
+    this.sendMIRV(bestTile);
   }
 }
+
+private mirvTileScore(tile: TileRef, targets: Unit[]): number {
+  // MIRV has ~1500 range, score structures within range
+  const dist = euclDistFN(tile, 1500, false);
+  return targets
+    .filter((unit) => dist(this.mg, unit.tile()))
+    .reduce((score, unit) => {
+      switch (unit.type()) {
+        case UnitType.City: return score + 25_000;
+        case UnitType.MissileSilo: return score + 50_000;
+        case UnitType.Port: return score + 10_000;
+        case UnitType.DefensePost: return score + 5_000;
+        default: return score;
+      }
+    }, 0);
+}
🧹 Nitpick comments (2)
src/core/execution/FakeHumanExecution.ts (2)

738-748: Simplify cooldown check: the loop is redundant.

After removeOldMIRVEvents() removes entries older than 600 ticks, any remaining entry is guaranteed to be within the 600-tick cooldown window. The loop checks every entry but could simply check if the array is non-empty.

     // Check MIRV cooldown to prevent spam
     this.removeOldMIRVEvents();
-    const currentTick = this.mg.ticks();
     const cooldownTicks = FakeHumanExecution.MIRV_COOLDOWN_TICKS;
 
-    // Check if any recent MIRV launches are still in cooldown
-    for (const [tick] of this.lastMIRVSent) {
-      if (currentTick - tick < cooldownTicks) {
-        return false; // Still in cooldown
-      }
-    }
+    // After pruning, any remaining entry is within cooldown
+    if (this.lastMIRVSent.length > 0) return false;

796-821: Consider guarding against division by zero.

While unlikely in practice, totalLand could theoretically be 0. Adding a guard improves defensive coding.

 private selectVictoryDenialTarget(): Player | null {
   if (this.player === null) throw new Error("not initialized");
   const totalLand = this.mg.numLandTiles();
+  if (totalLand === 0) return null;
   let best: { p: Player; severity: number } | null = null;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0d34e05 and 7fb9b22.

📒 Files selected for processing (1)
  • src/core/execution/FakeHumanExecution.ts (7 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-23T08:03:44.914Z
Learnt from: ElMelchizedek
PR: openfrontio/OpenFrontIO#1876
File: src/core/execution/FakeHumanExecution.ts:547-592
Timestamp: 2025-08-23T08:03:44.914Z
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/FakeHumanExecution.ts (2)
src/core/game/Game.ts (2)
  • Tick (22-22)
  • Player (517-651)
src/core/execution/MIRVExecution.ts (2)
  • owner (179-181)
  • MirvExecution (16-190)
🔇 Additional comments (4)
src/core/execution/FakeHumanExecution.ts (4)

22-22: LGTM! Clean constant definitions and imports.

The MIRV constants are well-named and documented. The import and tracking array additions follow the existing patterns in the file.

Also applies to: 31-39, 55-55


244-244: Good: MIRV decision now short-circuits the attack flow.

This prevents stacking MIRV with nukes and boat/land attacks in the same tick, as requested in past reviews.


337-346: Good: Memory pruning prevents unbounded growth.

The MIRV event tracking correctly prunes old entries before adding new ones, addressing the past review concern about unbounded array growth.

Also applies to: 725-731


774-784: Good refactoring: eliminates code duplication.

Extracting the repeated player-filtering logic into a helper makes the three strategy methods cleaner and easier to maintain.

@TheGiraffe3 TheGiraffe3 linked an issue Oct 18, 2025 that may be closed by this pull request
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (4)
src/core/execution/FakeHumanExecution.ts (4)

699-727: Consider minor performance optimization (optional).

The method iterates all territory tiles twice: once to compute the center, once to find the closest tile. For large territories, you could combine into a single pass or use x * x instead of Math.pow(x, 2).

Optional refactor using squared distances without Math.pow:

   for (const tile of tiles) {
-    const distance = Math.sqrt(
-      Math.pow(this.mg.x(tile) - centerX, 2) +
-        Math.pow(this.mg.y(tile) - centerY, 2),
-    );
+    const dx = this.mg.x(tile) - centerX;
+    const dy = this.mg.y(tile) - centerY;
+    const distance = dx * dx + dy * dy; // Compare squared distances, no sqrt needed
     if (distance < closestDistance) {
       closestDistance = distance;
       closestTile = tile;
     }
   }

Note: Since we're only comparing distances, we don't need the Math.sqrt at all.


748-754: Minor redundancy: removeOldMIRVEvents called twice per MIRV launch.

Both considerMIRV (line 761) and sendMIRV (line 750) call removeOldMIRVEvents(). Since sendMIRV is only called from considerMIRV via maybeSendMIRV, the second call is redundant.

Optional: remove the call from sendMIRV since considerMIRV already prunes at the start:

   private sendMIRV(tile: TileRef): void {
     if (this.player === null) throw new Error("not initialized");
-    this.removeOldMIRVEvents();
     const tick = this.mg.ticks();
     this.lastMIRVSent.push([tick, tile]);
     this.mg.addExecution(new MirvExecution(this.player, tile));
   }

763-763: Consider more explicit cooldown check for clarity.

The cooldown is currently enforced by checking lastMIRVSent.length > 0 after pruning. While this works, it's less explicit than directly checking the time since the last MIRV.

Optional: make the cooldown logic more explicit:

     this.removeOldMIRVEvents();
 
-    if (this.lastMIRVSent.length > 0) return false;
+    // Enforce cooldown: don't send another MIRV if one was sent recently
+    if (this.lastMIRVSent.length > 0) {
+      const lastMIRVTick = this.lastMIRVSent[this.lastMIRVSent.length - 1][0];
+      if (this.mg.ticks() - lastMIRVTick < FakeHumanExecution.MIRV_COOLDOWN_TICKS) {
+        return false;
+      }
+    }

This makes the cooldown logic explicit and uses the constant directly.


789-794: Consider excluding PlayerType.Bot from MIRV targets.

MIRVs are expensive strategic weapons. The similar maybeSendNuke method (line 277) explicitly excludes PlayerType.Bot targets, presumably because basic bot players can be defeated with conventional attacks.

Consider adding the Bot exclusion:

   private getValidMirvTargetPlayers(): Player[] {
     if (this.player === null) throw new Error("not initialized");
     return this.mg.players().filter((p) => {
-      return p !== this.player && p.isPlayer() && !this.player!.isOnSameTeam(p);
+      return (
+        p !== this.player &&
+        p.isPlayer() &&
+        !this.player!.isOnSameTeam(p) &&
+        p.type() !== PlayerType.Bot
+      );
     });
   }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e022879 and df9e406.

📒 Files selected for processing (1)
  • src/core/execution/FakeHumanExecution.ts (8 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-10-17T19:16:57.669Z
Learnt from: sambokai
PR: openfrontio/OpenFrontIO#2225
File: src/core/execution/FakeHumanExecution.ts:711-723
Timestamp: 2025-10-17T19:16:57.669Z
Learning: In the OpenFrontIO game, MIRVs (Multiple Independently targetable Reentry Vehicles) have an area of effect with a radius of 1500 tiles around the aim point. To maximize efficiency, bots should aim at the center of the target player's territory rather than selecting random tiles. This differs from the initial understanding that MIRVs covered an entire player's territory regardless of aim point.

Applied to files:

  • src/core/execution/FakeHumanExecution.ts
📚 Learning: 2025-08-24T11:13:08.495Z
Learnt from: DevelopingTom
PR: openfrontio/OpenFrontIO#1900
File: src/core/execution/SAMLauncherExecution.ts:103-111
Timestamp: 2025-08-24T11:13:08.495Z
Learning: In SAMLauncherExecution.ts, the cached target bug can only occur if: 1) SAM is not on cooldown when nuke is in range, 2) SAM goes on cooldown right after computing trajectory, 3) SAM becomes available again before nuke explodes. This is not possible with current cooldown values but the fix is still valuable for robustness.

Applied to files:

  • src/core/execution/FakeHumanExecution.ts
📚 Learning: 2025-08-23T08:03:44.914Z
Learnt from: ElMelchizedek
PR: openfrontio/OpenFrontIO#1876
File: src/core/execution/FakeHumanExecution.ts:547-592
Timestamp: 2025-08-23T08:03:44.914Z
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/FakeHumanExecution.ts (3)
src/core/game/Game.ts (2)
  • Tick (22-22)
  • Player (517-651)
src/core/game/GameMap.ts (2)
  • TileRef (3-3)
  • x (122-124)
src/core/execution/MIRVExecution.ts (2)
  • owner (179-181)
  • MirvExecution (16-190)

coderabbitai[bot]
coderabbitai bot previously approved these changes Oct 18, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (2)
src/core/execution/FakeHumanExecution.ts (2)

738-771: Early-game edge case: MIRV spam when no one else has cities.

If secondHighest === 0 (e.g., early game where only one player has built cities), then threshold = 0 * 1.3 = 0 at line 764. This means any player with 1+ cities would trigger a MIRV launch at line 766, potentially wasting expensive MIRVs.

Add a guard to prevent premature MIRV usage:

   let secondHighest = 0;
   for (const { p, cityCount } of allPlayers) {
     if (p !== topTarget.p) {
       secondHighest = cityCount;
       break;
     }
   }

+  // Avoid MIRV spam in early game when no one else has cities
+  if (secondHighest === 0) return null;
+
   const threshold =
     secondHighest * FakeHumanExecution.STEAMROLL_CITY_GAP_MULTIPLIER;

774-779: Add bot filter to getValidMirvTargetPlayers() for consistency with nuke targeting

The inconsistency you identified is real. maybeSendNuke() explicitly excludes PlayerType.Bot at line 276, but getValidMirvTargetPlayers() does not. Since both are weapons, they should follow the same targeting rules. The p.isPlayer() check is not sufficient—nuke has an additional explicit bot type check, indicating these filters serve different purposes.

Apply the suggested fix to keep weapon targeting logic consistent:

 private getValidMirvTargetPlayers(): Player[] {
   if (this.player === null) throw new Error("not initialized");
   return this.mg.players().filter((p) => {
-    return p !== this.player && p.isPlayer() && !this.player!.isOnSameTeam(p);
+    return (
+      p !== this.player &&
+      p.isPlayer() &&
+      !this.player!.isOnSameTeam(p) &&
+      p.type() !== PlayerType.Bot
+    );
   });
 }
🧹 Nitpick comments (1)
src/core/execution/FakeHumanExecution.ts (1)

781-826: Helper methods are well-implemented.

isInboundMIRVFrom() correctly checks enemy MIRV targets. calculateTerritoryCenter() uses a centroid approach that aligns with the 1500-tile MIRV radius from the learnings - aiming at the center maximizes coverage.

Optional: Use Math.hypot() for clearer distance calculation at lines 815-818:

     for (const tile of tiles) {
-      const distance = Math.sqrt(
-        Math.pow(this.mg.x(tile) - centerX, 2) +
-          Math.pow(this.mg.y(tile) - centerY, 2),
-      );
+      const distance = Math.hypot(
+        this.mg.x(tile) - centerX,
+        this.mg.y(tile) - centerY,
+      );
       if (distance < closestDistance) {
         closestDistance = distance;
         closestTile = tile;
       }
     }

Based on learnings

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b22cfb0 and 1bd4f1a.

📒 Files selected for processing (3)
  • src/core/execution/FakeHumanExecution.ts (6 hunks)
  • src/core/execution/MIRVExecution.ts (1 hunks)
  • src/core/game/Game.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/core/execution/MIRVExecution.ts
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: sambokai
PR: openfrontio/OpenFrontIO#2225
File: src/core/execution/FakeHumanExecution.ts:172-173
Timestamp: 2025-10-18T17:54:01.293Z
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.
📚 Learning: 2025-05-18T23:36:12.847Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:143-159
Timestamp: 2025-05-18T23:36:12.847Z
Learning: In this codebase, NukeType is a union type derived from UnitType values (specifically bomb-related values like AtomBomb, HydrogenBomb, MIRV, and MIRVWarhead) rather than a separate enum. This means comparing NukeType values against UnitType values in switch statements is valid and intentional.

Applied to files:

  • src/core/game/Game.ts
📚 Learning: 2025-05-18T23:36:12.847Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#784
File: src/core/game/StatsImpl.ts:143-159
Timestamp: 2025-05-18T23:36:12.847Z
Learning: In this codebase, NukeType is a union type derived from specific UnitType enum values (AtomBomb, HydrogenBomb, MIRV, MIRVWarhead). This means that comparisons in switch statements between a NukeType parameter and UnitType enum values are valid and will work correctly at runtime.

Applied to files:

  • src/core/game/Game.ts
📚 Learning: 2025-10-18T17:54:01.293Z
Learnt from: sambokai
PR: openfrontio/OpenFrontIO#2225
File: src/core/execution/FakeHumanExecution.ts:172-173
Timestamp: 2025-10-18T17:54:01.293Z
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
📚 Learning: 2025-10-17T19:16:57.669Z
Learnt from: sambokai
PR: openfrontio/OpenFrontIO#2225
File: src/core/execution/FakeHumanExecution.ts:711-723
Timestamp: 2025-10-17T19:16:57.669Z
Learning: In the OpenFrontIO game, MIRVs (Multiple Independently targetable Reentry Vehicles) have an area of effect with a radius of 1500 tiles around the aim point. To maximize efficiency, bots should aim at the center of the target player's territory rather than selecting random tiles. This differs from the initial understanding that MIRVs covered an entire player's territory regardless of aim point.

Applied to files:

  • src/core/execution/FakeHumanExecution.ts
📚 Learning: 2025-08-23T08:03:44.914Z
Learnt from: ElMelchizedek
PR: openfrontio/OpenFrontIO#1876
File: src/core/execution/FakeHumanExecution.ts:547-592
Timestamp: 2025-08-23T08:03:44.914Z
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/FakeHumanExecution.ts (4)
src/core/game/Game.ts (2)
  • Tick (22-22)
  • Player (519-653)
src/core/game/GameMap.ts (2)
  • TileRef (3-3)
  • x (122-124)
src/core/game/PlayerImpl.ts (3)
  • team (740-742)
  • target (514-517)
  • tiles (278-280)
src/core/execution/MIRVExecution.ts (2)
  • owner (181-183)
  • MirvExecution (16-192)
🔇 Additional comments (7)
src/core/game/Game.ts (1)

266-268: LGTM - Type signature aligns with other nuke types.

The optional targetTile field matches the pattern used by AtomBomb, HydrogenBomb, and MIRVWarhead. This enables MIRV execution to specify a target destination.

src/core/execution/FakeHumanExecution.ts (6)

22-22: Well-structured MIRV configuration.

The constants are clearly named and documented. The cooldown (600 ticks) and thresholds (85% team, 70% individual) provide reasonable strategic balance. The lastMIRVSent state field follows the same pattern as lastNukeSent.

Also applies to: 31-39, 55-55


172-172: Correct integration - MIRV doesn't block other actions.

The return value is intentionally not used to short-circuit maybeAttack(), allowing both MIRV and ground attacks in the same tick.

Based on learnings


671-698: MIRV decision flow is well-structured.

The three-strategy priority system (counter-MIRV, victory denial, steamroll stop) provides strategic depth. The cooldown check at line 677 correctly gates repeated MIRV launches.


700-708: Counter-MIRV strategy is sound.

Prioritizing the largest attacker when multiple players have inbound MIRVs is a reasonable defensive strategy.


710-736: Victory denial strategy handles edge cases correctly.

The totalLand === 0 guard at line 713 prevents division-by-zero. The lower thresholds (85% team, 70% individual) compared to actual win conditions allow strategic preemptive strikes.


829-865: MIRV execution methods are correct.

The fallback logic in maybeSendMIRV() (center first, then any tile) ensures MIRVs are launched when possible. sendMIRV() correctly prunes old events before adding new ones (line 850), preventing unbounded memory growth. The cooldown implementation uses the defined constant and correct <= comparison.

coderabbitai[bot]
coderabbitai bot previously approved these changes Oct 18, 2025
coderabbitai[bot]
coderabbitai bot previously approved these changes Oct 19, 2025
coderabbitai[bot]
coderabbitai bot previously approved these changes Oct 19, 2025
coderabbitai[bot]
coderabbitai bot previously approved these changes Oct 19, 2025
@sambokai sambokai changed the title feat: implement MIRV strategies for bots Feature: Enable Bots to Launch MIRVs Strategically Oct 19, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
src/core/execution/FakeHumanExecution.ts (1)

743-776: Guard against early-game MIRV spam when secondHighest === 0.

If only one player has built cities, secondHighest will be 0 (line 763), making threshold = 0 * 1.3 = 0. Line 771 then approves any player with 1+ cities, potentially triggering MIRVs too early before strategic dominance is established.

Add a guard after line 766:

     for (const { p, cityCount } of allPlayers) {
       if (p !== topTarget.p) {
         secondHighest = cityCount;
         break;
       }
     }
+
+    // Avoid MIRV in early game when no one else has cities
+    if (secondHighest === 0) return null;
 
     const threshold =
       secondHighest * FakeHumanExecution.STEAMROLL_CITY_GAP_MULTIPLIER;
🧹 Nitpick comments (1)
src/core/execution/FakeHumanExecution.ts (1)

784-801: Consider filtering out PlayerType.Bot targets for consistency.

Line 796 doesn't filter PlayerType.Bot, but maybeSendNuke (line 276) explicitly excludes bots from targeting. This inconsistency could lead to bots launching MIRVs at each other.

If bot-on-bot MIRV attacks are undesired, apply this filter:

     const players = this.mg.players().filter((p) => {
-      return p !== this.player && p.isPlayer() && !this.player!.isOnSameTeam(p);
+      return (
+        p !== this.player &&
+        p.isPlayer() &&
+        !this.player!.isOnSameTeam(p) &&
+        p.type() !== PlayerType.Bot
+      );
     });
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8ce87bf and 73d25b1.

📒 Files selected for processing (1)
  • src/core/execution/FakeHumanExecution.ts (6 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: sambokai
PR: openfrontio/OpenFrontIO#2225
File: src/core/execution/FakeHumanExecution.ts:172-173
Timestamp: 2025-10-18T17:54:01.293Z
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.
📚 Learning: 2025-10-18T17:54:01.293Z
Learnt from: sambokai
PR: openfrontio/OpenFrontIO#2225
File: src/core/execution/FakeHumanExecution.ts:172-173
Timestamp: 2025-10-18T17:54:01.293Z
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
📚 Learning: 2025-10-17T19:16:57.669Z
Learnt from: sambokai
PR: openfrontio/OpenFrontIO#2225
File: src/core/execution/FakeHumanExecution.ts:711-723
Timestamp: 2025-10-17T19:16:57.669Z
Learning: In the OpenFrontIO game, MIRVs (Multiple Independently targetable Reentry Vehicles) have an area of effect with a radius of 1500 tiles around the aim point. To maximize efficiency, bots should aim at the center of the target player's territory rather than selecting random tiles. This differs from the initial understanding that MIRVs covered an entire player's territory regardless of aim point.

Applied to files:

  • src/core/execution/FakeHumanExecution.ts
📚 Learning: 2025-08-23T08:03:44.914Z
Learnt from: ElMelchizedek
PR: openfrontio/OpenFrontIO#1876
File: src/core/execution/FakeHumanExecution.ts:547-592
Timestamp: 2025-08-23T08:03:44.914Z
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/FakeHumanExecution.ts (4)
src/core/game/Game.ts (2)
  • Tick (22-22)
  • Player (519-653)
src/core/game/GameMap.ts (2)
  • TileRef (3-3)
  • x (122-124)
src/core/game/PlayerImpl.ts (3)
  • team (740-742)
  • target (514-517)
  • tiles (278-280)
src/core/game/GameImpl.ts (3)
  • x (780-782)
  • players (402-404)
  • target (574-580)
🔇 Additional comments (6)
src/core/execution/FakeHumanExecution.ts (6)

31-39: LGTM! Well-defined constants for MIRV strategy.

The constants are clearly named and align with the PR objectives. The 10-minute cooldown (6000 ticks) and 35% failure rate provide good strategic balance.


55-55: Good pattern reuse for MIRV tracking.

Mirrors the lastNukeSent structure, maintaining consistency in the codebase.


172-172: Correct: MIRV and ground attacks can occur in same tick.

The implementation correctly allows both actions, as clarified in previous reviews.

Based on learnings.


671-703: Strategic MIRV decision flow is well-structured.

The method correctly:

  • Enforces prerequisites and cooldown
  • Simulates hesitation via failure rate
  • Prioritizes strategies logically (retaliation → prevention → balance)

820-850: Efficient territory center calculation with squared-distance optimization.

Correctly calculates the centroid and finds the nearest tile without unnecessary square roots. This implementation properly aims MIRVs at the center of the target's territory for maximum 1500-tile radius coverage.

Based on learnings.


853-897: MIRV execution and cooldown management implemented correctly.

The methods properly:

  • Prioritize center-tile targeting with sensible fallback
  • Trigger cooldown before launching (including for failure simulation)
  • Prune old events using the constant with correct <= comparison

The fallback tile in triggerMIRVCooldown (line 884) is safe since only the tick matters for cooldown logic.

coderabbitai[bot]
coderabbitai bot previously approved these changes Oct 19, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
src/core/execution/FakeHumanExecution.ts (1)

743-776: Steamroll: guard early game (secondHighest === 0) to avoid always-trigger.

If the next best cityCount is 0, threshold becomes 0 and the top target is always picked. Add a guard.

     if (allPlayers.length < 2) return null;
@@
-    const threshold =
+    // Avoid MIRV spam when no other player has cities yet
+    if (secondHighest === 0) return null;
+
+    const threshold =
       secondHighest * FakeHumanExecution.STEAMROLL_CITY_GAP_MULTIPLIER;
🧹 Nitpick comments (2)
src/core/execution/FakeHumanExecution.ts (1)

784-801: Align MIRV targeting with nuke policy (optional): exclude basic bots.

maybeSendNuke skips PlayerType.Bot; MIRV target filter currently includes them. Consider making both consistent.

-    const players = this.mg.players().filter((p) => {
-      return p !== this.player && p.isPlayer() && !this.player!.isOnSameTeam(p);
-    });
+    const players = this.mg.players().filter((p) =>
+      p !== this.player &&
+      p.isPlayer() &&
+      p.type() !== PlayerType.Bot &&
+      !this.player!.isOnSameTeam(p)
+    );
tests/FakeHumanMIRV.test.ts (1)

95-119: Make the test deterministic and simpler: register the execution and tick the game.

Avoid manual tick math and seed hunting. Attach FakeHumanExecution to the game and run ticks; or stub RNG to bypass the failure path.

-    let retaliationSuccessful = false;
-
-    for (const gameId of gameIds) {
-      const testExecution = new FakeHumanExecution(gameId, fakehumanNation);
-      testExecution.init(game);
-
-      // Execute fakehuman's tick logic...
-      for (let tick = 0; tick < 200; tick++) {
-        testExecution.tick(game.ticks() + tick);
-        if (tick % 10 === 0) {
-          game.executeNextTick();
-        }
-        if (fakehuman.units(UnitType.MIRV).length > mirvCountBefore) {
-          retaliationSuccessful = true;
-          break;
-        }
-      }
-      if (retaliationSuccessful) break;
-    }
+    const testExecution = new FakeHumanExecution("game_test", fakehumanNation);
+    testExecution.init(game);
+    // Run inside the engine like real gameplay
+    game.addExecution(testExecution as any);
+    executeTicks(game, 400);
+    const retaliationSuccessful =
+      fakehuman.units(UnitType.MIRV).length > mirvCountBefore;

Optionally, stub the failure to remove flakiness:

// jest.spyOn(PseudoRandom.prototype, "nextInt").mockImplementation((min, max) => {
//   if (min === 0 && max === 100) return 99; // ensure failure check is false
//   return Math.floor((min + max) / 2);
// });
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 73d25b1 and 7bda321.

📒 Files selected for processing (2)
  • src/core/execution/FakeHumanExecution.ts (7 hunks)
  • tests/FakeHumanMIRV.test.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: sambokai
PR: openfrontio/OpenFrontIO#2225
File: src/core/execution/FakeHumanExecution.ts:172-173
Timestamp: 2025-10-18T17:54:01.293Z
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.
📚 Learning: 2025-10-18T17:54:01.293Z
Learnt from: sambokai
PR: openfrontio/OpenFrontIO#2225
File: src/core/execution/FakeHumanExecution.ts:172-173
Timestamp: 2025-10-18T17:54:01.293Z
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
  • tests/FakeHumanMIRV.test.ts
📚 Learning: 2025-10-17T19:16:57.669Z
Learnt from: sambokai
PR: openfrontio/OpenFrontIO#2225
File: src/core/execution/FakeHumanExecution.ts:711-723
Timestamp: 2025-10-17T19:16:57.669Z
Learning: In the OpenFrontIO game, MIRVs (Multiple Independently targetable Reentry Vehicles) have an area of effect with a radius of 1500 tiles around the aim point. To maximize efficiency, bots should aim at the center of the target player's territory rather than selecting random tiles. This differs from the initial understanding that MIRVs covered an entire player's territory regardless of aim point.

Applied to files:

  • src/core/execution/FakeHumanExecution.ts
📚 Learning: 2025-08-23T08:03:44.914Z
Learnt from: ElMelchizedek
PR: openfrontio/OpenFrontIO#1876
File: src/core/execution/FakeHumanExecution.ts:547-592
Timestamp: 2025-08-23T08:03:44.914Z
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 (2)
src/core/execution/FakeHumanExecution.ts (5)
src/core/PseudoRandom.ts (1)
  • PseudoRandom (3-76)
src/core/execution/utils/BotBehavior.ts (1)
  • BotBehavior (25-340)
src/core/game/Game.ts (3)
  • Tick (22-22)
  • Nation (296-302)
  • Player (519-653)
src/core/game/PlayerImpl.ts (3)
  • team (740-742)
  • target (514-517)
  • tiles (278-280)
src/core/execution/MIRVExecution.ts (2)
  • owner (181-183)
  • MirvExecution (16-192)
tests/FakeHumanMIRV.test.ts (5)
tests/util/Setup.ts (1)
  • setup (23-80)
src/core/game/Game.ts (2)
  • PlayerInfo (397-417)
  • Cell (304-326)
src/core/execution/MIRVExecution.ts (1)
  • MirvExecution (16-192)
tests/util/utils.ts (1)
  • executeTicks (32-36)
src/core/execution/FakeHumanExecution.ts (1)
  • FakeHumanExecution (30-906)

@sambokai sambokai changed the title Feature: Enable Bots to Launch MIRVs Strategically Feature: Enable FakeHumans (Nation Bots) to Launch MIRVs Strategically Oct 19, 2025
@sambokai sambokai changed the title Feature: Enable FakeHumans (Nation Bots) to Launch MIRVs Strategically Feature: Enable FakeHumans ("Nation Bots") to Launch MIRVs Strategically Oct 19, 2025
coderabbitai[bot]
coderabbitai bot previously approved these changes Oct 19, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (2)
src/core/execution/FakeHumanExecution.ts (1)

871-875: Bug: TileRef truthiness on centerTile/canBuild (tile 0 ignored).

TileRef is a number; 0 is valid but falsy. Compare explicitly:

-    const centerTile = this.calculateTerritoryCenter(enemy);
-    if (centerTile && this.player.canBuild(UnitType.MIRV, centerTile)) {
-      this.sendMIRV(centerTile);
-      return;
-    }
+    const centerTile = this.calculateTerritoryCenter(enemy);
+    if (centerTile !== null) {
+      const spawn = this.player.canBuild(UnitType.MIRV, centerTile);
+      if (spawn !== false) {
+        this.sendMIRV(centerTile);
+        return;
+      }
+    }
tests/FakeHumanMIRV.test.ts (1)

96-103: Advance the game every iteration and use game.ticks().

Current loop advances only every 10 steps and calls tick(tick). Make it deterministic:

-      for (let tick = 0; tick < 200; tick++) {
-        testExecution.tick(tick);
-        // Allow the game to process executions
-        if (tick % 10 === 0) {
-          game.executeNextTick();
-        }
+      for (let i = 0; i < 200; i++) {
+        game.executeNextTick();
+        testExecution.tick(game.ticks());
         }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c335588 and 7e8c999.

📒 Files selected for processing (5)
  • src/core/execution/FakeHumanExecution.ts (7 hunks)
  • src/core/execution/MIRVExecution.ts (1 hunks)
  • src/core/execution/Util.ts (2 hunks)
  • src/core/game/Game.ts (1 hunks)
  • tests/FakeHumanMIRV.test.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/core/execution/Util.ts
  • src/core/game/Game.ts
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
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.
Learnt from: sambokai
PR: openfrontio/OpenFrontIO#2225
File: src/core/execution/FakeHumanExecution.ts:770-795
Timestamp: 2025-10-27T09:47:26.376Z
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.
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.
📚 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
  • tests/FakeHumanMIRV.test.ts
  • src/core/execution/MIRVExecution.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/FakeHumanExecution.ts
  • tests/FakeHumanMIRV.test.ts
📚 Learning: 2025-10-20T11:02:16.969Z
Learnt from: sambokai
PR: openfrontio/OpenFrontIO#2225
File: src/core/execution/FakeHumanExecution.ts:57-57
Timestamp: 2025-10-20T11:02:16.969Z
Learning: In src/core/execution/FakeHumanExecution.ts, the correct MIRV victory denial thresholds are VICTORY_DENIAL_TEAM_THRESHOLD = 0.8 (80% for team games) and VICTORY_DENIAL_INDIVIDUAL_THRESHOLD = 0.65 (65% for individual players), not 0.85 and 0.7 as might be mentioned in some documentation.

Applied to files:

  • src/core/execution/FakeHumanExecution.ts
  • tests/FakeHumanMIRV.test.ts
📚 Learning: 2025-10-27T09:47:26.376Z
Learnt from: sambokai
PR: openfrontio/OpenFrontIO#2225
File: src/core/execution/FakeHumanExecution.ts:770-795
Timestamp: 2025-10-27T09:47:26.376Z
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/FakeHumanExecution.ts
  • tests/FakeHumanMIRV.test.ts
📚 Learning: 2025-10-17T19:16:57.699Z
Learnt from: sambokai
PR: openfrontio/OpenFrontIO#2225
File: src/core/execution/FakeHumanExecution.ts:711-723
Timestamp: 2025-10-17T19:16:57.699Z
Learning: In the OpenFrontIO game, MIRVs (Multiple Independently targetable Reentry Vehicles) have an area of effect with a radius of 1500 tiles around the aim point. To maximize efficiency, bots should aim at the center of the target player's territory rather than selecting random tiles. This differs from the initial understanding that MIRVs covered an entire player's territory regardless of aim point.

Applied to files:

  • src/core/execution/FakeHumanExecution.ts
📚 Learning: 2025-08-23T08:03:44.914Z
Learnt from: ElMelchizedek
PR: openfrontio/OpenFrontIO#1876
File: src/core/execution/FakeHumanExecution.ts:547-592
Timestamp: 2025-08-23T08:03:44.914Z
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
📚 Learning: 2025-10-26T15:37:07.700Z
Learnt from: GlacialDrift
PR: openfrontio/OpenFrontIO#2298
File: src/client/graphics/layers/TerritoryLayer.ts:200-210
Timestamp: 2025-10-26T15:37:07.700Z
Learning: In GameImpl.ts lines 124-139, team assignment logic varies by number of teams: when numPlayerTeams < 8, teams are assigned ColoredTeams values (Red, Blue, Yellow, Green, Purple, Orange, Teal); when numPlayerTeams >= 8, teams are assigned generic string identifiers like "Team 1", "Team 2", etc., which are not members of ColoredTeams.

Applied to files:

  • tests/FakeHumanMIRV.test.ts
🧬 Code graph analysis (2)
src/core/execution/FakeHumanExecution.ts (7)
src/core/game/Game.ts (4)
  • Execution (346-351)
  • Tick (23-23)
  • PlayerID (22-22)
  • Player (520-656)
src/core/PseudoRandom.ts (1)
  • PseudoRandom (3-76)
src/core/execution/utils/BotBehavior.ts (1)
  • BotBehavior (25-340)
src/core/game/GameImpl.ts (5)
  • ticks (326-328)
  • x (780-782)
  • players (402-404)
  • owner (165-167)
  • target (574-580)
src/core/game/PlayerImpl.ts (3)
  • team (770-772)
  • tiles (279-281)
  • target (522-525)
src/core/execution/MIRVExecution.ts (2)
  • owner (181-183)
  • MirvExecution (16-192)
src/core/execution/Util.ts (1)
  • calculateTerritoryCenter (84-132)
tests/FakeHumanMIRV.test.ts (5)
tests/util/Setup.ts (1)
  • setup (23-80)
src/core/game/Game.ts (2)
  • PlayerInfo (401-415)
  • Cell (308-330)
src/core/execution/FakeHumanExecution.ts (2)
  • FakeHumanExecution (31-912)
  • tick (135-187)
src/core/execution/MIRVExecution.ts (1)
  • MirvExecution (16-192)
tests/util/utils.ts (1)
  • executeTicks (32-36)
🔇 Additional comments (4)
src/core/execution/MIRVExecution.ts (1)

71-73: Good: pass target tile into MIRV spawn.

This aligns MIRV unit params with Game.ts and enables targetTile() queries. Looks correct.

src/core/execution/FakeHumanExecution.ts (3)

185-186: OK: MIRVs and ground actions in same tick.

Design choice acknowledged and matches intent.

Based on learnings


894-901: Cooldown pruning looks correct.

Uses MIRV_COOLDOWN_TICKS and ≤ to avoid off‑by‑one. Good.


829-836: Target filtering is sensible.

Excludes self, teammates, and bots; caches briefly. Good.

Comment on lines +260 to +265
for (let tick = 0; tick < 200; tick++) {
testExecution.tick(game.ticks());
// Allow the game to process executions
if (tick % 10 === 0) {
game.executeNextTick();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Same fix: drive one tick per loop.

Apply the same per-iteration advancement and call tick(game.ticks()) as above.

🤖 Prompt for AI Agents
In tests/FakeHumanMIRV.test.ts around lines 260 to 265, the loop advances ticks
in batches and only calls game.executeNextTick() every 10 iterations; change it
to drive one tick per loop by calling game.executeNextTick() every iteration and
calling testExecution.tick(game.ticks()) once per iteration (i.e., remove the
modulo conditional and invoke executeNextTick() inside the loop each tick) so
the test advances one tick per loop and keeps testExecution in sync.

Comment on lines +413 to +418
for (let tick = 0; tick < 200; tick++) {
testExecution.tick(game.ticks());
// Allow the game to process executions
if (tick % 10 === 0) {
game.executeNextTick();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Same fix in steamroll test.

Advance every iteration and pass game.ticks() to avoid missing the processing window.

🤖 Prompt for AI Agents
In tests/FakeHumanMIRV.test.ts around lines 413-418, the loop only calls
game.executeNextTick() every 10 iterations which can miss processing windows;
modify the loop to advance the game every iteration and call
testExecution.tick(game.ticks()) each iteration, and call game.executeNextTick()
on every tick (not only when tick % 10 === 0) so the test advances synchronously
and doesn't skip processing.

Comment on lines +698 to +703
for (let tick = 0; tick < 200; tick++) {
testExecution.tick(game.ticks());
// Allow the game to process executions
if (tick % 10 === 0) {
game.executeNextTick();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Same fix in team victory denial test.

Advance one tick each loop and use game.ticks() for consistency.

🤖 Prompt for AI Agents
In tests/FakeHumanMIRV.test.ts around lines 698 to 703, the loop currently
advances testExecution.tick by a fixed value and only occasionally calls
game.executeNextTick; change it so each iteration advances by one tick using
testExecution.tick(game.ticks()) and call game.executeNextTick() on every
iteration (i.e., move game.executeNextTick() outside the if block) to keep tick
progression consistent with game.ticks().

@evanpelle evanpelle merged commit af86a92 into openfrontio:main Oct 29, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MIRV for bots

5 participants