-
Notifications
You must be signed in to change notification settings - Fork 789
feat: Lobby presets #3045
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: Lobby presets #3045
Conversation
WalkthroughAdds a lobby presets feature: client-side preset store with CRUD and auto-apply, host modal UI and handlers for saving/loading/deleting presets, tests and a storage shim for testing, plus a server default change (game difficulty set to Easy). Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Modal as HostLobbyModal
participant Presets as LobbyPresets
participant Storage as localStorage
participant Server as GameServer
User->>Modal: Open host modal
Modal->>Presets: listPresets()
Presets->>Storage: read LOBBY_PRESET_STORAGE_KEY
Storage-->>Presets: stored JSON
Presets-->>Modal: presets array
Modal->>Modal: render preset selector
User->>Modal: Select preset
Modal->>Presets: get preset by id
Presets-->>Modal: preset config
Modal->>Modal: applyGameConfigPatch(config)
User->>Modal: Save preset (name)
Modal->>Modal: buildGameConfigPatch()
Modal->>Presets: upsertPreset(name, config)
Presets->>Storage: saveLobbyPresetStore()
Storage-->>Presets: persisted
Presets-->>Modal: saved preset
User->>Modal: Create lobby
Modal->>Modal: buildFullGameConfig()
Modal->>Server: POST /createLobby with gameConfig
Server-->>Modal: game created
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 |
a19e429 to
94ca6cc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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);
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