Fix alliance renewal popup not being removed when alliance is broken (#2722)

Resolves #2464

## Description

This PR fixes a bug where the alliance renewal popup remained visible
after an alliance was broken or betrayed.
The issue occurred because renewal UI events were tied to player
identifiers instead of the unique allianceId.
When a player had multiple alliances, breaking one alliance did not
correctly remove the associated renewal popup.
This change ensures that renewal popups are correctly removed only for
the specific alliance that was broken.


### What was wrong
Alliance renewal UI events were previously associated implicitly with
players.
This caused incorrect behavior when a player had **multiple alliances**,
because **players are not unique identifiers of an alliance**.

As a result, breaking one alliance could leave stale renewal popups
visible.

### What was changed
- Alliance break logic now relies on **allianceId**, not player IDs
- Renewal popups are removed **only for the specific broken alliance**
- Alliances involving the same player but different allianceIds are
unaffected
- Added tests to ensure this bug cannot reappear

### Result
- Renewal popup disappears immediately when an alliance is
betrayed/broken
- No unintended removal of other alliance renewal popups
- Correct behavior even when a player is involved in multiple alliances

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

assessin.
This commit is contained in:
Achim Marius
2025-12-28 23:35:05 +02:00
committed by GitHub
parent 63acbf1043
commit 5a065d71c5
5 changed files with 164 additions and 10 deletions
@@ -70,6 +70,7 @@ interface GameEvent {
focusID?: number;
unitView?: UnitView;
shouldDelete?: (game: GameView) => boolean;
allianceID?: number;
}
@customElement("events-display")
@@ -336,6 +337,7 @@ export class EventsDisplay extends LitElement implements Layer {
highlight: true,
createdAt: this.game.ticks(),
focusID: other.smallID(),
allianceID: alliance.id,
});
}
}
@@ -361,6 +363,16 @@ export class EventsDisplay extends LitElement implements Layer {
renderLayer(): void {}
private removeAllianceRenewalEvents(allianceID: number) {
this.events = this.events.filter(
(event) =>
!(
event.type === MessageType.RENEW_ALLIANCE &&
event.allianceID === allianceID
),
);
}
onDisplayMessageEvent(event: DisplayMessageUpdate) {
const myPlayer = this.game.myPlayer();
if (
@@ -552,6 +564,9 @@ export class EventsDisplay extends LitElement implements Layer {
const myPlayer = this.game.myPlayer();
if (!myPlayer) return;
this.removeAllianceRenewalEvents(update.allianceID);
this.requestUpdate();
const betrayed = this.game.playerBySmallID(update.betrayedID) as PlayerView;
const traitor = this.game.playerBySmallID(update.traitorID) as PlayerView;
+4 -9
View File
@@ -632,7 +632,7 @@ export class GameImpl implements Game {
});
}
public breakAlliance(breaker: Player, alliance: Alliance) {
public breakAlliance(breaker: Player, alliance: MutableAlliance) {
let other: Player;
if (alliance.requestor() === breaker) {
other = alliance.recipient();
@@ -648,18 +648,13 @@ export class GameImpl implements Game {
breaker.markTraitor();
}
const breakerSet = new Set(breaker.alliances());
const alliances = other.alliances().filter((a) => breakerSet.has(a));
if (alliances.length !== 1) {
throw new Error(
`must have exactly one alliance, have ${alliances.length}`,
);
}
this.alliances_ = this.alliances_.filter((a) => a !== alliances[0]);
this.alliances_ = this.alliances_.filter((a) => a !== alliance);
this.addUpdate({
type: GameUpdateType.BrokeAlliance,
traitorID: breaker.smallID(),
betrayedID: other.smallID(),
allianceID: alliance.id(),
});
}
+1
View File
@@ -202,6 +202,7 @@ export interface BrokeAllianceUpdate {
type: GameUpdateType.BrokeAlliance;
traitorID: number;
betrayedID: number;
allianceID: number;
}
export interface AllianceExpiredUpdate {
+1 -1
View File
@@ -436,7 +436,7 @@ export class PlayerImpl implements Player {
return delta >= this.mg.config().allianceRequestCooldown();
}
breakAlliance(alliance: Alliance): void {
breakAlliance(alliance: MutableAlliance): void {
this.mg.breakAlliance(this, alliance);
}
@@ -0,0 +1,143 @@
jest.mock("lit", () => ({
html: () => {},
LitElement: class {},
}));
jest.mock("lit/decorators.js", () => ({
customElement: () => (clazz: any) => clazz,
query: () => () => {},
state: () => () => {},
property: () => () => {},
}));
jest.mock("lit/directive.js", () => ({
DirectiveResult: class {},
}));
jest.mock("lit/directives/unsafe-html.js", () => ({
unsafeHTML: () => {},
UnsafeHTMLDirective: class {},
}));
import { EventsDisplay } from "../../../../src/client/graphics/layers/EventsDisplay";
import { MessageType } from "../../../../src/core/game/Game";
describe("EventsDisplay - alliance renewal cleanup (allianceID based)", () => {
function makeRenewal(
allianceID: number,
focusID: number,
description = "Alliance about to expire",
) {
return {
description,
type: MessageType.RENEW_ALLIANCE,
allianceID,
focusID,
createdAt: 0,
};
}
test("removes ONLY renewal events for the broken alliance", () => {
const display = new EventsDisplay();
const allianceAB = 1;
const allianceAC = 2;
const allianceBC = 3;
(display as any).events = [
makeRenewal(allianceAB, 1), // AB
makeRenewal(allianceAC, 1), // AC
makeRenewal(allianceBC, 2), // BC
];
// Break alliance AB
(display as any).removeAllianceRenewalEvents(allianceAB);
const remaining = (display as any).events;
// AB renewal removed
expect(remaining.some((e: any) => e.allianceID === allianceAB)).toBe(false);
// Other alliances untouched
expect(remaining.some((e: any) => e.allianceID === allianceAC)).toBe(true);
expect(remaining.some((e: any) => e.allianceID === allianceBC)).toBe(true);
});
test("does NOT remove renewals just because the same player is involved", () => {
const display = new EventsDisplay();
const allianceAB = 10;
const allianceAC = 11;
(display as any).events = [
makeRenewal(allianceAB, 1), // Player 1 involved
makeRenewal(allianceAC, 1), // Same player, different alliance
];
(display as any).removeAllianceRenewalEvents(allianceAB);
const remaining = (display as any).events;
expect(remaining.length).toBe(1);
expect(remaining[0].allianceID).toBe(allianceAC);
});
test("breaking one alliance does not affect renewals between other players", () => {
const display = new EventsDisplay();
const allianceAB = 100;
const allianceCD = 200;
(display as any).events = [
makeRenewal(allianceAB, 1), // AB
makeRenewal(allianceCD, 3), // CD
];
(display as any).removeAllianceRenewalEvents(allianceAB);
const remaining = (display as any).events;
expect(remaining.length).toBe(1);
expect(remaining[0].allianceID).toBe(allianceCD);
});
test("does not affect non-RENEW_ALLIANCE events", () => {
const display = new EventsDisplay();
(display as any).events = [
{
description: "Alliance broken",
type: MessageType.ALLIANCE_BROKEN,
createdAt: 0,
},
{
description: "Alliance accepted",
type: MessageType.ALLIANCE_ACCEPTED,
createdAt: 0,
},
{
description: "Renewal",
type: MessageType.RENEW_ALLIANCE,
allianceID: 999,
createdAt: 0,
},
];
(display as any).removeAllianceRenewalEvents(999);
const remaining = (display as any).events;
expect(
remaining.some((e: any) => e.type === MessageType.ALLIANCE_BROKEN),
).toBe(true);
expect(
remaining.some((e: any) => e.type === MessageType.ALLIANCE_ACCEPTED),
).toBe(true);
expect(
remaining.some((e: any) => e.type === MessageType.RENEW_ALLIANCE),
).toBe(false);
});
});