Fix oversight: non-human player never responds to alliance renewal request (#1536)

## Description:

**--Fix oversight in v24--**
In v24, alliance renewal was introduced. But a Bot or Nation never
answers to it. So the Event Panel expiration message + clicking to Renew
and waiting, is all in vain if the other player is not a human. Like in
Single player in all cases. The message after ~30 seconds is always
"Alliance with xxx expired". This feels illogical and there's no purpose
for showing a Request to Renew button if it then always expires.

Also reported by players like here:
https://discord.com/channels/1284581928254701718/1284581928833388619/1398249123093676094

This PR fixes it by having the non-human reciever of the request, say
yes to depending on attiude towards the human and chance. This feels
more realistic.

The requestor is always the human player because they click a button in
EventsDisplay. So there is always already an extension request which the
bot can react to with another extension request to have the alliance be
extended.

**--Add tests--** 
It adds tests for extending alliance between human and non-human player.
One for AllianceExtensionExecution simply testing if alliance between
human and non-human can be extended. And one in BotBehavior, testing if
it correctly handles an extension request by adding a new
AllianceExtensionExecution.

**--Fix silent bug in existing test--**
Adding the new test for human and non-human for
AllianceExtensionExecution, i ran into a bug in the existing test for
extending alliance between humans. Which made the test always pass
because expirationAt wasn't fetched correctly. Had to fix that too,
without intending that for this PR beforehand. And then had to include
the bugfix from PR #1582 (v25) in it too to have the alliance actually
extended. More details below:

(-- The existing test would always return 'all passed' because it did
not get the expiresAt() but got the createdAt +
config.AllianceDuration() for both expirationBefore and expirationAfter.
createdAt is immutable so before and after would be the same. And then
it did not test for toBeGreaterThan, which would have failed. But it
tested wrongfully for toBeGreaterThanOrEqual which was a pass even when
the expirationBefore and expirationAfter would be the same and no
extension had taken place.

-- The bugfix from PR 1582 needed to be included now too. Because only
with those changes, the existing test has its alliance truly extended
and only with that the expirationAt actually changed. Actually, checking
if extend() was called isn't needed anymore, since we now check the
expirationAt correctly which on its own tells us if extend() was
succesful. But left this addition from PR 1582 in since it can't do any
harm.)

## 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
- [ ] I have read and accepted the CLA agreement (only required once).

## Please put your Discord username so you can be contacted if a bug or
regression is found:

tryout33

---------

Co-authored-by: evanpelle <openfrontio@gmail.com>
Co-authored-by: Drills Kibo <59177241+drillskibo@users.noreply.github.com>

src/core/execution/BotExecution.ts

@@ -57,6 +57,7 @@ export class BotExecution implements Execution {
     }
 
     this.behavior.handleAllianceRequests();
+    this.behavior.handleAllianceExtensionRequests();
     this.maybeAttack();
   }
 

src/core/execution/FakeHumanExecution.ts

@@ -155,6 +155,7 @@ export class FakeHumanExecution implements Execution {
 
     this.updateRelationsFromEmbargos();
     this.behavior.handleAllianceRequests();
+    this.behavior.handleAllianceExtensionRequests();
     this.handleUnits();
     this.handleEmbargoesToHostileNations();
     this.maybeAttack();

src/core/execution/alliance/AllianceExtensionExecution.ts

@@ -39,7 +39,7 @@ export class AllianceExtensionExecution implements Execution {
     // Mark this player's intent to extend
     alliance.addExtensionRequest(this.from);
 
-    if (alliance.canExtend()) {
+    if (alliance.bothAgreedToExtend()) {
       alliance.extend();
 
       mg.displayMessage(

src/core/execution/utils/BotBehavior.ts

@@ -9,6 +9,7 @@ import {
 } from "../../game/Game";
 import { PseudoRandom } from "../../PseudoRandom";
 import { flattenedEmojiTable } from "../../Util";
+import { AllianceExtensionExecution } from "../alliance/AllianceExtensionExecution";
 import { AttackExecution } from "../AttackExecution";
 import { EmojiExecution } from "../EmojiExecution";
 
@@ -37,6 +38,28 @@ export class BotBehavior {
     }
   }
 
+  handleAllianceExtensionRequests() {
+    for (const alliance of this.player.alliances()) {
+      // Alliance expiration tracked by Events Panel, only human ally can click Request to Renew
+      // Skip if no expiration yet/ ally didn't request extension yet/ bot already agreed to extend
+      if (!alliance.onlyOneAgreedToExtend()) continue;
+
+      // Nation is either Friendly or Neutral as an ally. Bot has no attitude
+      // If Friendly or Bot, always agree to extend. If Neutral, have random chance decide
+      const human = alliance.other(this.player);
+      if (
+        this.player.type() === PlayerType.FakeHuman &&
+        this.player.relation(human) === Relation.Neutral
+      ) {
+        if (!this.random.chance(1.5)) continue;
+      }
+
+      this.game.addExecution(
+        new AllianceExtensionExecution(this.player, human.id()),
+      );
+    }
+  }
+
   private emoji(player: Player, emoji: number) {
     if (player.type() !== PlayerType.Human) return;
     this.game.addExecution(new EmojiExecution(this.player, player.id(), emoji));

src/core/game/AllianceImpl.ts

@@ -47,12 +47,21 @@ export class AllianceImpl implements MutableAlliance {
     }
   }
 
-  canExtend(): boolean {
+  bothAgreedToExtend(): boolean {
     return (
       this.extensionRequestedRequestor_ && this.extensionRequestedRecipient_
     );
   }
 
+  onlyOneAgreedToExtend(): boolean {
+    // Requestor / Recipient of the original alliance request, not of the extension request
+    // False if: no expiration or neither requested extension yet (both false), or both agreed to extend (both true)
+    // True if: one requested extension, other didn't yet or actively ignored (one true, one false)
+    return (
+      this.extensionRequestedRequestor_ !== this.extensionRequestedRecipient_
+    );
+  }
+
   public id(): number {
     return this.id_;
   }

src/core/game/Game.ts

@@ -362,10 +362,11 @@ export interface Alliance {
 export interface MutableAlliance extends Alliance {
   expire(): void;
   other(player: Player): Player;
-  canExtend(): boolean;
+  bothAgreedToExtend(): boolean;
   addExtensionRequest(player: Player): void;
   id(): number;
   extend(): void;
+  onlyOneAgreedToExtend(): boolean;
 }
 
 export class PlayerInfo {

tests/AllianceExtensionExecution.test.ts

@@ -7,6 +7,7 @@ import { playerInfo, setup } from "./util/Setup";
 let game: Game;
 let player1: Player;
 let player2: Player;
+let player3: Player;
 
 describe("AllianceExtensionExecution", () => {
   beforeEach(async () => {
@@ -20,18 +21,20 @@ describe("AllianceExtensionExecution", () => {
       [
         playerInfo("player1", PlayerType.Human),
         playerInfo("player2", PlayerType.Human),
+        playerInfo("player3", PlayerType.FakeHuman),
       ],
     );
 
     player1 = game.player("player1");
     player2 = game.player("player2");
+    player3 = game.player("player3");
 
     while (game.inSpawnPhase()) {
       game.executeNextTick();
     }
   });
 
-  test("Successfully extends existing alliance", () => {
+  test("Successfully extends existing alliance between Humans", () => {
     jest.spyOn(player1, "canSendAllianceRequest").mockReturnValue(true);
     jest.spyOn(player2, "isAlive").mockReturnValue(true);
     jest.spyOn(player1, "isAlive").mockReturnValue(true);
@@ -51,8 +54,8 @@ describe("AllianceExtensionExecution", () => {
 
     const allianceBefore = player1.allianceWith(player2)!;
     const allianceSpy = jest.spyOn(allianceBefore, "extend");
-    const expirationBefore =
-      allianceBefore.createdAt() + game.config().allianceDuration();
+
+    const expirationBefore = allianceBefore.expiresAt();
 
     game.addExecution(new AllianceExtensionExecution(player1, player2.id()));
     game.executeNextTick();
@@ -64,10 +67,9 @@ describe("AllianceExtensionExecution", () => {
 
     expect(allianceAfter.id()).toBe(allianceBefore.id());
 
-    const expirationAfter =
-      allianceAfter.createdAt() + game.config().allianceDuration();
+    const expirationAfter = allianceAfter.expiresAt();
 
-    expect(expirationAfter).toBeGreaterThanOrEqual(expirationBefore);
+    expect(expirationAfter).toBeGreaterThan(expirationBefore);
     expect(allianceSpy).toHaveBeenCalledTimes(1);
   });
 
@@ -78,4 +80,43 @@ describe("AllianceExtensionExecution", () => {
     expect(player1.allianceWith(player2)).toBeFalsy();
     expect(player2.allianceWith(player1)).toBeFalsy();
   });
+
+  test("Successfully extends existing alliance between Human and non-Human", () => {
+    //test of handleAllianceExtensions is done in BotBehavior tests
+    jest.spyOn(player1, "canSendAllianceRequest").mockReturnValue(true);
+    jest.spyOn(player3, "isAlive").mockReturnValue(true);
+    jest.spyOn(player1, "isAlive").mockReturnValue(true);
+
+    game.addExecution(new AllianceRequestExecution(player1, player3.id()));
+    game.executeNextTick();
+    game.executeNextTick();
+
+    game.addExecution(
+      new AllianceRequestReplyExecution(player1.id(), player3, true),
+    );
+    game.executeNextTick();
+    game.executeNextTick();
+
+    expect(player1.allianceWith(player3)).toBeTruthy();
+    expect(player3.allianceWith(player1)).toBeTruthy();
+
+    const allianceBefore = player1.allianceWith(player3)!;
+    const allianceSpy = jest.spyOn(allianceBefore, "extend");
+    const expirationBefore = allianceBefore.expiresAt();
+
+    game.addExecution(new AllianceExtensionExecution(player1, player3.id()));
+    game.executeNextTick();
+    expect(allianceSpy).toHaveBeenCalledTimes(0); // both players must agree to extend
+    game.addExecution(new AllianceExtensionExecution(player3, player1.id()));
+    game.executeNextTick();
+
+    const allianceAfter = player1.allianceWith(player3)!;
+
+    expect(allianceAfter.id()).toBe(allianceBefore.id());
+
+    const expirationAfter = allianceAfter.expiresAt();
+
+    expect(expirationAfter).toBeGreaterThan(expirationBefore);
+    expect(allianceSpy).toHaveBeenCalledTimes(1);
+  });
 });

tests/BotBehavior.test.ts

@@ -1,3 +1,4 @@
+import { AllianceExtensionExecution } from "../src/core/execution/alliance/AllianceExtensionExecution";
 import { BotBehavior } from "../src/core/execution/utils/BotBehavior";
 import {
   AllianceRequest,
@@ -5,6 +6,7 @@ import {
   Player,
   PlayerInfo,
   PlayerType,
+  Relation,
   Tick,
 } from "../src/core/game/Game";
 import { PseudoRandom } from "../src/core/PseudoRandom";
@@ -149,3 +151,79 @@ describe("BotBehavior.handleAllianceRequests", () => {
     expect(request.reject).toHaveBeenCalled();
   });
 });
+
+describe("BotBehavior.handleAllianceExtensionRequests", () => {
+  let mockGame: any;
+  let mockPlayer: any;
+  let mockAlliance: any;
+  let mockHuman: any;
+  let mockRandom: any;
+  let botBehavior: BotBehavior;
+
+  beforeEach(() => {
+    mockGame = { addExecution: jest.fn() };
+    mockHuman = { id: jest.fn(() => "human_id") };
+    mockAlliance = {
+      onlyOneAgreedToExtend: jest.fn(() => true),
+      other: jest.fn(() => mockHuman),
+    };
+    mockRandom = { chance: jest.fn() };
+
+    mockPlayer = {
+      alliances: jest.fn(() => [mockAlliance]),
+      relation: jest.fn(),
+      id: jest.fn(() => "bot_id"),
+      type: jest.fn(() => PlayerType.FakeHuman),
+    };
+
+    botBehavior = new BotBehavior(
+      mockRandom,
+      mockGame,
+      mockPlayer,
+      0.5,
+      0.5,
+      0.2,
+    );
+  });
+
+  it("should NOT request extension if onlyOneAgreedToExtend is false (no expiration yet or both already agreed)", () => {
+    mockAlliance.onlyOneAgreedToExtend.mockReturnValue(false);
+    botBehavior.handleAllianceExtensionRequests();
+    expect(mockGame.addExecution).not.toHaveBeenCalled();
+  });
+
+  it("should always extend if type Bot", () => {
+    mockPlayer.type.mockReturnValue(PlayerType.Bot);
+    botBehavior.handleAllianceExtensionRequests();
+    expect(mockGame.addExecution).toHaveBeenCalledTimes(1);
+    expect(mockGame.addExecution.mock.calls[0][0]).toBeInstanceOf(
+      AllianceExtensionExecution,
+    );
+  });
+
+  it("should always extend if Nation and relation is Friendly", () => {
+    mockPlayer.relation.mockReturnValue(Relation.Friendly);
+    botBehavior.handleAllianceExtensionRequests();
+    expect(mockGame.addExecution).toHaveBeenCalledTimes(1);
+    expect(mockGame.addExecution.mock.calls[0][0]).toBeInstanceOf(
+      AllianceExtensionExecution,
+    );
+  });
+
+  it("should extend if Nation, relation is Neutral and random chance is true", () => {
+    mockPlayer.relation.mockReturnValue(Relation.Neutral);
+    mockRandom.chance.mockReturnValue(true);
+    botBehavior.handleAllianceExtensionRequests();
+    expect(mockGame.addExecution).toHaveBeenCalledTimes(1);
+    expect(mockGame.addExecution.mock.calls[0][0]).toBeInstanceOf(
+      AllianceExtensionExecution,
+    );
+  });
+
+  it("should NOT extend if Nation, relation is Neutral and random chance is false", () => {
+    mockPlayer.relation.mockReturnValue(Relation.Neutral);
+    mockRandom.chance.mockReturnValue(false);
+    botBehavior.handleAllianceExtensionRequests();
+    expect(mockGame.addExecution).not.toHaveBeenCalled();
+  });
+});