From 13b350513fa1c347931550888b467fe396d7bfc8 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) 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. - [ ] 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 regression is found: evan --- eslint.config.js | 2 +- 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 +++++++++++++++++++ 9 files changed, 142 insertions(+), 23 deletions(-) create mode 100644 tests/AllianceRequestExecution.test.ts diff --git a/eslint.config.js b/eslint.config.js index 166d7e035..d2d5df4ec 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -107,7 +107,7 @@ export default [ "function-call-argument-newline": ["error", "consistent"], "max-depth": ["error", { max: 5 }], "max-len": ["error", { code: 120 }], - "max-lines": ["error", { max: 676, skipBlankLines: true, skipComments: true }], + "max-lines": ["error", { max: 677, skipBlankLines: true, skipComments: true }], "max-lines-per-function": ["error", { max: 561 }], "no-loss-of-precision": "error", "no-multi-spaces": "error", diff --git a/src/client/graphics/layers/EventsDisplay.ts b/src/client/graphics/layers/EventsDisplay.ts index 193e5f314..66771cfbc 100644 --- a/src/client/graphics/layers/EventsDisplay.ts +++ b/src/client/graphics/layers/EventsDisplay.ts @@ -65,6 +65,7 @@ type GameEvent = { duration?: Tick; focusID?: number; unitView?: UnitView; + shouldDelete?: (game: GameView) => boolean; }; @customElement("events-display") @@ -196,7 +197,8 @@ export class EventsDisplay extends LitElement implements Layer { let remainingEvents = this.events.filter((event) => { if (this.game === undefined) return; 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(); } @@ -464,12 +466,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 16bf6bf3f..fe1e2525a 100644 --- a/src/core/configuration/Config.ts +++ b/src/core/configuration/Config.ts @@ -121,6 +121,7 @@ export type 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 ba7ba83b6..eddf481fe 100644 --- a/src/core/configuration/DefaultConfig.ts +++ b/src/core/configuration/DefaultConfig.ts @@ -533,6 +533,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 ee2e929a3..b73cd50c6 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 readonly 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 6fbb7bd0c..591c383c9 100644 --- a/src/core/game/AllianceRequestImpl.ts +++ b/src/core/game/AllianceRequestImpl.ts @@ -3,6 +3,8 @@ import { AllianceRequestUpdate, GameUpdateType } from "./GameUpdates"; import { GameImpl } from "./GameImpl"; export class AllianceRequestImpl implements AllianceRequest { + private status_: "pending" | "accepted" | "rejected" = "pending"; + constructor( private readonly requestor_: Player, private readonly recipient_: Player, @@ -10,6 +12,10 @@ export class AllianceRequestImpl implements AllianceRequest { private readonly 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 713b46b2c..2486e57ac 100644 --- a/src/core/game/Game.ts +++ b/src/core/game/Game.ts @@ -354,6 +354,7 @@ export type AllianceRequest = { requestor(): Player; recipient(): Player; createdAt(): Tick; + status(): "pending" | "accepted" | "rejected"; }; export type Alliance = { diff --git a/src/core/game/PlayerImpl.ts b/src/core/game/PlayerImpl.ts index 0483b2064..e1716bce1 100644 --- a/src/core/game/PlayerImpl.ts +++ b/src/core/game/PlayerImpl.ts @@ -397,9 +397,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(); + }); +});