refactor cosmetics out of PlayerInfo (#1299)

## Description:

Remove Cosmetics from PlayerInfo. The game engine should have no
knowledge of cosmetics since they shouldn't affect game play at all.
Instead pass player cosmetics into the GameView.

## 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

@@ -150,9 +150,10 @@ export async function createClientGame(
   const gameView = new GameView(
     worker,
     config,
-    gameMap.gameMap,
+    gameMap,
     lobbyConfig.clientID,
     lobbyConfig.gameStartInfo.gameID,
+    lobbyConfig.gameStartInfo.players,
   );
 
   console.log("going to init path finder");

src/client/graphics/layers/NameLayer.ts

@@ -198,8 +198,8 @@ export class NameLayer implements Layer {
       element.style.aspectRatio = "3/4";
     };
 
-    if (player.flag()) {
-      const flag = player.flag();
+    if (player.cosmetics.flag) {
+      const flag = player.cosmetics.flag;
       if (flag !== undefined && flag !== null && flag.startsWith("!")) {
         const flagWrapper = document.createElement("div");
         applyFlagStyles(flagWrapper);

src/client/graphics/layers/PlayerInfoOverlay.ts

@@ -207,21 +207,21 @@ export class PlayerInfoOverlay extends LitElement implements Layer {
             ? "text-green-500"
             : "text-white"}"
         >
-          ${player.flag()
-            ? player.flag()!.startsWith("!")
+          ${player.cosmetics.flag
+            ? player.cosmetics.flag!.startsWith("!")
               ? html`<div
                   class="h-8 mr-1 aspect-[3/4] player-flag"
                   ${ref((el) => {
                     if (el instanceof HTMLElement) {
                       requestAnimationFrame(() => {
-                        renderPlayerFlag(player.flag()!, el);
+                        renderPlayerFlag(player.cosmetics.flag!, el);
                       });
                     }
                   })}
                 ></div>`
               : html`<img
                   class="h-8 mr-1 aspect-[3/4]"
-                  src=${"/flags/" + player.flag()! + ".svg"}
+                  src=${"/flags/" + player.cosmetics.flag! + ".svg"}
                 />`
             : html``}
           ${player.name()}

src/client/graphics/layers/TerritoryLayer.ts

@@ -307,7 +307,7 @@ export class TerritoryLayer implements Layer {
         this.paintTile(tile, useBorderColor, 255);
       }
     } else {
-      const pattern = owner.pattern();
+      const pattern = owner.cosmetics.pattern;
       const patternsEnabled = this.cachedTerritoryPatternsEnabled ?? false;
       if (pattern === undefined || patternsEnabled === false) {
         this.paintTile(tile, this.theme.territoryColor(owner), 150);

src/core/GameRunner.ts

@@ -42,8 +42,6 @@ export async function createGameRunner(
   const humans = gameStart.players.map(
     (p) =>
       new PlayerInfo(
-        p.pattern,
-        p.flag,
         p.clientID === clientID
           ? sanitize(p.username)
           : fixProfaneUsername(sanitize(p.username)),
@@ -60,14 +58,7 @@ export async function createGameRunner(
           new Nation(
             new Cell(n.coordinates[0], n.coordinates[1]),
             n.strength,
-            new PlayerInfo(
-              undefined,
-              n.flag || "",
-              n.name,
-              PlayerType.FakeHuman,
-              null,
-              random.nextID(),
-            ),
+            new PlayerInfo(n.name, PlayerType.FakeHuman, null, random.nextID()),
           ),
       );
 

src/core/execution/BotSpawner.ts

@@ -46,14 +46,7 @@ export class BotSpawner {
       }
     }
     return new SpawnExecution(
-      new PlayerInfo(
-        undefined,
-        "",
-        botName,
-        PlayerType.Bot,
-        null,
-        this.random.nextID(),
-      ),
+      new PlayerInfo(botName, PlayerType.Bot, null, this.random.nextID()),
       tile,
     );
   }

src/core/game/Game.ts

@@ -350,8 +350,6 @@ export class PlayerInfo {
   public readonly clan: string | null;
 
   constructor(
-    public readonly pattern: string | undefined,
-    public readonly flag: string | undefined,
     public readonly name: string,
     public readonly playerType: PlayerType,
     // null if bot.

src/core/game/GameUpdates.ts

@@ -133,8 +133,6 @@ export interface PlayerUpdate {
   type: GameUpdateType.Player;
   nameViewData?: NameViewData;
   clientID: ClientID | null;
-  pattern: string | undefined;
-  flag: string | undefined;
   name: string;
   displayName: string;
   id: PlayerID;

src/core/game/GameView.ts

@@ -1,6 +1,6 @@
 import { Config } from "../configuration/Config";
 import { PatternDecoder } from "../PatternDecoder";
-import { ClientID, GameID } from "../Schemas";
+import { ClientID, GameID, Player } from "../Schemas";
 import { createRandomName } from "../Util";
 import { WorkerClient } from "../worker/WorkerClient";
 import {
@@ -9,11 +9,9 @@ import {
   GameUpdates,
   Gold,
   NameViewData,
-  Player,
   PlayerActions,
   PlayerBorderTiles,
   PlayerID,
-  PlayerInfo,
   PlayerProfile,
   PlayerType,
   Team,
@@ -32,12 +30,18 @@ import {
   PlayerUpdate,
   UnitUpdate,
 } from "./GameUpdates";
+import { TerrainMapData } from "./TerrainMapLoader";
 import { TerraNulliusImpl } from "./TerraNulliusImpl";
 import { UnitGrid } from "./UnitGrid";
 import { UserSettings } from "./UserSettings";
 
 const userSettings: UserSettings = new UserSettings();
 
+interface PlayerCosmetics {
+  pattern?: string | undefined;
+  flag?: string | undefined;
+}
+
 export class UnitView {
   public _wasUpdated = true;
   public lastPos: TileRef[] = [];
@@ -145,6 +149,7 @@ export class PlayerView {
     private game: GameView,
     public data: PlayerUpdate,
     public nameData: NameViewData,
+    public cosmetics: PlayerCosmetics,
   ) {
     if (data.clientID === game.myClientID()) {
       this.anonymousName = this.data.name;
@@ -155,7 +160,9 @@ export class PlayerView {
       );
     }
     this.decoder =
-      data.pattern === undefined ? undefined : new PatternDecoder(data.pattern);
+      this.cosmetics.pattern === undefined
+        ? undefined
+        : new PatternDecoder(this.cosmetics.pattern);
   }
 
   patternDecoder(): PatternDecoder | undefined {
@@ -202,13 +209,6 @@ export class PlayerView {
   smallID(): number {
     return this.data.smallID;
   }
-  flag(): string | undefined {
-    return this.data.flag;
-  }
-
-  pattern(): string | undefined {
-    return this.data.pattern;
-  }
 
   name(): string {
     return this.anonymousName !== null && userSettings.anonymousNames()
@@ -236,7 +236,7 @@ export class PlayerView {
   isAlive(): boolean {
     return this.data.isAlive;
   }
-  isPlayer(): this is Player {
+  isPlayer(): this is PlayerView {
     return true;
   }
   numTilesOwned(): number {
@@ -306,16 +306,7 @@ export class PlayerView {
   outgoingEmojis(): EmojiMessage[] {
     return this.data.outgoingEmojis;
   }
-  info(): PlayerInfo {
-    return new PlayerInfo(
-      this.pattern(),
-      this.flag(),
-      this.name(),
-      this.type(),
-      this.clientID(),
-      this.id(),
-    );
-  }
+
   hasSpawned(): boolean {
     return this.data.hasSpawned;
   }
@@ -338,16 +329,35 @@ export class GameView implements GameMap {
 
   private toDelete = new Set<number>();
 
+  private _cosmetics: Map<string, PlayerCosmetics> = new Map();
+
+  private _map: GameMap;
+
   constructor(
     public worker: WorkerClient,
     private _config: Config,
-    private _map: GameMap,
+    private _mapData: TerrainMapData,
     private _myClientID: ClientID,
     private _gameID: GameID,
+    private _hunans: Player[],
   ) {
+    this._map = this._mapData.gameMap;
     this.lastUpdate = null;
-    this.unitGrid = new UnitGrid(_map);
+    this.unitGrid = new UnitGrid(this._map);
+    this._cosmetics = new Map(
+      this._hunans.map((h) => [
+        h.clientID,
+        { flag: h.flag, pattern: h.pattern } satisfies PlayerCosmetics,
+      ]),
+    );
+    for (const nation of this._mapData.manifest.nations) {
+      // Nations don't have client ids, so we use their name as the key instead.
+      this._cosmetics.set(nation.name, {
+        flag: nation.flag,
+      });
+    }
   }
+
   isOnEdgeOfMap(ref: TileRef): boolean {
     return this._map.isOnEdgeOfMap(ref);
   }
@@ -379,7 +389,15 @@ export class GameView implements GameMap {
       } else {
         this._players.set(
           pu.id,
-          new PlayerView(this, pu, gu.playerNameViewData[pu.id]),
+          new PlayerView(
+            this,
+            pu,
+            gu.playerNameViewData[pu.id],
+            // First check human by clientID, then check nation by name.
+            this._cosmetics.get(pu.clientID ?? "") ??
+              this._cosmetics.get(pu.name) ??
+              {},
+          ),
         );
       }
     });

src/core/game/PlayerImpl.ts

@@ -128,8 +128,6 @@ export class PlayerImpl implements Player {
     return {
       type: GameUpdateType.Player,
       clientID: this.clientID(),
-      pattern: this.pattern(),
-      flag: this.flag(),
       name: this.name(),
       displayName: this.displayName(),
       id: this.id(),
@@ -177,14 +175,6 @@ export class PlayerImpl implements Player {
     return this._smallID;
   }
 
-  pattern(): string | undefined {
-    return this.playerInfo.pattern;
-  }
-
-  flag(): string | undefined {
-    return this.playerInfo.flag;
-  }
-
   name(): string {
     return this._name;
   }

tests/Attack.test.ts

@@ -33,8 +33,6 @@ describe("Attack", () => {
       infiniteTroops: true,
     });
     const attackerInfo = new PlayerInfo(
-      undefined,
-      "us",
       "attacker dude",
       PlayerType.Human,
       null,
@@ -42,8 +40,6 @@ describe("Attack", () => {
     );
     game.addPlayer(attackerInfo);
     const defenderInfo = new PlayerInfo(
-      undefined,
-      "us",
       "defender dude",
       PlayerType.Human,
       null,

tests/BotBehavior.test.ts

@@ -23,16 +23,12 @@ describe("BotBehavior.handleAllianceRequests", () => {
     });
 
     const playerInfo = new PlayerInfo(
-      undefined,
-      "us",
       "player_id",
       PlayerType.Bot,
       null,
       "player_id",
     );
     const requestorInfo = new PlayerInfo(
-      undefined,
-      "fr",
       "requestor_id",
       PlayerType.Human,
       null,

tests/Disconnected.test.ts

@@ -16,8 +16,6 @@ describe("Disconnected", () => {
     });
 
     const player1Info = new PlayerInfo(
-      undefined,
-      "us",
       "Active Player",
       PlayerType.Human,
       null,
@@ -25,8 +23,6 @@ describe("Disconnected", () => {
     );
 
     const player2Info = new PlayerInfo(
-      undefined,
-      "fr",
       "Disconnected Player",
       PlayerType.Human,
       null,

tests/MissileSilo.test.ts

@@ -33,8 +33,6 @@ describe("MissileSilo", () => {
   beforeEach(async () => {
     game = await setup("plains", { infiniteGold: true, instantBuild: true });
     const attacker_info = new PlayerInfo(
-      undefined,
-      "fr",
       "attacker_id",
       PlayerType.Human,
       null,

tests/PlayerInfo.test.ts

@@ -4,8 +4,6 @@ describe("PlayerInfo", () => {
   describe("clan", () => {
     test("should extract clan from name when format is [XX]Name", () => {
       const playerInfo = new PlayerInfo(
-        undefined,
-        "fr",
         "[CL]PlayerName",
         PlayerType.Human,
         null,
@@ -16,8 +14,6 @@ describe("PlayerInfo", () => {
 
     test("should extract clan from name when format is [XXX]Name", () => {
       const playerInfo = new PlayerInfo(
-        undefined,
-        "fr",
         "[ABC]PlayerName",
         PlayerType.Human,
         null,
@@ -28,8 +24,6 @@ describe("PlayerInfo", () => {
 
     test("should extract clan from name when format is [XXXX]Name", () => {
       const playerInfo = new PlayerInfo(
-        undefined,
-        "fr",
         "[ABCD]PlayerName",
         PlayerType.Human,
         null,
@@ -40,8 +34,6 @@ describe("PlayerInfo", () => {
 
     test("should extract clan from name when format is [XXXXX]Name", () => {
       const playerInfo = new PlayerInfo(
-        undefined,
-        "fr",
         "[ABCDE]PlayerName",
         PlayerType.Human,
         null,
@@ -52,8 +44,6 @@ describe("PlayerInfo", () => {
 
     test("should extract clan from name when format is [xxxxx]Name", () => {
       const playerInfo = new PlayerInfo(
-        undefined,
-        "fr",
         "[abcde]PlayerName",
         PlayerType.Human,
         null,
@@ -64,8 +54,6 @@ describe("PlayerInfo", () => {
 
     test("should extract clan from name when format is [XxXxX]Name", () => {
       const playerInfo = new PlayerInfo(
-        undefined,
-        "fr",
         "[AbCdE]PlayerName",
         PlayerType.Human,
         null,
@@ -76,8 +64,6 @@ describe("PlayerInfo", () => {
 
     test("should return null when name doesn't start with [", () => {
       const playerInfo = new PlayerInfo(
-        undefined,
-        "fr",
         "PlayerName",
         PlayerType.Human,
         null,
@@ -88,8 +74,6 @@ describe("PlayerInfo", () => {
 
     test("should return null when name doesn't contain ]", () => {
       const playerInfo = new PlayerInfo(
-        undefined,
-        "fr",
         "[ABCPlayerName",
         PlayerType.Human,
         null,
@@ -100,8 +84,6 @@ describe("PlayerInfo", () => {
 
     test("should return null when clan tag is not 2-5 uppercase letters", () => {
       const playerInfo = new PlayerInfo(
-        undefined,
-        "fr",
         "[A]PlayerName",
         PlayerType.Human,
         null,
@@ -112,8 +94,6 @@ describe("PlayerInfo", () => {
 
     test("should return null when clan tag contains non alphanumeric characters", () => {
       const playerInfo = new PlayerInfo(
-        undefined,
-        "fr",
         "[A1c]PlayerName",
         PlayerType.Human,
         null,
@@ -124,8 +104,6 @@ describe("PlayerInfo", () => {
 
     test("should return null when clan tag is too long", () => {
       const playerInfo = new PlayerInfo(
-        undefined,
-        "fr",
         "[ABCDEF]PlayerName",
         PlayerType.Human,
         null,

tests/Stats.test.ts

@@ -19,22 +19,8 @@ describe("Stats", () => {
   beforeEach(async () => {
     stats = new StatsImpl();
     game = await setup("half_land_half_ocean", {}, [
-      new PlayerInfo(
-        undefined,
-        "us",
-        "boat dude",
-        PlayerType.Human,
-        "client1",
-        "player_1_id",
-      ),
-      new PlayerInfo(
-        undefined,
-        "us",
-        "boat dude",
-        PlayerType.Human,
-        "client2",
-        "player_2_id",
-      ),
+      new PlayerInfo("boat dude", PlayerType.Human, "client1", "player_1_id"),
+      new PlayerInfo("boat dude", PlayerType.Human, "client2", "player_2_id"),
     ]);
 
     while (game.inSpawnPhase()) {

tests/TeamAssignment.test.ts

@@ -7,8 +7,6 @@ describe("assignTeams", () => {
   const createPlayer = (id: string, clan?: string): PlayerInfo => {
     const name = clan ? `[${clan}]Player ${id}` : `Player ${id}`;
     return new PlayerInfo(
-      undefined,
-      "🏳️", // flag
       name,
       PlayerType.Human,
       null, // clientID (null for testing)

tests/TerritoryCapture.test.ts

@@ -6,14 +6,7 @@ describe("Territory management", () => {
   test("player owns the tile it spawns on", async () => {
     const game = await setup("plains");
     game.addPlayer(
-      new PlayerInfo(
-        undefined,
-        "us",
-        "test_player",
-        PlayerType.Human,
-        null,
-        "test_id",
-      ),
+      new PlayerInfo("test_player", PlayerType.Human, null, "test_id"),
     );
     const spawnTile = game.map().ref(50, 50);
     game.addExecution(

tests/UnitGrid.test.ts

@@ -11,14 +11,7 @@ async function checkRange(
   const game = await setup(mapName, { infiniteGold: true, instantBuild: true });
   const grid = new UnitGrid(game.map());
   const player = game.addPlayer(
-    new PlayerInfo(
-      undefined,
-      "us",
-      "test_player",
-      PlayerType.Human,
-      null,
-      "test_id",
-    ),
+    new PlayerInfo("test_player", PlayerType.Human, null, "test_id"),
   );
   const unitTile = game.map().ref(unitPosX, 0);
   grid.addUnit(player.buildUnit(UnitType.DefensePost, unitTile, {}));
@@ -41,14 +34,7 @@ async function nearbyUnits(
   const game = await setup(mapName, { infiniteGold: true, instantBuild: true });
   const grid = new UnitGrid(game.map());
   const player = game.addPlayer(
-    new PlayerInfo(
-      undefined,
-      "us",
-      "test_player",
-      PlayerType.Human,
-      null,
-      "test_id",
-    ),
+    new PlayerInfo("test_player", PlayerType.Human, null, "test_id"),
   );
   const unitTile = game.map().ref(unitPosX, 0);
   for (const unitType of unitTypes) {
@@ -122,14 +108,7 @@ describe("Unit Grid range tests", () => {
       });
       const grid = new UnitGrid(game.map());
       const player = game.addPlayer(
-        new PlayerInfo(
-          undefined,
-          "us",
-          "test_player",
-          PlayerType.Human,
-          null,
-          "test_id",
-        ),
+        new PlayerInfo("test_player", PlayerType.Human, null, "test_id"),
       );
       const unitTile = game.map().ref(0, 0);
       grid.addUnit(player.buildUnit(UnitType.City, unitTile, {}));
@@ -146,14 +125,7 @@ describe("Unit Grid range tests", () => {
       });
       const grid = new UnitGrid(game.map());
       const player = game.addPlayer(
-        new PlayerInfo(
-          undefined,
-          "us",
-          "test_player",
-          PlayerType.Human,
-          null,
-          "test_id",
-        ),
+        new PlayerInfo("test_player", PlayerType.Human, null, "test_id"),
       );
       const unitType = UnitType.City;
       const unitTile = game.map().ref(0, 0);

tests/Warship.test.ts

@@ -24,22 +24,8 @@ describe("Warship", () => {
         instantBuild: true,
       },
       [
-        new PlayerInfo(
-          undefined,
-          "us",
-          "boat dude",
-          PlayerType.Human,
-          null,
-          "player_1_id",
-        ),
-        new PlayerInfo(
-          undefined,
-          "us",
-          "boat dude",
-          PlayerType.Human,
-          null,
-          "player_2_id",
-        ),
+        new PlayerInfo("boat dude", PlayerType.Human, null, "player_1_id"),
+        new PlayerInfo("boat dude", PlayerType.Human, null, "player_2_id"),
       ],
     );
 

tests/core/executions/NukeExecution.test.ts

@@ -25,8 +25,6 @@ describe("NukeExecution", () => {
       outer: 10,
     }));
     const player_info = new PlayerInfo(
-      undefined,
-      "us",
       "player_id",
       PlayerType.Human,
       null,

tests/core/executions/SAMLauncherExecution.test.ts

@@ -25,32 +25,24 @@ describe("SAM", () => {
       instantBuild: true,
     });
     const defender_info = new PlayerInfo(
-      undefined,
-      "us",
       "defender_id",
       PlayerType.Human,
       null,
       "defender_id",
     );
     const middle_defender_info = new PlayerInfo(
-      undefined,
-      "us",
       "middle_defender_id",
       PlayerType.Human,
       null,
       "middle_defender_id",
     );
     const far_defender_info = new PlayerInfo(
-      undefined,
-      "us",
       "far_defender_id",
       PlayerType.Human,
       null,
       "far_defender_id",
     );
     const attacker_info = new PlayerInfo(
-      undefined,
-      "fr",
       "attacker_id",
       PlayerType.Human,
       null,

tests/util/Setup.ts

@@ -83,5 +83,5 @@ export async function setup(
 }
 
 export function playerInfo(name: string, type: PlayerType): PlayerInfo {
-  return new PlayerInfo(undefined, "fr", name, type, null, name);
+  return new PlayerInfo(name, type, null, name);
 }