upgrade unit when building a unit of same type (#1328)

## Description:

When building a structure in the same location as a nearby structure, it
will update the existing structure instead of creating a new one.

Also fix ctrl+click shortcut to bring up the build menu.

## 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
- [x] I understand that submitting code with bugs that could have been
caught through manual testing blocks releases and new features for all
contributors

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

evan

src/client/graphics/GameRenderer.ts

@@ -71,6 +71,7 @@ export function createRenderer(
   }
   buildMenu.game = game;
   buildMenu.eventBus = eventBus;
+  buildMenu.transformHandler = transformHandler;
 
   const leaderboard = document.querySelector("leader-board") as Leaderboard;
   if (!emojiTable || !(leaderboard instanceof Leaderboard)) {

src/client/graphics/layers/BuildMenu.ts

@@ -13,11 +13,27 @@ import missileSiloIcon from "../../../../resources/non-commercial/svg/MissileSil
 import samlauncherIcon from "../../../../resources/non-commercial/svg/SamLauncherIconWhite.svg";
 import { translateText } from "../../../client/Utils";
 import { EventBus } from "../../../core/EventBus";
-import { Cell, Gold, PlayerActions, UnitType } from "../../../core/game/Game";
+import {
+  BuildableUnit,
+  Cell,
+  Gold,
+  PlayerActions,
+  UnitType,
+} from "../../../core/game/Game";
 import { TileRef } from "../../../core/game/GameMap";
 import { GameView } from "../../../core/game/GameView";
-import { BuildUnitIntentEvent } from "../../Transport";
+import {
+  CloseViewEvent,
+  MouseDownEvent,
+  ShowBuildMenuEvent,
+  ShowEmojiMenuEvent,
+} from "../../InputHandler";
+import {
+  BuildUnitIntentEvent,
+  SendUpgradeStructureIntentEvent,
+} from "../../Transport";
 import { renderNumber } from "../../Utils";
+import { TransformHandler } from "../TransformHandler";
 import { Layer } from "./Layer";
 
 export interface BuildItemDisplay {
@@ -113,6 +129,30 @@ export class BuildMenu extends LitElement implements Layer {
   private clickedTile: TileRef;
   public playerActions: PlayerActions | null;
   private filteredBuildTable: BuildItemDisplay[][] = buildTable;
+  public transformHandler: TransformHandler;
+
+  init() {
+    this.eventBus.on(ShowBuildMenuEvent, (e) => {
+      const clickedCell = this.transformHandler.screenToWorldCoordinates(
+        e.x,
+        e.y,
+      );
+      if (clickedCell === null) {
+        return;
+      }
+      if (!this.game.isValidCoord(clickedCell.x, clickedCell.y)) {
+        return;
+      }
+      const tile = this.game.ref(clickedCell.x, clickedCell.y);
+      if (!this.game.myPlayer()?.isAlive()) {
+        return;
+      }
+      this.showMenu(tile);
+    });
+    this.eventBus.on(CloseViewEvent, () => this.hideMenu());
+    this.eventBus.on(ShowEmojiMenuEvent, () => this.hideMenu());
+    this.eventBus.on(MouseDownEvent, () => this.hideMenu());
+  }
 
   tick() {
     if (!this._hidden) {
@@ -312,7 +352,7 @@ export class BuildMenu extends LitElement implements Layer {
   @state()
   private _hidden = true;
 
-  public canBuild(item: BuildItemDisplay): boolean {
+  public canBuildOrUpgrade(item: BuildItemDisplay): boolean {
     if (this.game?.myPlayer() === null || this.playerActions === null) {
       return false;
     }
@@ -321,7 +361,7 @@ export class BuildMenu extends LitElement implements Layer {
     if (unit.length === 0) {
       return false;
     }
-    return unit[0].canBuild !== false;
+    return unit[0].canBuild !== false || unit[0].canUpgrade !== false;
   }
 
   public cost(item: BuildItemDisplay): Gold {
@@ -342,15 +382,27 @@ export class BuildMenu extends LitElement implements Layer {
     return player.units(item.unitType).length.toString();
   }
 
-  public onBuildSelected = (item: BuildItemDisplay) => {
-    this.eventBus.emit(
-      new BuildUnitIntentEvent(
-        item.unitType,
-        new Cell(this.game.x(this.clickedTile), this.game.y(this.clickedTile)),
-      ),
-    );
+  public sendBuildOrUpgrade(buildableUnit: BuildableUnit, tile: TileRef): void {
+    if (buildableUnit === null) {
+      return;
+    }
+    if (buildableUnit.canUpgrade !== false) {
+      this.eventBus.emit(
+        new SendUpgradeStructureIntentEvent(
+          buildableUnit.canUpgrade,
+          buildableUnit.type,
+        ),
+      );
+    } else if (buildableUnit.canBuild) {
+      this.eventBus.emit(
+        new BuildUnitIntentEvent(
+          buildableUnit.type,
+          new Cell(this.game.x(tile), this.game.y(tile)),
+        ),
+      );
+    }
     this.hideMenu();
-  };
+  }
 
   render() {
     return html`
@@ -361,13 +413,23 @@ export class BuildMenu extends LitElement implements Layer {
         ${this.filteredBuildTable.map(
           (row) => html`
             <div class="build-row">
-              ${row.map(
-                (item) => html`
+              ${row.map((item) => {
+                const buildableUnit = this.playerActions?.buildableUnits.find(
+                  (bu) => bu.type === item.unitType,
+                );
+                if (buildableUnit === undefined) {
+                  return html``;
+                }
+                const enabled =
+                  buildableUnit.canBuild !== false ||
+                  buildableUnit.canUpgrade !== false;
+                return html`
                   <button
                     class="build-button"
-                    @click=${() => this.onBuildSelected(item)}
-                    ?disabled=${!this.canBuild(item)}
-                    title=${!this.canBuild(item)
+                    @click=${() =>
+                      this.sendBuildOrUpgrade(buildableUnit, this.clickedTile)}
+                    ?disabled=${!enabled}
+                    title=${!enabled
                       ? translateText("build_menu.not_enough_money")
                       : ""}
                   >
@@ -402,8 +464,8 @@ export class BuildMenu extends LitElement implements Layer {
                         </div>`
                       : ""}
                   </button>
-                `,
-              )}
+                `;
+              })}
             </div>
           `,
         )}

src/client/graphics/layers/MainRadialMenu.ts

@@ -127,6 +127,7 @@ export class MainRadialMenu extends LitElement implements Layer {
       playerPanel: this.playerPanel,
       chatIntegration: this.chatIntegration,
       closeMenu: () => this.closeMenu(),
+      eventBus: this.eventBus,
     };
 
     this.radialMenu.setRootMenuItems(rootMenuItems, centerButtonElement);

src/client/graphics/layers/PlayerActionHandler.ts

@@ -1,14 +1,8 @@
 import { EventBus } from "../../../core/EventBus";
-import {
-  Cell,
-  PlayerActions,
-  PlayerID,
-  UnitType,
-} from "../../../core/game/Game";
+import { Cell, PlayerActions, PlayerID } from "../../../core/game/Game";
 import { TileRef } from "../../../core/game/GameMap";
 import { PlayerView } from "../../../core/game/GameView";
 import {
-  BuildUnitIntentEvent,
   SendAllianceRequestIntentEvent,
   SendAttackIntentEvent,
   SendBoatAttackIntentEvent,
@@ -67,13 +61,6 @@ export class PlayerActionHandler {
   ): Promise<TileRef | false> {
     return await player.bestTransportShipSpawn(tile);
   }
-
-  handleBuildUnit(unitType: UnitType, cellX: number, cellY: number) {
-    this.eventBus.emit(
-      new BuildUnitIntentEvent(unitType, new Cell(cellX, cellY)),
-    );
-  }
-
   handleSpawn(spawnCell: Cell) {
     this.eventBus.emit(new SendSpawnIntentEvent(spawnCell));
   }

src/client/graphics/layers/RadialMenuElements.ts

@@ -26,6 +26,7 @@ import emojiIcon from "../../../../resources/images/EmojiIconWhite.svg";
 import infoIcon from "../../../../resources/images/InfoIcon.svg";
 import targetIcon from "../../../../resources/images/TargetIconWhite.svg";
 import traitorIcon from "../../../../resources/images/TraitorIconWhite.svg";
+import { EventBus } from "../../../core/EventBus";
 
 export interface MenuElementParams {
   myPlayer: PlayerView;
@@ -38,6 +39,7 @@ export interface MenuElementParams {
   playerActionHandler: PlayerActionHandler;
   playerPanel: PlayerPanel;
   chatIntegration: ChatIntegration;
+  eventBus: EventBus;
   closeMenu: () => void;
 }
 
@@ -371,8 +373,10 @@ export const buildMenuElement: MenuElement = {
           ? item.key.replace("unit_type.", "")
           : item.unitType.toString(),
         disabled: (params: MenuElementParams) =>
-          !params.buildMenu.canBuild(item),
-        color: params.buildMenu.canBuild(item) ? COLORS.building : undefined,
+          !params.buildMenu.canBuildOrUpgrade(item),
+        color: params.buildMenu.canBuildOrUpgrade(item)
+          ? COLORS.building
+          : undefined,
         icon: item.icon,
         tooltipItems: [
           { text: translateText(item.key || ""), className: "title" },
@@ -389,11 +393,15 @@ export const buildMenuElement: MenuElement = {
             : null,
         ].filter((item): item is TooltipItem => item !== null),
         action: (params: MenuElementParams) => {
-          params.playerActionHandler.handleBuildUnit(
-            item.unitType,
-            params.game.x(params.tile),
-            params.game.y(params.tile),
+          const buildableUnit = params.playerActions.buildableUnits.find(
+            (bu) => bu.type === item.unitType,
           );
+          if (buildableUnit === undefined) {
+            return;
+          }
+          if (params.buildMenu.canBuildOrUpgrade(item)) {
+            params.buildMenu.sendBuildOrUpgrade(buildableUnit, params.tile);
+          }
           params.closeMenu();
         },
       }));

src/core/configuration/DefaultConfig.ts

@@ -430,7 +430,6 @@ export class DefaultConfig implements Config {
                 ),
           territoryBound: true,
           constructionDuration: this.instantBuild() ? 0 : 5 * 10,
-          upgradable: true,
         };
       case UnitType.SAMLauncher:
         return {
@@ -478,6 +477,7 @@ export class DefaultConfig implements Config {
           constructionDuration: this.instantBuild() ? 0 : 2 * 10,
           canBuildTrainStation: true,
           experimental: true,
+          upgradable: true,
         };
       case UnitType.Construction:
         return {

src/core/execution/ExecutionManager.ts

@@ -107,6 +107,7 @@ export class Executor {
       case "embargo":
         return new EmbargoExecution(player, intent.targetID, intent.action);
       case "build_unit":
+        // TODO: fix this
         return new ConstructionExecution(
           player,
           this.mg.ref(intent.x, intent.y),

src/core/game/Game.ts

@@ -522,6 +522,7 @@ export interface Player {
     spawnTile: TileRef,
     params: UnitParams<T>,
   ): Unit;
+
   upgradeUnit(unit: Unit): void;
 
   captureUnit(unit: Unit): void;
@@ -682,6 +683,8 @@ export interface PlayerActions {
 
 export interface BuildableUnit {
   canBuild: TileRef | false;
+  // unit id of the existing unit that can be upgraded, or false if it cannot be upgraded.
+  canUpgrade: number | false;
   type: UnitType;
   cost: Gold;
 }

src/core/game/PlayerImpl.ts

@@ -794,6 +794,27 @@ export class PlayerImpl implements Player {
     return b;
   }
 
+  // Returns the existing unit that can be upgraded,
+  // or false if it cannot be upgraded.
+  // New units of the same type can upgrade existing units.
+  // e.g. if a place a new city here, will it upgrade an existing city?
+  private canUpgradeExistingUnit(
+    type: UnitType,
+    targetTile: TileRef,
+  ): Unit | false {
+    if (!this.mg.config().unitInfo(type).upgradable) {
+      return false;
+    }
+    const range = this.mg.config().structureMinDist();
+    const existing = this.mg
+      .nearbyUnits(targetTile, range, type)
+      .sort((a, b) => a.distSquared - b.distSquared);
+    if (existing.length > 0) {
+      return existing[0].unit;
+    }
+    return false;
+  }
+
   upgradeUnit(unit: Unit) {
     const cost = this.mg.unitInfo(unit.type()).cost(this);
     this.removeGold(cost);
@@ -803,11 +824,19 @@ export class PlayerImpl implements Player {
   public buildableUnits(tile: TileRef): BuildableUnit[] {
     const validTiles = this.validStructureSpawnTiles(tile);
     return Object.values(UnitType).map((u) => {
+      let canUpgrade: number | false = false;
+      if (!this.mg.inSpawnPhase()) {
+        const existingUnit = this.canUpgradeExistingUnit(u, tile);
+        if (existingUnit !== false) {
+          canUpgrade = existingUnit.id();
+        }
+      }
       return {
         type: u,
         canBuild: this.mg.inSpawnPhase()
           ? false
           : this.canBuild(u, tile, validTiles),
+        canUpgrade: canUpgrade,
         cost: this.mg.config().unitInfo(u).cost(this),
       } as BuildableUnit;
     });

tests/PlayerImpl.test.ts

@@ -0,0 +1,48 @@
+import {
+  Game,
+  Player,
+  PlayerInfo,
+  PlayerType,
+  UnitType,
+} from "../src/core/game/Game";
+import { setup } from "./util/Setup";
+
+let game: Game;
+let player: Player;
+
+describe("PlayerImpl", () => {
+  beforeEach(async () => {
+    game = await setup(
+      "plains",
+      {
+        infiniteGold: true,
+        instantBuild: true,
+      },
+      [new PlayerInfo("player", PlayerType.Human, null, "player_id")],
+    );
+
+    while (game.inSpawnPhase()) {
+      game.executeNextTick();
+    }
+
+    player = game.player("player_id");
+  });
+
+  test("City can be upgraded", () => {
+    const city = player.buildUnit(UnitType.City, game.ref(0, 0), {});
+    const buCity = player
+      .buildableUnits(game.ref(0, 0))
+      .find((bu) => bu.type === UnitType.City);
+    expect(buCity).toBeDefined();
+    expect(buCity!.canUpgrade).toBe(city.id());
+  });
+
+  test("DefensePost cannot be upgraded", () => {
+    player.buildUnit(UnitType.DefensePost, game.ref(0, 0), {});
+    const buDefensePost = player
+      .buildableUnits(game.ref(0, 0))
+      .find((bu) => bu.type === UnitType.DefensePost);
+    expect(buDefensePost).toBeDefined();
+    expect(buDefensePost!.canUpgrade).toBeFalsy();
+  });
+});