Fix: remove alliances on death (#3168)

## Description:

- Remove alliances on death: after death, alliances would stay active
including countdown timers and (when dead player kept spectating) icons.
Now remove them when player becomes inActive.

- Moved code to private method within PlayerExecution + added comments
in NationExecution and BotExecution for more clarity as to where
removals are performed from at death

- Remove renewal request from Events Display when Alliance doesn't exist
anymore (after death or otherwise).

- Also cleanup this.alliancesCheckedAt when alliance doesn't exist
anymore. Before, old/broken alliance id's would accumulate in it during
a game.

- Removed now-redundant isAlive check in EventsDisplay. Both the
alliances array as the isAlive are updated in the same tick from
PlayerUpdates so now alliance is removed from alliances array on player
death, the other.isAlive() check is no longer needed. Of course we could
keep it in just to be very safe, so just let me know when you're
doubtful about this.

- Attack.test.ts: fix failing test. Player B dies because of the attack,
meaning the alliance now gets removed. Prevent this by gving both a
different, adjecent, starting tile. And to be more clear about what is
needed for the test to pass, add isAlive check for both of them after
the attacks.

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

src/client/graphics/layers/EventsDisplay.ts

@@ -251,7 +251,11 @@ export class EventsDisplay extends LitElement implements Layer {
     const myPlayer = this.game.myPlayer();
     if (!myPlayer?.isAlive()) return;
 
+    const currentAllianceIds = new Set<number>();
+
     for (const alliance of myPlayer.alliances()) {
+      currentAllianceIds.add(alliance.id);
+
       if (
         alliance.expiresAt >
         this.game.ticks() + this.game.config().allianceExtensionPromptOffset()
@@ -270,7 +274,6 @@ export class EventsDisplay extends LitElement implements Layer {
       this.alliancesCheckedAt.set(alliance.id, this.game.ticks());
 
       const other = this.game.player(alliance.other) as PlayerView;
-      if (!other.isAlive()) continue;
 
       this.addEvent({
         description: translateText("events_display.about_to_expire", {
@@ -305,6 +308,13 @@ export class EventsDisplay extends LitElement implements Layer {
         allianceID: alliance.id,
       });
     }
+
+    for (const [allianceId] of this.alliancesCheckedAt) {
+      if (!currentAllianceIds.has(allianceId)) {
+        this.removeAllianceRenewalEvents(allianceId);
+        this.alliancesCheckedAt.delete(allianceId);
+      }
+    }
   }
 
   private addEvent(event: GameEvent) {
@@ -530,6 +540,7 @@ export class EventsDisplay extends LitElement implements Layer {
     if (!myPlayer) return;
 
     this.removeAllianceRenewalEvents(update.allianceID);
+    this.alliancesCheckedAt.delete(update.allianceID);
     this.requestUpdate();
 
     const betrayed = this.game.playerBySmallID(update.betrayedID) as PlayerView;

src/core/execution/BotExecution.ts

@@ -39,6 +39,7 @@ export class BotExecution implements Execution {
     if (ticks % this.attackRate !== this.attackTick) return;
 
     if (!this.bot.isAlive()) {
+      //removeOnDeath is called from bot's PlayerExecution
       this.active = false;
       return;
     }

src/core/execution/NationExecution.ts

@@ -144,6 +144,7 @@ export class NationExecution implements Execution {
     }
 
     if (!this.player.isAlive()) {
+      //removeOnDeath is called from nation's PlayerExecution
       this.active = false;
       return;
     }

src/core/execution/PlayerExecution.ts

@@ -60,19 +60,7 @@ export class PlayerExecution implements Execution {
     }
 
     if (!this.player.isAlive()) {
-      // Player has no tiles, delete any remaining units and gold
-      const gold = this.player.gold();
-      this.player.removeGold(gold);
-      this.player.units().forEach((u) => {
-        if (
-          u.type() !== UnitType.AtomBomb &&
-          u.type() !== UnitType.HydrogenBomb &&
-          u.type() !== UnitType.MIRVWarhead &&
-          u.type() !== UnitType.MIRV
-        ) {
-          u.delete();
-        }
-      });
+      this.removeOnDeath();
       this.active = false;
       this.mg.stats().playerKilled(this.player, ticks);
       return;
@@ -400,4 +388,24 @@ export class PlayerExecution implements Execution {
 
     return result;
   }
+
+  private removeOnDeath(): void {
+    // Player (bot, human, nation) has no tiles
+    // Delete any remaining gold, non-nuke units and alliances
+    const gold = this.player.gold();
+    this.player.removeGold(gold);
+
+    this.player.units().forEach((u) => {
+      if (
+        u.type() !== UnitType.AtomBomb &&
+        u.type() !== UnitType.HydrogenBomb &&
+        u.type() !== UnitType.MIRVWarhead &&
+        u.type() !== UnitType.MIRV
+      ) {
+        u.delete();
+      }
+    });
+
+    this.player.removeAllAlliances();
+  }
 }

src/core/game/Game.ts

@@ -663,6 +663,7 @@ export interface Player {
   allianceWith(other: Player): MutableAlliance | null;
   canSendAllianceRequest(other: Player): boolean;
   breakAlliance(alliance: Alliance): void;
+  removeAllAlliances(): void;
   createAllianceRequest(recipient: Player): AllianceRequest | null;
   betrayals(): number;
 

src/core/game/GameImpl.ts

@@ -719,6 +719,12 @@ export class GameImpl implements Game {
     });
   }
 
+  public removeAlliancesByPlayerSilently(player: Player): void {
+    this.alliances_ = this.alliances_.filter(
+      (a) => a.requestor() !== player && a.recipient() !== player,
+    );
+  }
+
   public isSpawnImmunityActive(): boolean {
     return (
       this.config().numSpawnPhaseTurns() +

src/core/game/PlayerImpl.ts

@@ -450,6 +450,10 @@ export class PlayerImpl implements Player {
     this.mg.breakAlliance(this, alliance);
   }
 
+  removeAllAlliances(): void {
+    this.mg.removeAlliancesByPlayerSilently(this);
+  }
+
   isTraitor(): boolean {
     return this.getTraitorRemainingTicks() > 0;
   }

tests/Attack.test.ts

@@ -183,7 +183,7 @@ describe("Attack race condition with alliance requests", () => {
       null,
       "playerB_id",
     );
-    playerB = addPlayerToGame(playerBInfo, game, game.ref(0, 10));
+    playerB = addPlayerToGame(playerBInfo, game, game.ref(0, 11));
 
     while (game.inSpawnPhase()) {
       game.executeNextTick();
@@ -224,6 +224,9 @@ describe("Attack race condition with alliance requests", () => {
       game.executeNextTick();
     }
 
+    expect(playerA.isAlive()).toBe(true);
+    expect(playerB.isAlive()).toBe(true);
+
     // Player A should not be marked as traitor because the alliance was formed after the attack started
     expect(playerA.isTraitor()).toBe(false);