Refactored enums to string unions #1883
Conversation
…io#1597) ## Description: This PR implements a new feature allowing automatic upgrade of the nearest building using the middle mouse button. This feature greatly simplifies the upgrade process that previously required a right-click + building recreation. ## 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 have read and accepted the CLA agreement (only required once). ## Please put your Discord username so you can be contacted if a bug or regression is found: Kipstzz --------- Co-authored-by: Scott Anderson <662325+scottanderson@users.noreply.github.com>
…tio#1643) ## Description: Checks that each lang file’s lang_code matches its filename and that the flag SVG exists. Reports all errors in a single test run for easier debugging. ## 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 have read and accepted the CLA agreement (only required once). ## Please put your Discord username so you can be contacted if a bug or regression is found: aotumuri Co-authored-by: Drills Kibo <59177241+drillskibo@users.noreply.github.com>
## Description: Added a red delete button with trash can icon to the right-click radial menu that allows players to voluntarily delete their own units. ## 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 have read and accepted the CLA agreement (only required once). ## Please put your Discord username so you can be contacted if a bug or regression is found: Kipstz <img width="286" height="209" alt="image" src="https://github.com/user-attachments/assets/85142be3-2aa5-4c84-ab30-0c68289c8f85" /> --------- Co-authored-by: Drills Kibo <59177241+drillskibo@users.noreply.github.com>
## Description: Strict mode is causing docker deployments to fail. This commit reverts only the strict mode setting itself, while keeping the code changes, so that we can re-enable it more easily in the future. ## 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 - [ ] I have read and accepted the CLA agreement (only required once).
Fixes openfrontio#1559 ## Description: The current Mars map has several issues: - The map is very underdetailed - The coastlines are very smooth and visually unappealing - The map was created from an incompatible source image, which results in the height data being very wrong - As a result the map hardly resembles Mars at all. - There is too much land - The major landmass is almost entirely covered in plains - The ocean doesn't match the heightmap - The gameplay is boring - The map is very succeptable to snowballing issues - The map seems commonly disliked in the community I attempted to remedy this. - The water level and elevation are calculated from accurate height data - Water level is much higher making the map much more interesting, both in the gameplay and visual aspect. - The map has several contitents which should slow down snowballing - The map is much more detailed overall - I added a lot more NPC's. 8 for such a big map just wasn't enough. <img width="2500" height="1150" alt="Mars_Out2" src="https://github.com/user-attachments/assets/7b7009f7-6376-4059-b731-86931df8f926" /> - [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: aceralex --------- Co-authored-by: Drills Kibo <59177241+drillskibo@users.noreply.github.com>
…ntio#1680) ## Description: Switches away from ts-jest in favor of @swc/jest. On my local I observe a ten-fold decrease in how long it takes the test suite to run. No changes are required to how our existing tests are written. Benchmarking old: 24.658s new: 2.268s ts-jest (old) ``` Test Suites: 29 passed, 29 total Tests: 215 passed, 215 total Snapshots: 0 total Time: 24.658 s Ran all test suites. ``` swc-jest (new) ``` Test Suites: 29 passed, 29 total Tests: 215 passed, 215 total Snapshots: 0 total Time: 2.268 s Ran all test suites. ``` Fixes openfrontio#1679 ## 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 have read and accepted the CLA agreement (only required once). ## Please put your Discord username so you can be contacted if a bug or regression is found: slyty
## Description: This pull request adds support for ICU (Intl MessageFormat) syntax in the translation system. Existing translation files may need to be updated to fully leverage ICU features. ## 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 have read and accepted the CLA agreement (only required once). ## Please put your Discord username so you can be contacted if a bug or regression is found: DISCORD_USERNAME
…values is in a single file. - If, when merging, there is no need for it, drop this commit.
I had to change 'TradeShipExecution.test.js', because it used 'type: 2', but since there is no longer an enum, I changed it to 'type: PathFindResultTypeSchema.enum.Completed'.
|
|
WalkthroughLarge refactor replacing enums with Zod schemas and string-literal unions across client, core, and server. Update types, unit types, player types, terrain, phases, and HTTP limiter moved to schemas/strings. Replay speed now schema-backed with value/label maps. Some flows tweaked (server start delay, worker prefix routing, unit selection UX). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as ReplayPanel
participant LS as LocalServer
participant GM as GameManager (client)
Note over UI: User clicks a speed button
UI->>UI: onReplaySpeedChange(value)
UI->>LS: emit ReplaySpeedChangeEvent(value)
Note over LS: Map key -> number via ReplaySpeedValues
LS->>LS: set replaySpeed = ReplaySpeedValues[value]
LS-->>GM: apply new tick multiplier
sequenceDiagram
autonumber
participant Core as GameImpl
participant GV as GameView
participant Layers as Client Layers
rect rgba(230,240,255,0.4)
Note right of Core: Emit updates with string types
Core-->>GV: { type: "Unit" | "Tile" | "Win" | ... }
end
GV->>Layers: updates["Unit"]/["Win"]/...
Layers->>Layers: Branch via string keys
Note over Layers: No GameUpdateType enum used
sequenceDiagram
autonumber
participant Cli as Client
participant W as Worker
participant GK as Gatekeeper
participant S as GameServer
rect rgba(235,255,235,0.4)
Cli->>W: HTTP request /w{id}/api/...
W->>W: validate and strip /w{id} prefix
W->>GK: httpHandler("get"|"post"|"put", handler)
GK-->>W: throttle/allow
W->>S: route to game handler
S-->>Cli: response
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (26)
src/core/execution/ConstructionExecution.ts (1)
85-95: Off-by-one: construction completes one tick lateticksUntilComplete is checked before decrement, so a duration of N actually finishes in N+1 ticks. Decrement first, then compare ≤ 0.
Apply:
- if (this.ticksUntilComplete === 0) { - this.player = this.construction.owner(); - this.construction.delete(false); - // refund the cost so player has the gold to build the unit - this.player.addGold(this.cost); - this.completeConstruction(); - this.active = false; - return; - } - this.ticksUntilComplete--; + this.ticksUntilComplete--; + if (this.ticksUntilComplete <= 0) { + this.player = this.construction.owner(); + this.construction.delete(false); + // refund the cost so player has the gold to build the unit + this.player.addGold(this.cost); + this.completeConstruction(); + this.active = false; + return; + }src/client/graphics/AnimatedSpriteLoader.ts (1)
110-118: Add missing FxType entries for loader keysThe
AnimatedSpriteLoader.tsuses"Dust"and"Conquest"as keys, but these aren’t listed in theFxTypeSchemaz.enum—so the inferredFxTypeunion doesn’t include them. This will break anysatisfies FxTypechecks. Please update the enum insrc/client/graphics/fx/Fx.tsso it covers all used keys.• File: src/client/graphics/fx/Fx.ts
Update the z.enum list underFxTypeSchemato include:
"Dust""Conquest"Suggested diff:
export const FxTypeSchema = z.enum([ "MiniFire", "MiniSmoke", "MiniBigSmoke", "MiniSmokeAndFire", "MiniExplosion", "UnitExplosion", "SinkingShip", "Nuke", "SAMExplosion", + "Dust", + "Conquest", "UnderConstruction", // …others as needed ]);src/client/graphics/layers/BuildMenu.ts (1)
49-119: Prefer translation key for alt text and accessibility.Images use
alt="${item.unitType}", which exposes internal type strings and is not localized. Use the existingkey(already a translate key) to power alt text for screen readers.Apply outside-selected-lines change at Lines 433-435:
- <img - src=${item.icon} - alt="${item.unitType}" + <img + src=${item.icon} + alt="${item.key ? translateText(item.key) : item.unitType}"src/core/execution/FakeHumanExecution.ts (2)
93-106: Bug: comparing string relations with <=/>= yields wrong logic.player.relation(other) now returns a string union ("Hostile" | "Neutral" | ...). Using <= or >= does lexicographic comparisons, not rank. This will misbehave (e.g., "Neutral" >= "Neutral" is true, but "Ally" semantics aren’t captured).
Use equality or an explicit rank map. Minimal safe fix: start embargo only when Hostile; stop embargo when not Hostile.
- if ( - player.relation(other) <= "Hostile" && - !player.hasEmbargoAgainst(other) - ) { + if ( + player.relation(other) === "Hostile" && + !player.hasEmbargoAgainst(other) + ) { player.addEmbargo(other.id(), false); - } else if ( - player.relation(other) >= "Neutral" && - player.hasEmbargoAgainst(other) - ) { + } else if ( + player.relation(other) !== "Hostile" && + player.hasEmbargoAgainst(other) + ) { player.stopEmbargo(other.id()); }If you need graded behavior, introduce an ordering:
const RELATION_RANK: Record<"Hostile" | "Neutral" | "Ally", number> = { Hostile: 0, Neutral: 1, Ally: 2 }; // then compare RELATION_RANK[player.relation(other)] >= RELATION_RANK.Neutral
93-106: Fix string-based relation comparisons with a proper orderingDirectly comparing relation() results as strings (using
<,<=,>,>=) breaks when the labels don’t sort in the intended order. Instead, define a typed union for Relation and a numeric order map, then use a compare helper or numeric checks.Problematic spots:
- src/core/execution/utils/BotBehavior.ts:90
if (this.player.relation(ally) < "Friendly") {- src/core/execution/FakeHumanExecution.ts:96–97
if ( player.relation(other) <= "Hostile" && !player.hasEmbargoAgainst(other) ) { … }- src/core/execution/FakeHumanExecution.ts:101–102
else if ( player.relation(other) >= "Neutral" && player.hasEmbargoAgainst(other) ) { … }Suggested refactor:
- Define a
Relationunion and order map:// src/core/types/Relation.ts export type Relation = 'Hostile' | 'Neutral' | 'Friendly'; export const RelationOrder: Record<Relation, number> = { Hostile: 0, Neutral: 1, Friendly: 2, }; export function compareRelation(a: Relation, b: Relation): number { return RelationOrder[a] - RelationOrder[b]; }- Replace string comparisons with numeric checks:
// BotBehavior.ts (around line 90) if (compareRelation(this.player.relation(ally), 'Friendly') < 0) { continue; } // FakeHumanExecution.ts (around lines 96 and 101) if ( compareRelation(player.relation(other), 'Hostile') <= 0 && !player.hasEmbargoAgainst(other) ) { player.addEmbargo(other.id(), false); } else if ( compareRelation(player.relation(other), 'Neutral') >= 0 && player.hasEmbargoAgainst(other) ) { player.stopEmbargo(other.id()); }- Import and use these in each file; remove all
</<=/>/>=against string literals.This ensures comparisons follow your intended game-logic order and uses clean, idiomatic TypeScript with typed unions.
src/client/PublicLobby.ts (1)
63-71: Validate map type with the schema before loading assetsCasting a string to GameMapType can mask invalid values and crash the loader. Prefer schema validation and bail out early on bad input.
- private async loadMapImage(gameID: GameID, gameMap: string) { + private async loadMapImage(gameID: GameID, gameMap: string) { try { - // Convert string to GameMapType value - const mapType = gameMap as GameMapType; - const data = terrainMapFileLoader.getMapData(mapType); + // Validate string -> GameMapType using schema + // (aligns with the repo-wide Zod-backed unions) + const { GameMapTypeSchema } = await import("../core/Schemas"); + const parsed = GameMapTypeSchema.safeParse(gameMap); + if (!parsed.success) return; + const data = terrainMapFileLoader.getMapData(parsed.data as GameMapType); this.mapImages.set(gameID, await data.webpPath()); this.requestUpdate();If you prefer static imports (tree-shaken), I can provide an alternate diff. Do you want me to wire this with a top-level import instead of dynamic?
src/client/graphics/layers/AlertFrame.ts (1)
91-97: Ensure Alert triggers by comparing player IDs, not object referencesUsing strict equality (
betrayed === myPlayer) can silently fail ifplayerBySmallIDreturns a freshPlayerViewinstance rather than the exact same object. Instead, compare their IDs:• File src/client/graphics/layers/AlertFrame.ts, lines 91–97
const betrayed = this.game.playerBySmallID(update.betrayedID); // Only trigger alert if the current player is the betrayed one - if (betrayed === myPlayer) { + if (betrayed?.id() === myPlayer.id()) { this.activateAlert(); }If you’d rather use small numeric IDs throughout for consistency, you can compare
betrayed?.smallID() === myPlayer.smallID()instead.src/core/execution/MIRVExecution.ts (2)
64-69: Bug: wrong coordinate call (mg.x(mg.x(...))) will throw.
mg.x()expects a TileRef, but a number is passed due to the double call. This will cause a runtime error and wrong pathing for MIRV.Fix the coordinate math:
- const x = Math.floor( - (this.mg.x(this.dst) + this.mg.x(this.mg.x(this.nuke.tile()))) / 2, - ); + const x = Math.floor( + (this.mg.x(this.dst) + this.mg.x(this.nuke.tile())) / 2 + );Consider extracting to a small helper for readability.
71-77: Internationalize MIRV inbound warning and introduce a typed event constantIt looks like there’s no existing translation key for the MIRV inbound warning, nor a typed constant for its display event. We need to add both before calling
displayIncomingUnit.• Add
mirv.inboundWarningto youren.json(or other locale files), e.g.:{ // … "mirv": { "inboundWarning": "⚠️⚠️⚠️ {player} – MIRV INBOUND ⚠️⚠️⚠️" } }• Declare a new member in your event-type union/enum—e.g. in
src/core/game/eventTypes.ts(or wherever you keepDISPLAY_EVENT_TYPES):export type DISPLAY_EVENT_TYPES = | /* … */ | "MIRV_INBOUND"; export const DISPLAY_EVENT_TYPES = { /* … */ MIRV_INBOUND: "MIRV_INBOUND" as const, };• Update
src/core/execution/MIRVExecution.tsat lines 71–77:this.mg.displayIncomingUnit( this.nuke.id(), - // TODO TranslateText - `⚠️⚠️⚠️ ${this.player.name()} - MIRV INBOUND ⚠️⚠️⚠️`, + translateText("mirv.inboundWarning", { player: this.player.name() }), - "MIRV_INBOUND", + DISPLAY_EVENT_TYPES.MIRV_INBOUND, this.targetPlayer.id(), );• Adjust imports at the top of
MIRVExecution.ts:-import { translateText } from "…"; // wherever your i18n helper lives +import { translateText } from "src/client/i18n"; -import { DISPLAY_EVENT_TYPES } from "…"; // wherever your event types live +import { DISPLAY_EVENT_TYPES } from "src/core/game/eventTypes";This ensures the warning is fully localizable and the event type is strongly typed.
src/core/execution/PortExecution.ts (1)
79-88: Fix spawn rate parameter mismatchThe call in
shouldSpawnTradeShip()is passing the global trade ship count intotradeShipSpawnRate, but the config signature and default implementation expect the number of ports. This likely inverts the intended spawn curve.• In
src/core/execution/PortExecution.ts(lines 79–88):
- Replace
withconst numTradeShips = this.mg.unitCount("Trade Ship"); const spawnRate = this.mg.config().tradeShipSpawnRate(numTradeShips);const numPorts = this.mg.unitCount("Port"); const spawnRate = this.mg.config().tradeShipSpawnRate(numPorts);• Verify in
src/core/configuration/Config.tsthattradeShipSpawnRate(numberOfPorts: number)and its docs align with using port count, or adjust the signature/docs if you really intend to throttle by ship count.
• Insrc/core/configuration/DefaultConfig.ts, update the parameter name fromnumTradeShipstonumberOfPortsfor clarity.Ensuring the config and calling code agree will restore the correct spawn curve behavior.
src/server/GameManager.ts (2)
71-88: Private games won’t start; multiple timers can be scheduled every tick.phase() returns "LOBBY" for private games until started (see GameServer.phase). Your code only schedules start when phase === "ACTIVE", so private games never start. Also, tick runs every second and can enqueue many setTimeouts before the first fires.
Apply this patch to (1) schedule from "LOBBY" or "ACTIVE", and (2) dedupe timers:
tick() { const active = new Map<GameID, GameServer>(); for (const [id, game] of this.games) { const phase = game.phase(); - if (phase === "ACTIVE") { - if (!game.hasStarted()) { - // Prestart tells clients to start loading the game. - game.prestart(); - // Start game on delay to allow time for clients to connect. - setTimeout(() => { - try { - game.start(); - } catch (error) { - this.log.error(`error starting game ${id}: ${error}`); - } - }, 2000); - } - } + if (!game.hasStarted() && (phase === "LOBBY" || phase === "ACTIVE")) { + // Prestart tells clients to start loading the game. + if (!this._startTimers.has(id)) { + game.prestart(); + // Start game on delay to allow time for clients to connect. + const timer = setTimeout(() => { + try { + if (!game.hasStarted()) { + game.start(); + } + } catch (error) { + this.log.error( + `error starting game ${id}: ${error instanceof Error ? error.stack ?? error.message : String(error)}`, + ); + } finally { + this._startTimers.delete(id); + } + }, 2000); + this._startTimers.set(id, timer); + } + }Add this field to the class (outside selected range):
private _startTimers: Map<GameID, NodeJS.Timeout> = new Map();
90-101: Clear pending start timers when a game finishesWe verified that
GameServer.getPhase()does return"LOBBY"for private (non-public) games before they start, so no change is needed there—private games correctly sit in the lobby phase. To avoid leaking timers or accidentally restarting a game that just ended, please updatesrc/server/GameManager.tsaround the FINISHED check as follows:--- a/src/server/GameManager.ts +++ b/src/server/GameManager.ts @@ lines 90-101 - if (phase === "FINISHED") { + if (phase === "FINISHED") { try { game.end(); } catch (error) { - this.log.error(`error ending game ${id}: ${error}`); + this.log.error( + `error ending game ${id}: ${ + error instanceof Error ? error.stack ?? error.message : String(error) + }`, + ); } + // clear any pending start timer for this game + const timer = this._startTimers.get(id); + if (timer) { + clearTimeout(timer); + this._startTimers.delete(id); + } } else { active.set(id, game); } } this.games = active;This keeps your code idiomatic and leak-free by composing the timer cleanup logic right after ending the game.
src/client/graphics/layers/StructureLayer.ts (1)
100-104: Fix: Object.entries on Partial<Record<...>> can yield undefined values — guard before load.With Partial<Record<UnitType, UnitRenderConfig>>,
Object.entries(this.unitConfigs)hasconfig: UnitRenderConfig | undefined. Current code passesconfigdirectly, which isn’t type-safe and may break under stricter TS settings.Apply this diff to guard and keep type-safety:
private loadIconData() { - Object.entries(this.unitConfigs).forEach(([unitType, config]) => { - this.loadIcon(unitType, config); - }); + Object.entries(this.unitConfigs).forEach(([unitType, config]) => { + if (config) { + this.loadIcon(unitType as UnitType, config); + } + }); }Additionally, to tighten types and avoid stringly-typed keys, update the map and method signature outside this hunk:
// Prefer UnitType over string keys private unitIcons: Map<UnitType, HTMLImageElement> = new Map(); private loadIcon(unitType: UnitType, config: UnitRenderConfig) { /* ... */ }I can generate a follow-up patch if you want this applied across the file.
src/server/Worker.ts (3)
146-164: Bug: start_game route returns without responding when game not found.This leaves the client hanging. Return a 404.
app.post( "/api/start_game/:id", gatekeeper.httpHandler("post", async (req, res) => { log.info(`starting private lobby with id ${req.params.id}`); const game = gm.game(req.params.id); - if (!game) { - return; - } + if (!game) { + return res.status(404).json({ error: "Game not found" }); + }
206-212: Bug: exists endpoint treats undefined as “exists: true”.
gm.game(lobbyId) !== nullis true forundefined. Ifgm.gamecan returnundefined, the endpoint will lie.app.get( "/api/game/:id/exists", gatekeeper.httpHandler("get", async (req, res) => { const lobbyId = req.params.id; res.json({ - exists: gm.game(lobbyId) !== null, + exists: gm.game(lobbyId) != null, // null or undefined => false }); }), );If
gm.gamenever returnsundefined, tighten its return type; otherwise this guard is correct.
215-224: Bug: null-only check can still pass undefined; may crash on game.gameInfo().Use a falsy check to cover both null and undefined.
app.get( "/api/game/:id", gatekeeper.httpHandler("get", async (req, res) => { const game = gm.game(req.params.id); - if (game === null) { + if (!game) { log.info(`lobby ${req.params.id} not found`); return res.status(404).json({ error: "Game not found" }); } res.json(game.gameInfo()); }), );src/core/execution/WarshipExecution.ts (1)
214-216: Return type can be undefined
this.warship?.isActive()yieldsboolean | undefined, but the method is typed asboolean. Coerce to boolean to satisfy the signature.- isActive(): boolean { - return this.warship?.isActive(); - } + isActive(): boolean { + return !!this.warship?.isActive(); + }src/core/GameRunner.ts (1)
125-139: Bug: isExecuting remains true after an exception, blocking all future ticksIf
this.game.executeNextTick()throws, we early-return without resettingthis.isExecutingto false. This will soft-deadlock the runner.Apply this diff to reset the flag on error:
try { updates = this.game.executeNextTick(); } catch (error: unknown) { if (error instanceof Error) { console.error("Game tick error:", error.message); this.callBack({ errMsg: error.message, stack: error.stack, } as ErrorUpdate); } else { console.error("Game tick error:", error); } - return; + this.isExecuting = false; + return; }src/core/configuration/ConfigLoader.ts (1)
16-26: Make getConfig work in both browser and server (SSR) environments
getConfigalways callsgetServerConfigFromClient()(does a fetch). On the server this can fail or slow SSR. We already havegetServerConfigFromServer()— use it conditionally.Apply this diff:
- const sc = await getServerConfigFromClient(); + const sc = + typeof window === "undefined" + ? getServerConfigFromServer() + : await getServerConfigFromClient();Also, the default-case error message references
process.env.GAME_ENV(may be undefined on the client). Prefer the actualsc.env()value.- throw Error(`unsupported server configuration: ${process.env.GAME_ENV}`); + throw Error(`unsupported server configuration: ${sc.env?.() ?? "unknown"}`);src/core/game/UnitImpl.ts (1)
105-111: Duplicate getters/setters for target tile; keep one API and alias the old namesWe currently have both
setTileTarget/tileTargetandsetTargetTile/targetTile. Duplication invites mistakes. Keep the new names and make the old names thin aliases (marked deprecated) to preserve compatibility.Apply this diff to alias old names to the new ones:
- setTileTarget(tile: TileRef | undefined): void { - this._targetTile = tile; - } - tileTarget(): TileRef | undefined { - return this._targetTile; - } + /** @deprecated use setTargetTile */ + setTileTarget(tile: TileRef | undefined): void { + this.setTargetTile(tile); + } + /** @deprecated use targetTile */ + tileTarget(): TileRef | undefined { + return this.targetTile(); + }No change needed for the new
setTargetTile/targetTilemethods.Also applies to: 326-329
src/core/game/GameUpdates.ts (3)
224-231: Unify DisplayMessageUpdate with Game.displayMessage signature.Game.displayMessage(playerID: PlayerID | null, goldAmount?: bigint). Here you use number for playerID and raw bigint for gold. Prefer the PlayerID alias and (optionally) Gold alias for consistency.
export interface DisplayMessageUpdate { type: "DisplayEvent"; message: string; messageType: MessageType; - goldAmount?: bigint; - playerID: number | null; + goldAmount?: Gold; + playerID: PlayerID | null; params?: Record<string, string | number>; }
255-260: Use PlayerID for UnitIncomingUpdate.playerID to match Game.displayIncomingUnit.Game.displayIncomingUnit takes playerID: PlayerID | null. Using number here will drift.
export interface UnitIncomingUpdate { type: "UnitIncoming"; unitID: number; message: string; messageType: MessageType; - playerID: number; + playerID: PlayerID; }
114-136: Clarify numeric owner IDs in UnitUpdate and remove stale TODOTo avoid mixing the internal numeric “smallID” domain with the external
PlayerID(string) domain—and to clean up the outdated comment—rename the numeric fields inUnitUpdateand delete the “tileref” TODO. Then update all call sites accordingly.• In src/core/game/GameUpdates.ts
- Change
ownerID: number;→ownerSmallID: number;- Change
lastOwnerID?: number;→lastOwnerSmallID?: number;- Remove the
// TODO: make these tilerefscomment• Update all references (e.g. in
TerritoryLayer.ts,TransportShipUtils.ts,UnitImpl.ts, etc.) to use the new property names (ownerSmallID/lastOwnerSmallID).Diff snippet:
export interface UnitUpdate { type: "Unit"; unitType: UnitType; troops: number; id: number; - ownerID: number; - lastOwnerID?: number; - // TODO: make these tilerefs + ownerSmallID: number; + lastOwnerSmallID?: number; pos: TileRef; lastPos: TileRef; isActive: boolean; reachedTarget: boolean; retreating: boolean; targetable: boolean; targetUnitId?: number; // Only for trade ships targetTile?: TileRef; // Only for nukes health?: number; constructionType?: UnitType; missileTimerQueue: number[]; level: number; hasTrainStation: boolean; trainType?: TrainType; // Only for trains loaded?: boolean; // Only for trains }Mandatory updates are needed in each call site where
update.ownerIDorupdate.lastOwnerIDappears. Make sure you also adjustUnitImpl(where smallID() is used),TerritoryLayer.ts:92-93,TransportShipUtils.ts:110, and any other occurrences identified by your ripgrep search.src/core/game/Game.ts (1)
675-681: Align GameUpdates payload interfaces with Game.display method signatures*The update interfaces in
src/core/game/GameUpdates.tsaren’t matching thedisplayMessage,displayChatanddisplayIncomingUnitsignatures inGame.ts. They’re missing required fields (and one interface is missing entirely), which forces downstream casts.Please update the following:
DisplayMessageUpdate (around line 223):
Add thetype: MessageType,playerID: PlayerID | null,goldAmount?: bigintandparams?: Record<string, string | number>properties to match
Game.displayMessage(message: string, type: MessageType, playerID: PlayerID | null, goldAmount?: bigint, params?: Record<string, string|number>).DisplayChatMessageUpdate:
Define this interface (it’s currently missing) so it mirrors
Game.displayChat(message: string, category: string, playerID: PlayerID | null, goldAmount?: bigint, params?: Record<string, string|number>).UnitIncomingUpdate (around line 254):
Extend it withmessage: string,type: MessageType,playerID: PlayerID | null,goldAmount?: bigintandparams?: Record<string, string|number>to match
Game.displayIncomingUnit(unitID: number, message: string, type: MessageType, playerID: PlayerID | null, goldAmount?: bigint, params?: Record<string|number>).Suggested diff:
export interface DisplayMessageUpdate { - type: "DisplayEvent"; - message: string; + type: "DisplayEvent"; + message: string; + eventType: MessageType; + playerID: PlayerID | null; + goldAmount?: bigint; + params?: Record<string, string | number>; } +export interface DisplayChatMessageUpdate { + type: "DisplayChat"; + message: string; + category: string; + playerID: PlayerID | null; + goldAmount?: bigint; + params?: Record<string, string | number>; +} export interface UnitIncomingUpdate { - type: "UnitIncoming"; - unitID: number; + type: "UnitIncoming"; + unitID: number; + message: string; + eventType: MessageType; + playerID: PlayerID | null; + goldAmount?: bigint; + params?: Record<string, string | number>; }src/client/graphics/layers/RadialMenuElements.ts (2)
349-353: Don’t compare PlayerView objects by reference; compare by id
params.selected === params.myPlayeris brittle. These are often different object instances representing the same player. Use ids to decide ownership, like you do elsewhere.Apply this diff:
- const unitTypes: Set<UnitType> = getAllEnabledUnits( - params.selected === params.myPlayer, - params.game.config(), - ); + const isMe = + params.selected !== null && + params.selected.id() === params.myPlayer.id(); + const unitTypes: Set<UnitType> = getAllEnabledUnits( + isMe, + params.game.config(), + );Bonus: you can remove
getAllEnabledUnitsentirely and rely on
params.buildMenu.canBuildOrUpgrade(item)plusATTACK_UNIT_TYPESto decide attack vs build — single source of truth, less duplication. I can draft that refactor if you want.
574-579: Ensure a stable “ally” slot in the radial menu
The radial layout relies on element id values matching theSlotSchemaentries ("info","boat","build","attack","ally","back","delete"). Right now you push eitherallyRequestElement(id: "ally_request") orallyBreakElement(id: "ally_break") directly, so neither matches"ally"and the menu can shift or drop that slot.Recommended fix (mandatory refactor):
Define a wrapper
MenuElementwithid: "ally"and asubMenuthat returns the correct child element:const allyMenuElement: MenuElement = { id: "ally", name: "ally", icon: allianceIcon, color: COLORS.alliance, disabled: () => false, subMenu: (params) => [ params.selected?.isAlliedWith(params.myPlayer) ? allyBreakElement : allyRequestElement, ], };In
rootMenuElement.subMenu, replace the inlinelet ally = …logic and its push with:const menuItems: (MenuElement | null)[] = [ infoMenuElement, boatMenuElement, - ally, // remove this + allyMenuElement, // use wrapper instead ];This ensures the top-level slot id always stays
"ally", preserving radial positions and any CSS hooks.
| const key = GameMapTypeSchema.options.find( | ||
| (option) => (option as GameMapType) === map, | ||
| ); | ||
| const fileName = key?.toLowerCase(); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Stop using schema.options.find + toLowerCase for file paths; add a clear, typed map-dir resolver.
.options.find(...)is redundant:mapis already a GameMapType (string union) and will always be present.toLowerCase()alone is risky. Many map folder names won’t match a raw lowercase string with spaces (e.g., "Giant World Map", "Gateway to the Atlantic"). This can 404 at runtime.- Make the directory name deterministic and explicit. Either:
- Provide a Record<GameMapType, string> mapping, or
- Use a single slugify/toMapDir helper that mirrors your actual resources folder naming scheme.
Apply this minimal diff and add a helper (see the extra code block below):
- const key = GameMapTypeSchema.options.find(
- (option) => (option as GameMapType) === map,
- );
- const fileName = key?.toLowerCase();
+ const fileName = toMapDir(map);If you adopt this change, you can also drop the unused GameMapTypeSchema import on Line 1:
-import { GameMapType, GameMapTypeSchema } from "./Game";
+import { GameMapType } from "./Game";Add this helper near the top of the file (or in a shared util), tuned to your actual folder naming:
// Keep this in one place to avoid silent path mismatches across loaders.
function toMapDir(map: GameMapType): string {
// Example slug: "Giant World Map" -> "giant-world-map"
// Adjust to match resources/maps/* directory names.
return map
.toLowerCase()
.normalize("NFKD") // strip diacritics
.replace(/[\u0300-\u036f]/g, "")
.replace(/[^a-z0-9]+/g, "-")
.replace(/^-+|-+$/g, "");
}Run this script to verify the computed directories align with actual folders:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) List actual map directories
echo "Actual directories under resources/maps:"
fd -t d . resources/maps -d 1 | sed 's#resources/maps/##' | sort > /tmp/actual_dirs.txt
cat /tmp/actual_dirs.txt
# 2) Extract GameMapType options from Game.ts
rg -nPU '(?s)GameMapTypeSchema\s*=\s*z\.enum\(\[\s*(.*?)\s*\]\);' src/core/game/Game.ts -r '$1' \
| tr ',' '\n' \
| sed -E 's/["'\'' ]//g' \
| sed '/^$/d' > /tmp/map_types_raw.txt
# Re-extract with quotes preserved for node processing
rg -nPU '(?s)GameMapTypeSchema\s*=\s*z\.enum\(\[\s*(.*?)\s*\]\);' src/core/game/Game.ts -r '$1' > /tmp/map_types_quoted.txt
# 3) Compute slugs via node (same as toMapDir)
node - <<'NODE'
const fs = require('fs');
const raw = fs.readFileSync('/tmp/map_types_quoted.txt', 'utf8');
const names = raw.match(/"([^"]+)"/g)?.map(s => s.slice(1, -1)) ?? [];
const slug = s => s.toLowerCase()
.normalize('NFKD').replace(/[\u0300-\u036f]/g, '')
.replace(/[^a-z0-9]+/g, '-').replace(/^-+|-+$/g, '');
const computed = names.map(n => ({ name: n, slug: slug(n), lower: n.toLowerCase() }));
fs.writeFileSync('/tmp/computed_slugs.txt', computed.map(o => o.slug).join('\n'));
fs.writeFileSync('/tmp/computed_lower.txt', computed.map(o => o.lower).join('\n'));
console.log('Sample mapping (first 10):');
console.log(computed.slice(0,10));
NODE
# 4) Check matches (slug style)
echo
echo "Unmatched (slug style):"
comm -23 <(sort /tmp/computed_slugs.txt) <(sort /tmp/actual_dirs.txt) || true
# 5) Check matches (lowercase with spaces)
echo
echo "Unmatched (lowercase-as-is style):"
comm -23 <(sort /tmp/computed_lower.txt) <(sort /tmp/actual_dirs.txt) || true
echo
echo "If either list above is non-empty, adjust toMapDir() or provide an explicit Record<GameMapType, string>."Length of output: 2194
Align the map-dir resolver with actual folder names
The script shows that using a slug helper (with hyphens) or raw lowercase strings (with spaces) does not match the real directories under resources/maps (e.g. gatewaytotheatlantic vs. gateway-to-the-atlantic or "gateway to the atlantic")—you’ll get 404s at runtime. Instead, switch to a single, explicit mapping or a helper that lowercases and strips all non-alphanumeric characters so it matches the existing folder names exactly.
• In src/core/game/BinaryLoaderGameMapLoader.ts, replace lines 34–37:
- const key = GameMapTypeSchema.options.find(
- (option) => (option as GameMapType) === map,
- );
- const fileName = key?.toLowerCase();
+ const fileName = toMapDir(map);• Drop the unused import on line 1:
-import { GameMapType, GameMapTypeSchema } from "./Game";
+import { GameMapType } from "./Game";• Add this helper (tuned to the existing resources/maps/* names) either at the top of this file or in a shared util:
// Always match the actual folder names under resources/maps
function toMapDir(map: GameMapType): string {
return map
.toLowerCase()
.normalize("NFKD") // strip accents
.replace(/[\u0300-\u036f]/g, "")
.replace(/[^a-z0-9]/g, ""); // drop everything not a–z or 0–9
}• Alternatively, if you prefer an explicit lookup (safer if names diverge further), use:
const mapDirMap: Record<GameMapType, string> = {
World: "world",
"Giant World Map": "giantworldmap",
"Gateway to the Atlantic": "gatewaytotheatlantic",
// …and so on for each GameMapType
};
function toMapDir(map: GameMapType): string {
return mapDirMap[map];
}This guarantees no silent mismatches and keeps your loader deterministic.
🤖 Prompt for AI Agents
In src/core/game/BinaryLoaderGameMapLoader.ts around lines 34 to 37: replace the
current logic that picks a key and lowercases it with a deterministic map-dir
resolver that matches actual folders under resources/maps; remove the now-unused
import on line 1; add a helper function toMapDir(map: GameMapType) that returns
the directory name by either (a) lowercasing, normalizing and stripping all
non-alphanumeric characters (use .toLowerCase(), Unicode NFKD normalization,
remove combining marks, then remove anything not a–z0–9) or (b) use an explicit
Record<GameMapType,string> lookup mapping each GameMapType to its exact folder
name, and then use toMapDir(map) wherever fileName was used.
| ["Transport Ship"]: { | ||
| troops?: number; | ||
| destination?: TileRef; | ||
| }; | ||
|
|
||
| [UnitType.Warship]: { | ||
| ["Warship"]: { | ||
| patrolTile: TileRef; | ||
| }; | ||
|
|
||
| [UnitType.Shell]: Record<string, never>; | ||
| ["Shell"]: Record<string, never>; | ||
|
|
||
| [UnitType.SAMMissile]: Record<string, never>; | ||
| ["SAM Missile"]: Record<string, never>; | ||
|
|
||
| [UnitType.Port]: Record<string, never>; | ||
| ["Port"]: Record<string, never>; | ||
|
|
||
| [UnitType.AtomBomb]: { | ||
| ["Atom Bomb"]: { | ||
| targetTile?: number; | ||
| trajectory: TrajectoryTile[]; | ||
| }; | ||
|
|
||
| [UnitType.HydrogenBomb]: { | ||
| ["Hydrogen Bomb"]: { | ||
| targetTile?: number; | ||
| trajectory: TrajectoryTile[]; | ||
| }; | ||
|
|
||
| [UnitType.TradeShip]: { | ||
| ["Trade Ship"]: { | ||
| targetUnit: Unit; | ||
| lastSetSafeFromPirates?: number; | ||
| }; | ||
|
|
||
| [UnitType.Train]: { | ||
| ["Train"]: { | ||
| trainType: TrainType; | ||
| targetUnit?: Unit; | ||
| loaded?: boolean; | ||
| }; | ||
|
|
||
| [UnitType.Factory]: Record<string, never>; | ||
| ["Factory"]: Record<string, never>; | ||
|
|
||
| [UnitType.MissileSilo]: Record<string, never>; | ||
| ["Missile Silo"]: Record<string, never>; | ||
|
|
||
| [UnitType.DefensePost]: Record<string, never>; | ||
| ["Defense Post"]: Record<string, never>; | ||
|
|
||
| [UnitType.SAMLauncher]: Record<string, never>; | ||
| ["SAM Launcher"]: Record<string, never>; | ||
|
|
||
| [UnitType.City]: Record<string, never>; | ||
| ["City"]: Record<string, never>; | ||
|
|
||
| [UnitType.MIRV]: Record<string, never>; | ||
| ["MIRV"]: Record<string, never>; | ||
|
|
||
| [UnitType.MIRVWarhead]: { | ||
| ["MIRV Warhead"]: { | ||
| targetTile?: number; | ||
| }; | ||
|
|
||
| [UnitType.Construction]: Record<string, never>; | ||
| ["Construction"]: Record<string, never>; | ||
| } |
There was a problem hiding this comment.
Use TileRef instead of number for targetTile in nuke-related params.
These should be TileRef to match the rest of the file and avoid subtle mismatches.
["Atom Bomb"]: {
- targetTile?: number;
+ targetTile?: TileRef;
trajectory: TrajectoryTile[];
};
["Hydrogen Bomb"]: {
- targetTile?: number;
+ targetTile?: TileRef;
trajectory: TrajectoryTile[];
};
["MIRV Warhead"]: {
- targetTile?: number;
+ targetTile?: TileRef;
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ["Transport Ship"]: { | |
| troops?: number; | |
| destination?: TileRef; | |
| }; | |
| [UnitType.Warship]: { | |
| ["Warship"]: { | |
| patrolTile: TileRef; | |
| }; | |
| [UnitType.Shell]: Record<string, never>; | |
| ["Shell"]: Record<string, never>; | |
| [UnitType.SAMMissile]: Record<string, never>; | |
| ["SAM Missile"]: Record<string, never>; | |
| [UnitType.Port]: Record<string, never>; | |
| ["Port"]: Record<string, never>; | |
| [UnitType.AtomBomb]: { | |
| ["Atom Bomb"]: { | |
| targetTile?: number; | |
| trajectory: TrajectoryTile[]; | |
| }; | |
| [UnitType.HydrogenBomb]: { | |
| ["Hydrogen Bomb"]: { | |
| targetTile?: number; | |
| trajectory: TrajectoryTile[]; | |
| }; | |
| [UnitType.TradeShip]: { | |
| ["Trade Ship"]: { | |
| targetUnit: Unit; | |
| lastSetSafeFromPirates?: number; | |
| }; | |
| [UnitType.Train]: { | |
| ["Train"]: { | |
| trainType: TrainType; | |
| targetUnit?: Unit; | |
| loaded?: boolean; | |
| }; | |
| [UnitType.Factory]: Record<string, never>; | |
| ["Factory"]: Record<string, never>; | |
| [UnitType.MissileSilo]: Record<string, never>; | |
| ["Missile Silo"]: Record<string, never>; | |
| [UnitType.DefensePost]: Record<string, never>; | |
| ["Defense Post"]: Record<string, never>; | |
| [UnitType.SAMLauncher]: Record<string, never>; | |
| ["SAM Launcher"]: Record<string, never>; | |
| [UnitType.City]: Record<string, never>; | |
| ["City"]: Record<string, never>; | |
| [UnitType.MIRV]: Record<string, never>; | |
| ["MIRV"]: Record<string, never>; | |
| [UnitType.MIRVWarhead]: { | |
| ["MIRV Warhead"]: { | |
| targetTile?: number; | |
| }; | |
| [UnitType.Construction]: Record<string, never>; | |
| ["Construction"]: Record<string, never>; | |
| } | |
| ["Atom Bomb"]: { | |
| targetTile?: TileRef; | |
| trajectory: TrajectoryTile[]; | |
| }; | |
| ["Hydrogen Bomb"]: { | |
| targetTile?: TileRef; | |
| trajectory: TrajectoryTile[]; | |
| }; | |
| ["MIRV Warhead"]: { | |
| targetTile?: TileRef; | |
| }; |
🤖 Prompt for AI Agents
In src/core/game/Game.ts around lines 195 to 248, the nuke-related type fields
use targetTile?: number; change them to targetTile?: TileRef; for the ["Atom
Bomb"], ["Hydrogen Bomb"] and ["MIRV Warhead"] entries to match the rest of the
file; update any imports/exports if necessary and adjust any code that assigns
or reads these fields to use TileRef values instead of numbers.
| export interface BonusEventUpdate { | ||
| type: GameUpdateType.BonusEvent; | ||
| type: "BonusEvent"; | ||
| player: PlayerID; | ||
| tile: TileRef; | ||
| gold: number; | ||
| troops: number; | ||
| } | ||
|
|
There was a problem hiding this comment.
Use Gold (bigint) for BonusEvent.gold to match the rest of the economy types.
You already import Gold. Using number here will cause mixing bigint and number downstream.
export interface BonusEventUpdate {
type: "BonusEvent";
player: PlayerID;
tile: TileRef;
- gold: number;
+ gold: Gold;
troops: number;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export interface BonusEventUpdate { | |
| type: GameUpdateType.BonusEvent; | |
| type: "BonusEvent"; | |
| player: PlayerID; | |
| tile: TileRef; | |
| gold: number; | |
| troops: number; | |
| } | |
| export interface BonusEventUpdate { | |
| type: "BonusEvent"; | |
| player: PlayerID; | |
| tile: TileRef; | |
| gold: Gold; | |
| troops: number; | |
| } |
🤖 Prompt for AI Agents
In src/core/game/GameUpdates.ts around lines 72 to 79, the BonusEventUpdate.gold
field is typed as number but should use the Gold type (bigint) to match the rest
of the economy types; change the gold property type from number to Gold and
ensure the Gold symbol is imported at the top of the file, then update any code
that constructs or manipulates BonusEventUpdate objects to use bigint values
(BigInt literals or conversions) and bigint-safe arithmetic or explicit
conversions where numeric values are required.
| type: "TargetPlayer"; | ||
| playerID: number; | ||
| targetID: number; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Align TargetPlayerUpdate IDs with PlayerID type or rename as small IDs.
Both fields are numbers. If these are PlayerIDs, switch to PlayerID. If they’re small IDs, rename to playerSmallID/targetSmallID to avoid confusion.
-export interface TargetPlayerUpdate {
- type: "TargetPlayer";
- playerID: number;
- targetID: number;
-}
+export interface TargetPlayerUpdate {
+ type: "TargetPlayer";
+ playerSmallID: number;
+ targetSmallID: number;
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| type: "TargetPlayer"; | |
| playerID: number; | |
| targetID: number; | |
| } | |
| export interface TargetPlayerUpdate { | |
| type: "TargetPlayer"; | |
| playerSmallID: number; | |
| targetSmallID: number; | |
| } |
🤖 Prompt for AI Agents
In src/core/game/GameUpdates.ts around lines 213 to 216, the TargetPlayer update
type defines playerID and targetID as plain numbers which is ambiguous; if these
fields represent the existing PlayerID type change their types from number to
PlayerID, otherwise rename them to playerSmallID and targetSmallID (and update
all usages) so their intent is clear. Update the type declaration accordingly
and search/replace all references to these fields across the codebase to use the
new type or new names, adjusting any serialization/validation logic to match.
| gameType: "Public", | ||
| difficulty: "Medium", | ||
| infiniteGold: false, | ||
| infiniteTroops: false, | ||
| instantBuild: false, | ||
| disableNPCs: mode === GameMode.Team, | ||
| disableNPCs: mode === "Team", | ||
| gameMode: mode, | ||
| playerTeams, | ||
| bots: 400, | ||
| disabledUnits: [UnitType.Train, UnitType.Factory], | ||
| disabledUnits: ["Train", "Factory"], | ||
| } satisfies GameConfig; |
There was a problem hiding this comment.
Type-narrow disabledUnits to UnitType[] to satisfy GameConfig.
As written, ["Train", "Factory"] is inferred as string[] and may fail the satisfies GameConfig check if GameConfig.disabledUnits expects UnitType[].
Minimal fix without extra imports:
- disabledUnits: ["Train", "Factory"],
+ disabledUnits: ["Train", "Factory"] as GameConfig["disabledUnits"],Alternative (if you prefer explicit typing):
import type { UnitType } from "../core/game/Game";
// ...
disabledUnits: ["Train", "Factory"] as const satisfies ReadonlyArray<UnitType>,🤖 Prompt for AI Agents
In src/server/MapPlaylist.ts around lines 79 to 89, the disabledUnits array is
inferred as string[] which can fail the satisfies GameConfig check; narrow its
type to UnitType[] by making the literal a typed readonly array or casting it:
either import UnitType and use the array literal with a readonly satisfies
annotation (e.g., as const satisfies ReadonlyArray<UnitType>) or, if you want
the minimal change, cast the literal to UnitType[] (e.g., ... as UnitType[]).
Ensure the resulting disabledUnits value matches GameConfig.disabledUnits type.
github-project-automation
bot
moved this from Triage
to Development
in OpenFront Release Management
| export const DifficultyDescription: Record<Difficulty, string> = { | ||
| Easy: "Relaxed", | ||
| Medium: "Balanced", | ||
| Hard: "Intense", | ||
| Impossible: "Impossible", | ||
| } as const; |
There was a problem hiding this comment.
I agree with the related coderabbit review, the strings "Relaxed", "Balanced" and "Intense" should only exist in en.json, so they can be localized.
| import { terrainMapFileLoader } from "../TerrainMapFileLoader"; | ||
| import { translateText } from "../Utils"; | ||
|
|
||
| // Add map descriptions | ||
| export const MapDescription: Record<keyof typeof GameMapType, string> = { | ||
| export const MapDescription: Record<GameMapType, string> = { |
There was a problem hiding this comment.
Same comment; these strings should be moved to en.json so they can be localized.
| ${DifficultySchema.options.map( | ||
| (value) => html` | ||
| <div | ||
| class="option-card ${this.selectedDifficulty === value | ||
| ? "selected" | ||
| : ""}" | ||
| @click=${() => this.handleDifficultySelection(value)} | ||
| > | ||
| <difficulty-display | ||
| .difficultyKey=${value} | ||
| ></difficulty-display> | ||
| <p class="option-card-title"> | ||
| ${translateText( | ||
| `difficulty.${DifficultyDescription[value]}`, | ||
| )} | ||
| </p> | ||
| </div> | ||
| `, | ||
| )} |
There was a problem hiding this comment.
I think we should take this opportunity to correct the oversight of having localized text in the keys for en.json.
"difficulty": {
"difficulty": "Difficulty",
"easy": "Relaxed",
"medium": "Balanced",
"hard": "Intense",
"impossible": "Impossible"
},
|
Please resolve conflicts with main |
Description:
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
assessin.