AFK team mate v2: better ship handling + tests + bugfix (#2396)

## Description:

See PR https://github.com/openfrontio/OpenFrontIO/pull/2203

It was reverted. This unreverts it, with an added fix for boat troops
not getting returned to owner. And small comment updates. And a const
for boatOwner to re-use.

The added bugfix is check for this.sourceTile === null in the retreat()
function in AttackExecution. A boat attack always sets removeTroops to
false because the troops were already removed from owner troops when the
boat departed. They don't have to be removed again in AttackExecution
init, when the boat lands and the attack starts. But at the end of the
attack, in retreat() in AttackExecution, the starting/boat troops still
need to be returned to the owner. That's why even if removeTroops is
false, when sourceTile is not null (only when it's a boat attack) we add
back the troops to the owner.

## 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

## Please put your Discord username so you can be contacted if a bug or
regression is found:

tryout33

---------

Co-authored-by: Fx Morin <28154542+FxMorin@users.noreply.github.com>
Co-authored-by: Ryan <7389646+ryanbarlow97@users.noreply.github.com>

src/core/configuration/DefaultConfig.ts

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

src/core/execution/AttackExecution.ts

@@ -181,6 +181,13 @@ export class AttackExecution implements Execution {
         this._owner.id(),
       );
     }
+    if (this.removeTroops === false && this.sourceTile === null) {
+      // 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
+      // boat attacks (sourceTile !== null) are the exception: troops were removed at departure and must be returned after attack still
+      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,13 +33,17 @@ 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;
@@ -173,11 +177,44 @@ export class TransportShipExecution implements Execution {
     }
     this.lastMove = ticks;
 
-    if (this.boat.retreating()) {
-      this.dst = this.src!; // src is guaranteed to be set at this point
+    // Team mate can conquer disconnected player and get their ships
+    // captureUnit has changed the owner of the unit, now update attacker
+    const boatOwner = this.boat.owner();
+    if (
+      this.originalOwner.isDisconnected() &&
+      boatOwner !== this.originalOwner &&
+      boatOwner.isOnSameTeam(this.originalOwner)
+    ) {
+      this.attacker = boatOwner;
+      this.originalOwner = boatOwner; // for when this owner disconnects too
+    }
 
-      if (this.boat.targetTile() !== this.dst) {
-        this.boat.setTargetTile(this.dst);
+    if (this.boat.retreating()) {
+      // Ensure retreat source is still valid for (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;
+        }
+      }
+
+      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);
+        }
       }
     }
 

src/core/execution/WarshipExecution.ts

@@ -55,10 +55,6 @@ 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) {
@@ -93,7 +89,7 @@ export class WarshipExecution implements Execution {
       if (
         unit.owner() === this.warship.owner() ||
         unit === this.warship ||
-        unit.owner().isFriendly(this.warship.owner()) ||
+        unit.owner().isFriendly(this.warship.owner(), true) ||
         this.alreadySentShell.has(unit)
       ) {
         continue;

src/core/game/Game.ts

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

src/core/game/GameImpl.ts

@@ -895,6 +895,20 @@ 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

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

src/core/game/TransportShipUtils.ts

@@ -148,6 +148,8 @@ 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,12 +1,25 @@
+import { AttackExecution } from "../src/core/execution/AttackExecution";
 import { MarkDisconnectedExecution } from "../src/core/execution/MarkDisconnectedExecution";
 import { SpawnExecution } from "../src/core/execution/SpawnExecution";
-import { Game, Player, PlayerInfo, PlayerType } from "../src/core/game/Game";
+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 { 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 () => {
@@ -158,4 +171,311 @@ 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,6 +25,7 @@ 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 +71,7 @@ export async function setup(
     randomSpawn: false,
     ..._gameConfig,
   };
-  const config = new TestConfig(
+  const config = new ConfigClass(
     serverConfig,
     gameConfig,
     new UserSettings(),

tests/util/TestConfig.ts

@@ -85,3 +85,26 @@ 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,
+    );
+  }
+}