Open
Conversation
|
|
Contributor
WalkthroughAdds a lobby presets feature: client-side preset store, UI component, integration into host and single-player modals using patch-based GameConfig flows, form-state utilities, server default config factories, tests, and a test storage shim. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Modal as HostLobbyModal / SinglePlayerModal
participant UI as LobbyPresetControls
participant Store as LobbyPresets
participant Storage as localStorage
participant Server as GameServer
User->>Modal: open modal
Modal->>Store: listPresets()
Store->>Storage: read LOBBY_PRESET_STORAGE_KEY
Storage-->>Store: JSON
Store-->>Modal: presets[]
Modal->>UI: render presets selector
User->>UI: select preset
UI->>Store: get preset by id
Store-->>UI: preset
UI-->>Modal: apply preset id
Modal->>Modal: buildGameConfigPatch()
Modal->>Modal: applyGameConfigPatch(patch)
User->>UI: save preset (name)
UI->>Modal: getConfigPatch()
Modal-->>UI: patch
UI->>Store: upsertPreset(name, config)
Store->>Storage: save LOBBY_PRESET_STORAGE_KEY
User->>Modal: create/start game
Modal->>Modal: buildFullGameConfig()
Modal->>Server: POST /createLobby (body: GameConfig)
Server-->>Modal: GameInfo
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
MitchZinck
force-pushed
the
2566-lobby-presets
branch
from
a19e429 to
94ca6cc
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/client/HostLobbyModal.ts`:
- Around line 1217-1229: In syncPresetsFromStore, avoid pre-selecting last-used
preset when auto-apply is off: set this.autoApplyLastUsedPreset from
store.autoApplyLastUsed first, then compute selectionId so it is
preferredSelectionId if provided, otherwise use store.lastUsedPresetId only when
store.autoApplyLastUsed is true; otherwise leave selectionId undefined. Update
assignment to this.selectedPresetId and this.presetNameInput to use that
selectionId/selectedPreset logic (references: syncPresetsFromStore,
autoApplyLastUsedPreset, preferredSelectionId, store.lastUsedPresetId,
selectedPresetId, presetNameInput).
github-project-automation
bot
moved this from Triage
to Development
in OpenFront Release Management
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/client/HostLobbyModal.ts`:
- Around line 1157-1165: When the preset lookup fails in HostLobbyModal
(loadLobbyPresetStore + preset lookup using this.selectedPresetId), clear the
stale UI state: set this.selectedPresetId = undefined (already done) and also
clear any "last-used" preset state and input control bound to the preset
selection (e.g., reset lastUsedPresetId or the form input controlling the
preset) before returning; keep the existing showMessage(translateText(...),
"red") behavior so the user is notified and the UI remains consistent.
🧹 Nitpick comments (3)
src/client/LobbyPresets.ts (2)
31-51: Clear invalid stored data so future loads recover cleanly.If JSON/schema parsing fails, the bad payload stays and every load re-parses. Removing the key once avoids repeated failures and allows clean recovery.
♻️ Suggested tweak
try { const parsed = JSON.parse(raw); const result = LobbyPresetStoreSchema.safeParse(parsed); if (result.success) { return result.data; } + localStorage.removeItem(LOBBY_PRESET_STORAGE_KEY); } catch { + localStorage.removeItem(LOBBY_PRESET_STORAGE_KEY); return emptyLobbyPresetStore(); }
71-72: Return a copy to avoid accidental mutation.Returning the internal array lets callers mutate it without saving, which can create confusing state.
♻️ Suggested tweak
export function listPresets(): LobbyPreset[] { - return loadLobbyPresetStore().presets; + return [...loadLobbyPresetStore().presets]; }tests/client/LobbyPresets.test.ts (1)
1-13: Avoid hardcoding the storage key in tests.Using the shared constant prevents drift if the key changes. This needs the constant exported from LobbyPresets.
♻️ Suggested tweak
import { deletePreset, loadLobbyPresetStore, setAutoApplyLastUsed, setLastUsedPresetId, upsertPreset, + LOBBY_PRESET_STORAGE_KEY, } from "../../src/client/LobbyPresets"; @@ - localStorage.removeItem("lobbyPresets.v1"); + localStorage.removeItem(LOBBY_PRESET_STORAGE_KEY);
evanpelle
reviewed
Jan 30, 2026
src/client/HostLobbyModal.ts
Outdated
| } | ||
|
|
||
| private async putGameConfig() { | ||
| private async handlePresetSelectionChange(e: Event) { |
Collaborator
There was a problem hiding this comment.
can the preset logic be moved into it's own lit element in client/components. Then it can be reused in singleplayer modal as well
src/client/HostLobbyModal.ts
Outdated
| import { renderUnitTypeOptions } from "./utilities/RenderUnitTypeOptions"; | ||
| import randomMap from "/images/RandomMap.webp?url"; | ||
|
|
||
| const DEFAULT_PRIVATE_GAME_CONFIG: GameConfig = { |
Collaborator
There was a problem hiding this comment.
Maybe all the game config storage/handling could go in its own file? One issue is that adding an new config option requires changing the code in many different places. I think if we had one registry that could make things easier.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/client/components/LobbyPresetControls.ts`:
- Around line 102-121: handlePresetSaveClick currently passes a config patch
with undefined fields which JSON.stringify will drop, so disabled toggle fields
(maxTimerValue, goldMultiplier, startingGold, spawnImmunityDuration) get lost;
before calling upsertPreset replace undefined toggle values with explicit nulls
(e.g. map any toggle-off fields to null) so the preset persists the "disabled"
intent, and update the preset schema/types and the consumers
(applyCommonGameConfigPatch / applyHostLobbyGameConfigPatch) to treat null as
the disabled sentinel when applying patches. Ensure you update upsertPreset
usage and any type definitions so stored presets can contain null for those
fields and the apply* functions check for field presence and null to clear
toggles.
61d0696 to
a9967c2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
If this PR fixes an issue, link it below. If not, delete these two lines.
Resolves #2566
Description:
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
mitchfz