Feature/reuse private lobby#4536
Conversation
When a private game ends, the host's win modal now shows a "New lobby"
button. Clicking it lets the whole group hop to a fresh private lobby
without anyone re-sharing a link: the server spins up a successor lobby,
broadcasts its id to everyone still connected, non-hosts get a one-click
"Join" banner, and the host lands back in the host view.
Design decision — the game server creates and broadcasts the successor,
not the host's client:
The finished game's server mints the successor lobby (reusing the game's
stored creatorPersistentID) and broadcasts the new game id to every
connected client. Because the server is the one that creates the lobby,
the id everyone is redirected to is authoritative and verified: it is a
real lobby the server just created for the authenticated lobby creator,
not an arbitrary id a client could inject. The alternative (the host's
browser calls /api/create_game, then asks the server to relay the id)
would have the server broadcasting an unverified id it never created.
The server also authorizes the request — only the client whose clientID
matches the lobby creator may trigger it — and is idempotent: a repeat
click re-broadcasts the same successor instead of spawning another lobby.
Supporting decisions:
- The successor is a brand-new lobby (a GameServer runs exactly one game),
created with default settings and the same creator; the host reconfigures
in the host view.
- The successor factory is injected only on the private /api/create_game
path (Worker.ts), so only private games can spawn successors — the
feature is inherently private-only.
- The host returns via a `?host` URL flag + reload into HostLobbyModal
attach mode (mirrors the win modal's existing Requeue full-navigation),
which cleanly tears down the finished game. Non-hosts get a dismissible
banner surfaced independently of the win modal, so it still works after
they close it to keep spectating.
Two new Zod wire messages in src/core/Schemas.ts (client create_next_lobby,
server new_lobby) carry the request and the broadcast. Tests cover the schema
round-trips, the server's authorization / broadcast / idempotency against a
real GameServer, and the win-modal button gating.
Resolves openfrontio#4476
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Follow-ups to the reuse-private-lobby feature (openfrontio#4476): - Fix successor lobbies not chaining past one generation. A spawned successor never received its own createSuccessorLobby factory, so the second "New lobby" click (in game 2) did nothing and the button stayed greyed out. Extract the factory into a recursive, dependency-injected helper (SuccessorLobby.ts) so every successor can spawn the next, preserving the same creator, indefinitely. Covered by SuccessorLobby.test.ts (three generations plus the failure paths). - Add an in-game "New lobby" button to the top-right bar, next to the pause button, so the host can start a fresh lobby without waiting to die or for the game to end. Shown only to the lobby creator of a private game; it emits the same create_next_lobby flow as the win-screen button. - Guard that button with a confirmation (matching the adjacent Exit button) so a stray click next to pause/exit doesn't pull everyone into a new lobby mid-game. - Move the non-host "new lobby" join prompt to the top of the screen. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The in-game "New lobby" button next to the pause controls now lets the host reuse the lobby at any time, so the win-screen button (which only appeared once the game ended or the host died) is redundant. Remove it along with its supporting state, getter and handler. The create_next_lobby flow is unchanged and the pause-bar button is now the sole entry point. The win_modal.new_lobby string stays — it's reused as the pause-bar button's tooltip. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a private successor-lobby flow. Hosts can create a new lobby through the sidebar, the server stores and broadcasts its ID, and clients either reattach as host or receive a prompt to join. ChangesSuccessor lobby reuse
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Host
participant GameRightSidebar
participant Worker
participant GameServer
participant ClientGameRunner
participant NewLobbyPrompt
Host->>GameRightSidebar: confirm new lobby
GameRightSidebar->>Worker: request successor lobby
Worker->>GameServer: validate and create successor
GameServer->>ClientGameRunner: broadcast new_lobby
ClientGameRunner->>NewLobbyPrompt: emit NewLobbyEvent
NewLobbyPrompt->>Host: redirect or show join prompt
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/server/Worker.ts (1)
175-179: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate id-minting logic.
The mint-id-with-warn-on-failure logic at Line 175-179 is repeated verbatim inside the
mintIdcallback (Lines 193-198). Extracting a small helper would remove the duplication.♻️ Proposed extraction
+ const mintWorkerGameId = (): GameID | null => { + const newId = ServerEnv.generateGameIdForWorker(workerId); + if (newId === null) { + log.warn(`Failed to mint game id on worker ${workerId}`); + } + return newId; + }; + app.post("/api/create_game", async (req, res) => { ... - const id = ServerEnv.generateGameIdForWorker(workerId); - if (id === null) { - log.warn(`Failed to mint game id on worker ${workerId}`); - return res.status(500).json({ error: "Could not allocate game id" }); - } + const id = mintWorkerGameId(); + if (id === null) { + return res.status(500).json({ error: "Could not allocate game id" }); + } ... wireSuccessorLobby(game, creatorPersistentID, { - mintId: () => { - const successorId = ServerEnv.generateGameIdForWorker(workerId); - if (successorId === null) { - log.warn(`Failed to mint successor game id on worker ${workerId}`); - } - return successorId; - }, + mintId: mintWorkerGameId, createGame: (successorId, creator) => gm.createGame(successorId, undefined, creator), });Also applies to: 187-203
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/Worker.ts` around lines 175 - 179, The game-id minting and warning/500 response logic is duplicated in the Worker request handler and the mintId callback; extract that repeated flow into a small helper so both call sites reuse the same behavior. Use the existing ServerEnv.generateGameIdForWorker, log.warn, and res.status(500).json paths as the shared implementation, and have both the main handler and mintId callback delegate to it instead of inlining the same null-check and response logic.src/server/GameServer.ts (1)
136-147: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider an explicit
isPublic()guard as defense-in-depth.The private-only restriction currently relies entirely on
createSuccessorLobbybeing left unwired for public games (Worker.ts). If wiring ever changes or another creation path forgets to skip public games,handleCreateNextLobbywould silently allow it. An explicit early-return here would make the invariant self-enforcing withinGameServeritself, matching how other intents (e.g.update_game_config,toggle_game_start_timer) explicitly rejectisPublic().🛡️ Proposed defense-in-depth check
private handleCreateNextLobby(client: Client) { + if (this.isPublic()) { + this.log.warn("create_next_lobby not allowed on public games, ignoring"); + return; + } if (client.clientID !== this.lobbyCreatorID) {Also applies to: 836-874
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/GameServer.ts` around lines 136 - 147, Add an explicit early `isPublic()` check in `GameServer.handleCreateNextLobby` before any successor lobby is created or reused. Right now the private-only behavior depends on `createSuccessorLobby` being unwired for public games, so make the invariant self-enforcing by rejecting public games inside `GameServer` itself, similar to the existing guards used by other intents like `update_game_config` and `toggle_game_start_timer`. Keep the existing `successorLobbyId` reuse behavior for private games unchanged.src/client/HostLobbyModal.ts (1)
562-587: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated lobby-binding logic vs. the
createLobby()success path.
attachToExistingLobbyre-implements the same show-invite-button / build-url / update-history / copy-link / dispatch-join-lobby sequence as thecreateLobby().then()handler above (Lines 526-538). Extracting a shared helper would remove the duplication and keep both paths in sync going forward.♻️ Proposed refactor
- private async attachToExistingLobby(lobbyId: string): Promise<void> { - if (!isValidGameID(lobbyId)) { - throw new Error(`Invalid lobby ID format: ${lobbyId}`); - } - this.lobbyId = lobbyId; - crazyGamesSDK.showInviteButton(this.lobbyId); - - const url = await this.constructUrl(); - this.updateLobbyHistory(url); - await this.updateComplete; - void (this.querySelector("copy-button") as CopyButton)?.handleCopy(); - - this.dispatchEvent( - new CustomEvent("join-lobby", { - detail: { - gameID: this.lobbyId, - source: "host", - } as JoinLobbyEvent, - bubbles: true, - composed: true, - }), - ); - } + private async attachToExistingLobby(lobbyId: string): Promise<void> { + if (!isValidGameID(lobbyId)) { + throw new Error(`Invalid lobby ID format: ${lobbyId}`); + } + await this.bindLobbyAndAnnounce(lobbyId); + } + + // Shared by createLobby()'s success path and attachToExistingLobby(): binds + // the modal to a lobby id, copies the share link, and tells Main.ts to join. + private async bindLobbyAndAnnounce(lobbyId: string): Promise<void> { + this.lobbyId = lobbyId; + crazyGamesSDK.showInviteButton(this.lobbyId); + + const url = await this.constructUrl(); + this.updateLobbyHistory(url); + await this.updateComplete; + void (this.querySelector("copy-button") as CopyButton)?.handleCopy(); + + this.dispatchEvent( + new CustomEvent("join-lobby", { + detail: { + gameID: this.lobbyId, + source: "host", + } as JoinLobbyEvent, + bubbles: true, + composed: true, + }), + ); + }The
createLobby()handler (Lines 525-539) can then callawait this.bindLobbyAndAnnounce(this.lobbyId)instead of repeating the body inline.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/client/HostLobbyModal.ts` around lines 562 - 587, `attachToExistingLobby` duplicates the same lobby-binding and announcement flow used by the `createLobby()` success handler. Extract the shared sequence into a helper such as `bindLobbyAndAnnounce` on `HostLobbyModal`, and have both `attachToExistingLobby` and the `createLobby()` success path call it so the show-invite-button, URL/history update, copy-button, and join-lobby dispatch logic stay in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/client/HostLobbyModal.ts`:
- Around line 507-520: The existingLobbyId branch in HostLobbyModal.onOpen calls
attachToExistingLobby with void, so an invalid id can reject silently without
the same failure handling used by createLobby(). Update the onOpen flow to
handle the async result from attachToExistingLobby explicitly, adding a catch
path that matches the existing createLobby error handling (including clipboard
cleanup and user-facing failure handling) so bad lobby ids do not create an
unhandled promise rejection.
In `@src/client/hud/layers/GameRightSidebar.ts`:
- Around line 205-218: The new lobby flow in onNewLobbyButtonClick can leave
newLobbyRequested stuck true if SendCreateNextLobbyEvent never leads to a
new_lobby broadcast, so add a timeout-based fallback in GameRightSidebar to
reset the request state and refresh the UI when confirmation does not arrive.
Use the existing onNewLobbyButtonClick path and newLobbyRequested/requestUpdate
handling to schedule a recovery reset, and ensure the fallback is cleared if the
success broadcast arrives first so the host does not remain permanently
disabled.
---
Nitpick comments:
In `@src/client/HostLobbyModal.ts`:
- Around line 562-587: `attachToExistingLobby` duplicates the same lobby-binding
and announcement flow used by the `createLobby()` success handler. Extract the
shared sequence into a helper such as `bindLobbyAndAnnounce` on
`HostLobbyModal`, and have both `attachToExistingLobby` and the `createLobby()`
success path call it so the show-invite-button, URL/history update, copy-button,
and join-lobby dispatch logic stay in one place.
In `@src/server/GameServer.ts`:
- Around line 136-147: Add an explicit early `isPublic()` check in
`GameServer.handleCreateNextLobby` before any successor lobby is created or
reused. Right now the private-only behavior depends on `createSuccessorLobby`
being unwired for public games, so make the invariant self-enforcing by
rejecting public games inside `GameServer` itself, similar to the existing
guards used by other intents like `update_game_config` and
`toggle_game_start_timer`. Keep the existing `successorLobbyId` reuse behavior
for private games unchanged.
In `@src/server/Worker.ts`:
- Around line 175-179: The game-id minting and warning/500 response logic is
duplicated in the Worker request handler and the mintId callback; extract that
repeated flow into a small helper so both call sites reuse the same behavior.
Use the existing ServerEnv.generateGameIdForWorker, log.warn, and
res.status(500).json paths as the shared implementation, and have both the main
handler and mintId callback delegate to it instead of inlining the same
null-check and response logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 543f5a48-6c16-4ca2-8f84-5bb9f82012cf
📒 Files selected for processing (16)
index.htmlresources/lang/en.jsonsrc/client/ClientGameRunner.tssrc/client/HostLobbyModal.tssrc/client/Main.tssrc/client/Transport.tssrc/client/hud/GameRenderer.tssrc/client/hud/layers/GameRightSidebar.tssrc/client/hud/layers/NewLobbyPrompt.tssrc/core/Schemas.tssrc/server/GameServer.tssrc/server/SuccessorLobby.tssrc/server/Worker.tstests/NewLobbyMessages.test.tstests/server/CreateNextLobby.test.tstests/server/SuccessorLobby.test.ts
…te-lobby # Conflicts: # src/client/HostLobbyModal.ts
- Catch attachToExistingLobby rejection in HostLobbyModal so an invalid successor lobby id clears the clipboard instead of rejecting silently, matching the createLobby() failure path. - Add a 10s fallback in GameRightSidebar that re-enables the in-game "New lobby" button if the server never confirms the successor lobby, cancelled when the NewLobbyEvent broadcast arrives. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Address evanpelle's review feedback: - Successor lobbies are now created through POST /wX/api/create_game?previous=<gameID> instead of a websocket message, so game creation stays behind its rate limits. The worker mints the game and tells the previous GameServer its successor id (setSuccessorLobby), which re-broadcasts "new_lobby" to everyone still connected. Idempotent: repeat requests return the same successor. - Removed the injected createSuccessorLobby callback, SuccessorLobby.ts, and the create_next_lobby client message/schema/Transport event. The host now navigates straight to the new host view from the HTTP response, so the singleplayer/isLocal websocket path no longer exists and the 10s fallback timer is replaced by real error handling. - The in-game "New lobby" confirmation uses showInGameConfirm (native prompts are disallowed inside the CrazyGames frame); showInGameConfirm gained an options param for non-destructive styling. - NewLobbyPrompt's join now uses the CrazyGames invite link (top-level navigation) on CrazyGames, since the stale invite param would otherwise route joiners back into the finished lobby. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/client/hud/layers/GameRightSidebar.ts`:
- Around line 206-231: Add English translation entries for
new_lobby_prompt.confirm, new_lobby_prompt.failed, and win_modal.new_lobby in
resources/lang/en.json, using clear user-facing text matching the existing
new-lobby confirmation, failure alert, and win-modal action references in
onNewLobbyButtonClick.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8acd7391-394a-4b1d-8a29-fe7e158248a3
📒 Files selected for processing (11)
resources/lang/en.jsonsrc/client/Api.tssrc/client/InGameModal.tssrc/client/Transport.tssrc/client/hud/layers/GameRightSidebar.tssrc/client/hud/layers/NewLobbyPrompt.tssrc/core/Schemas.tssrc/server/GameServer.tssrc/server/Worker.tstests/NewLobbyMessages.test.tstests/server/CreateNextLobby.test.ts
💤 Files with no reviewable changes (1)
- src/client/Transport.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- resources/lang/en.json
- src/client/hud/layers/NewLobbyPrompt.ts
Add approved & assigned issue number here:
Resolves #4476
Description:
Lets a private-lobby host reuse the same group for back-to-back games without
re-sharing the invite link.
Flow: in a private game, the host clicks a "New lobby" button in the
top-right bar (next to pause). The game's server creates a fresh private
lobby (same creator, default settings) and broadcasts its id to everyone still
connected. Non-hosts get a one-click "Join" banner at the top of the screen;
the host is taken straight back to the host view for the new lobby. The chain
can repeat indefinitely.
Key design decision: the server creates and broadcasts the successor lobby
The successor lobby is minted by the finished game's server, not the host's
browser. The old game's server is the only thing still connected to every player, so it has to be what announces the new
lobby; because it also creates that lobby, the id everyone is redirected to is
authoritative. A real lobby the server just made for the authenticated
creator, not an id a client handed it to trust and fan out. The request is
creator-only and idempotent per game, and every successor is wired the
same way, so the group can keep playing game after game.
How it works
sends a
create_next_lobbymessage.private lobby on the same worker, stores it (idempotent), and broadcasts a
new_lobbymessage with the new id to all connected clients.(
/…/game/<id>?host); everyone else sees a dismissible "Host started a newlobby — Join" banner that navigates to the join URL in one click.
Two new Zod wire messages in
src/core/Schemas.ts(create_next_lobby,new_lobby) carry the request and the broadcast.Screenshots
1. In-game New lobby button (top-right, next to pause) — shown only to the host of a private lobby
2. Confirmation so a stray click doesn't pull everyone into a new lobby
3. Everyone else gets a one-click Join banner at the top of the screen
4. The host lands back in the host view for the brand-new lobby
Design Decisions
A. The server creates & broadcasts the successor, not the client.
The finished game's server mints the successor and broadcasts its id. Why this
and not "host's browser calls
POST /api/create_game, then asks the server torelay the id"?
just created for the authenticated lobby creator (creator identity comes
from the JWT the game already holds), not an arbitrary id a client hands the
server to fan out to everyone.
so it must be the one to broadcast. Having it also create the lobby keeps it
to one authoritative round-trip instead of "client creates, then client asks
server to trust an id it didn't make."
B. The successor starts with default settings (not a copy of the old game).
A deliberate scope choice. The host lands in the normal host view and
reconfigures. Copying the exact config would mean reverse-mapping every
GameConfigfield back into the host-modal controls, which I thought would be out of scope for this PR. Same creatoris preserved; same settings intentionally is not.
C. It's a brand-new lobby, not the same game resurrected.
This directly follows @evanpelle's guidance on the issue: "A 'lobby' is really
just a game that hasn't started yet. So making a persistent lobby isn't really
possible. I think instead having a simple way to transfer players to a new lobby
is probably the way to go." A
GameServerruns exactly one game(start → end → archive), so rather than reworking that lifecycle to resurrect the
old game, the server spins up a fresh successor lobby and transfers the group
into it, which is also why the feature is framed as "reuse the group," not
"reuse the game object."
D. The host returns via a
?hostURL flag + full reload ("attach mode").Navigating to a normal join URL (
/game/<id>) always lands you in the joinview, which has no Start button. So the host can't just use the join URL. The
?hostflag routes the creator to the host view instead(
Main.handleUrl→HostLobbyModalin "attach" mode, which binds to theexisting lobby id and skips creating a new one). A full reload is used because
it cleanly tears down the finished game and mirrors the existing win-screen
"Requeue" button's
window.location.hrefpattern.E. Each successor can spawn its own successor (recursive factory).
The first version only chained one generation. A spawned lobby had no
factory of its own, so the second "New lobby" click did nothing (button just
greyed out). Fixed by
wireSuccessorLobby(a small dependency-injected helper)that wires every successor the same way. This is the whole point of the issue
("back-to-back games"), so it has its own regression test.
F. Private-only.
The successor factory is installed only on the private
POST /api/create_gamepath inWorker.ts. Public games (scheduled by the master)and singleplayer never get a factory, so
handleCreateNextLobbyis a no-op forthem. The client button is also gated on
isLobbyCreator && isPrivateLobby.G. In-game button + confirm; the win-modal button was removed.
An earlier version put the "New lobby" button on the win screen. I moved it to
the in-game bar so the host can reuse the lobby at any time (without dying
or waiting for the game to end), and added a confirm (matching the adjacent
Exit button) so a stray click next to pause/exit doesn't yank everyone into a
new lobby. The win-screen button became redundant and was removed.
Testing
(
tests/NewLobbyMessages.test.ts); the server handler — authorisation,broadcast, idempotency — against a real
GameServer(
tests/server/CreateNextLobby.test.ts); and successor chaining acrossmultiple generations (
tests/server/SuccessorLobby.test.ts).npm testpasses — 1782 tests across 154 files.consecutive games via the button; verified non-hosts get the Join banner, the
host lands back in the host view, and the chain works for 3+ games in a row.
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
MushroomLamp