fix bad tile crash (#1237)

## Description:

Sending invalid coords can cause game to crash. Make sure to validate
tile ref.

## 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/ClientGameRunner.ts

@@ -12,7 +12,7 @@ import {
 import { createGameRecord } from "../core/Util";
 import { ServerConfig } from "../core/configuration/Config";
 import { getConfig } from "../core/configuration/ConfigLoader";
-import { Cell, PlayerActions, UnitType } from "../core/game/Game";
+import { PlayerActions, UnitType } from "../core/game/Game";
 import { TileRef } from "../core/game/GameMap";
 import {
   ErrorUpdate,
@@ -404,7 +404,7 @@ export class ClientGameRunner {
           ),
         );
       } else if (this.canBoatAttack(actions, tile)) {
-        this.sendBoatAttackIntent(tile, cell);
+        this.sendBoatAttackIntent(tile);
       }
 
       const owner = this.gameView.owner(tile);
@@ -420,6 +420,9 @@ export class ClientGameRunner {
     if (!this.isActive || !this.lastMousePosition) {
       return;
     }
+    if (this.gameView.inSpawnPhase()) {
+      return;
+    }
     const cell = this.renderer.transformHandler.screenToWorldCoordinates(
       this.lastMousePosition.x,
       this.lastMousePosition.y,
@@ -427,11 +430,7 @@ export class ClientGameRunner {
     if (!this.gameView.isValidCoord(cell.x, cell.y)) {
       return;
     }
-
     const tile = this.gameView.ref(cell.x, cell.y);
-    if (this.gameView.inSpawnPhase()) {
-      return;
-    }
 
     if (this.myPlayer === null) {
       const myPlayer = this.gameView.playerByClientID(this.lobby.clientID);
@@ -441,7 +440,7 @@ export class ClientGameRunner {
 
     this.myPlayer.actions(tile).then((actions) => {
       if (!actions.canAttack && this.canBoatAttack(actions, tile)) {
-        this.sendBoatAttackIntent(tile, cell);
+        this.sendBoatAttackIntent(tile);
       }
     });
   }
@@ -461,26 +460,20 @@ export class ClientGameRunner {
     );
   }
 
-  private sendBoatAttackIntent(tile: TileRef, cell: Cell) {
+  private sendBoatAttackIntent(tile: TileRef) {
     if (!this.myPlayer) return;
 
-    this.myPlayer
-      .bestTransportShipSpawn(this.gameView.ref(cell.x, cell.y))
-      .then((spawn: number | false) => {
-        if (this.myPlayer === null) throw new Error("not initialized");
-        let spawnCell: Cell | null = null;
-        if (spawn !== false) {
-          spawnCell = new Cell(this.gameView.x(spawn), this.gameView.y(spawn));
-        }
-        this.eventBus.emit(
-          new SendBoatAttackIntentEvent(
-            this.gameView.owner(tile).id(),
-            cell,
-            this.myPlayer.troops() * this.renderer.uiState.attackRatio,
-            spawnCell,
-          ),
-        );
-      });
+    this.myPlayer.bestTransportShipSpawn(tile).then((spawn: number | false) => {
+      if (this.myPlayer === null) throw new Error("not initialized");
+      this.eventBus.emit(
+        new SendBoatAttackIntentEvent(
+          this.gameView.owner(tile).id(),
+          tile,
+          this.myPlayer.troops() * this.renderer.uiState.attackRatio,
+          spawn === false ? null : spawn,
+        ),
+      );
+    });
   }
 
   private shouldBoat(tile: TileRef, src: TileRef) {

src/client/Transport.ts

@@ -10,6 +10,7 @@ import {
   Tick,
   UnitType,
 } from "../core/game/Game";
+import { TileRef } from "../core/game/GameMap";
 import { PlayerView } from "../core/game/GameView";
 import {
   AllPlayersStats,
@@ -75,9 +76,9 @@ export class SendAttackIntentEvent implements GameEvent {
 export class SendBoatAttackIntentEvent implements GameEvent {
   constructor(
     public readonly targetID: PlayerID | null,
-    public readonly dst: Cell,
+    public readonly dst: TileRef,
     public readonly troops: number,
-    public readonly src: Cell | null = null,
+    public readonly src: TileRef | null = null,
   ) {}
 }
 
@@ -437,10 +438,8 @@ export class Transport {
       clientID: this.lobbyConfig.clientID,
       targetID: event.targetID,
       troops: event.troops,
-      dstX: event.dst.x,
-      dstY: event.dst.y,
-      srcX: event.src?.x ?? null,
-      srcY: event.src?.y ?? null,
+      dst: event.dst,
+      src: event.src,
     });
   }
 

src/client/graphics/layers/PlayerActionHandler.ts

@@ -48,13 +48,13 @@ export class PlayerActionHandler {
   handleBoatAttack(
     player: PlayerView,
     targetId: PlayerID | null,
-    targetCell: Cell,
-    spawnTile: Cell | null,
+    targetTile: TileRef,
+    spawnTile: TileRef | null,
   ) {
     this.eventBus.emit(
       new SendBoatAttackIntentEvent(
         targetId,
-        targetCell,
+        targetTile,
         this.uiState.attackRatio * player.troops(),
         spawnTile,
       ),

src/client/graphics/layers/RadialMenuElements.ts

@@ -1,6 +1,5 @@
 import {
   AllPlayers,
-  Cell,
   PlayerActions,
   TerraNullius,
   UnitType,
@@ -378,16 +377,11 @@ export const boatMenuElement: MenuElement = {
       params.tile,
     );
 
-    let spawnTile: Cell | null = null;
-    if (spawn !== false) {
-      spawnTile = new Cell(params.game.x(spawn), params.game.y(spawn));
-    }
-
     params.playerActionHandler.handleBoatAttack(
       params.myPlayer,
       params.selected?.id() || null,
-      new Cell(params.game.x(params.tile), params.game.y(params.tile)),
-      spawnTile,
+      params.tile,
+      spawn !== false ? spawn : null,
     );
 
     params.closeMenu();

src/core/Schemas.ts

@@ -210,10 +210,8 @@ export const BoatAttackIntentSchema = BaseIntentSchema.extend({
   type: z.literal("boat"),
   targetID: ID.nullable(),
   troops: z.number(),
-  dstX: z.number(),
-  dstY: z.number(),
-  srcX: z.number().nullable(),
-  srcY: z.number().nullable(),
+  dst: z.number(),
+  src: z.number().nullable(),
 });
 
 export const AllianceRequestIntentSchema = BaseIntentSchema.extend({

src/core/execution/ExecutionManager.ts

@@ -1,5 +1,4 @@
 import { Execution, Game } from "../game/Game";
-import { TileRef } from "../game/GameMap";
 import { PseudoRandom } from "../PseudoRandom";
 import { ClientID, GameID, Intent, Turn } from "../Schemas";
 import { simpleHash } from "../Util";
@@ -72,16 +71,12 @@ export class Executor {
           this.mg.ref(intent.x, intent.y),
         );
       case "boat":
-        let src: TileRef | null = null;
-        if (intent.srcX !== null && intent.srcY !== null) {
-          src = this.mg.ref(intent.srcX, intent.srcY);
-        }
         return new TransportShipExecution(
           player,
           intent.targetID,
-          this.mg.ref(intent.dstX, intent.dstY),
+          intent.dst,
           intent.troops,
-          src,
+          intent.src,
         );
       case "allianceRequest":
         return new AllianceRequestExecution(player, intent.recipient);

src/core/execution/MoveWarshipExecution.ts

@@ -9,6 +9,10 @@ export class MoveWarshipExecution implements Execution {
   ) {}
 
   init(mg: Game, ticks: number): void {
+    if (!mg.isValidRef(this.position)) {
+      console.warn(`MoveWarshipExecution: position ${this.position} not valid`);
+      return;
+    }
     const warship = this.owner
       .units(UnitType.Warship)
       .find((u) => u.id() === this.unitId);

src/core/execution/TransportShipExecution.ts

@@ -51,6 +51,16 @@ export class TransportShipExecution implements Execution {
       this.active = false;
       return;
     }
+    if (!mg.isValidRef(this.ref)) {
+      console.warn(`TransportShipExecution: ref ${this.ref} not valid`);
+      this.active = false;
+      return;
+    }
+    if (this.src !== null && !mg.isValidRef(this.src)) {
+      console.warn(`TransportShipExecution: src ${this.src} not valid`);
+      this.active = false;
+      return;
+    }
 
     this.lastMove = ticks;
     this.mg = mg;

src/core/game/GameImpl.ts

@@ -688,6 +688,9 @@ export class GameImpl implements Game {
   ref(x: number, y: number): TileRef {
     return this._map.ref(x, y);
   }
+  isValidRef(ref: TileRef): boolean {
+    return this._map.isValidRef(ref);
+  }
   x(ref: TileRef): number {
     return this._map.x(ref);
   }

src/core/game/GameMap.ts

@@ -5,7 +5,7 @@ export type TileUpdate = bigint;
 
 export interface GameMap {
   ref(x: number, y: number): TileRef;
-
+  isValidRef(ref: TileRef): boolean;
   x(ref: TileRef): number;
   y(ref: TileRef): number;
   cell(ref: TileRef): Cell;
@@ -117,6 +117,10 @@ export class GameMapImpl implements GameMap {
     return this.yToRef[y] + x;
   }
 
+  isValidRef(ref: TileRef): boolean {
+    return this.isValidCoord(this.x(ref), this.y(ref));
+  }
+
   x(ref: TileRef): number {
     return this.refToX[ref];
   }

src/core/game/GameView.ts

@@ -496,6 +496,9 @@ export class GameView implements GameMap {
   ref(x: number, y: number): TileRef {
     return this._map.ref(x, y);
   }
+  isValidRef(ref: TileRef): boolean {
+    return this._map.isValidRef(ref);
+  }
   x(ref: TileRef): number {
     return this._map.x(ref);
   }