From 07e13b3479fef50873b8cbb3a4aea3c7229819e7 Mon Sep 17 00:00:00 2001 From: VariableVince <24507472+VariableVince@users.noreply.github.com> Date: Thu, 12 Feb 2026 20:01:08 +0100 Subject: [PATCH] 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 | 13 +++++++- src/core/execution/BotExecution.ts | 1 + src/core/execution/NationExecution.ts | 1 + src/core/execution/PlayerExecution.ts | 34 +++++++++++++-------- src/core/game/Game.ts | 1 + src/core/game/GameImpl.ts | 6 ++++ src/core/game/PlayerImpl.ts | 4 +++ tests/Attack.test.ts | 5 ++- 8 files changed, 50 insertions(+), 15 deletions(-) diff --git a/src/client/graphics/layers/EventsDisplay.ts b/src/client/graphics/layers/EventsDisplay.ts index b39d3f8de..31f496c05 100644 --- a/src/client/graphics/layers/EventsDisplay.ts +++ b/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(); + 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; diff --git a/src/core/execution/BotExecution.ts b/src/core/execution/BotExecution.ts index 85fabb34a..491bf1b21 100644 --- a/src/core/execution/BotExecution.ts +++ b/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; } diff --git a/src/core/execution/NationExecution.ts b/src/core/execution/NationExecution.ts index 466adcdaa..45abd3729 100644 --- a/src/core/execution/NationExecution.ts +++ b/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; } diff --git a/src/core/execution/PlayerExecution.ts b/src/core/execution/PlayerExecution.ts index e1d80d0b4..1e9ae4a10 100644 --- a/src/core/execution/PlayerExecution.ts +++ b/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(); + } } diff --git a/src/core/game/Game.ts b/src/core/game/Game.ts index 1057e7e6b..d0a1d3f97 100644 --- a/src/core/game/Game.ts +++ b/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; diff --git a/src/core/game/GameImpl.ts b/src/core/game/GameImpl.ts index cab7e6061..45d77878f 100644 --- a/src/core/game/GameImpl.ts +++ b/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() + diff --git a/src/core/game/PlayerImpl.ts b/src/core/game/PlayerImpl.ts index dd72a3307..d00948d44 100644 --- a/src/core/game/PlayerImpl.ts +++ b/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; } diff --git a/tests/Attack.test.ts b/tests/Attack.test.ts index 8659a809d..ff875870d 100644 --- a/tests/Attack.test.ts +++ b/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);