From 081735bf55c39e26d1e27d3f94495df9dfc02538 Mon Sep 17 00:00:00 2001 From: evanpelle Date: Tue, 19 Aug 2025 12:52:54 -0700 Subject: [PATCH] Improve alliance UX, prevent hung alliance requests (#1868) ## Description: This PR does two things: 1. Allows you to send an alliance request to approve an existing request, ex: * player A sends req to player B * now player B can send an ally request to player A, which accepts the request from player A. This way even if you lose or don't see the alliance notification, you can still accept the alliance. 2. Have AllianceRequestExecution reject the request if not accepted or rejected. There is a bug where sometimes the EventDisplay does not trigger the delete() function, resulting in hung alliance requests. I couldn't figure out why the delete() function is sometimes not called, but I think it's better design to have core/ itself handle abandoned alliance requests, this was UI bugs can't break the game state. ## Please complete the following: - [ ] I have added screenshots for all UI updates - [ ] I process any text displayed to the user through translateText() and I've added it to the en.json file - [ ] I have added relevant tests to the test directory - [ ] 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: evan --- src/client/graphics/layers/EventsDisplay.ts | 14 ++-- src/core/configuration/Config.ts | 1 + src/core/configuration/DefaultConfig.ts | 3 + .../alliance/AllianceRequestExecution.ts | 53 +++++++++---- src/core/game/AllianceRequestImpl.ts | 8 ++ src/core/game/Game.ts | 1 + src/core/game/PlayerImpl.ts | 6 +- tests/AllianceRequestExecution.test.ts | 77 +++++++++++++++++++ 8 files changed, 141 insertions(+), 22 deletions(-) create mode 100644 tests/AllianceRequestExecution.test.ts diff --git a/src/client/graphics/layers/EventsDisplay.ts b/src/client/graphics/layers/EventsDisplay.ts index 388486909..44beb58b6 100644 --- a/src/client/graphics/layers/EventsDisplay.ts +++ b/src/client/graphics/layers/EventsDisplay.ts @@ -66,6 +66,7 @@ interface GameEvent { duration?: Tick; focusID?: number; unitView?: UnitView; + shouldDelete?: (game: GameView) => boolean; } @customElement("events-display") @@ -195,7 +196,8 @@ export class EventsDisplay extends LitElement implements Layer { let remainingEvents = this.events.filter((event) => { const shouldKeep = - this.game.ticks() - event.createdAt < (event.duration ?? 600); + this.game.ticks() - event.createdAt < (event.duration ?? 600) && + !event.shouldDelete?.(this.game); if (!shouldKeep && event.onDelete) { event.onDelete(); } @@ -456,12 +458,12 @@ export class EventsDisplay extends LitElement implements Layer { highlight: true, type: MessageType.ALLIANCE_REQUEST, createdAt: this.game.ticks(), - onDelete: () => - this.eventBus.emit( - new SendAllianceReplyIntentEvent(requestor, recipient, false), - ), priority: 0, - duration: 150, + duration: this.game.config().allianceRequestDuration() - 20, // 2 second buffer + shouldDelete: (game) => { + // Recipient sent a separate request, so they became allied without the recipient responding. + return requestor.isAlliedWith(recipient); + }, focusID: update.requestorID, }); } diff --git a/src/core/configuration/Config.ts b/src/core/configuration/Config.ts index 263d92f6e..f06724999 100644 --- a/src/core/configuration/Config.ts +++ b/src/core/configuration/Config.ts @@ -119,6 +119,7 @@ export interface Config { shellLifetime(): number; boatMaxNumber(): number; allianceDuration(): Tick; + allianceRequestDuration(): Tick; allianceRequestCooldown(): Tick; temporaryEmbargoDuration(): Tick; targetDuration(): Tick; diff --git a/src/core/configuration/DefaultConfig.ts b/src/core/configuration/DefaultConfig.ts index 95af6e07d..7a7fee7f1 100644 --- a/src/core/configuration/DefaultConfig.ts +++ b/src/core/configuration/DefaultConfig.ts @@ -520,6 +520,9 @@ export class DefaultConfig implements Config { targetCooldown(): Tick { return 15 * 10; } + allianceRequestDuration(): Tick { + return 20 * 10; + } allianceRequestCooldown(): Tick { return 30 * 10; } diff --git a/src/core/execution/alliance/AllianceRequestExecution.ts b/src/core/execution/alliance/AllianceRequestExecution.ts index 419b8b92c..ba36dbcd0 100644 --- a/src/core/execution/alliance/AllianceRequestExecution.ts +++ b/src/core/execution/alliance/AllianceRequestExecution.ts @@ -1,8 +1,15 @@ -import { Execution, Game, Player, PlayerID } from "../../game/Game"; +import { + AllianceRequest, + Execution, + Game, + Player, + PlayerID, +} from "../../game/Game"; export class AllianceRequestExecution implements Execution { + private req: AllianceRequest | null = null; private active = true; - private recipient: Player | null = null; + private mg: Game; constructor( private requestor: Player, @@ -10,29 +17,49 @@ export class AllianceRequestExecution implements Execution { ) {} init(mg: Game, ticks: number): void { + this.mg = mg; if (!mg.hasPlayer(this.recipientID)) { console.warn( `AllianceRequestExecution recipient ${this.recipientID} not found`, ); - this.active = false; return; } - this.recipient = mg.player(this.recipientID); + const recipient = mg.player(this.recipientID); + + if (!this.requestor.canSendAllianceRequest(recipient)) { + console.warn("cannot send alliance request"); + this.active = false; + } else { + const incoming = recipient + .outgoingAllianceRequests() + .find((r) => r.recipient() === this.requestor); + if (incoming) { + // If the recipient already has pending alliance request, + // then accept it instead of creating a new one. + this.active = false; + incoming.accept(); + } else { + this.req = this.requestor.createAllianceRequest(recipient); + } + } } tick(ticks: number): void { - if (this.recipient === null) { - throw new Error("Not initialized"); + if ( + this.req?.status() === "accepted" || + this.req?.status() === "rejected" + ) { + this.active = false; + return; } - if (this.requestor.isFriendly(this.recipient)) { - console.warn("already allied"); - } else if (!this.requestor.canSendAllianceRequest(this.recipient)) { - console.warn("recent or pending alliance request"); - } else { - this.requestor.createAllianceRequest(this.recipient); + if ( + this.mg.ticks() - (this.req?.createdAt() ?? 0) > + this.mg.config().allianceRequestDuration() + ) { + this.req?.reject(); + this.active = false; } - this.active = false; } isActive(): boolean { diff --git a/src/core/game/AllianceRequestImpl.ts b/src/core/game/AllianceRequestImpl.ts index 6d51acdb3..b40247329 100644 --- a/src/core/game/AllianceRequestImpl.ts +++ b/src/core/game/AllianceRequestImpl.ts @@ -3,6 +3,8 @@ import { GameImpl } from "./GameImpl"; import { AllianceRequestUpdate, GameUpdateType } from "./GameUpdates"; export class AllianceRequestImpl implements AllianceRequest { + private status_: "pending" | "accepted" | "rejected" = "pending"; + constructor( private requestor_: Player, private recipient_: Player, @@ -10,6 +12,10 @@ export class AllianceRequestImpl implements AllianceRequest { private game: GameImpl, ) {} + status(): "pending" | "accepted" | "rejected" { + return this.status_; + } + requestor(): Player { return this.requestor_; } @@ -23,9 +29,11 @@ export class AllianceRequestImpl implements AllianceRequest { } accept(): void { + this.status_ = "accepted"; this.game.acceptAllianceRequest(this); } reject(): void { + this.status_ = "rejected"; this.game.rejectAllianceRequest(this); } diff --git a/src/core/game/Game.ts b/src/core/game/Game.ts index 4f6e834db..7e7197290 100644 --- a/src/core/game/Game.ts +++ b/src/core/game/Game.ts @@ -349,6 +349,7 @@ export interface AllianceRequest { requestor(): Player; recipient(): Player; createdAt(): Tick; + status(): "pending" | "accepted" | "rejected"; } export interface Alliance { diff --git a/src/core/game/PlayerImpl.ts b/src/core/game/PlayerImpl.ts index a63e14613..7157e412f 100644 --- a/src/core/game/PlayerImpl.ts +++ b/src/core/game/PlayerImpl.ts @@ -392,9 +392,9 @@ export class PlayerImpl implements Player { return false; } - const hasPending = - this.incomingAllianceRequests().some((ar) => ar.requestor() === other) || - this.outgoingAllianceRequests().some((ar) => ar.recipient() === other); + const hasPending = this.outgoingAllianceRequests().some( + (ar) => ar.recipient() === other, + ); if (hasPending) { return false; diff --git a/tests/AllianceRequestExecution.test.ts b/tests/AllianceRequestExecution.test.ts new file mode 100644 index 000000000..c88edd5a0 --- /dev/null +++ b/tests/AllianceRequestExecution.test.ts @@ -0,0 +1,77 @@ +import { AllianceRequestExecution } from "../src/core/execution/alliance/AllianceRequestExecution"; +import { AllianceRequestReplyExecution } from "../src/core/execution/alliance/AllianceRequestReplyExecution"; +import { Game, Player, PlayerType } from "../src/core/game/Game"; +import { playerInfo, setup } from "./util/Setup"; + +let game: Game; +let player1: Player; +let player2: Player; + +describe("AllianceRequestExecution", () => { + beforeEach(async () => { + game = await setup( + "plains", + { + infiniteGold: true, + instantBuild: true, + infiniteTroops: true, + }, + [ + playerInfo("player1", PlayerType.Human), + playerInfo("player2", PlayerType.Human), + playerInfo("player3", PlayerType.FakeHuman), + ], + ); + + player1 = game.player("player1"); + player1.conquer(game.ref(0, 0)); + + player2 = game.player("player2"); + player2.conquer(game.ref(0, 1)); + + while (game.inSpawnPhase()) { + game.executeNextTick(); + } + }); + + test("Can create alliance by replying", () => { + game.addExecution(new AllianceRequestExecution(player1, player2.id())); + game.executeNextTick(); + + game.addExecution( + new AllianceRequestReplyExecution(player1.id(), player2, true), + ); + game.executeNextTick(); + game.executeNextTick(); + + expect(player1.isAlliedWith(player2)).toBeTruthy(); + expect(player2.isAlliedWith(player1)).toBeTruthy(); + }); + + test("Can create alliance by sending alliance request back", () => { + game.addExecution(new AllianceRequestExecution(player1, player2.id())); + game.executeNextTick(); + + game.addExecution(new AllianceRequestExecution(player2, player1.id())); + game.executeNextTick(); + + expect(player1.isAlliedWith(player2)).toBeTruthy(); + expect(player2.isAlliedWith(player1)).toBeTruthy(); + }); + + test("Alliance request expires", () => { + game.config().allianceRequestDuration = () => 5; + game.addExecution(new AllianceRequestExecution(player1, player2.id())); + game.executeNextTick(); + + expect(player1.outgoingAllianceRequests().length).toBe(1); + + for (let i = 0; i < 6; i++) { + game.executeNextTick(); + } + + expect(player1.outgoingAllianceRequests().length).toBe(0); + expect(player1.isAlliedWith(player2)).toBeFalsy(); + expect(player2.isAlliedWith(player1)).toBeFalsy(); + }); +});