Revert "fix: traitor bug when attacking immediately after initiating an alliance (#2044)"

This reverts commit 6f96788406.

src/core/execution/AttackExecution.ts

@@ -16,6 +16,8 @@ import { FlatBinaryHeap } from "./utils/FlatBinaryHeap"; // adjust path if neede
 
 const malusForRetreat = 25;
 export class AttackExecution implements Execution {
+  private breakAlliance = false;
+  private wasAlliedAtInit = false; // Store alliance state at initialization
   private active: boolean = true;
   private toConquer = new FlatBinaryHeap();
 
@@ -60,24 +62,6 @@ export class AttackExecution implements Execution {
         ? mg.terraNullius()
         : mg.player(this._targetID);
 
-    if (this._owner === this.target) {
-      console.error(`Player ${this._owner} cannot attack itself`);
-      this.active = false;
-      return;
-    }
-
-    // ALLIANCE CHECK — block attacks on friendly (ally or same team)
-    if (this.target.isPlayer()) {
-      const targetPlayer = this.target as Player;
-      if (this._owner.isFriendly(targetPlayer)) {
-        console.warn(
-          `${this._owner.displayName()} cannot attack ${targetPlayer.displayName()} because they are friendly (allied or same team)`,
-        );
-        this.active = false;
-        return;
-      }
-    }
-
     if (this.target && this.target.isPlayer()) {
       const targetPlayer = this.target as Player;
       if (
@@ -86,10 +70,15 @@ export class AttackExecution implements Execution {
       ) {
         // Don't let bots embargo since they can't trade anyway.
         targetPlayer.addEmbargo(this._owner, true);
-        this.rejectIncomingAllianceRequests(targetPlayer);
       }
     }
 
+    if (this._owner === this.target) {
+      console.error(`Player ${this._owner} cannot attack itself`);
+      this.active = false;
+      return;
+    }
+
     if (this.target.isPlayer()) {
       if (
         this.mg.config().numSpawnPhaseTurns() +
@@ -159,6 +148,11 @@ export class AttackExecution implements Execution {
     }
 
     if (this.target.isPlayer()) {
+      // Store the alliance state at initialization time to prevent race conditions
+      this.wasAlliedAtInit = this._owner.isAlliedWith(this.target);
+      if (this.wasAlliedAtInit) {
+        this.breakAlliance = true;
+      }
       this.target.updateRelation(this._owner, -80);
     }
   }
@@ -227,8 +221,20 @@ export class AttackExecution implements Execution {
       return;
     }
 
-    if (targetPlayer && this._owner.isFriendly(targetPlayer)) {
+    const alliance = targetPlayer
+      ? this._owner.allianceWith(targetPlayer)
+      : null;
+    if (this.breakAlliance && alliance !== null) {
+      this.breakAlliance = false;
+      this._owner.breakAlliance(alliance);
+    }
+    if (
+      targetPlayer &&
+      this._owner.isAlliedWith(targetPlayer) &&
+      !this.wasAlliedAtInit
+    ) {
       // In this case a new alliance was created AFTER the attack started.
+      // We should retreat to avoid the attacker becoming a traitor.
       this.retreat();
       return;
     }
@@ -289,15 +295,6 @@ export class AttackExecution implements Execution {
     }
   }
 
-  private rejectIncomingAllianceRequests(target: Player) {
-    const request = this._owner
-      .incomingAllianceRequests()
-      .find((ar) => ar.requestor() === target);
-    if (request !== undefined) {
-      request.reject();
-    }
-  }
-
   private addNeighbors(tile: TileRef) {
     if (this.attack === null) {
       throw new Error("Attack not initialized");

src/core/execution/BotExecution.ts

@@ -69,13 +69,6 @@ export class BotExecution implements Execution {
     if (toAttack !== null) {
       const odds = this.bot.isFriendly(toAttack) ? 6 : 3;
       if (this.random.chance(odds)) {
-        // Check and break alliance before attacking if needed
-        const alliance = this.bot.allianceWith(toAttack);
-
-        if (alliance !== null) {
-          this.bot.breakAlliance(alliance);
-        }
-
         this.behavior.sendAttack(toAttack);
         return;
       }

src/core/execution/FakeHumanExecution.ts

@@ -161,30 +161,6 @@ export class FakeHumanExecution implements Execution {
     this.maybeAttack();
   }
 
-  /**
-   * TODO: Implement strategic betrayal logic
-   * Currently this just breaks alliances without strategic consideration.
-   * Future implementation should consider:
-   * - Relative strength (troop count, territory size) compared to target
-   * - Risk vs reward of betrayal
-   * - Potential impact on relations with other players
-   * - Timing (don't betray when already fighting other enemies)
-   * - Strategic value of target's territory
-   * - If target is distracted
-   */
-  private maybeConsiderBetrayal(target: Player): boolean {
-    if (this.player === null) throw new Error("not initialized");
-
-    const alliance = this.player.allianceWith(target);
-
-    if (!alliance) return false;
-
-    this.player.breakAlliance(alliance);
-
-    // Successfully broken an alliance
-    return true;
-  }
-
   private maybeAttack() {
     if (this.player === null || this.behavior === null) {
       throw new Error("not initialized");
@@ -232,7 +208,6 @@ export class FakeHumanExecution implements Execution {
     const toAttack = this.random.chance(2)
       ? enemies[0]
       : this.random.randElement(enemies);
-
     if (this.shouldAttack(toAttack)) {
       this.behavior.sendAttack(toAttack);
       return;
@@ -253,17 +228,9 @@ export class FakeHumanExecution implements Execution {
 
   private shouldAttack(other: Player): boolean {
     if (this.player === null) throw new Error("not initialized");
-
     if (this.player.isOnSameTeam(other)) {
       return false;
     }
-
-    // Consider betrayal for allies
-    if (this.player.isAlliedWith(other)) {
-      const canProceed = this.maybeConsiderBetrayal(other);
-      return canProceed;
-    }
-
     if (this.player.isFriendly(other)) {
       if (this.shouldDiscourageAttack(other)) {
         return this.random.chance(200);
@@ -429,7 +396,7 @@ export class FakeHumanExecution implements Execution {
 
   private maybeSendBoatAttack(other: Player) {
     if (this.player === null) throw new Error("not initialized");
-    if (this.player.isFriendly(other)) return;
+    if (this.player.isOnSameTeam(other)) return;
     const closest = closestTwoTiles(
       this.mg,
       Array.from(this.player.borderTiles()).filter((t) =>

src/core/execution/utils/BotBehavior.ts

@@ -230,9 +230,7 @@ export class BotBehavior {
   }
 
   sendAttack(target: Player | TerraNullius) {
-    // Skip attacking friendly targets (allies or teammates) - decision to break alliances should be made by caller
-    if (target.isPlayer() && this.player.isFriendly(target)) return;
-
+    if (target.isPlayer() && this.player.isOnSameTeam(target)) return;
     const maxTroops = this.game.config().maxTroops(this.player);
     const reserveRatio = target.isPlayer()
       ? this.reserveRatio
@@ -244,7 +242,7 @@ export class BotBehavior {
       new AttackExecution(
         troops,
         this.player,
-        target.isPlayer() ? target.id() : this.game.terraNullius().id(),
+        target.isPlayer() ? target.id() : null,
       ),
     );
   }