From ca522a5937622c00d505a081feec1196be25bb88 Mon Sep 17 00:00:00 2001 From: evanpelle Date: Sat, 28 Jun 2025 12:33:19 -0700 Subject: [PATCH] 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 | 3 +- src/client/graphics/layers/NameLayer.ts | 4 +- .../graphics/layers/PlayerInfoOverlay.ts | 8 +-- src/client/graphics/layers/TerritoryLayer.ts | 2 +- src/core/GameRunner.ts | 11 +-- src/core/execution/BotSpawner.ts | 9 +-- src/core/game/Game.ts | 2 - src/core/game/GameUpdates.ts | 2 - src/core/game/GameView.ts | 68 ++++++++++++------- src/core/game/PlayerImpl.ts | 10 --- tests/Attack.test.ts | 4 -- tests/BotBehavior.test.ts | 4 -- tests/Disconnected.test.ts | 4 -- tests/MissileSilo.test.ts | 2 - tests/PlayerInfo.test.ts | 22 ------ tests/Stats.test.ts | 18 +---- tests/TeamAssignment.test.ts | 2 - tests/TerritoryCapture.test.ts | 9 +-- tests/UnitGrid.test.ts | 36 ++-------- tests/Warship.test.ts | 18 +---- tests/core/executions/NukeExecution.test.ts | 2 - .../executions/SAMLauncherExecution.test.ts | 8 --- tests/util/Setup.ts | 2 +- 23 files changed, 64 insertions(+), 186 deletions(-) diff --git a/src/client/ClientGameRunner.ts b/src/client/ClientGameRunner.ts index 0614732cd..12e21b1d6 100644 --- a/src/client/ClientGameRunner.ts +++ b/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"); diff --git a/src/client/graphics/layers/NameLayer.ts b/src/client/graphics/layers/NameLayer.ts index bb5056b3b..6b7af3197 100644 --- a/src/client/graphics/layers/NameLayer.ts +++ b/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); diff --git a/src/client/graphics/layers/PlayerInfoOverlay.ts b/src/client/graphics/layers/PlayerInfoOverlay.ts index 5ae2c81a0..ca2c97354 100644 --- a/src/client/graphics/layers/PlayerInfoOverlay.ts +++ b/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`
{ if (el instanceof HTMLElement) { requestAnimationFrame(() => { - renderPlayerFlag(player.flag()!, el); + renderPlayerFlag(player.cosmetics.flag!, el); }); } })} >
` : html`` : html``} ${player.name()} diff --git a/src/client/graphics/layers/TerritoryLayer.ts b/src/client/graphics/layers/TerritoryLayer.ts index 28d5def51..6fdd7a48b 100644 --- a/src/client/graphics/layers/TerritoryLayer.ts +++ b/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); diff --git a/src/core/GameRunner.ts b/src/core/GameRunner.ts index f2740c3db..93ad45fe6 100644 --- a/src/core/GameRunner.ts +++ b/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()), ), ); diff --git a/src/core/execution/BotSpawner.ts b/src/core/execution/BotSpawner.ts index 9b9a00bb2..134a7c666 100644 --- a/src/core/execution/BotSpawner.ts +++ b/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, ); } diff --git a/src/core/game/Game.ts b/src/core/game/Game.ts index d2bbade2f..b62c62b02 100644 --- a/src/core/game/Game.ts +++ b/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. diff --git a/src/core/game/GameUpdates.ts b/src/core/game/GameUpdates.ts index 0c76f3a95..4f2be287c 100644 --- a/src/core/game/GameUpdates.ts +++ b/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; diff --git a/src/core/game/GameView.ts b/src/core/game/GameView.ts index 47f0e7128..f50bd6c85 100644 --- a/src/core/game/GameView.ts +++ b/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(); + private _cosmetics: Map = 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) ?? + {}, + ), ); } }); diff --git a/src/core/game/PlayerImpl.ts b/src/core/game/PlayerImpl.ts index b9f045d00..ddc0658dd 100644 --- a/src/core/game/PlayerImpl.ts +++ b/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; } diff --git a/tests/Attack.test.ts b/tests/Attack.test.ts index 71beae6b5..8974f6250 100644 --- a/tests/Attack.test.ts +++ b/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, diff --git a/tests/BotBehavior.test.ts b/tests/BotBehavior.test.ts index e296892bb..9710770ac 100644 --- a/tests/BotBehavior.test.ts +++ b/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, diff --git a/tests/Disconnected.test.ts b/tests/Disconnected.test.ts index 709c8104a..e03138efa 100644 --- a/tests/Disconnected.test.ts +++ b/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, diff --git a/tests/MissileSilo.test.ts b/tests/MissileSilo.test.ts index 6e58ba2c5..c3a1f5ab1 100644 --- a/tests/MissileSilo.test.ts +++ b/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, diff --git a/tests/PlayerInfo.test.ts b/tests/PlayerInfo.test.ts index eca14e173..72ca1cceb 100644 --- a/tests/PlayerInfo.test.ts +++ b/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, diff --git a/tests/Stats.test.ts b/tests/Stats.test.ts index b988210bf..c53804b4f 100644 --- a/tests/Stats.test.ts +++ b/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()) { diff --git a/tests/TeamAssignment.test.ts b/tests/TeamAssignment.test.ts index bea1ea74f..5d9f5bcab 100644 --- a/tests/TeamAssignment.test.ts +++ b/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) diff --git a/tests/TerritoryCapture.test.ts b/tests/TerritoryCapture.test.ts index d547010ff..e46678c08 100644 --- a/tests/TerritoryCapture.test.ts +++ b/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( diff --git a/tests/UnitGrid.test.ts b/tests/UnitGrid.test.ts index ffd4ff9f6..9a0869ace 100644 --- a/tests/UnitGrid.test.ts +++ b/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); diff --git a/tests/Warship.test.ts b/tests/Warship.test.ts index 16a72e397..ee6c556de 100644 --- a/tests/Warship.test.ts +++ b/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"), ], ); diff --git a/tests/core/executions/NukeExecution.test.ts b/tests/core/executions/NukeExecution.test.ts index 435d388b4..5a614ebea 100644 --- a/tests/core/executions/NukeExecution.test.ts +++ b/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, diff --git a/tests/core/executions/SAMLauncherExecution.test.ts b/tests/core/executions/SAMLauncherExecution.test.ts index 7ceb08e1a..993488399 100644 --- a/tests/core/executions/SAMLauncherExecution.test.ts +++ b/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, diff --git a/tests/util/Setup.ts b/tests/util/Setup.ts index 7c6e86daa..c552d7d4f 100644 --- a/tests/util/Setup.ts +++ b/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); }