Refactor UnitSpecific info => AllUnitParams type union (#701)

## Description:

By using a type union we get better type safety, enforcing each unit
type have the appropriate params when initializing

## Please complete the following:

- [x] I have added screenshots for all UI updates
- [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

---------

Co-authored-by: evan <openfrontio@gmail.com>

src/client/ClientGameRunner.ts

@@ -239,6 +239,7 @@ export class ClientGameRunner {
           this.lobby.gameStartInfo.gameID,
           this.lobby.clientID,
         );
+        console.error(gu.stack);
         this.stop(true);
         return;
       }

src/core/execution/CityExecution.ts

@@ -38,7 +38,7 @@ export class CityExecution implements Execution {
         this.active = false;
         return;
       }
-      this.city = this.player.buildUnit(UnitType.City, 0, spawnTile);
+      this.city = this.player.buildUnit(UnitType.City, spawnTile, {});
     }
     if (!this.city.isActive()) {
       this.active = false;

src/core/execution/ConstructionExecution.ts

@@ -60,8 +60,8 @@ export class ConstructionExecution implements Execution {
       }
       this.construction = this.player.buildUnit(
         UnitType.Construction,
-        0,
         spawnTile,
+        {},
       );
       this.cost = this.mg.unitInfo(this.constructionType).cost(this.player);
       this.player.removeGold(this.cost);

src/core/execution/DefensePostExecution.ts

@@ -65,7 +65,7 @@ export class DefensePostExecution implements Execution {
         this.active = false;
         return;
       }
-      this.post = this.player.buildUnit(UnitType.DefensePost, 0, spawnTile);
+      this.post = this.player.buildUnit(UnitType.DefensePost, spawnTile, {});
     }
     if (!this.post.isActive()) {
       this.active = false;

src/core/execution/MIRVExecution.ts

@@ -70,7 +70,7 @@ export class MirvExecution implements Execution {
         this.active = false;
         return;
       }
-      this.nuke = this.player.buildUnit(UnitType.MIRV, 0, spawn);
+      this.nuke = this.player.buildUnit(UnitType.MIRV, spawn, {});
       const x = Math.floor(
         (this.mg.x(this.dst) + this.mg.x(this.mg.x(this.nuke.tile()))) / 2,
       );

src/core/execution/MissileSiloExecution.ts

@@ -41,7 +41,7 @@ export class MissileSiloExecution implements Execution {
         this.active = false;
         return;
       }
-      this.silo = this.player.buildUnit(UnitType.MissileSilo, 0, spawn, {
+      this.silo = this.player.buildUnit(UnitType.MissileSilo, spawn, {
         cooldownDuration: this.mg.config().SiloCooldown(),
       });
 

src/core/execution/NukeExecution.ts

@@ -95,7 +95,7 @@ export class NukeExecution implements Execution {
         this.active = false;
         return;
       }
-      this.nuke = this.player.buildUnit(this.type, 0, spawn, {
+      this.nuke = this.player.buildUnit(this.type, spawn, {
         detonationDst: this.dst,
       });
       if (this.mg.hasOwner(this.dst)) {

src/core/execution/PortExecution.ts

@@ -45,7 +45,7 @@ export class PortExecution implements Execution {
         this.active = false;
         return;
       }
-      this.port = player.buildUnit(UnitType.Port, 0, spawn);
+      this.port = player.buildUnit(UnitType.Port, spawn, {});
     }
 
     if (!this.port.isActive()) {

src/core/execution/SAMLauncherExecution.ts

@@ -99,7 +99,7 @@ export class SAMLauncherExecution implements Execution {
         this.active = false;
         return;
       }
-      this.sam = this.player.buildUnit(UnitType.SAMLauncher, 0, spawnTile, {
+      this.sam = this.player.buildUnit(UnitType.SAMLauncher, spawnTile, {
         cooldownDuration: this.mg.config().SAMCooldown(),
       });
     }

src/core/execution/SAMMissileExecution.ts

@@ -33,8 +33,8 @@ export class SAMMissileExecution implements Execution {
     if (this.SAMMissile == null) {
       this.SAMMissile = this._owner.buildUnit(
         UnitType.SAMMissile,
-        0,
         this.spawn,
+        {},
       );
     }
     if (!this.SAMMissile.isActive()) {

src/core/execution/ShellExecution.ts

@@ -24,7 +24,7 @@ export class ShellExecution implements Execution {
 
   tick(ticks: number): void {
     if (this.shell == null) {
-      this.shell = this._owner.buildUnit(UnitType.Shell, 0, this.spawn);
+      this.shell = this._owner.buildUnit(UnitType.Shell, this.spawn, {});
     }
     if (!this.shell.isActive()) {
       this.active = false;

src/core/execution/TradeShipExecution.ts

@@ -45,7 +45,7 @@ export class TradeShipExecution implements Execution {
         this.active = false;
         return;
       }
-      this.tradeShip = this.origOwner.buildUnit(UnitType.TradeShip, 0, spawn, {
+      this.tradeShip = this.origOwner.buildUnit(UnitType.TradeShip, spawn, {
         dstPort: this._dstPort,
         lastSetSafeFromPirates: ticks,
       });

src/core/execution/TransportShipExecution.ts

@@ -139,11 +139,9 @@ export class TransportShipExecution implements Execution {
       }
     }
 
-    this.boat = this.attacker.buildUnit(
-      UnitType.TransportShip,
-      this.troops,
-      this.src,
-    );
+    this.boat = this.attacker.buildUnit(UnitType.TransportShip, this.src, {
+      troops: this.troops,
+    });
   }
 
   tick(ticks: number) {

src/core/execution/WarshipExecution.ts

@@ -119,7 +119,7 @@ export class WarshipExecution implements Execution {
         this.active = false;
         return;
       }
-      this.warship = this._owner.buildUnit(UnitType.Warship, 0, spawn);
+      this.warship = this._owner.buildUnit(UnitType.Warship, spawn, {});
       return;
     }
     if (!this.warship.isActive()) {

src/core/game/Game.ts

@@ -148,6 +148,51 @@ export enum UnitType {
   Construction = "Construction",
 }
 
+export interface UnitParamsMap {
+  [UnitType.TransportShip]: {
+    troops?: number;
+    destination?: TileRef;
+  };
+
+  [UnitType.Warship]: {};
+
+  [UnitType.Shell]: {};
+
+  [UnitType.SAMMissile]: {};
+
+  [UnitType.Port]: {};
+
+  [UnitType.AtomBomb]: {};
+
+  [UnitType.HydrogenBomb]: {};
+
+  [UnitType.TradeShip]: {
+    dstPort: Unit;
+    lastSetSafeFromPirates?: number;
+  };
+
+  [UnitType.MissileSilo]: {
+    cooldownDuration?: number;
+  };
+
+  [UnitType.DefensePost]: {};
+
+  [UnitType.SAMLauncher]: {};
+
+  [UnitType.City]: {};
+
+  [UnitType.MIRV]: {};
+
+  [UnitType.MIRVWarhead]: {};
+
+  [UnitType.Construction]: {};
+}
+
+// Type helper to get params type for a specific unit type
+export type UnitParams<T extends UnitType> = UnitParamsMap[T];
+
+export type AllUnitParams = UnitParamsMap[keyof UnitParamsMap];
+
 export const nukeTypes = [
   UnitType.AtomBomb,
   UnitType.HydrogenBomb,
@@ -276,15 +321,6 @@ export class PlayerInfo {
   }
 }
 
-// Some units have info specific to them
-export interface UnitSpecificInfos {
-  dstPort?: Unit; // Only for trade ships
-  lastSetSafeFromPirates?: number; // Only for trade ships
-  detonationDst?: TileRef; // Only for nukes
-  warshipTarget?: Unit;
-  cooldownDuration?: number;
-}
-
 export interface Unit {
   id(): number;
 
@@ -391,12 +427,12 @@ export interface Player {
   unitsIncludingConstruction(type: UnitType): Unit[];
   buildableUnits(tile: TileRef): BuildableUnit[];
   canBuild(type: UnitType, targetTile: TileRef): TileRef | false;
-  buildUnit(
-    type: UnitType,
-    troops: number,
-    tile: TileRef,
-    unitSpecificInfos?: UnitSpecificInfos,
+  buildUnit<T extends UnitType>(
+    type: T,
+    spawnTile: TileRef,
+    params: UnitParams<T>,
   ): Unit;
+
   captureUnit(unit: Unit): void;
 
   // Relations & Diplomacy

src/core/game/PlayerImpl.ts

@@ -35,7 +35,7 @@ import {
   TerraNullius,
   Tick,
   Unit,
-  UnitSpecificInfos,
+  UnitParams,
   UnitType,
 } from "./Game";
 import { GameImpl } from "./GameImpl";
@@ -703,11 +703,10 @@ export class PlayerImpl implements Player {
     );
   }
 
-  buildUnit(
-    type: UnitType,
-    troops: number,
+  buildUnit<T extends UnitType>(
+    type: T,
     spawnTile: TileRef,
-    unitSpecificInfos: UnitSpecificInfos = {},
+    params: UnitParams<T>,
   ): UnitImpl {
     if (this.mg.config().isUnitDisabled(type)) {
       throw new Error(
@@ -720,14 +719,13 @@ export class PlayerImpl implements Player {
       type,
       this.mg,
       spawnTile,
-      troops,
       this.mg.nextUnitID(),
       this,
-      unitSpecificInfos,
+      params,
     );
     this._units.push(b);
     this.removeGold(cost);
-    this.removeTroops(troops);
+    this.removeTroops("troops" in params ? params.troops : 0);
     this.mg.addUpdate(b.toUpdate());
     this.mg.addUnit(b);
 

src/core/game/UnitImpl.ts

@@ -1,11 +1,11 @@
 import { simpleHash, toInt, withinInt } from "../Util";
 import {
+  AllUnitParams,
   MessageType,
   Player,
   Tick,
   Unit,
   UnitInfo,
-  UnitSpecificInfos,
   UnitType,
 } from "./Game";
 import { GameImpl } from "./GameImpl";
@@ -24,6 +24,7 @@ export class UnitImpl implements Unit {
   private _lastSetSafeFromPirates: number; // Only for trade ships
   private _constructionType: UnitType = undefined;
 
+  private _troops: number;
   private _cooldownTick: Tick | null = null;
   private _dstPort: Unit | null = null; // Only for trade ships
   private _detonationDst: TileRef | null = null; // Only for nukes
@@ -34,21 +35,22 @@ export class UnitImpl implements Unit {
     private _type: UnitType,
     private mg: GameImpl,
     private _tile: TileRef,
-    private _troops: number,
     private _id: number,
     public _owner: PlayerImpl,
-    unitsSpecificInfos: UnitSpecificInfos = {},
+    params: AllUnitParams = {},
   ) {
-    this._health = toInt(this.mg.unitInfo(_type).maxHealth ?? 1);
     this._lastTile = _tile;
-    this._dstPort = unitsSpecificInfos.dstPort;
-    this._detonationDst = unitsSpecificInfos.detonationDst;
-    this._warshipTarget = unitsSpecificInfos.warshipTarget;
-    this._cooldownDuration = unitsSpecificInfos.cooldownDuration;
-    this._lastSetSafeFromPirates = unitsSpecificInfos.lastSetSafeFromPirates;
+    this._health = toInt(this.mg.unitInfo(_type).maxHealth ?? 1);
     this._safeFromPiratesCooldown = this.mg
       .config()
       .safeFromPiratesCooldownMax();
+
+    this._troops = "troops" in params ? params.troops : 0;
+    this._dstPort = "dstPort" in params ? params.dstPort : null;
+    this._cooldownDuration =
+      "cooldownDuration" in params ? params.cooldownDuration : null;
+    this._lastSetSafeFromPirates =
+      "lastSetSafeFromPirates" in params ? params.lastSetSafeFromPirates : 0;
   }
 
   id() {

tests/SAM.test.ts

@@ -50,9 +50,9 @@ describe("SAM", () => {
   });
 
   test("one sam should take down one nuke", async () => {
-    const sam = defender.buildUnit(UnitType.SAMLauncher, 0, game.ref(1, 1));
+    const sam = defender.buildUnit(UnitType.SAMLauncher, game.ref(1, 1), {});
     game.addExecution(new SAMLauncherExecution(defender.id(), null, sam));
-    attacker.buildUnit(UnitType.AtomBomb, 0, game.ref(1, 1));
+    attacker.buildUnit(UnitType.AtomBomb, game.ref(1, 1), {});
 
     executeTicks(game, 3);
 
@@ -60,10 +60,14 @@ describe("SAM", () => {
   });
 
   test("sam should only get one nuke at a time", async () => {
-    const sam = defender.buildUnit(UnitType.SAMLauncher, 0, game.ref(1, 1));
+    const sam = defender.buildUnit(UnitType.SAMLauncher, game.ref(1, 1), {});
     game.addExecution(new SAMLauncherExecution(defender.id(), null, sam));
-    attacker.buildUnit(UnitType.AtomBomb, 0, game.ref(2, 1));
-    attacker.buildUnit(UnitType.AtomBomb, 0, game.ref(1, 2));
+    attacker.buildUnit(UnitType.AtomBomb, game.ref(2, 1), {
+      detonationDst: game.ref(2, 1),
+    });
+    attacker.buildUnit(UnitType.AtomBomb, game.ref(1, 2), {
+      detonationDst: game.ref(1, 2),
+    });
     expect(attacker.units(UnitType.AtomBomb)).toHaveLength(2);
 
     executeTicks(game, 3);
@@ -72,10 +76,12 @@ describe("SAM", () => {
   });
 
   test("sam should cooldown as long as configured", async () => {
-    const sam = defender.buildUnit(UnitType.SAMLauncher, 0, game.ref(1, 1));
+    const sam = defender.buildUnit(UnitType.SAMLauncher, game.ref(1, 1), {});
     game.addExecution(new SAMLauncherExecution(defender.id(), null, sam));
     expect(sam.isCooldown()).toBeFalsy();
-    const nuke = attacker.buildUnit(UnitType.AtomBomb, 0, game.ref(1, 2));
+    const nuke = attacker.buildUnit(UnitType.AtomBomb, game.ref(1, 2), {
+      detonationDst: game.ref(1, 2),
+    });
 
     executeTicks(game, 3);
 
@@ -91,11 +97,15 @@ describe("SAM", () => {
   });
 
   test("two sams should not target twice same nuke", async () => {
-    const sam1 = defender.buildUnit(UnitType.SAMLauncher, 0, game.ref(1, 1));
+    const sam1 = defender.buildUnit(UnitType.SAMLauncher, game.ref(1, 1), {
+      cooldownDuration: 10,
+    });
     game.addExecution(new SAMLauncherExecution(defender.id(), null, sam1));
-    const sam2 = defender.buildUnit(UnitType.SAMLauncher, 0, game.ref(1, 2));
+    const sam2 = defender.buildUnit(UnitType.SAMLauncher, game.ref(1, 2), {});
     game.addExecution(new SAMLauncherExecution(defender.id(), null, sam2));
-    const nuke = attacker.buildUnit(UnitType.AtomBomb, 0, game.ref(2, 2));
+    const nuke = attacker.buildUnit(UnitType.AtomBomb, game.ref(2, 2), {
+      detonationDst: game.ref(2, 2),
+    });
 
     executeTicks(game, 3);
 

tests/Warship.test.ts

@@ -59,11 +59,11 @@ describe("Warship", () => {
   test("Warship heals only if player has port", async () => {
     const maxHealth = game.config().unitInfo(UnitType.Warship).maxHealth;
 
-    const port = player1.buildUnit(UnitType.Port, 0, game.ref(coastX, 10));
+    const port = player1.buildUnit(UnitType.Port, game.ref(coastX, 10), {});
     const warship = player1.buildUnit(
       UnitType.Warship,
-      0,
       game.ref(coastX + 1, 10),
+      {},
     );
 
     game.executeNextTick();
@@ -91,8 +91,10 @@ describe("Warship", () => {
     // we can obviously directly add it to the player)
     const tradeShip = player2.buildUnit(
       UnitType.TradeShip,
-      0,
       game.ref(coastX + 1, 7),
+      {
+        dstPort: null,
+      },
     );
 
     expect(tradeShip.owner().id()).toBe(player2.id());
@@ -113,8 +115,10 @@ describe("Warship", () => {
     // we can obviously directly add it to the player)
     const tradeShip = player2.buildUnit(
       UnitType.TradeShip,
-      0,
       game.ref(coastX + 1, 11),
+      {
+        dstPort: null,
+      },
     );
 
     expect(tradeShip.owner().id()).toBe(player2.id());