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

@@ -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,
     });
   }

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;

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;
   }

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 {

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);
   }
 

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 {

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;

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();
+  });
+});