Resolve review comments

Author: deshack

src/client/graphics/layers/EventsDisplay.ts

@@ -12,6 +12,7 @@ import {
 } from "../../../core/game/Game";
 import {
   AllianceExpiredUpdate,
+  AllianceExtensionUpdate,
   AllianceRequestReplyUpdate,
   AllianceRequestUpdate,
   BrokeAllianceUpdate,
@@ -175,26 +176,18 @@ export class EventsDisplay extends LitElement implements Layer {
     [GameUpdateType.Emoji, this.onEmojiMessageEvent.bind(this)],
     [GameUpdateType.UnitIncoming, this.onUnitIncomingEvent.bind(this)],
     [GameUpdateType.AllianceExpired, this.onAllianceExpiredEvent.bind(this)],
+    [
+      GameUpdateType.AllianceExtension,
+      this.onAllianceExtensionEvent.bind(this),
+    ],
   ] as const;
 
   constructor() {
     super();
     this.events = [];
   }
 
-  init() {
-    this.eventBus.on(SendAllianceExtensionIntentEvent, (e) => {
-      const myPlayer = this.game.myPlayer();
-      if (!myPlayer) return;
-      const alliance = myPlayer
-        .alliances()
-        .find((a) => a.other === e.recipient.id());
-      if (alliance) {
-        this.removeAllianceRenewalEvents(alliance.id);
-        this.requestUpdate();
-      }
-    });
-  }
+  init() {}
 
   tick() {
     this.active = true;
@@ -622,6 +615,11 @@ export class EventsDisplay extends LitElement implements Layer {
     });
   }
 
+  private onAllianceExtensionEvent(update: AllianceExtensionUpdate) {
+    this.removeAllianceRenewalEvents(update.allianceID);
+    this.requestUpdate();
+  }
+
   onTargetPlayerEvent(event: TargetPlayerUpdate) {
     const other = this.game.playerBySmallID(event.playerID) as PlayerView;
     const myPlayer = this.game.myPlayer() as PlayerView;

src/client/graphics/layers/PlayerPanel.ts

@@ -122,8 +122,8 @@ export class PlayerPanel extends LitElement implements Layer {
       const myPlayer = this.g.myPlayer();
       if (myPlayer !== null && myPlayer.isAlive()) {
         this.actions = await myPlayer.actions(this.tile);
-        if (this.actions?.interaction?.allianceExpiresAt !== undefined) {
-          const expiresAt = this.actions.interaction.allianceExpiresAt;
+        if (this.actions?.interaction?.allianceInfo?.expiresAt !== undefined) {
+          const expiresAt = this.actions.interaction.allianceInfo.expiresAt;
           const remainingTicks = expiresAt - this.g.ticks();
           const remainingSeconds = Math.max(0, Math.floor(remainingTicks / 10)); // 10 ticks per second
 

src/client/graphics/layers/RadialMenu.ts

@@ -1287,8 +1287,8 @@ export class RadialMenu implements Layer {
     params: MenuElementParams,
   ): string {
     const interaction = params.playerActions?.interaction;
-    const myAgreed = interaction?.myPlayerAgreedToExtend ?? false;
-    const otherAgreed = interaction?.otherPlayerAgreedToExtend ?? false;
+    const myAgreed = interaction?.allianceInfo?.myPlayerAgreedToExtend ?? false;
+    const otherAgreed = interaction?.allianceInfo?.otherAgreedToExtend ?? false;
     return `${disabled}:${myAgreed}:${otherAgreed}`;
   }
 
@@ -1307,8 +1307,8 @@ export class RadialMenu implements Layer {
     }
 
     const interaction = params.playerActions?.interaction;
-    const myAgreed = interaction?.myPlayerAgreedToExtend ?? false;
-    const otherAgreed = interaction?.otherPlayerAgreedToExtend ?? false;
+    const myAgreed = interaction?.allianceInfo?.myPlayerAgreedToExtend ?? false;
+    const otherAgreed = interaction?.allianceInfo?.otherAgreedToExtend ?? false;
 
     const ns = "http://www.w3.org/2000/svg";
     const smallSize = iconSize * 0.8;

src/client/graphics/layers/RadialMenuElements.ts

@@ -218,7 +218,7 @@ const allyExtendElement: MenuElement = {
   id: "ally_extend",
   name: "extend",
   displayed: (params: MenuElementParams) =>
-    !!params.playerActions?.interaction?.inAllianceExtensionWindow,
+    !!params.playerActions?.interaction?.allianceInfo?.inExtensionWindow,
   disabled: (params: MenuElementParams) =>
     !params.playerActions?.interaction?.canExtendAlliance,
   color: COLORS.ally,
@@ -230,10 +230,10 @@ const allyExtendElement: MenuElement = {
   },
   timerFraction: (params: MenuElementParams) => {
     const interaction = params.playerActions?.interaction;
-    if (!interaction?.allianceExpiresAt) return 1;
+    if (!interaction?.allianceInfo?.expiresAt) return 1;
     const remaining = Math.max(
       0,
-      interaction.allianceExpiresAt - params.game.ticks(),
+      interaction.allianceInfo.expiresAt - params.game.ticks(),
     );
     const extensionWindow = Math.max(
       1,
@@ -664,7 +664,7 @@ export const rootMenuElement: MenuElement = {
       (tileOwner as PlayerView).id() === params.myPlayer.id();
 
     const inExtensionWindow =
-      params.playerActions.interaction?.inAllianceExtensionWindow;
+      params.playerActions.interaction?.allianceInfo?.inExtensionWindow;
 
     const menuItems: (MenuElement | null)[] = [
       infoMenuElement,

src/core/GameRunner.ts

@@ -212,20 +212,8 @@ export class GameRunner {
         canDonateTroops: player.canDonateTroops(other),
         canEmbargo: !player.hasEmbargoAgainst(other),
       };
-      const alliance = player.allianceWith(other as Player);
-      if (alliance) {
-        actions.interaction.allianceExpiresAt = alliance.expiresAt();
-        const inWindow =
-          alliance.expiresAt() <=
-          this.game.ticks() +
-            this.game.config().allianceExtensionPromptOffset();
-        actions.interaction.inAllianceExtensionWindow = inWindow;
-        actions.interaction.myPlayerAgreedToExtend =
-          alliance.agreedToExtend(player);
-        actions.interaction.otherPlayerAgreedToExtend = alliance.agreedToExtend(
-          other as Player,
-        );
-      }
+      actions.interaction.allianceInfo =
+        player.allianceInfo(other as Player) ?? undefined;
     }
 
     return actions;

src/core/execution/alliance/AllianceExtensionExecution.ts

@@ -5,6 +5,7 @@ import {
   Player,
   PlayerID,
 } from "../../game/Game";
+import { GameUpdateType } from "../../game/GameUpdates";
 
 export class AllianceExtensionExecution implements Execution {
   constructor(
@@ -42,6 +43,12 @@ export class AllianceExtensionExecution implements Execution {
     // Mark this player's intent to extend
     alliance.addExtensionRequest(this.from);
 
+    mg.addUpdate({
+      type: GameUpdateType.AllianceExtension,
+      playerID: this.from.smallID(),
+      allianceID: alliance.id(),
+    });
+
     if (alliance.bothAgreedToExtend()) {
       alliance.extend();
 

src/core/game/Game.ts

@@ -661,6 +661,7 @@ export interface Player {
   allies(): Player[];
   isAlliedWith(other: Player): boolean;
   allianceWith(other: Player): MutableAlliance | null;
+  allianceInfo(other: Player): AllianceInfo | null;
   canSendAllianceRequest(other: Player): boolean;
   canExtendAlliance(other: Player): boolean;
   breakAlliance(alliance: Alliance): void;
@@ -860,6 +861,13 @@ export interface PlayerBorderTiles {
   borderTiles: ReadonlySet<TileRef>;
 }
 
+export interface AllianceInfo {
+  expiresAt: Tick;
+  inExtensionWindow: boolean;
+  myPlayerAgreedToExtend: boolean;
+  otherAgreedToExtend: boolean;
+}
+
 export interface PlayerInteraction {
   sharedBorder: boolean;
   canSendEmoji: boolean;
@@ -870,10 +878,7 @@ export interface PlayerInteraction {
   canDonateGold: boolean;
   canDonateTroops: boolean;
   canEmbargo: boolean;
-  allianceExpiresAt?: Tick;
-  inAllianceExtensionWindow?: boolean;
-  myPlayerAgreedToExtend?: boolean;
-  otherPlayerAgreedToExtend?: boolean;
+  allianceInfo?: AllianceInfo;
 }
 
 export interface EmojiMessage {

src/core/game/PlayerImpl.ts

@@ -12,6 +12,7 @@ import {
 import { AttackImpl } from "./AttackImpl";
 import {
   Alliance,
+  AllianceInfo,
   AllianceRequest,
   AllPlayers,
   Attack,
@@ -402,6 +403,22 @@ export class PlayerImpl implements Player {
     );
   }
 
+  allianceInfo(other: Player): AllianceInfo | null {
+    const alliance = this.allianceWith(other);
+    if (!alliance) {
+      return null;
+    }
+    const inExtensionWindow =
+      alliance.expiresAt() <=
+      this.mg.ticks() + this.mg.config().allianceExtensionPromptOffset();
+    return {
+      expiresAt: alliance.expiresAt(),
+      inExtensionWindow,
+      myPlayerAgreedToExtend: alliance.agreedToExtend(this),
+      otherAgreedToExtend: alliance.agreedToExtend(other),
+    };
+  }
+
   canExtendAlliance(other: Player): boolean {
     if (other === this) {
       return false;

tests/client/graphics/RadialMenuElements.test.ts

@@ -364,10 +364,13 @@ describe("RadialMenuElements", () => {
       mockPlayerActions.interaction = {
         ...mockPlayerActions.interaction,
         canBreakAlliance: true,
-        inAllianceExtensionWindow: true,
         canExtendAlliance: false,
-        myPlayerAgreedToExtend: true,
-        otherPlayerAgreedToExtend: false,
+        allianceInfo: {
+          expiresAt: 100,
+          inExtensionWindow: true,
+          myPlayerAgreedToExtend: true,
+          otherAgreedToExtend: false,
+        },
       };
 
       const subMenu = rootMenuElement.subMenu!(mockParams);
@@ -387,8 +390,13 @@ describe("RadialMenuElements", () => {
       mockPlayerActions.interaction = {
         ...mockPlayerActions.interaction,
         canBreakAlliance: true,
-        inAllianceExtensionWindow: false,
         canExtendAlliance: false,
+        allianceInfo: {
+          expiresAt: 100,
+          inExtensionWindow: false,
+          myPlayerAgreedToExtend: false,
+          otherAgreedToExtend: false,
+        },
       };
 
       const subMenu = rootMenuElement.subMenu!(mockParams);
@@ -408,10 +416,13 @@ describe("RadialMenuElements", () => {
       mockPlayerActions.interaction = {
         ...mockPlayerActions.interaction,
         canBreakAlliance: true,
-        inAllianceExtensionWindow: true,
         canExtendAlliance: false,
-        myPlayerAgreedToExtend: true,
-        otherPlayerAgreedToExtend: false,
+        allianceInfo: {
+          expiresAt: 100,
+          inExtensionWindow: true,
+          myPlayerAgreedToExtend: true,
+          otherAgreedToExtend: false,
+        },
       };
 
       const subMenu = rootMenuElement.subMenu!(mockParams);
@@ -432,10 +443,13 @@ describe("RadialMenuElements", () => {
       mockPlayerActions.interaction = {
         ...mockPlayerActions.interaction,
         canBreakAlliance: true,
-        inAllianceExtensionWindow: true,
         canExtendAlliance: true,
-        myPlayerAgreedToExtend: false,
-        otherPlayerAgreedToExtend: false,
+        allianceInfo: {
+          expiresAt: 100,
+          inExtensionWindow: true,
+          myPlayerAgreedToExtend: false,
+          otherAgreedToExtend: false,
+        },
       };
 
       const subMenu = rootMenuElement.subMenu!(mockParams);