Improve host lobby modal #2318
Improve host lobby modal #2318
Conversation
WalkthroughRefactors host and single-player modals into component-driven light‑DOM UI, adds preset persistence/manager, many shared Lit components (pickers, sliders, map browser, advanced options), safer map loading, invite ID visibility/copy flows, and expands English translations. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Modal as Host/Single Modal
participant Presets as PresetsManager
participant Storage as localStorage
participant Server as API
Note over User,Presets: Save a preset (client-only flow)
User->>Modal: click "Save preset" (name)
Modal->>Presets: call getSettings()
Presets->>Storage: persist preset JSON
Presets-->>Modal: emit apply-preset (settings)
Modal->>Modal: applySettings(settings)
alt lobbyId present
Modal->>Server: PUT config via config.workerPath(lobbyId)
Server-->>Modal: ack
end
sequenceDiagram
participant User
participant InviteUI as Invite Bar
participant Modal
participant Clipboard
Note over User,InviteUI: Reveal and copy lobby invite
User->>InviteUI: toggle "show id"
InviteUI->>Modal: toggleInviteVisibility(true)
Modal->>InviteUI: render full id
User->>InviteUI: click copy
InviteUI->>Clipboard: write invite URL
Clipboard-->>User: copied
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (12)
src/client/components/shared/DifficultyControls.ts (1)
10-12: Consider simplifying the keyOf lookup.The
Object.keys(Difficulty).find()pattern works but relies on TypeScript enum behavior. For clarity and type safety, consider usingObject.entries:- private keyOf(value: Difficulty) { - return Object.keys(Difficulty).find((k) => Difficulty[k] === value) ?? ""; - } + private keyOf(value: Difficulty) { + const entry = Object.entries(Difficulty).find(([_, v]) => v === value); + return entry?.[0] ?? ""; + }This makes it explicit that you're matching on values and extracting keys.
src/client/components/shared/GameModePicker.ts (1)
43-45: Consider explicit translation mapping for all game modes.The current logic assumes only FFA and one other mode:
${mode === GameMode.FFA ? translateText("game_mode.ffa") : translateText("game_mode.teams")}If additional game modes are added in the future, this will incorrectly label them as "teams". Consider using an explicit mapping:
${translateText(`game_mode.${mode === GameMode.FFA ? "ffa" : "teams"}`)}Or better, a lookup object if more modes are expected.
src/client/components/shared/SettingsSummary.ts (1)
15-17: Harden enum-to-key lookup for numeric enumsUsing Object.keys() on TS enums includes reverse mappings. Safer/simpler: use the reverse map directly for numeric enums.
Apply:
- private keyOf(enumObj, value) { - return Object.keys(enumObj).find((k) => enumObj[k] === value) ?? ""; - } + private enumName<E extends Record<string, string | number>>(e: E, v: number | string): string { + return typeof v === "number" ? (e as any)[v] ?? "" : String(v); + }And update usages:
- const mapKey = this.keyOf(GameMapType, this.selectedMap); - const diffKey = this.keyOf(Difficulty, this.selectedDifficulty); + const mapKey = this.enumName(GameMapType, this.selectedMap); + const diffKey = this.enumName(Difficulty, this.selectedDifficulty);src/client/components/Maps.ts (1)
71-91: Optional: cancel in-flight loads on disconnect or new keyToken guard prevents stale state, but network work still continues. Consider AbortController to actually cancel requests.
Sketch:
+ private _abort?: AbortController; private async loadMapData() { if (!this.mapKey) return; const myToken = ++this._loadToken; + this._abort?.abort(); + this._abort = new AbortController(); this.isLoading = true; this.mapWebpPath = null; try { // ... - const [webpPath, manifest] = await Promise.all([data.webpPath(), data.manifest()]); + const [webpPath, manifest] = await Promise.all([ + data.webpPath({ signal: this._abort.signal }), + data.manifest({ signal: this._abort.signal }), + ]); if (myToken !== this._loadToken) return; this.mapWebpPath = webpPath; this.mapName = manifest.name; } catch (error) { console.error("Failed to load map data:", error); } finally { this.isLoading = false; } }(Adjust loader signatures if needed.)
src/client/components/shared/MapBrowserPane.ts (2)
72-79: Localize and tighten list semanticsLocalize the aria-label and declare a listbox container to pair with option roles.
- <aside - aria-label="Map Browser" + <aside + aria-label=${translateText("map.browser")} class="min-h-80 flex flex-col overflow-hidden rounded-xl border border-white/15 bg-zinc-900/40" >And around the maps grid:
- <div class="grid flex-1 grid-cols-1 gap-4 overflow-auto p-3"> + <div class="grid flex-1 grid-cols-1 gap-4 overflow-auto p-3" role="listbox" aria-multiselectable="false">Ensure map items are role="option" as in the previous comment.
133-147: Use lit repeat() for stable keyed renderingImproves performance and DOM stability during search/filter.
+ import { repeat } from "lit/directives/repeat.js"; ... - ${maps.length - ? maps.map(({ value, key, name }) => { + ${maps.length + ? repeat( + maps, + ({ value }) => value, + ({ value, key, name }) => { const selected = !this.useRandomMap && this.selectedMap === value; - return html` <div + return html` <div @click=${() => this.selectMap(value)} class="w-full h-full cursor-pointer" > <map-display .mapKey=${key} .selected=${selected} .translation=${name} ></map-display> </div>`; - }) + }, + ) : html`<div class="col-span-full text-sm text-zinc-400">src/client/components/shared/AdvancedOptions.ts (1)
87-97: Optional: unify rule rendering to reduce duplicationYou already have renderRulesOptions; consider extending its config to include donate* rules when provided, instead of a separate renderRule method here.
Also applies to: 98-111
src/client/components/shared/BotsSlider.ts (1)
27-33: Type willUpdate with PropertyValues for better TS checksAligns with Lit’s typing and catches mistakes.
-import { customElement, property, state } from "lit/decorators.js"; +import { customElement, property, state } from "lit/decorators.js"; +import type { PropertyValues } from "lit"; ... - willUpdate(changed: Map<string, unknown>) { + willUpdate(changed: PropertyValues<this>) {src/client/components/shared/PresetsBar.ts (1)
33-35: Localize aria-label/title strings for accessibilityHardcoded English labels (“Select preset”, “Preset name”) bypass i18n. Use translateText to keep accessibility strings localized.
- aria-label="Select preset" - title="Select preset" + aria-label=${translateText("single_modal.select_placeholder")} + title=${translateText("single_modal.select_placeholder")} ... - aria-label="Preset name" + aria-label=${translateText("common.name")}Also applies to: 55-60, 62-71, 74-81, 85-92
src/client/HostLobbyModal.ts (1)
104-105: Prevent overlapping polls; add error handlingA 1s interval can stack fetches if a request lags. Gate with an in‑flight flag and catch errors.
- private playersInterval: ReturnType<typeof setInterval> | null = null; + private playersInterval: ReturnType<typeof setInterval> | null = null; + private isPolling = false; @@ private async pollPlayers() { - if (!this.lobbyId) return; - const config = await getServerConfigFromClient(); - fetch(`/${config.workerPath(this.lobbyId)}/api/game/${this.lobbyId}`, { - method: "GET", - headers: { "Content-Type": "application/json" }, - }) - .then((response) => response.json()) - .then((data: GameInfo) => { - this.clients = data.clients ?? []; - }); + if (!this.lobbyId || this.isPolling) return; + this.isPolling = true; + try { + const config = await getServerConfigFromClient(); + const res = await fetch( + `/${config.workerPath(this.lobbyId)}/api/game/${this.lobbyId}`, + { method: "GET", headers: { "Content-Type": "application/json" } }, + ); + if (res.ok) { + const data: GameInfo = await res.json(); + this.clients = data.clients ?? []; + } + } catch (err) { + console.warn("pollPlayers failed:", err); + } finally { + this.isPolling = false; + } }Also applies to: 666-679
src/client/SinglePlayerModal.ts (2)
59-71: Avoid attribute reflection on internal stateful propsThese are controlled programmatically; reflecting to attributes can coerce types unexpectedly. Mark them non-attribute.
- @property({ type: Number }) selectedMap: GameMapType = GameMapType.World; - @property({ type: Number }) selectedDifficulty: Difficulty = + @property({ type: Number, attribute: false }) selectedMap: GameMapType = GameMapType.World; + @property({ type: Number, attribute: false }) selectedDifficulty: Difficulty = Difficulty.Medium; - @property({ type: Boolean }) disableNPCs = false; - @property({ type: Number }) bots = 400; - @property({ type: Boolean }) infiniteGold = false; - @property({ type: Boolean }) infiniteTroops = false; - @property({ type: Boolean }) compactMap = false; - @property({ type: Boolean }) instantBuild = false; - @property({ type: Boolean }) useRandomMap = false; - @property({ type: Number }) gameMode: GameMode = GameMode.FFA; - @property({ type: Number }) teamCount: TeamCountConfig = 2; + @property({ type: Boolean, attribute: false }) disableNPCs = false; + @property({ type: Number, attribute: false }) bots = 400; + @property({ type: Boolean, attribute: false }) infiniteGold = false; + @property({ type: Boolean, attribute: false }) infiniteTroops = false; + @property({ type: Boolean, attribute: false }) compactMap = false; + @property({ type: Boolean, attribute: false }) instantBuild = false; + @property({ type: Boolean, attribute: false }) useRandomMap = false; + @property({ type: Number, attribute: false }) gameMode: GameMode = GameMode.FFA; + @property({ type: Number, attribute: false }) teamCount: TeamCountConfig = 2;
139-146: Localize the quick start button titleUse translateText for the title tooltip to match the button label.
- title="Quick Start (Enter)" + title=${translateText("single_modal.quick_start")}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
resources/lang/en.json(6 hunks)src/client/HostLobbyModal.ts(11 hunks)src/client/SinglePlayerModal.ts(2 hunks)src/client/Transport.ts(1 hunks)src/client/components/Maps.ts(3 hunks)src/client/components/shared/AdvancedOptions.ts(1 hunks)src/client/components/shared/BotsSlider.ts(1 hunks)src/client/components/shared/DifficultyControls.ts(1 hunks)src/client/components/shared/DifficultyPicker.ts(1 hunks)src/client/components/shared/ExpandButton.ts(1 hunks)src/client/components/shared/GameModeControls.ts(1 hunks)src/client/components/shared/GameModePicker.ts(1 hunks)src/client/components/shared/MapBrowserPane.ts(1 hunks)src/client/components/shared/PresetsBar.ts(1 hunks)src/client/components/shared/PresetsManager.ts(1 hunks)src/client/components/shared/SettingsSummary.ts(1 hunks)src/client/components/shared/TeamCountPicker.ts(1 hunks)src/client/utilities/RenderRulesOptions.ts(1 hunks)src/client/utilities/RenderUnitTypeOptions.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-09T02:20:43.637Z
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1110
File: src/client/Main.ts:293-295
Timestamp: 2025-06-09T02:20:43.637Z
Learning: In src/client/Main.ts, during game start in the handleJoinLobby callback, UI elements are hidden using direct DOM manipulation with classList.add("hidden") for consistency. This includes modals, buttons, and error divs. The codebase follows this pattern rather than using component APIs for hiding elements during game transitions.
Applied to files:
src/client/HostLobbyModal.ts
🧬 Code graph analysis (12)
src/client/components/shared/DifficultyControls.ts (3)
src/client/HostLobbyModal.ts (1)
customElement(79-718)src/client/SinglePlayerModal.ts (1)
customElement(54-472)src/client/components/shared/DifficultyPicker.ts (1)
customElement(6-44)
src/client/components/shared/AdvancedOptions.ts (4)
src/client/HostLobbyModal.ts (1)
customElement(79-718)src/client/SinglePlayerModal.ts (1)
customElement(54-472)src/client/utilities/RenderRulesOptions.ts (1)
renderRulesOptions(25-91)src/client/utilities/RenderUnitTypeOptions.ts (1)
renderUnitTypeOptions(23-87)
src/client/components/shared/GameModePicker.ts (3)
src/client/HostLobbyModal.ts (1)
customElement(79-718)src/client/SinglePlayerModal.ts (1)
customElement(54-472)src/client/components/shared/GameModeControls.ts (1)
customElement(6-34)
src/client/components/shared/TeamCountPicker.ts (2)
src/core/Schemas.ts (1)
TeamCountConfig(153-153)src/core/game/Game.ts (3)
Duos(52-52)Trios(53-53)Quads(54-54)
src/client/components/shared/MapBrowserPane.ts (4)
src/core/game/Game.ts (1)
mapCategories(103-141)src/client/HostLobbyModal.ts (1)
customElement(79-718)src/client/SinglePlayerModal.ts (1)
customElement(54-472)src/client/components/Maps.ts (1)
customElement(42-146)
src/client/components/Maps.ts (2)
src/client/TerrainMapFileLoader.ts (1)
terrainMapFileLoader(4-4)src/client/Utils.ts (1)
translateText(92-147)
src/client/HostLobbyModal.ts (4)
src/core/Schemas.ts (1)
TeamCountConfig(153-153)src/core/game/UserSettings.ts (1)
UserSettings(6-201)src/client/Utils.ts (1)
translateText(92-147)src/core/configuration/ConfigLoader.ts (1)
getServerConfigFromClient(28-45)
src/client/components/shared/PresetsManager.ts (4)
src/client/HostLobbyModal.ts (1)
customElement(79-718)src/client/SinglePlayerModal.ts (1)
customElement(54-472)src/client/components/shared/PresetsBar.ts (1)
customElement(7-114)src/core/Util.ts (1)
generateID(245-251)
src/client/utilities/RenderUnitTypeOptions.ts (1)
src/client/HostLobbyModal.ts (1)
toggleUnit(599-605)
src/client/SinglePlayerModal.ts (3)
src/client/HostLobbyModal.ts (1)
customElement(79-718)src/client/components/Maps.ts (1)
customElement(42-146)src/client/utilities/RenderRulesOptions.ts (1)
RuleKey(4-9)
src/client/components/shared/DifficultyPicker.ts (3)
src/client/HostLobbyModal.ts (1)
customElement(79-718)src/client/SinglePlayerModal.ts (1)
customElement(54-472)src/client/components/shared/DifficultyControls.ts (1)
customElement(6-43)
src/client/components/shared/GameModeControls.ts (2)
src/client/components/shared/DifficultyControls.ts (1)
customElement(6-43)src/client/components/shared/GameModePicker.ts (1)
customElement(6-52)
🔇 Additional comments (7)
src/client/Transport.ts (3)
567-581: Good defensive improvement.The change from
this.socket!.readyStatetothis.socket?.readyStateon line 577 is correct. Since the else branch is reached when socket might be null, using optional chaining prevents a potential crash in logging.
583-597: Good defensive improvement.Same improvement as in
onSendWinnerEventabove. The change fromthis.socket!.readyStatetothis.socket?.readyStateon line 593 correctly uses optional chaining for safe logging when socket might be null.
336-349: No action needed — z.prettifyError is valid Zod 4 API.z.prettifyError is part of Zod 4's official API alongside other error helpers like z.flattenError and z.treeifyError. The code at line 340 is correct and requires no changes. This existing code (not modified in this PR) uses valid Zod 4 methods.
Likely an incorrect or invalid review comment.
src/client/components/shared/ExpandButton.ts (1)
9-18: Controlled component pattern relies on parent updating state.The
onClickhandler dispatches atoggleevent but doesn't updatethis.expandedinternally. This means the button is a controlled component—the parent must listen to the event and update the.expandedproperty:@toggle=${(e: CustomEvent<{ value: boolean }>) => (this.rightExpanded = e.detail.value)}This pattern is fine and matches the usage in HostLobbyModal.ts and SinglePlayerModal.ts. Just ensure all consumers follow this pattern to avoid the button appearing stuck.
src/client/utilities/RenderRulesOptions.ts (1)
1-91: LGTM! Clean implementation of rules options.This utility follows good practices:
- Type-safe
RuleKeyunion andRulesRenderContextinterface- Proper checkbox semantics (checked state matches
isOn, handler passescheckeddirectly)- Accessibility attributes (
aria-label,title)- Consistent styling with the unit options
- Clear on/off visual indicators
The straightforward
toggleRule(key, checked)logic is easier to understand than the inverted pattern in RenderUnitTypeOptions.ts.src/client/components/Maps.ts (1)
58-62: LGTM: reactive reload on mapKey changesupdated() correctly reloads on mapKey change and avoids redundant work.
src/client/components/shared/BotsSlider.ts (1)
14-25: LGTM: debounced slider with proper clamping and cleanupEvent semantics, clamping, and timer cleanup look solid. Light DOM styling is scoped via class selector—nice.
Also applies to: 35-74, 80-158
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/client/components/shared/MapBrowserPane.ts (1)
106-128: Add type="button" to the random toggle
Line 106 still falls back to the HTML default"submit". In a form, clicking this control would submit the form. Please addtype="button"just like the other buttons in this block.- <button + <button + type="button" class="h-9 rounded-full border px-3 flex items-center gap-1.5 transition-all ${this
🧹 Nitpick comments (2)
src/client/HostLobbyModal.ts (1)
34-77: Consider extracting shared preset structure to reduce duplication.The
HostLobbyPresettype (lines 34-55) andSinglePlayerPreset(in SinglePlayerModal.ts lines 46-52) share identical structure except for the settings content. You could define a genericPreset<TSettings>type in a shared file to eliminate duplication and ensure consistency.// In a shared types file type Preset<TSettings> = { id: string; name: string; createdAt: number; updatedAt: number; settings: TSettings; }; // Then use it type HostLobbyPreset = Preset<{ selectedMap: GameMapType; selectedDifficulty: Difficulty; // ... rest of settings }>;This would also align with the
GenericPreset<T>already used inPresetsManager.ts(line 15).src/client/SinglePlayerModal.ts (1)
287-299: Simplify static property access.Lines 288-290 use
(this.constructor as typeof SinglePlayerModal).PRESETS_KEYto access static properties. This works but is verbose. Consider storing static values in instance fields during construction or referencing the class directly:+ private readonly PRESETS_KEY = SinglePlayerModal.PRESETS_KEY; + private readonly MAX_PRESETS = SinglePlayerModal.MAX_PRESETS; ${html` <presets-manager - storageKey=${(this.constructor as typeof SinglePlayerModal).PRESETS_KEY} - .limit=${(this.constructor as typeof SinglePlayerModal).MAX_PRESETS} + storageKey=${this.PRESETS_KEY} + .limit=${this.MAX_PRESETS}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
resources/lang/en.json(7 hunks)src/client/HostLobbyModal.ts(10 hunks)src/client/SinglePlayerModal.ts(4 hunks)src/client/components/shared/AdvancedOptions.ts(1 hunks)src/client/components/shared/MapBrowserPane.ts(1 hunks)src/client/components/shared/PresetsManager.ts(1 hunks)src/client/utilities/RenderUnitTypeOptions.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (15)
📓 Common learnings
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1110
File: src/client/Main.ts:293-295
Timestamp: 2025-06-09T02:20:43.637Z
Learning: In src/client/Main.ts, during game start in the handleJoinLobby callback, UI elements are hidden using direct DOM manipulation with classList.add("hidden") for consistency. This includes modals, buttons, and error divs. The codebase follows this pattern rather than using component APIs for hiding elements during game transitions.
📚 Learning: 2025-06-09T02:20:43.637Z
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1110
File: src/client/Main.ts:293-295
Timestamp: 2025-06-09T02:20:43.637Z
Learning: In src/client/Main.ts, during game start in the handleJoinLobby callback, UI elements are hidden using direct DOM manipulation with classList.add("hidden") for consistency. This includes modals, buttons, and error divs. The codebase follows this pattern rather than using component APIs for hiding elements during game transitions.
Applied to files:
src/client/utilities/RenderUnitTypeOptions.tssrc/client/components/shared/AdvancedOptions.tssrc/client/HostLobbyModal.tssrc/client/SinglePlayerModal.ts
📚 Learning: 2025-08-12T00:31:50.144Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#1752
File: src/core/game/Game.ts:750-752
Timestamp: 2025-08-12T00:31:50.144Z
Learning: In the OpenFrontIO codebase, changes to the PlayerInteraction interface (like adding canDonateGold and canDonateTroops flags) do not require corresponding updates to src/core/Schemas.ts or server serialization code.
Applied to files:
src/client/components/shared/AdvancedOptions.tssrc/client/HostLobbyModal.tssrc/client/SinglePlayerModal.ts
📚 Learning: 2025-10-21T20:06:04.823Z
Learnt from: Saphereye
PR: openfrontio/OpenFrontIO#2233
File: src/client/HostLobbyModal.ts:891-891
Timestamp: 2025-10-21T20:06:04.823Z
Learning: For the HumansVsNations game mode in `src/client/HostLobbyModal.ts` and related files, the implementation strategy is to generate all nations and adjust their strength for balancing, rather than limiting lobby size based on the number of available nations on the map.
Applied to files:
src/client/HostLobbyModal.tssrc/client/SinglePlayerModal.ts
📚 Learning: 2025-06-02T14:27:37.609Z
Learnt from: andrewNiziolek
PR: openfrontio/OpenFrontIO#1007
File: resources/lang/de.json:115-115
Timestamp: 2025-06-02T14:27:37.609Z
Learning: For OpenFrontIO project: When localization keys are renamed in language JSON files, the maintainers separate technical changes from translation content updates. They wait for community translators to update the actual translation values rather than attempting to translate in the same PR. This allows technical changes to proceed while ensuring accurate translations from native speakers.
Applied to files:
src/client/HostLobbyModal.tsresources/lang/en.json
📚 Learning: 2025-10-26T15:37:07.732Z
Learnt from: GlacialDrift
PR: openfrontio/OpenFrontIO#2298
File: src/client/graphics/layers/TerritoryLayer.ts:200-210
Timestamp: 2025-10-26T15:37:07.732Z
Learning: In GameImpl.ts lines 124-139, team assignment logic varies by number of teams: when numPlayerTeams < 8, teams are assigned ColoredTeams values (Red, Blue, Yellow, Green, Purple, Orange, Teal); when numPlayerTeams >= 8, teams are assigned generic string identifiers like "Team 1", "Team 2", etc., which are not members of ColoredTeams.
Applied to files:
src/client/HostLobbyModal.ts
📚 Learning: 2025-10-20T20:15:28.858Z
Learnt from: sambokai
PR: openfrontio/OpenFrontIO#2225
File: src/core/execution/FakeHumanExecution.ts:51-51
Timestamp: 2025-10-20T20:15:28.858Z
Learning: In src/core/execution/FakeHumanExecution.ts, game balance constants like MIRV_COOLDOWN_TICKS, MIRV_HESITATION_ODDS, VICTORY_DENIAL_TEAM_THRESHOLD, VICTORY_DENIAL_INDIVIDUAL_THRESHOLD, and STEAMROLL_CITY_GAP_MULTIPLIER are experimental tuning parameters subject to frequent change during balance testing. Do not flag changes to these values as issues or compare them against previous values.
Applied to files:
src/client/HostLobbyModal.tssrc/client/SinglePlayerModal.ts
📚 Learning: 2025-06-16T03:03:59.778Z
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1192
File: src/client/graphics/layers/RadialMenuElements.ts:312-314
Timestamp: 2025-06-16T03:03:59.778Z
Learning: In RadialMenuElements.ts, when fixing undefined params errors in subMenu functions, use explicit checks like `if (params === undefined || params.selected === null)` rather than optional chaining, as it makes the intent clearer and matches the specific error scenarios where params can be undefined during spawn phase operations.
Applied to files:
src/client/HostLobbyModal.ts
📚 Learning: 2025-10-27T09:47:26.395Z
Learnt from: sambokai
PR: openfrontio/OpenFrontIO#2225
File: src/core/execution/FakeHumanExecution.ts:770-795
Timestamp: 2025-10-27T09:47:26.395Z
Learning: In src/core/execution/FakeHumanExecution.ts, the selectSteamrollStopTarget() method intentionally allows MIRV targeting when secondHighest city count is 0 (e.g., nuclear endgame scenarios where structures are destroyed). This is valid game design—"if you can afford it, you're good to go"—and should not be flagged as requiring a guard condition.
Applied to files:
src/client/HostLobbyModal.ts
📚 Learning: 2025-08-21T21:42:45.277Z
Learnt from: plazmaezio
PR: openfrontio/OpenFrontIO#1883
File: src/client/HostLobbyModal.ts:227-245
Timestamp: 2025-08-21T21:42:45.277Z
Learning: In openfrontio/OpenFrontIO, the en.json file contains difficulty translation keys using the descriptive names ("Relaxed", "Balanced", "Intense", "Impossible") rather than the enum values ("Easy", "Medium", "Hard", "Impossible"). The DifficultyDescription mapping in src/client/components/Difficulties.ts correctly maps enum values to these descriptive translation keys.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-08-16T10:52:08.292Z
Learnt from: TheGiraffe3
PR: openfrontio/OpenFrontIO#884
File: resources/lang/en.json:456-461
Timestamp: 2025-08-16T10:52:08.292Z
Learning: In OpenFrontIO, translation files in resources/lang/*.json (except en.json) should not be updated in regular PRs. Only dedicated translation PRs titled "mls" and made by Aotumori should update non-English locale files. Regular PRs should only update en.json when adding or modifying translation keys.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-05-30T03:53:52.231Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#949
File: resources/lang/en.json:8-10
Timestamp: 2025-05-30T03:53:52.231Z
Learning: For the OpenFrontIO project, do not suggest updating translation files in resources/lang/*.json except for en.json. The project has a dedicated translation team that handles all other locale files.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-08-27T08:12:19.610Z
Learnt from: mokizzz
PR: openfrontio/OpenFrontIO#1940
File: resources/lang/en.json:763-766
Timestamp: 2025-08-27T08:12:19.610Z
Learning: In OpenFrontIO, some country entries in src/client/data/countries.json may have similar names but different codes (e.g., "Empire of Japan" vs "Empire of Japan1"). Each unique code requires its own translation key in resources/lang/en.json after normalization. Always verify against countries.json before suggesting removal of translation keys.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-07-12T08:41:35.101Z
Learnt from: Aotumuri
PR: openfrontio/OpenFrontIO#1357
File: resources/lang/de.json:523-540
Timestamp: 2025-07-12T08:41:35.101Z
Learning: In OpenFrontIO project localization files, always check the en.json source file before flagging potential spelling errors in other language files, as some keys may intentionally use non-standard spellings that need to be consistent across all translations.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-08-19T11:00:55.422Z
Learnt from: TheGiraffe3
PR: openfrontio/OpenFrontIO#1864
File: resources/maps/arabianpeninsula/manifest.json:13-170
Timestamp: 2025-08-19T11:00:55.422Z
Learning: In OpenFrontIO, nation names in map manifests are displayed directly in the UI without translation. They do not need to be added to resources/lang/en.json or processed through translateText(). This is the established pattern across all existing maps including Europe, World, Asia, Africa, and others.
Applied to files:
resources/lang/en.json
🧬 Code graph analysis (6)
src/client/utilities/RenderUnitTypeOptions.ts (1)
src/client/HostLobbyModal.ts (1)
toggleUnit(606-615)
src/client/components/shared/PresetsManager.ts (4)
src/client/HostLobbyModal.ts (1)
customElement(79-749)src/client/SinglePlayerModal.ts (1)
customElement(54-489)src/client/components/shared/PresetsBar.ts (1)
customElement(7-114)src/core/Util.ts (1)
generateID(245-251)
src/client/components/shared/MapBrowserPane.ts (2)
src/core/game/Game.ts (1)
mapCategories(103-141)src/client/components/Maps.ts (1)
customElement(42-146)
src/client/components/shared/AdvancedOptions.ts (4)
src/client/HostLobbyModal.ts (1)
customElement(79-749)src/client/SinglePlayerModal.ts (1)
customElement(54-489)src/client/utilities/RenderRulesOptions.ts (1)
renderRulesOptions(25-91)src/client/utilities/RenderUnitTypeOptions.ts (1)
renderUnitTypeOptions(23-88)
src/client/HostLobbyModal.ts (4)
src/core/Schemas.ts (2)
TeamCountConfig(153-153)ClientInfo(131-134)src/core/game/UserSettings.ts (1)
UserSettings(6-201)src/client/Utils.ts (1)
translateText(92-147)src/core/configuration/ConfigLoader.ts (1)
getServerConfigFromClient(28-45)
src/client/SinglePlayerModal.ts (5)
src/client/HostLobbyModal.ts (1)
customElement(79-749)src/client/components/shared/PresetsManager.ts (1)
customElement(16-190)src/client/components/shared/AdvancedOptions.ts (1)
customElement(18-140)src/client/components/shared/MapBrowserPane.ts (1)
customElement(8-170)src/client/utilities/RenderRulesOptions.ts (1)
RuleKey(4-9)
🔇 Additional comments (4)
src/client/HostLobbyModal.ts (3)
516-534: Good defensive polling initialization.Deferring
setIntervaluntil after the lobby is created (line 520) prevents unnecessary API calls with an emptylobbyId. The guard aligns well with the checks inputGameConfig(line 575) andpollPlayers(line 698).
617-626: LGTM: Smart debouncing via event type.Only calling
putGameConfigon"change"events (line 624) avoids flooding the server with updates during slider drag. This pairs well with thedebounceMsprop in thebots-slidercomponent.
220-225: Translation keys are present and correctly implemented.All translation keys used in the code (
host_modal.hide_id,host_modal.show_id,host_modal.copy_invite,common.copied) exist inresources/lang/en.json. The code will display proper translated strings instead of raw keys. No changes needed.src/client/SinglePlayerModal.ts (1)
59-70: Verify @Property vs @State usage for modal configuration.Lines 59-70 use
@propertydecorator, which creates public, reflected attributes. This differs fromHostLobbyModal(lines 81-92) which uses@statefor the same fields.
@propertymakes these accessible externally as HTML attributes/properties. If external code shouldn't modify these settings, use@stateinstead to keep them internal. If they're meant to be configurable from outside,@propertyis correct but document this intent.
|
I like the new UI. I can't seem to scroll past the maps on mobile. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
src/client/components/Maps.ts (1)
114-138: Guarantee a real alt string (avoid"null")
When boththis.translationandthis.mapNameare absent,${this.translation || this.mapName}evaluates tonull, so the attribute literal becomesalt="null". That’s confusing for screen-reader users. Please fall back totranslateText("common.untitled")(or an empty string if you want it decorative) so the alt text is always meaningful.- alt="${this.translation || this.mapName}" + alt="${this.translation ?? + this.mapName ?? + translateText("common.untitled")}"src/client/components/shared/PresetsManager.ts (1)
25-35: Past review comment addressed.The early warning for missing
storageKeyhas been implemented as suggested in the previous review. The guard correctly warns and returns early whenstorageKeyis missing.src/client/HostLobbyModal.ts (2)
189-194: Past review comment on fallback remains relevant.The code at line 193 still calls
translateText("host_modal.generating")without checking if the key exists. As noted in the previous review,translateTextreturns the key itself when the translation is missing, so a hardcoded fallback would never apply.If the key
"host_modal.generating"is missing from translation files, users will see the raw key string instead of "Generating…".
624-631: Past review comment addressed.The random map selection now correctly filters to numeric enum values only, preventing invalid string values from being selected.
🧹 Nitpick comments (2)
src/client/components/shared/PresetsManager.ts (2)
101-130: Consider re-sorting after save for immediate UX consistency.After saving a new preset, it's appended to the array (line 127). On the next page load,
loadFromStoragesorts byupdatedAt(most recent first), but within the current session the new preset appears at the end of the dropdown.To improve UX, consider sorting immediately after adding:
this.presets = [...this.presets, preset]; +this.presets.sort((a, b) => b.updatedAt - a.updatedAt); this.selectedId = preset.id; this.persistToStorage();
132-155: Consider re-sorting after update for immediate UX consistency.Similar to the save flow, updating a preset changes its
updatedAttimestamp but doesn't re-sort the array in the current session. The preset won't move to the top of the list until the next page load.Apply the same sort after updating:
this.presets = [ ...this.presets.slice(0, i), updated, ...this.presets.slice(i + 1), ]; +this.presets.sort((a, b) => b.updatedAt - a.updatedAt); this.nameInput = updated.name; this.persistToStorage();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
resources/lang/en.json(7 hunks)src/client/HostLobbyModal.ts(10 hunks)src/client/SinglePlayerModal.ts(4 hunks)src/client/components/Maps.ts(3 hunks)src/client/components/shared/BotsSlider.ts(1 hunks)src/client/components/shared/GameModePicker.ts(1 hunks)src/client/components/shared/MapBrowserPane.ts(1 hunks)src/client/components/shared/PresetsBar.ts(1 hunks)src/client/components/shared/PresetsManager.ts(1 hunks)src/client/types/preset.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/client/components/shared/BotsSlider.ts
- src/client/components/shared/PresetsBar.ts
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1110
File: src/client/Main.ts:293-295
Timestamp: 2025-06-09T02:20:43.637Z
Learning: In src/client/Main.ts, during game start in the handleJoinLobby callback, UI elements are hidden using direct DOM manipulation with classList.add("hidden") for consistency. This includes modals, buttons, and error divs. The codebase follows this pattern rather than using component APIs for hiding elements during game transitions.
Learnt from: Saphereye
PR: openfrontio/OpenFrontIO#2233
File: src/client/HostLobbyModal.ts:891-891
Timestamp: 2025-10-21T20:06:04.823Z
Learning: For the HumansVsNations game mode in `src/client/HostLobbyModal.ts` and related files, the implementation strategy is to generate all nations and adjust their strength for balancing, rather than limiting lobby size based on the number of available nations on the map.
📚 Learning: 2025-06-09T02:20:43.637Z
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1110
File: src/client/Main.ts:293-295
Timestamp: 2025-06-09T02:20:43.637Z
Learning: In src/client/Main.ts, during game start in the handleJoinLobby callback, UI elements are hidden using direct DOM manipulation with classList.add("hidden") for consistency. This includes modals, buttons, and error divs. The codebase follows this pattern rather than using component APIs for hiding elements during game transitions.
Applied to files:
src/client/components/shared/MapBrowserPane.tssrc/client/components/shared/GameModePicker.tssrc/client/HostLobbyModal.tssrc/client/SinglePlayerModal.ts
📚 Learning: 2025-10-21T20:06:04.823Z
Learnt from: Saphereye
PR: openfrontio/OpenFrontIO#2233
File: src/client/HostLobbyModal.ts:891-891
Timestamp: 2025-10-21T20:06:04.823Z
Learning: For the HumansVsNations game mode in `src/client/HostLobbyModal.ts` and related files, the implementation strategy is to generate all nations and adjust their strength for balancing, rather than limiting lobby size based on the number of available nations on the map.
Applied to files:
src/client/HostLobbyModal.tssrc/client/SinglePlayerModal.ts
📚 Learning: 2025-08-12T00:31:50.144Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#1752
File: src/core/game/Game.ts:750-752
Timestamp: 2025-08-12T00:31:50.144Z
Learning: In the OpenFrontIO codebase, changes to the PlayerInteraction interface (like adding canDonateGold and canDonateTroops flags) do not require corresponding updates to src/core/Schemas.ts or server serialization code.
Applied to files:
src/client/HostLobbyModal.tssrc/client/SinglePlayerModal.ts
📚 Learning: 2025-06-02T14:27:37.609Z
Learnt from: andrewNiziolek
PR: openfrontio/OpenFrontIO#1007
File: resources/lang/de.json:115-115
Timestamp: 2025-06-02T14:27:37.609Z
Learning: For OpenFrontIO project: When localization keys are renamed in language JSON files, the maintainers separate technical changes from translation content updates. They wait for community translators to update the actual translation values rather than attempting to translate in the same PR. This allows technical changes to proceed while ensuring accurate translations from native speakers.
Applied to files:
src/client/HostLobbyModal.tsresources/lang/en.json
📚 Learning: 2025-10-26T15:37:07.732Z
Learnt from: GlacialDrift
PR: openfrontio/OpenFrontIO#2298
File: src/client/graphics/layers/TerritoryLayer.ts:200-210
Timestamp: 2025-10-26T15:37:07.732Z
Learning: In GameImpl.ts lines 124-139, team assignment logic varies by number of teams: when numPlayerTeams < 8, teams are assigned ColoredTeams values (Red, Blue, Yellow, Green, Purple, Orange, Teal); when numPlayerTeams >= 8, teams are assigned generic string identifiers like "Team 1", "Team 2", etc., which are not members of ColoredTeams.
Applied to files:
src/client/HostLobbyModal.ts
📚 Learning: 2025-10-20T20:15:28.858Z
Learnt from: sambokai
PR: openfrontio/OpenFrontIO#2225
File: src/core/execution/FakeHumanExecution.ts:51-51
Timestamp: 2025-10-20T20:15:28.858Z
Learning: In src/core/execution/FakeHumanExecution.ts, game balance constants like MIRV_COOLDOWN_TICKS, MIRV_HESITATION_ODDS, VICTORY_DENIAL_TEAM_THRESHOLD, VICTORY_DENIAL_INDIVIDUAL_THRESHOLD, and STEAMROLL_CITY_GAP_MULTIPLIER are experimental tuning parameters subject to frequent change during balance testing. Do not flag changes to these values as issues or compare them against previous values.
Applied to files:
src/client/HostLobbyModal.tssrc/client/SinglePlayerModal.ts
📚 Learning: 2025-06-16T03:03:59.778Z
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1192
File: src/client/graphics/layers/RadialMenuElements.ts:312-314
Timestamp: 2025-06-16T03:03:59.778Z
Learning: In RadialMenuElements.ts, when fixing undefined params errors in subMenu functions, use explicit checks like `if (params === undefined || params.selected === null)` rather than optional chaining, as it makes the intent clearer and matches the specific error scenarios where params can be undefined during spawn phase operations.
Applied to files:
src/client/HostLobbyModal.ts
📚 Learning: 2025-10-27T09:47:26.395Z
Learnt from: sambokai
PR: openfrontio/OpenFrontIO#2225
File: src/core/execution/FakeHumanExecution.ts:770-795
Timestamp: 2025-10-27T09:47:26.395Z
Learning: In src/core/execution/FakeHumanExecution.ts, the selectSteamrollStopTarget() method intentionally allows MIRV targeting when secondHighest city count is 0 (e.g., nuclear endgame scenarios where structures are destroyed). This is valid game design—"if you can afford it, you're good to go"—and should not be flagged as requiring a guard condition.
Applied to files:
src/client/HostLobbyModal.ts
📚 Learning: 2025-08-21T21:42:45.277Z
Learnt from: plazmaezio
PR: openfrontio/OpenFrontIO#1883
File: src/client/HostLobbyModal.ts:227-245
Timestamp: 2025-08-21T21:42:45.277Z
Learning: In openfrontio/OpenFrontIO, the en.json file contains difficulty translation keys using the descriptive names ("Relaxed", "Balanced", "Intense", "Impossible") rather than the enum values ("Easy", "Medium", "Hard", "Impossible"). The DifficultyDescription mapping in src/client/components/Difficulties.ts correctly maps enum values to these descriptive translation keys.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-08-16T10:52:08.292Z
Learnt from: TheGiraffe3
PR: openfrontio/OpenFrontIO#884
File: resources/lang/en.json:456-461
Timestamp: 2025-08-16T10:52:08.292Z
Learning: In OpenFrontIO, translation files in resources/lang/*.json (except en.json) should not be updated in regular PRs. Only dedicated translation PRs titled "mls" and made by Aotumori should update non-English locale files. Regular PRs should only update en.json when adding or modifying translation keys.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-08-27T08:12:19.610Z
Learnt from: mokizzz
PR: openfrontio/OpenFrontIO#1940
File: resources/lang/en.json:763-766
Timestamp: 2025-08-27T08:12:19.610Z
Learning: In OpenFrontIO, some country entries in src/client/data/countries.json may have similar names but different codes (e.g., "Empire of Japan" vs "Empire of Japan1"). Each unique code requires its own translation key in resources/lang/en.json after normalization. Always verify against countries.json before suggesting removal of translation keys.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-05-30T03:53:52.231Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#949
File: resources/lang/en.json:8-10
Timestamp: 2025-05-30T03:53:52.231Z
Learning: For the OpenFrontIO project, do not suggest updating translation files in resources/lang/*.json except for en.json. The project has a dedicated translation team that handles all other locale files.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-08-19T11:00:55.422Z
Learnt from: TheGiraffe3
PR: openfrontio/OpenFrontIO#1864
File: resources/maps/arabianpeninsula/manifest.json:13-170
Timestamp: 2025-08-19T11:00:55.422Z
Learning: In OpenFrontIO, nation names in map manifests are displayed directly in the UI without translation. They do not need to be added to resources/lang/en.json or processed through translateText(). This is the established pattern across all existing maps including Europe, World, Asia, Africa, and others.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-06-02T14:27:23.893Z
Learnt from: andrewNiziolek
PR: openfrontio/OpenFrontIO#1007
File: resources/lang/he.json:138-138
Timestamp: 2025-06-02T14:27:23.893Z
Learning: andrewNiziolek's project uses community translation for internationalization. When updating map names or other user-facing text, they update the keys programmatically but wait for community translators to provide accurate translations in each language rather than using machine translations.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-07-12T08:41:35.101Z
Learnt from: Aotumuri
PR: openfrontio/OpenFrontIO#1357
File: resources/lang/de.json:523-540
Timestamp: 2025-07-12T08:41:35.101Z
Learning: In OpenFrontIO project localization files, always check the en.json source file before flagging potential spelling errors in other language files, as some keys may intentionally use non-standard spellings that need to be consistent across all translations.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-07-12T06:35:10.457Z
Learnt from: Aotumuri
PR: openfrontio/OpenFrontIO#1357
File: resources/lang/ja.json:0-0
Timestamp: 2025-07-12T06:35:10.457Z
Learning: In OpenFrontIO project, "giantworldmap" is the correct localization key name for the giant world map, used consistently across all language files and TypeScript code. Do not suggest renaming this key.
Applied to files:
resources/lang/en.json
🧬 Code graph analysis (6)
src/client/components/shared/MapBrowserPane.ts (4)
src/core/game/Game.ts (1)
mapCategories(103-141)src/client/HostLobbyModal.ts (1)
customElement(74-753)src/client/SinglePlayerModal.ts (1)
customElement(52-490)src/client/components/Maps.ts (1)
customElement(42-144)
src/client/components/shared/GameModePicker.ts (4)
src/client/HostLobbyModal.ts (1)
customElement(74-753)src/client/SinglePlayerModal.ts (1)
customElement(52-490)src/client/components/shared/GameModeControls.ts (1)
customElement(6-34)src/client/components/shared/SettingsSummary.ts (1)
customElement(6-67)
src/client/HostLobbyModal.ts (5)
src/client/types/preset.ts (1)
Preset(2-8)src/core/Schemas.ts (2)
TeamCountConfig(153-153)ClientInfo(131-134)src/core/game/UserSettings.ts (1)
UserSettings(6-201)src/client/Utils.ts (1)
translateText(92-147)src/core/configuration/ConfigLoader.ts (1)
getServerConfigFromClient(28-45)
src/client/components/shared/PresetsManager.ts (4)
src/client/types/preset.ts (1)
Preset(2-8)src/client/HostLobbyModal.ts (1)
customElement(74-753)src/client/SinglePlayerModal.ts (1)
customElement(52-490)src/client/components/shared/PresetsBar.ts (1)
customElement(7-114)
src/client/components/Maps.ts (2)
src/client/TerrainMapFileLoader.ts (1)
terrainMapFileLoader(4-4)src/client/Utils.ts (1)
translateText(92-147)
src/client/SinglePlayerModal.ts (9)
src/core/Schemas.ts (1)
TeamCountConfig(153-153)src/client/types/preset.ts (1)
Preset(2-8)src/client/components/shared/BotsSlider.ts (1)
customElement(6-160)src/client/components/shared/MapBrowserPane.ts (1)
customElement(8-175)src/client/components/shared/PresetsBar.ts (1)
customElement(7-114)src/client/components/shared/PresetsManager.ts (1)
customElement(11-185)src/client/components/shared/AdvancedOptions.ts (1)
customElement(18-140)src/core/game/UserSettings.ts (1)
UserSettings(6-201)src/client/utilities/RenderRulesOptions.ts (1)
RuleKey(4-9)
🔇 Additional comments (1)
src/client/components/shared/PresetsManager.ts (1)
37-61: LGTM! Robust validation and error handling.The storage loading logic correctly validates the preset structure, sorts by most recent, and enforces the limit. Error handling is in place.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
src/client/components/Maps.ts (1)
115-120: Alt fallback still needs fixing (duplicate concern).The past review comment remains valid. Line 117 uses
||which can result inundefinedwhen both values are falsy, while the title at lines 133-139 correctly uses??with a translated fallback.- alt="${this.translation || this.mapName}" + alt="${this.translation ?? this.mapName ?? translateText("common.untitled")}"src/client/HostLobbyModal.ts (1)
192-196: Fallback for "Generating..." still won't work (duplicate concern).The past review comment remains valid. When
translateText("host_modal.generating")can't find the key, it returns the key string itself, notundefined, so any??fallback would never trigger. You need to check if the returned value equals the key.- const displayValue = id - ? this.lobbyIdVisible - ? id - : this.mask(id) - : translateText("host_modal.generating"); + const t = translateText("host_modal.generating"); + const generating = t !== "host_modal.generating" ? t : "Generating…"; + const displayValue = id + ? this.lobbyIdVisible + ? id + : this.mask(id) + : generating;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
resources/lang/en.json(10 hunks)src/client/HostLobbyModal.ts(10 hunks)src/client/SinglePlayerModal.ts(6 hunks)src/client/components/Maps.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1110
File: src/client/Main.ts:293-295
Timestamp: 2025-06-09T02:20:43.637Z
Learning: In src/client/Main.ts, during game start in the handleJoinLobby callback, UI elements are hidden using direct DOM manipulation with classList.add("hidden") for consistency. This includes modals, buttons, and error divs. The codebase follows this pattern rather than using component APIs for hiding elements during game transitions.
📚 Learning: 2025-06-09T02:20:43.637Z
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1110
File: src/client/Main.ts:293-295
Timestamp: 2025-06-09T02:20:43.637Z
Learning: In src/client/Main.ts, during game start in the handleJoinLobby callback, UI elements are hidden using direct DOM manipulation with classList.add("hidden") for consistency. This includes modals, buttons, and error divs. The codebase follows this pattern rather than using component APIs for hiding elements during game transitions.
Applied to files:
src/client/HostLobbyModal.tssrc/client/SinglePlayerModal.ts
📚 Learning: 2025-10-21T20:06:04.823Z
Learnt from: Saphereye
PR: openfrontio/OpenFrontIO#2233
File: src/client/HostLobbyModal.ts:891-891
Timestamp: 2025-10-21T20:06:04.823Z
Learning: For the HumansVsNations game mode in `src/client/HostLobbyModal.ts` and related files, the implementation strategy is to generate all nations and adjust their strength for balancing, rather than limiting lobby size based on the number of available nations on the map.
Applied to files:
src/client/HostLobbyModal.tssrc/client/SinglePlayerModal.ts
📚 Learning: 2025-06-02T14:27:37.609Z
Learnt from: andrewNiziolek
PR: openfrontio/OpenFrontIO#1007
File: resources/lang/de.json:115-115
Timestamp: 2025-06-02T14:27:37.609Z
Learning: For OpenFrontIO project: When localization keys are renamed in language JSON files, the maintainers separate technical changes from translation content updates. They wait for community translators to update the actual translation values rather than attempting to translate in the same PR. This allows technical changes to proceed while ensuring accurate translations from native speakers.
Applied to files:
src/client/HostLobbyModal.tsresources/lang/en.json
📚 Learning: 2025-10-26T15:37:07.732Z
Learnt from: GlacialDrift
PR: openfrontio/OpenFrontIO#2298
File: src/client/graphics/layers/TerritoryLayer.ts:200-210
Timestamp: 2025-10-26T15:37:07.732Z
Learning: In GameImpl.ts lines 124-139, team assignment logic varies by number of teams: when numPlayerTeams < 8, teams are assigned ColoredTeams values (Red, Blue, Yellow, Green, Purple, Orange, Teal); when numPlayerTeams >= 8, teams are assigned generic string identifiers like "Team 1", "Team 2", etc., which are not members of ColoredTeams.
Applied to files:
src/client/HostLobbyModal.ts
📚 Learning: 2025-10-20T20:15:28.858Z
Learnt from: sambokai
PR: openfrontio/OpenFrontIO#2225
File: src/core/execution/FakeHumanExecution.ts:51-51
Timestamp: 2025-10-20T20:15:28.858Z
Learning: In src/core/execution/FakeHumanExecution.ts, game balance constants like MIRV_COOLDOWN_TICKS, MIRV_HESITATION_ODDS, VICTORY_DENIAL_TEAM_THRESHOLD, VICTORY_DENIAL_INDIVIDUAL_THRESHOLD, and STEAMROLL_CITY_GAP_MULTIPLIER are experimental tuning parameters subject to frequent change during balance testing. Do not flag changes to these values as issues or compare them against previous values.
Applied to files:
src/client/HostLobbyModal.tssrc/client/SinglePlayerModal.ts
📚 Learning: 2025-06-16T03:03:59.778Z
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1192
File: src/client/graphics/layers/RadialMenuElements.ts:312-314
Timestamp: 2025-06-16T03:03:59.778Z
Learning: In RadialMenuElements.ts, when fixing undefined params errors in subMenu functions, use explicit checks like `if (params === undefined || params.selected === null)` rather than optional chaining, as it makes the intent clearer and matches the specific error scenarios where params can be undefined during spawn phase operations.
Applied to files:
src/client/HostLobbyModal.ts
📚 Learning: 2025-10-27T09:47:26.395Z
Learnt from: sambokai
PR: openfrontio/OpenFrontIO#2225
File: src/core/execution/FakeHumanExecution.ts:770-795
Timestamp: 2025-10-27T09:47:26.395Z
Learning: In src/core/execution/FakeHumanExecution.ts, the selectSteamrollStopTarget() method intentionally allows MIRV targeting when secondHighest city count is 0 (e.g., nuclear endgame scenarios where structures are destroyed). This is valid game design—"if you can afford it, you're good to go"—and should not be flagged as requiring a guard condition.
Applied to files:
src/client/HostLobbyModal.ts
📚 Learning: 2025-08-12T00:31:50.144Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#1752
File: src/core/game/Game.ts:750-752
Timestamp: 2025-08-12T00:31:50.144Z
Learning: In the OpenFrontIO codebase, changes to the PlayerInteraction interface (like adding canDonateGold and canDonateTroops flags) do not require corresponding updates to src/core/Schemas.ts or server serialization code.
Applied to files:
src/client/SinglePlayerModal.ts
📚 Learning: 2025-08-21T21:42:45.277Z
Learnt from: plazmaezio
PR: openfrontio/OpenFrontIO#1883
File: src/client/HostLobbyModal.ts:227-245
Timestamp: 2025-08-21T21:42:45.277Z
Learning: In openfrontio/OpenFrontIO, the en.json file contains difficulty translation keys using the descriptive names ("Relaxed", "Balanced", "Intense", "Impossible") rather than the enum values ("Easy", "Medium", "Hard", "Impossible"). The DifficultyDescription mapping in src/client/components/Difficulties.ts correctly maps enum values to these descriptive translation keys.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-08-16T10:52:08.292Z
Learnt from: TheGiraffe3
PR: openfrontio/OpenFrontIO#884
File: resources/lang/en.json:456-461
Timestamp: 2025-08-16T10:52:08.292Z
Learning: In OpenFrontIO, translation files in resources/lang/*.json (except en.json) should not be updated in regular PRs. Only dedicated translation PRs titled "mls" and made by Aotumori should update non-English locale files. Regular PRs should only update en.json when adding or modifying translation keys.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-08-27T08:12:19.610Z
Learnt from: mokizzz
PR: openfrontio/OpenFrontIO#1940
File: resources/lang/en.json:763-766
Timestamp: 2025-08-27T08:12:19.610Z
Learning: In OpenFrontIO, some country entries in src/client/data/countries.json may have similar names but different codes (e.g., "Empire of Japan" vs "Empire of Japan1"). Each unique code requires its own translation key in resources/lang/en.json after normalization. Always verify against countries.json before suggesting removal of translation keys.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-05-30T03:53:52.231Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#949
File: resources/lang/en.json:8-10
Timestamp: 2025-05-30T03:53:52.231Z
Learning: For the OpenFrontIO project, do not suggest updating translation files in resources/lang/*.json except for en.json. The project has a dedicated translation team that handles all other locale files.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-06-02T14:27:23.893Z
Learnt from: andrewNiziolek
PR: openfrontio/OpenFrontIO#1007
File: resources/lang/he.json:138-138
Timestamp: 2025-06-02T14:27:23.893Z
Learning: andrewNiziolek's project uses community translation for internationalization. When updating map names or other user-facing text, they update the keys programmatically but wait for community translators to provide accurate translations in each language rather than using machine translations.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-08-19T11:00:55.422Z
Learnt from: TheGiraffe3
PR: openfrontio/OpenFrontIO#1864
File: resources/maps/arabianpeninsula/manifest.json:13-170
Timestamp: 2025-08-19T11:00:55.422Z
Learning: In OpenFrontIO, nation names in map manifests are displayed directly in the UI without translation. They do not need to be added to resources/lang/en.json or processed through translateText(). This is the established pattern across all existing maps including Europe, World, Asia, Africa, and others.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-07-12T08:41:35.101Z
Learnt from: Aotumuri
PR: openfrontio/OpenFrontIO#1357
File: resources/lang/de.json:523-540
Timestamp: 2025-07-12T08:41:35.101Z
Learning: In OpenFrontIO project localization files, always check the en.json source file before flagging potential spelling errors in other language files, as some keys may intentionally use non-standard spellings that need to be consistent across all translations.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-07-12T06:35:10.457Z
Learnt from: Aotumuri
PR: openfrontio/OpenFrontIO#1357
File: resources/lang/ja.json:0-0
Timestamp: 2025-07-12T06:35:10.457Z
Learning: In OpenFrontIO project, "giantworldmap" is the correct localization key name for the giant world map, used consistently across all language files and TypeScript code. Do not suggest renaming this key.
Applied to files:
resources/lang/en.json
🧬 Code graph analysis (3)
src/client/components/Maps.ts (2)
src/client/TerrainMapFileLoader.ts (1)
terrainMapFileLoader(4-4)src/client/Utils.ts (1)
translateText(92-147)
src/client/HostLobbyModal.ts (4)
src/client/types/preset.ts (1)
Preset(2-8)src/core/Schemas.ts (2)
TeamCountConfig(155-155)ClientInfo(132-135)src/core/game/UserSettings.ts (1)
UserSettings(6-201)src/core/configuration/ConfigLoader.ts (1)
getServerConfigFromClient(28-45)
src/client/SinglePlayerModal.ts (3)
src/core/Schemas.ts (1)
TeamCountConfig(155-155)src/client/types/preset.ts (1)
Preset(2-8)src/client/utilities/RenderRulesOptions.ts (1)
RuleKey(4-9)
🪛 Biome (2.1.2)
resources/lang/en.json
[error] 283-283: The key max_timer was already declared.
This where a duplicated key was declared again.
If a key is defined multiple times, only the last definition takes effect. Previous definitions are ignored.
(lint/suspicious/noDuplicateObjectKeys)
🪛 GitHub Actions: 🧪 CI
src/client/HostLobbyModal.ts
[warning] 1-1: Code style issues found in 2 files. Run 'prettier --write' to fix code style issues in this file.
src/client/SinglePlayerModal.ts
[warning] 1-1: Code style issues found in 2 files. Run 'prettier --write' to fix code style issues in this file.
resources/lang/en.json
[error] 231-231: Prettier: SyntaxError: Unexpected token, expected "," (231:5).
🔇 Additional comments (35)
src/client/components/Maps.ts (4)
52-53: Good race condition prevention.The load token pattern correctly guards against stale async results when mapKey changes rapidly.
59-63: LGTM!The updated lifecycle correctly reloads when mapKey changes.
65-67: LGTM!Light DOM rendering aligns with the component-driven refactor across the PR.
72-98: Solid async load flow with race protection.The token-based guard at line 89 correctly prevents stale results, and the case-insensitive normalization with error throwing is a good safety measure.
src/client/HostLobbyModal.ts (12)
38-75: Clean type definitions.The preset types and allowed rule keys are well-structured with good type safety.
116-158: Well-structured settings management.The currentSettings/applySettings pattern is clean. Array spreading for disabledUnits prevents mutation issues.
167-179: LGTM!Component integration follows the new pattern consistently.
214-273: Translation keys now properly used.The hardcoded strings from the past review have been replaced with
translateText()calls. Ensure the corresponding keys exist inen.json(host_modal.hide_id, host_modal.show_id, host_modal.copy_invite, common.copied).
308-426: Component composition looks solid.The settings pane integrates multiple shared components cleanly with proper event handling and accessibility attributes.
508-545: Open flow is well-structured.The polling guard at line 519 (
??=) prevents duplicate intervals, and error handling is present.
573-612: LGTM!The guard at line 574 prevents invalid requests, and the HumansVsNations special case follows the established pattern.
625-634: Solid event handling.The dual event type handling with clamping is correct and matches the pattern in SinglePlayerModal.
636-643: Random map selection fixed.The past review concern has been addressed. The numeric filter at lines 637-639 correctly prevents picking invalid string enum values.
666-682: LGTM!Reset to defaults is straightforward and correctly calls putGameConfig.
684-711: Clean backdrop and clipboard handling.The backdrop click check and clipboard copy flow are implemented correctly.
728-753: Type-safe rule flag setting.The explicit switch is safer than dynamic property access and ensures all rule keys are handled.
resources/lang/en.json (8)
8-31: Common translations properly added.The past review concern about
common.no_resultshas been addressed. All new common keys (no_results, search, copied, etc.) support the new UI components.
32-43: LGTM!The game_options section properly supports the AdvancedOptions component.
158-161: LGTM!Settings summary keys support the new SettingsSummary component.
178-183: LGTM!Single player modal keys properly support the presets UI.
238-241: LGTM!Map component translations support error and loading states.
257-261: LGTM!Team mode translations are concise and clear.
298-301: Translation keys properly added (past concern resolved).The past review concern about missing host_modal keys (generating, hide_id, show_id, copy_invite) has been addressed.
331-339: Presets translations complete (past concern resolved).The past review concern about missing
presets.select_placeholderand other preset keys has been addressed.src/client/SinglePlayerModal.ts (11)
35-53: Clean preset types.The type definitions follow the same pattern as HostLobbyModal, maintaining consistency.
57-68: LGTM!Using
@propertyinstead of@stateallows parent components to configure the modal externally.
93-123: Settings management is solid.The currentSettings/applySettings pattern matches HostLobbyModal with proper array spreading.
125-267: Component composition is clean.The render helpers properly integrate shared components with good event handling patterns.
268-301: LGTM!Main render structure matches HostLobbyModal with proper presets integration.
332-339: Random map selection is correct.The numeric filter prevents picking invalid string enum values, matching the pattern in HostLobbyModal.
341-348: Unit toggle logic is sound.The toggle correctly manages the disabled units array with duplicate prevention.
350-358: Bots event handling is correct.The pattern matches HostLobbyModal but appropriately omits the server call for single player.
360-379: Type-safe rule flag setting.The explicit switch is safer than dynamic property access. Note the absence of donate rules is correct for single player.
395-479: Start game flow is complete.The config assembly with HumansVsNations override (lines 463-470) follows the established pattern. The duplicate
disableNPCsat lines 452 and 469 is intentional for the override.
481-501: LGTM!Reset and backdrop handling are straightforward and correct.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/client/components/shared/GameModePicker.ts (1)
24-51: LGTM! Enum iteration fix successfully applied.The previous issue with Object.keys(GameMode) has been correctly resolved. The code now filters to numeric enum values only, preventing spurious buttons and ensuring numeric mode values are dispatched.
Optional: Add explicit button type to prevent implicit form submission.
For safety, consider adding
type="button"to prevent these buttons from implicitly submitting forms if this component is ever placed inside a form context:return html` <button + type="button" class=${`h-10 px-4 transition-colors ${
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
resources/lang/en.json(10 hunks)src/client/HostLobbyModal.ts(10 hunks)src/client/SinglePlayerModal.ts(4 hunks)src/client/components/Maps.ts(3 hunks)src/client/components/shared/GameModePicker.ts(1 hunks)src/client/components/shared/MapBrowserPane.ts(1 hunks)src/client/components/shared/PresetsManager.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (16)
📓 Common learnings
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1110
File: src/client/Main.ts:293-295
Timestamp: 2025-06-09T02:20:43.637Z
Learning: In src/client/Main.ts, during game start in the handleJoinLobby callback, UI elements are hidden using direct DOM manipulation with classList.add("hidden") for consistency. This includes modals, buttons, and error divs. The codebase follows this pattern rather than using component APIs for hiding elements during game transitions.
📚 Learning: 2025-06-09T02:20:43.637Z
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1110
File: src/client/Main.ts:293-295
Timestamp: 2025-06-09T02:20:43.637Z
Learning: In src/client/Main.ts, during game start in the handleJoinLobby callback, UI elements are hidden using direct DOM manipulation with classList.add("hidden") for consistency. This includes modals, buttons, and error divs. The codebase follows this pattern rather than using component APIs for hiding elements during game transitions.
Applied to files:
src/client/components/shared/GameModePicker.tssrc/client/components/shared/MapBrowserPane.tssrc/client/HostLobbyModal.tssrc/client/SinglePlayerModal.ts
📚 Learning: 2025-10-26T15:37:07.732Z
Learnt from: GlacialDrift
PR: openfrontio/OpenFrontIO#2298
File: src/client/graphics/layers/TerritoryLayer.ts:200-210
Timestamp: 2025-10-26T15:37:07.732Z
Learning: In GameImpl.ts lines 124-139, team assignment logic varies by number of teams: when numPlayerTeams < 8, teams are assigned ColoredTeams values (Red, Blue, Yellow, Green, Purple, Orange, Teal); when numPlayerTeams >= 8, teams are assigned generic string identifiers like "Team 1", "Team 2", etc., which are not members of ColoredTeams.
Applied to files:
src/client/components/shared/GameModePicker.tssrc/client/HostLobbyModal.ts
📚 Learning: 2025-10-20T20:15:28.858Z
Learnt from: sambokai
PR: openfrontio/OpenFrontIO#2225
File: src/core/execution/FakeHumanExecution.ts:51-51
Timestamp: 2025-10-20T20:15:28.858Z
Learning: In src/core/execution/FakeHumanExecution.ts, game balance constants like MIRV_COOLDOWN_TICKS, MIRV_HESITATION_ODDS, VICTORY_DENIAL_TEAM_THRESHOLD, VICTORY_DENIAL_INDIVIDUAL_THRESHOLD, and STEAMROLL_CITY_GAP_MULTIPLIER are experimental tuning parameters subject to frequent change during balance testing. Do not flag changes to these values as issues or compare them against previous values.
Applied to files:
src/client/components/shared/GameModePicker.tssrc/client/HostLobbyModal.tssrc/client/SinglePlayerModal.ts
📚 Learning: 2025-10-21T20:06:04.823Z
Learnt from: Saphereye
PR: openfrontio/OpenFrontIO#2233
File: src/client/HostLobbyModal.ts:891-891
Timestamp: 2025-10-21T20:06:04.823Z
Learning: For the HumansVsNations game mode in `src/client/HostLobbyModal.ts` and related files, the implementation strategy is to generate all nations and adjust their strength for balancing, rather than limiting lobby size based on the number of available nations on the map.
Applied to files:
src/client/components/shared/GameModePicker.tssrc/client/HostLobbyModal.tssrc/client/SinglePlayerModal.ts
📚 Learning: 2025-06-10T09:56:44.473Z
Learnt from: Ble4Ch
PR: openfrontio/OpenFrontIO#1063
File: src/core/configuration/PastelThemeDark.ts:53-53
Timestamp: 2025-06-10T09:56:44.473Z
Learning: In ColorAllocator class in src/core/configuration/Colors.ts, the correct method names are assignColor(id: string): Colord for general color assignment and assignTeamColor(team: Team): Colord for team-specific colors. There are no assignPlayerColor() or assignBotColor() methods.
Applied to files:
src/client/components/shared/GameModePicker.ts
📚 Learning: 2025-06-02T14:27:37.609Z
Learnt from: andrewNiziolek
PR: openfrontio/OpenFrontIO#1007
File: resources/lang/de.json:115-115
Timestamp: 2025-06-02T14:27:37.609Z
Learning: For OpenFrontIO project: When localization keys are renamed in language JSON files, the maintainers separate technical changes from translation content updates. They wait for community translators to update the actual translation values rather than attempting to translate in the same PR. This allows technical changes to proceed while ensuring accurate translations from native speakers.
Applied to files:
src/client/HostLobbyModal.tsresources/lang/en.json
📚 Learning: 2025-06-16T03:03:59.778Z
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1192
File: src/client/graphics/layers/RadialMenuElements.ts:312-314
Timestamp: 2025-06-16T03:03:59.778Z
Learning: In RadialMenuElements.ts, when fixing undefined params errors in subMenu functions, use explicit checks like `if (params === undefined || params.selected === null)` rather than optional chaining, as it makes the intent clearer and matches the specific error scenarios where params can be undefined during spawn phase operations.
Applied to files:
src/client/HostLobbyModal.ts
📚 Learning: 2025-10-27T09:47:26.395Z
Learnt from: sambokai
PR: openfrontio/OpenFrontIO#2225
File: src/core/execution/FakeHumanExecution.ts:770-795
Timestamp: 2025-10-27T09:47:26.395Z
Learning: In src/core/execution/FakeHumanExecution.ts, the selectSteamrollStopTarget() method intentionally allows MIRV targeting when secondHighest city count is 0 (e.g., nuclear endgame scenarios where structures are destroyed). This is valid game design—"if you can afford it, you're good to go"—and should not be flagged as requiring a guard condition.
Applied to files:
src/client/HostLobbyModal.ts
📚 Learning: 2025-08-21T21:42:45.277Z
Learnt from: plazmaezio
PR: openfrontio/OpenFrontIO#1883
File: src/client/HostLobbyModal.ts:227-245
Timestamp: 2025-08-21T21:42:45.277Z
Learning: In openfrontio/OpenFrontIO, the en.json file contains difficulty translation keys using the descriptive names ("Relaxed", "Balanced", "Intense", "Impossible") rather than the enum values ("Easy", "Medium", "Hard", "Impossible"). The DifficultyDescription mapping in src/client/components/Difficulties.ts correctly maps enum values to these descriptive translation keys.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-08-16T10:52:08.292Z
Learnt from: TheGiraffe3
PR: openfrontio/OpenFrontIO#884
File: resources/lang/en.json:456-461
Timestamp: 2025-08-16T10:52:08.292Z
Learning: In OpenFrontIO, translation files in resources/lang/*.json (except en.json) should not be updated in regular PRs. Only dedicated translation PRs titled "mls" and made by Aotumori should update non-English locale files. Regular PRs should only update en.json when adding or modifying translation keys.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-05-30T03:53:52.231Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#949
File: resources/lang/en.json:8-10
Timestamp: 2025-05-30T03:53:52.231Z
Learning: For the OpenFrontIO project, do not suggest updating translation files in resources/lang/*.json except for en.json. The project has a dedicated translation team that handles all other locale files.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-08-19T11:00:55.422Z
Learnt from: TheGiraffe3
PR: openfrontio/OpenFrontIO#1864
File: resources/maps/arabianpeninsula/manifest.json:13-170
Timestamp: 2025-08-19T11:00:55.422Z
Learning: In OpenFrontIO, nation names in map manifests are displayed directly in the UI without translation. They do not need to be added to resources/lang/en.json or processed through translateText(). This is the established pattern across all existing maps including Europe, World, Asia, Africa, and others.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-08-27T08:12:19.610Z
Learnt from: mokizzz
PR: openfrontio/OpenFrontIO#1940
File: resources/lang/en.json:763-766
Timestamp: 2025-08-27T08:12:19.610Z
Learning: In OpenFrontIO, some country entries in src/client/data/countries.json may have similar names but different codes (e.g., "Empire of Japan" vs "Empire of Japan1"). Each unique code requires its own translation key in resources/lang/en.json after normalization. Always verify against countries.json before suggesting removal of translation keys.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-07-12T06:35:10.457Z
Learnt from: Aotumuri
PR: openfrontio/OpenFrontIO#1357
File: resources/lang/ja.json:0-0
Timestamp: 2025-07-12T06:35:10.457Z
Learning: In OpenFrontIO project, "giantworldmap" is the correct localization key name for the giant world map, used consistently across all language files and TypeScript code. Do not suggest renaming this key.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-08-12T00:31:50.144Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#1752
File: src/core/game/Game.ts:750-752
Timestamp: 2025-08-12T00:31:50.144Z
Learning: In the OpenFrontIO codebase, changes to the PlayerInteraction interface (like adding canDonateGold and canDonateTroops flags) do not require corresponding updates to src/core/Schemas.ts or server serialization code.
Applied to files:
src/client/SinglePlayerModal.ts
🧬 Code graph analysis (6)
src/client/components/shared/GameModePicker.ts (3)
src/client/HostLobbyModal.ts (1)
customElement(75-766)src/client/SinglePlayerModal.ts (1)
customElement(53-502)src/client/components/shared/GameModeControls.ts (1)
customElement(6-34)
src/client/components/Maps.ts (2)
src/client/TerrainMapFileLoader.ts (1)
terrainMapFileLoader(4-4)src/client/Utils.ts (1)
translateText(92-147)
src/client/components/shared/MapBrowserPane.ts (4)
src/core/game/Game.ts (1)
mapCategories(108-147)src/client/HostLobbyModal.ts (1)
customElement(75-766)src/client/SinglePlayerModal.ts (1)
customElement(53-502)src/client/components/Maps.ts (1)
customElement(43-147)
src/client/HostLobbyModal.ts (4)
src/client/types/preset.ts (1)
Preset(2-8)src/core/Schemas.ts (2)
TeamCountConfig(155-155)ClientInfo(132-135)src/core/game/UserSettings.ts (1)
UserSettings(6-201)src/core/configuration/ConfigLoader.ts (1)
getServerConfigFromClient(28-45)
src/client/SinglePlayerModal.ts (2)
src/client/types/preset.ts (1)
Preset(2-8)src/client/utilities/RenderRulesOptions.ts (1)
RuleKey(4-9)
src/client/components/shared/PresetsManager.ts (5)
src/client/types/preset.ts (1)
Preset(2-8)src/client/HostLobbyModal.ts (1)
customElement(75-766)src/client/SinglePlayerModal.ts (1)
customElement(53-502)src/client/components/shared/PresetsBar.ts (1)
customElement(7-114)src/core/Util.ts (1)
generateID(245-251)
🔇 Additional comments (12)
resources/lang/en.json (1)
8-338: LGTM! All previously flagged translation keys have been added.Verification confirms:
- ✅
common.no_resultsadded (line 30)- ✅
host_modal.generatingadded (line 297)- ✅
host_modal.hide_id,show_id,copy_inviteadded (lines 298-300)- ✅
presets.select_placeholderadded (line 337)- ✅ JSON syntax errors corrected (comma after
map.browser)All translation keys referenced by the new UI components are now present.
src/client/components/Maps.ts (2)
52-98: Excellent race condition handling with load tokens.The token-based pattern correctly prevents stale async results from overwriting newer data when
mapKeychanges rapidly. Early returns whenmyToken !== this._loadTokenensure only the most recent load updates the component state.
117-141: LGTM! Alt text fallback properly implemented.The three-level fallback chain (
translation ?? mapName ?? translateText("common.untitled")) ensures the image always has meaningful alt text. This addresses the previous accessibility concern and provides a safe final fallback via translation.src/client/components/shared/MapBrowserPane.ts (1)
133-167: LGTM! Accessibility issues resolved.All previous concerns have been addressed:
- ✅ Single
role="listbox"on outer container (duplicate removed)- ✅ Single
role="option"per item (no duplication with map-display)- ✅ Keyboard navigation implemented (Enter/Space on items)
- ✅ Proper ARIA states (
aria-selected,aria-pressed,aria-multiselectable)- ✅ Explicit
type="button"on all buttonsThe component is now fully keyboard-accessible and screen-reader friendly.
src/client/components/shared/PresetsManager.ts (2)
25-35: LGTM! StorageKey warning properly implemented.The early guard now provides clear feedback when
storageKeyis missing, helping integrators catch configuration errors during development. The warning message is descriptive and the early return prevents silent failures.
37-70: Robust localStorage handling with proper validation.The implementation correctly:
- Guards against missing
storageKeyin both load and save paths- Validates preset structure on load (id, name, timestamps, settings)
- Handles JSON parse/stringify errors gracefully
- Auto-cleans when exceeding limit
- Maintains sorted order (most recent first)
src/client/HostLobbyModal.ts (3)
117-159: Well-structured preset support with proper type safety.The
currentSettings()andapplySettings()methods correctly capture and restore all lobby configuration. The typeHostLobbyPreset["settings"]ensures compile-time verification that all settings are included.
637-644: LGTM! Random map selection correctly handles numeric enums.The filter to
typeof v === "number"prevents selecting string keys from the enum. The defensive fallback to[GameMapType.World]ensures a valid map is always returned. This addresses the previous critical issue.
219-273: LGTM! Button labels properly localized.All previously hardcoded English strings now use
translateText():
- Hide/Show ID buttons
- Copy invite button
- "Copied" confirmation
Translation keys are present in
en.json(verified in earlier review).src/client/SinglePlayerModal.ts (3)
94-124: Well-structured preset system matching HostLobbyModal pattern.The
currentSettings()andapplySettings()methods maintain all single-player configuration. Type safety viaSinglePlayerPreset["settings"]ensures completeness. Implementation is consistent with the host lobby approach.
333-340: LGTM! Consistent random map selection with HostLobbyModal.Correctly filters numeric enum values and provides a defensive fallback. Matches the pattern used in HostLobbyModal for consistency across the codebase.
342-380: Clean rule and unit toggling logic.Both
toggleUnitandsetRuleFlagcorrectly translate UI events into state updates:
checked=truemeans enabled (remove fromdisabledUnits)checked=falsemeans disabled (add todisabledUnits)- Switch statement exhaustively covers all
RuleKeyvaluesSimpler than HostLobbyModal (no server sync) but follows the same semantic patterns.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
src/client/components/shared/DifficultyPicker.ts (1)
27-40: Filter numeric enum values to avoid rendering string keys.TypeScript numeric enums include both string keys and numeric values in
Object.values(). Without filtering, you'll render buttons for string keys like"Easy","Medium", etc., in addition to the numeric values.Apply this diff to filter to numeric values only:
<div class="flex flex-wrap gap-2"> - ${(Object.values(Difficulty) as Difficulty[]).map( + ${(Object.values(Difficulty).filter( + (v) => typeof v === "number", + ) as Difficulty[]).map( (v) => html`This matches the pattern used in GameModePicker.ts (lines 30-32).
src/client/components/shared/MapBrowserPane.ts (1)
143-161: Verify no duplicate role="option" in map-display.This wrapper has
role="option"and keyboard handling. If<map-display>also rendersrole="option"internally, screen readers will announce each tile twice.Check that
map-displaydoes not render its ownrole="option"oraria-selected:#!/bin/bash # Search for role="option" in MapDisplay component rg -nP 'role=.*option' src/client/components/Maps.tsIf found, remove the role/aria from either the wrapper or
map-displayto keep a single interactive element per option.src/client/components/shared/GameModePicker.ts (1)
25-29: Filter enum values before rendering buttons.
Object.values(GameMode)returns both named keys ("FFA","Team") and numeric values (0,1) for TypeScript numeric enums. Without filtering, extra buttons render with undefined labels, andpick()dispatches string values instead of numbers.Apply this diff:
- const modes = Object.values(GameMode) as GameMode[]; + const modes = Object.values(GameMode).filter( + (m): m is GameMode => typeof m === "number", + );
🧹 Nitpick comments (6)
src/client/components/shared/TeamCountPicker.ts (1)
11-12: Simplify array property declarations.For array properties with complex element types, use
attribute: falseto skip Lit's built-in attribute-to-array conversion and rely on property binding.Apply this diff:
- @property({ type: Array }) numbers: TeamCountConfig[] = [2, 3, 4, 5, 6, 7]; - @property({ type: Array }) named: TeamCountConfig[] = [Duos, Trios, Quads]; + @property({ attribute: false }) numbers: TeamCountConfig[] = [2, 3, 4, 5, 6, 7]; + @property({ attribute: false }) named: TeamCountConfig[] = [Duos, Trios, Quads];This makes the intent clearer: these properties are set via JavaScript, not HTML attributes.
src/client/components/shared/MapBrowserPane.ts (1)
33-36: Optimize key lookup in map filtering.The nested
find()inside the double loop runs for every map. Pre-build a reverse lookup map once to avoid repeated searches.Add this helper before
getFilteredMaps():private mapValueToKey = new Map( (Object.keys(GameMapType) as Array<keyof typeof GameMapType>).map((k) => [ GameMapType[k], k, ]), );Then simplify the loop:
- const key = ( - Object.keys(GameMapType) as Array<keyof typeof GameMapType> - ).find((k) => GameMapType[k] === mapValue) as keyof typeof GameMapType; + const key = this.mapValueToKey.get(mapValue)!;src/client/components/shared/PresetsManager.ts (1)
44-46: Consider stricter preset validation.The filter checks
item && item.settings !== undefinedbut doesn't validate the full preset structure (id, name, timestamps).Add a type guard for more robust validation:
const isValidPreset = (item: any): item is GenericPreset<T> => item && typeof item.id === "string" && typeof item.name === "string" && typeof item.createdAt === "number" && typeof item.updatedAt === "number" && item.settings !== undefined; // Then use it: const arr = parsed.filter(isValidPreset);This prevents corrupted presets from breaking the UI.
src/client/SinglePlayerModal.ts (3)
2-2: Consider using @State for internal configuration instead of @Property.The fields like
selectedMap,selectedDifficulty,bots, etc. are internal modal state managed by user interactions, not attributes set by a parent component. Using@propertyunnecessarily exposes them as public attributes and creates reflected attributes in the DOM. Since this modal is self-contained and manages its own configuration,@statewould better encapsulate this internal state.Apply this pattern:
-@property({ type: String }) selectedMap: GameMapType = GameMapType.World; -@property({ type: String }) selectedDifficulty: Difficulty = Difficulty.Medium; -@property({ type: Boolean }) disableNPCs = false; -@property({ type: Number }) bots = 400; +@state() private selectedMap: GameMapType = GameMapType.World; +@state() private selectedDifficulty: Difficulty = Difficulty.Medium; +@state() private disableNPCs = false; +@state() private bots = 400;(Apply the same pattern to the other configuration properties.)
Also applies to: 55-70
343-350: Simplify the toggleUnit logic.The nested ternary is hard to follow. Here's a clearer approach:
private toggleUnit = (unit: UnitType, checked: boolean): void => { - // checked=true means the unit is enabled, so ensure it's NOT in disabledUnits - this.disabledUnits = checked - ? this.disabledUnits.filter((u) => u !== unit) - : this.disabledUnits.includes(unit) - ? this.disabledUnits - : [...this.disabledUnits, unit]; + if (checked) { + // Unit enabled: remove from disabled list + this.disabledUnits = this.disabledUnits.filter((u) => u !== unit); + } else if (!this.disabledUnits.includes(unit)) { + // Unit disabled: add to disabled list + this.disabledUnits = [...this.disabledUnits, unit]; + } };
363-381: Consider using object lookup instead of switch statement.The switch statement is verbose. An object lookup would be more concise and easier to maintain:
-private setRuleFlag(key: RuleKey, checked: boolean) { - switch (key) { - case "disableNPCs": - this.disableNPCs = checked; - break; - case "instantBuild": - this.instantBuild = checked; - break; - case "infiniteGold": - this.infiniteGold = checked; - break; - case "infiniteTroops": - this.infiniteTroops = checked; - break; - case "compactMap": - this.compactMap = checked; - break; - } -} +private setRuleFlag(key: RuleKey, checked: boolean) { + const ruleMap: Record<RuleKey, keyof this> = { + disableNPCs: 'disableNPCs', + instantBuild: 'instantBuild', + infiniteGold: 'infiniteGold', + infiniteTroops: 'infiniteTroops', + compactMap: 'compactMap', + }; + const prop = ruleMap[key]; + if (prop && prop in this) { + (this as any)[prop] = checked; + } +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/client/SinglePlayerModal.ts(4 hunks)src/client/components/shared/DifficultyControls.ts(1 hunks)src/client/components/shared/DifficultyPicker.ts(1 hunks)src/client/components/shared/GameModeControls.ts(1 hunks)src/client/components/shared/GameModePicker.ts(1 hunks)src/client/components/shared/MapBrowserPane.ts(1 hunks)src/client/components/shared/PresetsManager.ts(1 hunks)src/client/components/shared/SettingsSummary.ts(1 hunks)src/client/components/shared/TeamCountPicker.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/client/components/shared/DifficultyControls.ts
- src/client/components/shared/SettingsSummary.ts
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1110
File: src/client/Main.ts:293-295
Timestamp: 2025-06-09T02:20:43.637Z
Learning: In src/client/Main.ts, during game start in the handleJoinLobby callback, UI elements are hidden using direct DOM manipulation with classList.add("hidden") for consistency. This includes modals, buttons, and error divs. The codebase follows this pattern rather than using component APIs for hiding elements during game transitions.
📚 Learning: 2025-08-21T21:42:45.277Z
Learnt from: plazmaezio
PR: openfrontio/OpenFrontIO#1883
File: src/client/HostLobbyModal.ts:227-245
Timestamp: 2025-08-21T21:42:45.277Z
Learning: In openfrontio/OpenFrontIO, the en.json file contains difficulty translation keys using the descriptive names ("Relaxed", "Balanced", "Intense", "Impossible") rather than the enum values ("Easy", "Medium", "Hard", "Impossible"). The DifficultyDescription mapping in src/client/components/Difficulties.ts correctly maps enum values to these descriptive translation keys.
Applied to files:
src/client/components/shared/DifficultyPicker.ts
📚 Learning: 2025-10-26T15:37:07.732Z
Learnt from: GlacialDrift
PR: openfrontio/OpenFrontIO#2298
File: src/client/graphics/layers/TerritoryLayer.ts:200-210
Timestamp: 2025-10-26T15:37:07.732Z
Learning: In GameImpl.ts lines 124-139, team assignment logic varies by number of teams: when numPlayerTeams < 8, teams are assigned ColoredTeams values (Red, Blue, Yellow, Green, Purple, Orange, Teal); when numPlayerTeams >= 8, teams are assigned generic string identifiers like "Team 1", "Team 2", etc., which are not members of ColoredTeams.
Applied to files:
src/client/components/shared/DifficultyPicker.tssrc/client/components/shared/GameModePicker.tssrc/client/components/shared/TeamCountPicker.ts
📚 Learning: 2025-06-09T02:20:43.637Z
Learnt from: VariableVince
PR: openfrontio/OpenFrontIO#1110
File: src/client/Main.ts:293-295
Timestamp: 2025-06-09T02:20:43.637Z
Learning: In src/client/Main.ts, during game start in the handleJoinLobby callback, UI elements are hidden using direct DOM manipulation with classList.add("hidden") for consistency. This includes modals, buttons, and error divs. The codebase follows this pattern rather than using component APIs for hiding elements during game transitions.
Applied to files:
src/client/components/shared/GameModePicker.tssrc/client/SinglePlayerModal.tssrc/client/components/shared/MapBrowserPane.tssrc/client/components/shared/GameModeControls.ts
📚 Learning: 2025-10-20T20:15:28.858Z
Learnt from: sambokai
PR: openfrontio/OpenFrontIO#2225
File: src/core/execution/FakeHumanExecution.ts:51-51
Timestamp: 2025-10-20T20:15:28.858Z
Learning: In src/core/execution/FakeHumanExecution.ts, game balance constants like MIRV_COOLDOWN_TICKS, MIRV_HESITATION_ODDS, VICTORY_DENIAL_TEAM_THRESHOLD, VICTORY_DENIAL_INDIVIDUAL_THRESHOLD, and STEAMROLL_CITY_GAP_MULTIPLIER are experimental tuning parameters subject to frequent change during balance testing. Do not flag changes to these values as issues or compare them against previous values.
Applied to files:
src/client/components/shared/GameModePicker.tssrc/client/SinglePlayerModal.ts
📚 Learning: 2025-10-21T20:06:04.823Z
Learnt from: Saphereye
PR: openfrontio/OpenFrontIO#2233
File: src/client/HostLobbyModal.ts:891-891
Timestamp: 2025-10-21T20:06:04.823Z
Learning: For the HumansVsNations game mode in `src/client/HostLobbyModal.ts` and related files, the implementation strategy is to generate all nations and adjust their strength for balancing, rather than limiting lobby size based on the number of available nations on the map.
Applied to files:
src/client/components/shared/GameModePicker.tssrc/client/SinglePlayerModal.ts
📚 Learning: 2025-08-12T00:31:50.144Z
Learnt from: scottanderson
PR: openfrontio/OpenFrontIO#1752
File: src/core/game/Game.ts:750-752
Timestamp: 2025-08-12T00:31:50.144Z
Learning: In the OpenFrontIO codebase, changes to the PlayerInteraction interface (like adding canDonateGold and canDonateTroops flags) do not require corresponding updates to src/core/Schemas.ts or server serialization code.
Applied to files:
src/client/SinglePlayerModal.ts
🧬 Code graph analysis (7)
src/client/components/shared/DifficultyPicker.ts (1)
src/client/components/shared/DifficultyControls.ts (1)
customElement(6-43)
src/client/components/shared/GameModePicker.ts (3)
src/client/SinglePlayerModal.ts (1)
customElement(53-503)src/client/components/shared/GameModeControls.ts (1)
customElement(6-34)src/client/HostLobbyModal.ts (1)
customElement(75-766)
src/client/components/shared/TeamCountPicker.ts (2)
src/core/Schemas.ts (1)
TeamCountConfig(155-155)src/core/game/Game.ts (3)
Duos(53-53)Trios(54-54)Quads(55-55)
src/client/components/shared/PresetsManager.ts (5)
src/client/types/preset.ts (1)
Preset(2-8)src/client/SinglePlayerModal.ts (1)
customElement(53-503)src/client/components/shared/PresetsBar.ts (1)
customElement(7-114)src/client/HostLobbyModal.ts (1)
customElement(75-766)src/core/Util.ts (1)
generateID(245-251)
src/client/SinglePlayerModal.ts (10)
src/core/Schemas.ts (1)
TeamCountConfig(155-155)src/client/types/preset.ts (1)
Preset(2-8)src/client/components/shared/DifficultyControls.ts (1)
customElement(6-43)src/client/components/shared/DifficultyPicker.ts (1)
customElement(6-44)src/client/components/shared/GameModePicker.ts (1)
customElement(6-49)src/client/components/shared/MapBrowserPane.ts (1)
customElement(8-171)src/client/components/shared/PresetsManager.ts (1)
customElement(11-181)src/client/components/shared/AdvancedOptions.ts (1)
customElement(18-140)src/client/FlagInput.ts (1)
customElement(9-116)src/client/utilities/RenderRulesOptions.ts (1)
RuleKey(4-9)
src/client/components/shared/MapBrowserPane.ts (4)
src/core/game/Game.ts (1)
mapCategories(108-147)src/client/SinglePlayerModal.ts (1)
customElement(53-503)src/client/components/Maps.ts (1)
customElement(43-147)src/client/HostLobbyModal.ts (1)
customElement(75-766)
src/client/components/shared/GameModeControls.ts (3)
src/client/SinglePlayerModal.ts (1)
customElement(53-503)src/client/components/shared/GameModePicker.ts (1)
customElement(6-49)src/client/HostLobbyModal.ts (1)
customElement(75-766)
🔇 Additional comments (8)
src/client/components/shared/PresetsManager.ts (1)
113-124: LGTM!Clean preset creation logic with proper timestamp tracking, sorting, and persistence. The limit check prevents unbounded growth.
src/client/SinglePlayerModal.ts (7)
33-51: LGTM!The preset types and constants are well-defined and align with the PresetsManager integration.
95-125: LGTM!The preset helper methods correctly capture and restore all configuration state, with proper array cloning to prevent mutation.
270-303: LGTM!The dialog structure follows accessibility best practices with proper ARIA attributes, and the backdrop-click-to-close pattern is correctly implemented. The PresetsManager integration is properly wired.
305-315: LGTM!The light DOM rendering and direct style manipulation for modal visibility align with codebase patterns.
Based on learnings.
334-341: LGTM!The random map selection correctly filters numeric enum values with a safe fallback.
352-360: LGTM!The dual event handling and value clamping correctly manage bot count input from different sources.
257-268: No issues found. Code structure for scrolling is correct.The CSS properties for overflow and scrolling are properly configured:
- Main element has
flex-1 min-h-0 overflow-auto(correct flex pattern)- Header and settings pane use
min-h-0to allow shrinking- MapBrowserPane inner list has
overflow-autofor scrolling- No height constraints block scrolling
The conditional rendering at line 264 does not affect layout. However, on mobile devices with small viewports, space may be tight due to
inset-4margins andmin-h-[560px]minimum height on the section. If users report scrolling issues, verify the actual viewport height available after these constraints on target devices, as the code structure itself is sound.
|
|
||
| @customElement("difficulty-picker") | ||
| export class DifficultyPicker extends LitElement { | ||
| @property({ type: String }) value: Difficulty = Difficulty.Medium; |
There was a problem hiding this comment.
Fix property decorator type for numeric enum.
The @property({ type: String }) decorator is incorrect for Difficulty, which is a numeric enum. Lit uses the type option for built-in conversion, and String will coerce the numeric value to a string, breaking comparisons.
Apply this diff:
- @property({ type: String }) value: Difficulty = Difficulty.Medium;
+ @property({ type: Number }) value: Difficulty = Difficulty.Medium;🤖 Prompt for AI Agents
In src/client/components/shared/DifficultyPicker.ts around line 8, the @property
decorator uses { type: String } for the numeric Difficulty enum which coerces
values to strings and breaks comparisons; change the decorator to use { type:
Number } (or remove the type option to avoid coercion) so the numeric enum value
is preserved and comparisons work correctly.
|
|
||
| @customElement("game-mode-controls") | ||
| export class GameModeControls extends LitElement { | ||
| @property({ type: String }) value: GameMode = GameMode.FFA; |
There was a problem hiding this comment.
Fix property decorator type for numeric enum.
GameMode is a numeric enum, but @property({ type: String }) will coerce values to strings, breaking equality checks in the nested game-mode-picker.
Apply this diff:
- @property({ type: String }) value: GameMode = GameMode.FFA;
+ @property({ type: Number }) value: GameMode = GameMode.FFA;📝 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.
| @property({ type: String }) value: GameMode = GameMode.FFA; | |
| @property({ type: Number }) value: GameMode = GameMode.FFA; |
🤖 Prompt for AI Agents
In src/client/components/shared/GameModeControls.ts around line 8, the @property
decorator is using type: String which coerces the numeric GameMode enum to a
string and breaks comparisons; change the decorator to use type: Number so the
value remains a numeric enum (keep the default GameMode.FFA) and ensure any
attribute/serialization behavior aligns with numeric values.
|
|
||
| @customElement("game-mode-picker") | ||
| export class GameModePicker extends LitElement { | ||
| @property({ type: String }) value: GameMode = GameMode.FFA; |
There was a problem hiding this comment.
Fix property decorator type for numeric enum.
GameMode is a numeric enum, but @property({ type: String }) will coerce the numeric value to a string, breaking comparisons in line 37.
Apply this diff:
- @property({ type: String }) value: GameMode = GameMode.FFA;
+ @property({ type: Number }) value: GameMode = GameMode.FFA;📝 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.
| @property({ type: String }) value: GameMode = GameMode.FFA; | |
| @property({ type: Number }) value: GameMode = GameMode.FFA; |
🤖 Prompt for AI Agents
In src/client/components/shared/GameModePicker.ts around line 8, the @property
decorator is using type: String for GameMode which is a numeric enum; this
causes the value to be coerced to a string and breaks enum comparisons (e.g.
around line 37). Change the decorator to use type: Number so the property stays
a numeric enum value (keep the default GameMode.FFA) and ensure any
bindings/reads treat it as a number.
| @customElement("team-count-picker") | ||
| export class TeamCountPicker extends LitElement { | ||
| @property({ type: String }) mode: GameMode = GameMode.FFA; | ||
| @property({ type: Number }) value: TeamCountConfig = 2; |
There was a problem hiding this comment.
Fix property decorator type for union type.
TeamCountConfig is a union type (number | "Duos" | "Trios" | "Quads"), but @property({ type: Number }) will only convert string attributes to numbers, breaking string values like "Duos".
Apply this diff:
- @property({ type: Number }) value: TeamCountConfig = 2;
+ @property({ attribute: false }) value: TeamCountConfig = 2;Using attribute: false prevents automatic conversion and accepts the value directly via property binding (.value=${...}), which is how the component is used in SinglePlayerModal.ts and HostLobbyModal.ts.
📝 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.
| @property({ type: Number }) value: TeamCountConfig = 2; | |
| @property({ attribute: false }) value: TeamCountConfig = 2; |
🤖 Prompt for AI Agents
In src/client/components/shared/TeamCountPicker.ts around line 10, the property
decorator uses type: Number which forces attribute-to-property conversion and
breaks string union values like "Duos"/"Trios"/"Quads"; change the decorator to
use attribute: false so the property is assigned directly via JS property
binding (e.g. .value=...) and will accept the union type TeamCountConfig as-is
while keeping the default value of 2.
4 participants
Description:
Screenshots:
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
abodcraft1