Revert "AFK team mate: better ship handling + tests + bugfix (#2203)"

This reverts commit 896a8ebe92.

src/core/configuration/DefaultConfig.ts

@@ -680,7 +680,7 @@ export class DefaultConfig implements Config {
 
     if (attacker.isPlayer() && defender.isPlayer()) {
       if (defender.isDisconnected() && attacker.isOnSameTeam(defender)) {
-        // No troop loss if defender is disconnected and on same team
+        // No troop loss if defender is disconnected.
         mag = 0;
       }
       if (

src/core/execution/AttackExecution.ts

@@ -181,12 +181,6 @@ export class AttackExecution implements Execution {
         this._owner.id(),
       );
     }
-    if (this.removeTroops === false) {
-      // startTroops are always added to attack troops at init but not always removed from owner troops
-      // subtract startTroops from attack troops so we don't give back startTroops to owner that were never removed
-      this.attack.setTroops(this.attack.troops() - (this.startTroops ?? 0));
-    }
-
     const survivors = this.attack.troops() - deaths;
     this._owner.addTroops(survivors);
     this.attack.delete();

src/core/execution/TransportShipExecution.ts

@@ -33,17 +33,13 @@ export class TransportShipExecution implements Execution {
 
   private pathFinder: PathFinder;
 
-  private originalOwner: Player;
-
   constructor(
     private attacker: Player,
     private targetID: PlayerID | null,
     private ref: TileRef,
     private startTroops: number,
     private src: TileRef | null,
-  ) {
-    this.originalOwner = this.attacker;
-  }
+  ) {}
 
   activeDuringSpawnPhase(): boolean {
     return false;
@@ -177,43 +173,11 @@ export class TransportShipExecution implements Execution {
     }
     this.lastMove = ticks;
 
-    // Team mate can conquer disconnected player and get their ships
-    // captureUnit has changed the owner of the unit, now update attacker
-    if (
-      this.originalOwner.isDisconnected() &&
-      this.boat.owner() !== this.originalOwner &&
-      this.boat.owner().isOnSameTeam(this.originalOwner)
-    ) {
-      this.attacker = this.boat.owner();
-      this.originalOwner = this.boat.owner(); // for when this owner disconnects too
-    }
-
     if (this.boat.retreating()) {
-      // Ensure retreat source is valid for the new owner
-      if (this.mg.owner(this.src!) !== this.attacker) {
-        // Use bestTransportShipSpawn, not canBuild because of its max boats check etc
-        const newSrc = this.attacker.bestTransportShipSpawn(this.dst);
-        if (newSrc === false) {
-          this.src = null;
-        } else {
-          this.src = newSrc;
-        }
-      }
+      this.dst = this.src!; // src is guaranteed to be set at this point
 
-      if (this.src === null) {
-        console.warn(
-          `TransportShipExecution: retreating but no src found for new attacker`,
-        );
-        this.attacker.addTroops(this.boat.troops());
-        this.boat.delete(false);
-        this.active = false;
-        return;
-      } else {
-        this.dst = this.src;
-
-        if (this.boat.targetTile() !== this.dst) {
-          this.boat.setTargetTile(this.dst);
-        }
+      if (this.boat.targetTile() !== this.dst) {
+        this.boat.setTargetTile(this.dst);
       }
     }
 

src/core/execution/WarshipExecution.ts

@@ -55,6 +55,10 @@ export class WarshipExecution implements Execution {
       this.warship.delete();
       return;
     }
+    if (this.warship.owner().isDisconnected()) {
+      this.warship.delete();
+      return;
+    }
 
     const hasPort = this.warship.owner().unitCount(UnitType.Port) > 0;
     if (hasPort) {
@@ -89,7 +93,7 @@ export class WarshipExecution implements Execution {
       if (
         unit.owner() === this.warship.owner() ||
         unit === this.warship ||
-        unit.owner().isFriendly(this.warship.owner(), true) ||
+        unit.owner().isFriendly(this.warship.owner()) ||
         this.alreadySentShell.has(unit)
       ) {
         continue;

src/core/game/Game.ts

@@ -587,7 +587,7 @@ export interface Player {
   decayRelations(): void;
   isOnSameTeam(other: Player): boolean;
   // Either allied or on same team.
-  isFriendly(other: Player, treatAFKFriendly?: boolean): boolean;
+  isFriendly(other: Player): boolean;
   team(): Team | null;
   clan(): string | null;
   incomingAllianceRequests(): AllianceRequest[];

src/core/game/GameImpl.ts

@@ -877,20 +877,6 @@ export class GameImpl implements Game {
     return this._railNetwork;
   }
   conquerPlayer(conqueror: Player, conquered: Player) {
-    if (conquered.isDisconnected() && conqueror.isOnSameTeam(conquered)) {
-      const ships = conquered
-        .units()
-        .filter(
-          (u) =>
-            u.type() === UnitType.Warship ||
-            u.type() === UnitType.TransportShip,
-        );
-
-      for (const ship of ships) {
-        conqueror.captureUnit(ship);
-      }
-    }
-
     const gold = conquered.gold();
     this.displayMessage(
       `Conquered ${conquered.displayName()} received ${renderNumber(

src/core/game/PlayerImpl.ts

@@ -784,8 +784,8 @@ export class PlayerImpl implements Player {
     return this._team === other.team();
   }
 
-  isFriendly(other: Player, treatAFKFriendly: boolean = false): boolean {
-    if (other.isDisconnected() && !treatAFKFriendly) {
+  isFriendly(other: Player): boolean {
+    if (other.isDisconnected()) {
       return false;
     }
     return this.isOnSameTeam(other) || this.isAlliedWith(other);

src/core/game/TransportShipUtils.ts

@@ -148,8 +148,6 @@ export function bestShoreDeploymentSource(
   if (t === null) return false;
 
   const candidates = candidateShoreTiles(gm, player, t);
-  if (candidates.length === 0) return false;
-
   const aStar = new MiniAStar(gm, gm.miniMap(), candidates, t, 1_000_000, 1);
   const result = aStar.compute();
   if (result !== PathFindResultType.Completed) {

tests/Disconnected.test.ts

@@ -1,25 +1,12 @@
-import { AttackExecution } from "../src/core/execution/AttackExecution";
 import { MarkDisconnectedExecution } from "../src/core/execution/MarkDisconnectedExecution";
 import { SpawnExecution } from "../src/core/execution/SpawnExecution";
-import { TransportShipExecution } from "../src/core/execution/TransportShipExecution";
-import { WarshipExecution } from "../src/core/execution/WarshipExecution";
-import {
-  Game,
-  GameMode,
-  Player,
-  PlayerInfo,
-  PlayerType,
-  UnitType,
-} from "../src/core/game/Game";
-import { toInt } from "../src/core/Util";
+import { Game, Player, PlayerInfo, PlayerType } from "../src/core/game/Game";
 import { setup } from "./util/Setup";
-import { UseRealAttackLogic } from "./util/TestConfig";
 import { executeTicks } from "./util/utils";
 
 let game: Game;
 let player1: Player;
 let player2: Player;
-let enemy: Player;
 
 describe("Disconnected", () => {
   beforeEach(async () => {
@@ -171,311 +158,4 @@ describe("Disconnected", () => {
       expect(player1.isDisconnected()).toBe(true);
     });
   });
-
-  describe("Disconnected team member interactions", () => {
-    const coastX = 7;
-
-    beforeEach(async () => {
-      const player1Info = new PlayerInfo(
-        "[CLAN]Player1",
-        PlayerType.Human,
-        null,
-        "player_1_id",
-      );
-      const player2Info = new PlayerInfo(
-        "[CLAN]Player2",
-        PlayerType.Human,
-        null,
-        "player_2_id",
-      );
-
-      game = await setup(
-        "half_land_half_ocean",
-        {
-          infiniteGold: true,
-          instantBuild: true,
-          gameMode: GameMode.Team,
-          playerTeams: 2, // ignore player2 "kicked" console warn
-        },
-        [player1Info, player2Info],
-        undefined,
-        UseRealAttackLogic, // don't use TestConfig's mock attackLogic
-      );
-
-      game.addExecution(
-        new SpawnExecution(player1Info, game.map().ref(coastX - 2, 1)),
-        new SpawnExecution(player2Info, game.map().ref(coastX - 2, 4)),
-      );
-
-      while (game.inSpawnPhase()) {
-        game.executeNextTick();
-      }
-
-      player1 = game.player(player1Info.id);
-      player2 = game.player(player2Info.id);
-      player2.markDisconnected(false);
-
-      expect(player1.team()).not.toBeNull();
-      expect(player2.team()).not.toBeNull();
-      expect(player1.isOnSameTeam(player2)).toBe(true);
-    });
-
-    test("Team Warships should not attack disconnected team mate ships", () => {
-      const warship = player1.buildUnit(
-        UnitType.Warship,
-        game.map().ref(coastX + 1, 10),
-        {
-          patrolTile: game.map().ref(coastX + 1, 10),
-        },
-      );
-      game.addExecution(new WarshipExecution(warship));
-
-      const transportShip = player2.buildUnit(
-        UnitType.TransportShip,
-        game.map().ref(coastX + 1, 11),
-        {
-          troops: 100,
-        },
-      );
-
-      player2.markDisconnected(true);
-      executeTicks(game, 10);
-
-      expect(warship.targetUnit()).toBe(undefined);
-      expect(transportShip.isActive()).toBe(true);
-      expect(transportShip.owner()).toBe(player2);
-    });
-
-    test("Disconnected player Warship should not attack team members' ships", () => {
-      const warship = player2.buildUnit(
-        UnitType.Warship,
-        game.map().ref(coastX + 1, 5),
-        {
-          patrolTile: game.map().ref(coastX + 1, 10),
-        },
-      );
-      game.addExecution(new WarshipExecution(warship));
-
-      const transportShip = player1.buildUnit(
-        UnitType.TransportShip,
-        game.map().ref(coastX + 1, 6),
-        {
-          troops: 100,
-        },
-      );
-
-      player2.markDisconnected(true);
-      executeTicks(game, 10);
-
-      expect(warship.targetUnit()).toBe(undefined);
-      expect(transportShip.isActive()).toBe(true);
-      expect(transportShip.owner()).toBe(player1);
-    });
-
-    test("Player can attack disconnected team mate without troop loss", () => {
-      player2.conquer(game.map().ref(coastX - 2, 2));
-      player2.conquer(game.map().ref(coastX - 2, 3));
-      player2.markDisconnected(true);
-
-      const troopsBeforeAttack = player1.troops();
-      const startTroops = troopsBeforeAttack * 0.25;
-
-      game.addExecution(
-        new AttackExecution(startTroops, player1, player2.id(), null),
-      );
-
-      let expectedTotalGrowth = 0n;
-      let afterTickZero = false;
-
-      while (player2.isAlive()) {
-        if (afterTickZero) {
-          // No growth on tick 0, troop additions start from tick 1
-          const troopIncThisTick = game.config().troopIncreaseRate(player1);
-          expectedTotalGrowth += toInt(troopIncThisTick);
-        }
-
-        game.executeNextTick();
-        afterTickZero = true;
-      }
-
-      // Tick for retreat() in AttackExecution to add back startTtoops to owner troops
-      const troopIncThisTick1 = game.config().troopIncreaseRate(player1);
-      expectedTotalGrowth += toInt(troopIncThisTick1);
-
-      game.executeNextTick();
-
-      const expectedFinalTroops = Number(
-        toInt(troopsBeforeAttack) + expectedTotalGrowth,
-      );
-
-      // Verify no troop loss
-      expect(player1.troops()).toBe(expectedFinalTroops);
-    });
-
-    test("Conqueror gets conquered disconnected team member's transport- and warships", () => {
-      const warship = player2.buildUnit(
-        UnitType.Warship,
-        game.map().ref(coastX + 1, 1),
-        {
-          patrolTile: game.map().ref(coastX + 1, 1),
-        },
-      );
-      const transportShip = player2.buildUnit(
-        UnitType.TransportShip,
-        game.map().ref(coastX + 1, 3),
-        {
-          troops: 100,
-        },
-      );
-
-      player2.conquer(game.map().ref(coastX - 2, 1));
-      player2.markDisconnected(true);
-
-      game.addExecution(new AttackExecution(1000, player1, player2.id(), null));
-
-      executeTicks(game, 10);
-
-      expect(player2.isAlive()).toBe(false);
-      expect(warship.owner()).toBe(player1);
-      expect(transportShip.owner()).toBe(player1);
-    });
-
-    test("Captured transport ship landing attack should be in name of new owner", () => {
-      player2.conquer(game.map().ref(coastX, 1));
-      player2.conquer(game.map().ref(coastX - 1, 1));
-      player2.conquer(game.map().ref(coastX, 2));
-
-      const enemyShoreTile = game.map().ref(coastX, 15);
-
-      game.addExecution(
-        new TransportShipExecution(
-          player2,
-          null,
-          enemyShoreTile,
-          100,
-          game.map().ref(coastX, 1),
-        ),
-      );
-
-      executeTicks(game, 1);
-
-      expect(player2.isAlive()).toBe(true);
-      const transportShip = player2.units(UnitType.TransportShip)[0];
-      expect(player2.units(UnitType.TransportShip).length).toBe(1);
-
-      player2.markDisconnected(true);
-      game.addExecution(new AttackExecution(1000, player1, player2.id(), null));
-
-      executeTicks(game, 10);
-
-      expect(player2.isAlive()).toBe(false);
-      expect(transportShip.owner()).toBe(player1);
-
-      executeTicks(game, 30);
-
-      // Verify ship landed and tile ownership transferred to new ship owner
-      expect(game.owner(enemyShoreTile)).toBe(player1);
-    });
-
-    test("Captured transport ship should retreat to owner's shore tile", () => {
-      player1.conquer(game.map().ref(coastX, 4));
-      player2.conquer(game.map().ref(coastX, 1));
-
-      const enemyShoreTile = game.map().ref(coastX, 8);
-
-      game.addExecution(
-        new TransportShipExecution(
-          player2,
-          null,
-          enemyShoreTile,
-          100,
-          game.map().ref(coastX, 1),
-        ),
-      );
-      executeTicks(game, 1);
-
-      const transportShip = player2.units(UnitType.TransportShip)[0];
-      expect(player2.units(UnitType.TransportShip).length).toBe(1);
-
-      expect(transportShip.targetTile()).toBe(enemyShoreTile);
-
-      player2.markDisconnected(true);
-      game.addExecution(new AttackExecution(1000, player1, player2.id(), null));
-      executeTicks(game, 10);
-
-      expect(player2.isAlive()).toBe(false);
-      expect(transportShip.owner()).toBe(player1);
-
-      transportShip.orderBoatRetreat();
-      executeTicks(game, 2);
-
-      expect(transportShip.targetTile()).not.toBe(enemyShoreTile);
-      expect(game.owner(transportShip.targetTile()!)).toBe(player1);
-    });
-
-    test("Retreating transport ship is deleted if new owner has no shore tiles", () => {
-      player2.conquer(game.map().ref(coastX, 1));
-      player2.conquer(game.map().ref(coastX - 6, 2));
-      player1.conquer(game.map().ref(coastX - 6, 3));
-
-      const enemyShoreTile = game.map().ref(coastX, 15);
-
-      const boatTroops = 100;
-      game.addExecution(
-        new TransportShipExecution(
-          player2,
-          null,
-          enemyShoreTile,
-          boatTroops,
-          game.map().ref(coastX, 1),
-        ),
-      );
-      executeTicks(game, 1);
-
-      const transportShip = player2.units(UnitType.TransportShip)[0];
-      expect(player2.units(UnitType.TransportShip).length).toBe(1);
-
-      player2.markDisconnected(true);
-      game.addExecution(new AttackExecution(1000, player1, player2.id(), null));
-      executeTicks(game, 10);
-
-      expect(player2.isAlive()).toBe(false);
-      expect(transportShip.owner()).toBe(player1);
-
-      // Make sure player1 has no shore tiles for the ship to retreat to anymore
-      const enemyInfo = new PlayerInfo(
-        "Enemy",
-        PlayerType.Human,
-        null,
-        "enemy_id",
-      );
-      enemy = game.addPlayer(enemyInfo);
-
-      const shoreTiles = Array.from(player1.borderTiles()).filter((t) =>
-        game.isShore(t),
-      );
-      shoreTiles.forEach((tile) => {
-        enemy.conquer(tile);
-      });
-
-      expect(
-        Array.from(player1.borderTiles()).filter((t) => game.isShore(t)).length,
-      ).toBe(0);
-
-      executeTicks(game, 1);
-
-      const troopIncPerTick = game.config().troopIncreaseRate(player1);
-      const expectedTroopGrowth = toInt(troopIncPerTick * 1);
-      const expectedFinalTroops = Number(
-        toInt(player1.troops()) + expectedTroopGrowth,
-      );
-
-      transportShip.orderBoatRetreat();
-      executeTicks(game, 1);
-
-      expect(transportShip.isActive()).toBe(false);
-      // Also test if boat troops were returned to player1 as new ship owner
-      expect(player1.troops()).toBe(expectedFinalTroops + boatTroops);
-    });
-  });
 });

tests/util/Setup.ts

@@ -25,7 +25,6 @@ export async function setup(
   _gameConfig: Partial<GameConfig> = {},
   humans: PlayerInfo[] = [],
   currentDir: string = __dirname,
-  ConfigClass: typeof TestConfig = TestConfig,
 ): Promise<Game> {
   // Suppress console.debug for tests.
   console.debug = () => {};
@@ -70,7 +69,7 @@ export async function setup(
     instantBuild: false,
     ..._gameConfig,
   };
-  const config = new ConfigClass(
+  const config = new TestConfig(
     serverConfig,
     gameConfig,
     new UserSettings(),

tests/util/TestConfig.ts

@@ -81,26 +81,3 @@ export class TestConfig extends DefaultConfig {
     return 1;
   }
 }
-export class UseRealAttackLogic extends TestConfig {
-  // Override to use DefaultConfig's real attackLogic
-  attackLogic(
-    gm: Game,
-    attackTroops: number,
-    attacker: Player,
-    defender: Player | TerraNullius,
-    tileToConquer: TileRef,
-  ): {
-    attackerTroopLoss: number;
-    defenderTroopLoss: number;
-    tilesPerTickUsed: number;
-  } {
-    return DefaultConfig.prototype.attackLogic.call(
-      this,
-      gm,
-      attackTroops,
-      attacker,
-      defender,
-      tileToConquer,
-    );
-  }
-}