diff --git a/src/core/game/Game.ts b/src/core/game/Game.ts index 63b71fe8c..e6d5c710b 100644 --- a/src/core/game/Game.ts +++ b/src/core/game/Game.ts @@ -890,7 +890,6 @@ export interface Game extends GameMap { teamSpawnArea(team: Team): SpawnArea | undefined; // Alliances - alliances(): MutableAlliance[]; expireAlliance(alliance: Alliance): void; // Immunity timer diff --git a/src/core/game/GameImpl.ts b/src/core/game/GameImpl.ts index 465996049..6670d8584 100644 --- a/src/core/game/GameImpl.ts +++ b/src/core/game/GameImpl.ts @@ -89,7 +89,6 @@ export class GameImpl implements Game { _terraNullius: TerraNulliusImpl; allianceRequests: AllianceRequestImpl[] = []; - alliances_: AllianceImpl[] = []; private nextPlayerID = 1; private _nextUnitID = 1; @@ -227,10 +226,6 @@ export class GameImpl implements Game { return this.playerBySmallID(this.ownerID(ref)); } - alliances(): MutableAlliance[] { - return this.alliances_; - } - playerBySmallID(id: number): Player | TerraNullius { if (id === 0) { return this.terraNullius(); @@ -367,7 +362,8 @@ export class GameImpl implements Game { this._ticks, this.nextAllianceID++, ); - this.alliances_.push(alliance); + (alliance.requestor() as PlayerImpl)._alliances.push(alliance); + (alliance.recipient() as PlayerImpl)._alliances.push(alliance); (request.requestor() as PlayerImpl).pastOutgoingAllianceRequests.push( request, ); @@ -794,7 +790,7 @@ export class GameImpl implements Game { breaker.markTraitor(); } - this.alliances_ = this.alliances_.filter((a) => a !== alliance); + this.detachAlliance(alliance); this.addUpdate({ type: GameUpdateType.BrokeAlliance, @@ -815,7 +811,7 @@ export class GameImpl implements Game { `cannot expire alliance: must have exactly one alliance, have ${alliances.length}`, ); } - this.alliances_ = this.alliances_.filter((a) => a !== alliances[0]); + this.detachAlliance(alliances[0]); this.addUpdate({ type: GameUpdateType.AllianceExpired, player1ID: alliance.requestor().smallID(), @@ -824,9 +820,17 @@ export class GameImpl implements Game { } public removeAlliancesByPlayerSilently(player: Player): void { - this.alliances_ = this.alliances_.filter( - (a) => a.requestor() !== player && a.recipient() !== player, - ); + // Snapshot — detachAlliance reassigns the player's _alliances as it goes. + const removed = [...(player as PlayerImpl)._alliances]; + for (const alliance of removed) this.detachAlliance(alliance); + } + + /** Remove an alliance from both participants' per-player alliance lists. */ + private detachAlliance(alliance: Alliance): void { + const requestor = alliance.requestor() as PlayerImpl; + const recipient = alliance.recipient() as PlayerImpl; + requestor._alliances = requestor._alliances.filter((a) => a !== alliance); + recipient._alliances = recipient._alliances.filter((a) => a !== alliance); } public isSpawnImmunityActive(): boolean { diff --git a/src/core/game/PlayerImpl.ts b/src/core/game/PlayerImpl.ts index 5ac30a257..44ba4efb6 100644 --- a/src/core/game/PlayerImpl.ts +++ b/src/core/game/PlayerImpl.ts @@ -102,6 +102,8 @@ export class PlayerImpl implements Player { public _outgoingAttacks: Attack[] = []; public _outgoingLandAttacks: Attack[] = []; + public _alliances: MutableAlliance[] = []; + private _spawnTile: TileRef | undefined; private _isDisconnected = false; @@ -487,9 +489,7 @@ export class PlayerImpl implements Player { } alliances(): MutableAlliance[] { - return this.mg.alliances_.filter( - (a) => a.requestor() === this || a.recipient() === this, - ); + return this._alliances; } expiredAlliances(): Alliance[] { diff --git a/tests/PlayerAllianceList.test.ts b/tests/PlayerAllianceList.test.ts new file mode 100644 index 000000000..fc1166f25 --- /dev/null +++ b/tests/PlayerAllianceList.test.ts @@ -0,0 +1,130 @@ +import { AllianceRequestExecution } from "../src/core/execution/alliance/AllianceRequestExecution"; +import { BreakAllianceExecution } from "../src/core/execution/alliance/BreakAllianceExecution"; +import { Game, Player, PlayerType } from "../src/core/game/Game"; +import { playerInfo, setup } from "./util/Setup"; + +/** + * Tests for the per-player alliance list maintained on PlayerImpl + * (player.alliances()). It is updated incrementally as alliances form, break, + * and expire instead of scanning the global mg.alliances_ list, so the key + * invariant is that both participants' lists stay in sync with reality. + */ + +let game: Game; +let player1: Player; +let player2: Player; +let player3: Player; + +/** Form a mutual alliance via counter-requests, then tick to apply. */ +function ally(a: Player, b: Player): void { + game.addExecution(new AllianceRequestExecution(a, b.id())); + game.executeNextTick(); + game.addExecution(new AllianceRequestExecution(b, a.id())); + game.executeNextTick(); +} + +/** + * Break an alliance. Executions are inited on the tick they're added and only + * run on the following tick, so tick twice. + */ +function breakAlliance(a: Player, b: Player): void { + game.addExecution(new BreakAllianceExecution(a, b.id())); + game.executeNextTick(); + game.executeNextTick(); +} + +describe("per-player alliance list", () => { + beforeEach(async () => { + game = await setup( + "plains", + { infiniteGold: true, instantBuild: true, infiniteTroops: true }, + [ + playerInfo("player1", PlayerType.Human), + playerInfo("player2", PlayerType.Human), + playerInfo("player3", PlayerType.Human), + ], + ); + + player1 = game.player("player1"); + player1.conquer(game.ref(0, 0)); + player2 = game.player("player2"); + player2.conquer(game.ref(0, 1)); + player3 = game.player("player3"); + player3.conquer(game.ref(0, 2)); + }); + + test("forming an alliance adds it to both participants' lists", () => { + expect(player1.alliances()).toHaveLength(0); + expect(player2.alliances()).toHaveLength(0); + + ally(player1, player2); + + expect(player1.alliances()).toHaveLength(1); + expect(player2.alliances()).toHaveLength(1); + // Same underlying alliance object on both sides. + expect(player1.alliances()[0]).toBe(player2.alliances()[0]); + expect(player1.alliances()[0].other(player1)).toBe(player2); + expect(player2.alliances()[0].other(player2)).toBe(player1); + }); + + test("alliances() agrees with isAlliedWith / allianceWith", () => { + ally(player1, player2); + + expect(player1.isAlliedWith(player2)).toBe(true); + expect(player1.allianceWith(player2)).toBe(player1.alliances()[0]); + expect(player1.isAlliedWith(player3)).toBe(false); + expect(player3.alliances()).toHaveLength(0); + }); + + test("breaking an alliance removes it from both lists", () => { + ally(player1, player2); + expect(player1.alliances()).toHaveLength(1); + + breakAlliance(player1, player2); + + expect(player1.alliances()).toHaveLength(0); + expect(player2.alliances()).toHaveLength(0); + expect(player1.isAlliedWith(player2)).toBe(false); + }); + + test("expiring an alliance removes it from both lists", () => { + ally(player1, player2); + expect(player1.alliances()).toHaveLength(1); + + player1.alliances()[0].expire(); + + expect(player1.alliances()).toHaveLength(0); + expect(player2.alliances()).toHaveLength(0); + expect(player1.isAlliedWith(player2)).toBe(false); + }); + + test("a player tracks multiple alliances independently", () => { + ally(player1, player2); + ally(player1, player3); + + expect(player1.alliances()).toHaveLength(2); + const others = player1.alliances().map((a) => a.other(player1)); + expect(others).toContain(player2); + expect(others).toContain(player3); + + // Breaking one leaves the other intact. + breakAlliance(player1, player2); + + expect(player1.alliances()).toHaveLength(1); + expect(player1.alliances()[0].other(player1)).toBe(player3); + expect(player2.alliances()).toHaveLength(0); + expect(player3.alliances()).toHaveLength(1); + }); + + test("removeAllAlliances clears the player and every partner", () => { + ally(player1, player2); + ally(player1, player3); + expect(player1.alliances()).toHaveLength(2); + + player1.removeAllAlliances(); + + expect(player1.alliances()).toHaveLength(0); + expect(player2.alliances()).toHaveLength(0); + expect(player3.alliances()).toHaveLength(0); + }); +});