From 9c24d29824e6862ae3ae6af08393a0623b4420c1 Mon Sep 17 00:00:00 2001 From: VariableVince <24507472+VariableVince@users.noreply.github.com> Date: Tue, 18 Nov 2025 23:57:33 +0100 Subject: [PATCH] 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 | 2 +- src/core/execution/AttackExecution.ts | 7 + src/core/execution/TransportShipExecution.ts | 47 ++- src/core/execution/WarshipExecution.ts | 6 +- src/core/game/Game.ts | 2 +- src/core/game/GameImpl.ts | 14 + src/core/game/PlayerImpl.ts | 4 +- src/core/game/TransportShipUtils.ts | 2 + tests/Disconnected.test.ts | 322 ++++++++++++++++++- tests/util/Setup.ts | 3 +- tests/util/TestConfig.ts | 23 ++ 11 files changed, 416 insertions(+), 16 deletions(-) diff --git a/src/core/configuration/DefaultConfig.ts b/src/core/configuration/DefaultConfig.ts index 888d4dae5..df84be07f 100644 --- a/src/core/configuration/DefaultConfig.ts +++ b/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 ( diff --git a/src/core/execution/AttackExecution.ts b/src/core/execution/AttackExecution.ts index 13099b7b6..5997c1320 100644 --- a/src/core/execution/AttackExecution.ts +++ b/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(); diff --git a/src/core/execution/TransportShipExecution.ts b/src/core/execution/TransportShipExecution.ts index 913abb817..a54a040c0 100644 --- a/src/core/execution/TransportShipExecution.ts +++ b/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); + } } } diff --git a/src/core/execution/WarshipExecution.ts b/src/core/execution/WarshipExecution.ts index 67cb17aef..1ddb064cb 100644 --- a/src/core/execution/WarshipExecution.ts +++ b/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; diff --git a/src/core/game/Game.ts b/src/core/game/Game.ts index 3203b29a9..2d868b799 100644 --- a/src/core/game/Game.ts +++ b/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[]; diff --git a/src/core/game/GameImpl.ts b/src/core/game/GameImpl.ts index cccb25902..5854a1e5b 100644 --- a/src/core/game/GameImpl.ts +++ b/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( diff --git a/src/core/game/PlayerImpl.ts b/src/core/game/PlayerImpl.ts index d5e182cac..f85cc7aab 100644 --- a/src/core/game/PlayerImpl.ts +++ b/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); diff --git a/src/core/game/TransportShipUtils.ts b/src/core/game/TransportShipUtils.ts index b457ad94a..a0af53526 100644 --- a/src/core/game/TransportShipUtils.ts +++ b/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) { diff --git a/tests/Disconnected.test.ts b/tests/Disconnected.test.ts index e03138efa..ab630eb73 100644 --- a/tests/Disconnected.test.ts +++ b/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); + }); + }); }); diff --git a/tests/util/Setup.ts b/tests/util/Setup.ts index b93260895..963b134c1 100644 --- a/tests/util/Setup.ts +++ b/tests/util/Setup.ts @@ -25,6 +25,7 @@ export async function setup( _gameConfig: Partial = {}, humans: PlayerInfo[] = [], currentDir: string = __dirname, + ConfigClass: typeof TestConfig = TestConfig, ): Promise { // 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(), diff --git a/tests/util/TestConfig.ts b/tests/util/TestConfig.ts index 9e3e02e7b..c5f514ab0 100644 --- a/tests/util/TestConfig.ts +++ b/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, + ); + } +}