Perf: Maintain a per-player alliance list (#4172)

# Maintain a per-player alliance list (drop O(all-alliances) scan)

## Summary

`PlayerImpl.alliances()` was implemented as a full scan of the global
alliance
list on every call:

```ts
alliances(): MutableAlliance[] {
  return this.mg.alliances_.filter(
    (a) => a.requestor() === this || a.recipient() === this,
  );
}
```

This is O(all-alliances-in-game) **per call**, and it's called a lot —
most
notably twice per player per tick from `PlayerImpl.toFullUpdate()` (once
for
`allies`, once for `alliances`), which runs for every player every tick
on the
worker/core thread.

This PR makes each player own its alliance list: a per-player
`_alliances`
array (mirroring the existing `_incomingAttacks` / `_outgoingAttacks`
pattern),
maintained incrementally as alliances form/break/expire, so
`alliances()`
becomes an O(1) field read.

It turned out the global `mg.alliances_` list was only ever read by this
scan —
the `Game`-level `alliances()` getter had **zero callers** (all 17
`.alliances()`
callsites use the player-level accessor), and the list isn't used in
serialization. So rather than keep two structures in sync, this removes
the
global list entirely and makes the per-player lists the single source of
truth.

## Motivation

Profiling the worker/core thread showed `player.toFullUpdate` at ~**4%
of CPU**.
Breaking down where that time goes (microbenchmark, 100 players, ~100
alliances):

| Component | µs/tick | Share |
| --- | --- | --- |
| FULL (current: alliance scan ×2 + allocate collections) | 61.5 | 100%
|
| Alliance scan only (the two global `.filter()`s) | 41.7 | **~68%** |
| Allocation only (build arrays/objects, per-player list, no scan) | 6.4
| ~10% |

The global alliance scan — not the object allocation — is the dominant
cost, and
it gets *worse* with game size: the scan is O(players × total-alliances)
while
allocation is only O(players × own-alliances). Removing the scan targets
the
dominant ~2/3 of `toFullUpdate`'s cost.

It also speeds up `alliances()` everywhere, not just `toFullUpdate` —
it's called
in **17 places**, including AI hot paths (`NationAllianceBehavior`,
`PlayerExecution`).

> Note: this builds on the already-merged `diffPlayerUpdate`
typed-comparison
> change (commit `be87c76`), which addressed the diff/serialization
cost. This PR
> addresses the snapshot-construction cost.

## Changes

- **`PlayerImpl`**: add `public _alliances: MutableAlliance[]`;
`alliances()`
  returns it directly.
- **`GameImpl`**: remove the global `alliances_` field and the unused
`alliances()` getter. Maintain the per-player lists at the mutation
sites:
  - **add** — `acceptAllianceRequest` pushes the new alliance onto both
    participants.
  - **remove** — `breakAlliance`, `expireAlliance`, and
    `removeAlliancesByPlayerSilently` all funnel through a small
`detachAlliance()` helper that removes the alliance from both
participants.
- **`Game` interface**: drop `alliances(): MutableAlliance[]` (no
callers).

## Correctness notes

- `alliances()` now returns the internal array by reference. This
matches the
existing `outgoingAttacks()` / `incomingAttacks()` accessors, which
already do
the same. All 17 callsites were checked — none mutate the returned
array.
- `detachAlliance` reassigns the array (`filter`) rather than splicing
in place,
so the `for (const alliance of player.alliances())` loop in
`PlayerExecution`
(which can expire alliances mid-iteration) iterates a stable snapshot
and is
safe. `removeAlliancesByPlayerSilently` likewise snapshots the player's
list
  before detaching.

## Tests

New `tests/PlayerAllianceList.test.ts` asserts both participants' lists
stay in
sync through every mutation path:

- forming an alliance adds it to both lists
- `alliances()` agrees with `isAlliedWith` / `allianceWith`
- breaking removes it from both lists
- expiring removes it from both lists
- a player tracks multiple alliances independently (breaking one keeps
the other)
- `removeAllAlliances` clears the player and every partner

Full suite green: **1360 tests / 120 files**.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

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 {

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[] {

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);
+  });
+});