From eace5940da921272b192cbaf0606e94ad1bb8cdb Mon Sep 17 00:00:00 2001 From: tiennm99 Date: Mon, 21 Sep 2026 16:17:23 +0700 Subject: [PATCH] refactor(web): extract the online screen's request machine into a store Move the join/resume/quick-match/leave state machine out of routes/online/+page.svelte into stores/room-session.svelte.js, modelled on bot-session.svelte.js: no DOM, no runes beyond $state, so the resume time-box is something Vitest can drive directly. The page keeps layout, timers and wiring; the store keeps what the player asked for. fix(web): time-box the resume latch and stop leaking a left room's session A stale resume token used to get silence from the server, leaving `resuming` stuck true and every button on the join form disabled with no way out but a reload. `resuming` now clears five seconds after the socket opens if nothing has answered by then, same as it already does on an explicit error (which also covers a server new enough to send `session_not_resumable` instead of staying quiet). leave() now calls forgetSession(), matching the page-teardown path: without it, deliberately leaving a room left the token behind, and the next load of /online tried to resume into the room the player had just walked out of. fix(web): retry cancelQueue, leave and lobby actions instead of dropping them send() returns false while the socket is down, and cancelQueue/leave ignored that return value while ready/start/kick only reported it as a dead-looking button. All five now hold the request and resend it once the socket reopens, the way join/create already do, via room-session's held-action slot. Lobby's "reconnecting" banner is now driven by that held action instead of a local flag that never noticed a background retry had succeeded. Also: the lobby's chat panel now reopens once a game ends (phase 'over') instead of staying folded for the rest of the room's life after the first game, since phase never actually revisits 'lobby' on its own. --- web/src/lib/components/Lobby.svelte | 27 +-- web/src/lib/stores/room-session.svelte.js | 192 ++++++++++++++++++ web/src/routes/online/+page.svelte | 232 +++++++++++++--------- 3 files changed, 346 insertions(+), 105 deletions(-) create mode 100644 web/src/lib/stores/room-session.svelte.js diff --git a/web/src/lib/components/Lobby.svelte b/web/src/lib/components/Lobby.svelte index 5bce95e..d22e4d3 100644 --- a/web/src/lib/components/Lobby.svelte +++ b/web/src/lib/components/Lobby.svelte @@ -17,17 +17,21 @@ * above already carries the connection state and the away banners. * * The callbacks report whether the request actually reached the server. A - * socket that has just dropped answers `false`, and a button that silently - * did nothing is the fastest way to make a room look dead. + * socket that has just dropped answers `false` and holds the request for + * the caller to retry once the socket reopens — `actionHeld` is that + * retry showing here, so the banner clears itself once it lands rather + * than being a one-shot flag this component would have no way to know + * had gone stale. * @type {{ * compact?: boolean, + * actionHeld?: boolean, * onready: (ready: boolean) => boolean, * onstart: () => boolean, * onkick: (playerId: string) => boolean, * onleave: () => void * }} */ - let { compact = false, onready, onstart, onkick, onleave } = $props(); + let { compact = false, actionHeld = false, onready, onstart, onkick, onleave } = $props(); /** How long an armed kick waits before it goes back to being safe. */ const ARM_MS = 4000; @@ -55,22 +59,13 @@ let armedKick = $state(''); /** @type {ReturnType} */ let armTimer; - // Set when a request could not go out at all, which is a different thing - // from the server refusing it — that arrives as game.state.error. - let unsent = $state(false); - - /** @param {boolean} sent */ - function report(sent) { - unsent = !sent; - return sent; - } /** @param {string} playerId */ function armOrKick(playerId) { if (armedKick === playerId) { clearTimeout(armTimer); armedKick = ''; - report(onkick(playerId)); + onkick(playerId); return; } armedKick = playerId; @@ -201,7 +196,7 @@ aria-label={t.dismiss}>×

- {:else if unsent} + {:else if actionHeld} {/if} @@ -212,7 +207,7 @@ class="primary" disabled={!s.canStart || offline} data-testid="start-game" - onclick={() => report(onstart())} + onclick={() => onstart()} > {t.startGame} @@ -223,7 +218,7 @@ class:on={game.isReady} disabled={offline} data-testid="ready" - onclick={() => report(onready(!game.isReady))} + onclick={() => onready(!game.isReady)} > {game.isReady ? t.unready : t.ready} diff --git a/web/src/lib/stores/room-session.svelte.js b/web/src/lib/stores/room-session.svelte.js new file mode 100644 index 0000000..bb8750e --- /dev/null +++ b/web/src/lib/stores/room-session.svelte.js @@ -0,0 +1,192 @@ +/** + * Owns the /online screen's request machine: what the player has asked for + * that the socket has not yet carried, from opening a room through leaving + * one. Modelled on bot-session.svelte.js and for the same reason — a request + * is a thing the player did, not a condition inferred from the board, so it + * is stored as one rather than re-derived from `game.state`. + * + * No DOM and no runes beyond `$state`: every timer (the resume time-box, the + * stall timer, the quick-match elapsed clock) is a plain `setTimeout` or + * `setInterval` owned by the page's own `$effect`s, which call back into the + * plain setters here. That is what makes this testable under Vitest with no + * Svelte runtime involved, exactly as `ws/client.js` already is. + */ + +/** + * What the player asked for, held until the socket can carry it. + * @typedef {{ kind: 'create' } | { kind: 'join', code: string } | { kind: 'quickMatch' }} RoomRequest + */ + +/** + * A request made from inside a room that the socket refused to carry. + * Replacing rather than queuing: a later action is a later expression of the + * same intent (readying and then leaving before reconnecting means the + * leave is what should happen), and every one of these is safe to resend — + * `SetReady`, `StartGame`, `KickPlayer`, `LeaveRoom` and `CancelQuickMatch` + * are all refused rather than misapplied if they no longer make sense by the + * time they land. + * @typedef {{ kind: 'cancelQueue' } | { kind: 'leaveRoom' } | { kind: 'setReady', ready: boolean } | { kind: 'startGame' } | { kind: 'kickPlayer', playerId: string }} RoomAction + */ + +export function createRoomSession() { + const state = $state({ + /** @type {RoomRequest | null} */ + pending: null, + // True while the only reason this screen has a socket is to reclaim a + // game it might no longer be able to reclaim. + resuming: false, + // An invite link arrived before this player had a name. + needName: false, + // A held request that has been waiting on a socket for longer than a + // player will believe. + stalled: false, + // The resume timed out (or was refused) rather than succeeding. Shown + // instead of leaving the join form's own "connecting" label the only + // sign anything happened. + resumeFailed: false, + // Whole seconds spent in the quick-match queue, for the waiting + // panel's elapsed time and the bot nudge. Owned here rather than left + // as a raw interval in the page, alongside everything else this + // screen is waiting on. + queuedForS: 0, + /** @type {RoomAction | null} */ + heldAction: null + }); + + return { + state, + + /** + * Queues a request to open or join a room. Nothing is sent until the + * socket is open, which the caller still has to drive with `connect()` + * and `flush()` — this only records the intent. + * @param {RoomRequest} req + */ + request(req) { + state.pending = req; + state.resuming = false; + state.needName = false; + state.stalled = false; + state.resumeFailed = false; + }, + + /** Drops a queued request without sending it. */ + clearPending() { + state.pending = null; + }, + + /** + * Holds a join behind a resume already in progress, without touching + * the other latches `request()` resets — the resume owns those while + * it runs, and is what the held join is waiting on. + * @param {RoomRequest} req + */ + holdPendingJoin(req) { + state.pending = req; + }, + + /** @param {boolean} value */ + setNeedName(value) { + state.needName = value; + }, + + /** + * Sends the queued request if there is one and the socket can carry + * it. Nothing goes out while a resume is in flight: the seat this tab + * is reclaiming may be in the very room the held request names. + * @param {boolean} isOpen + * @param {{ + * create: () => boolean, + * join: (code: string) => boolean, + * quickMatch: () => boolean + * }} senders + * @returns {boolean} whether a request was sent + */ + flush(isOpen, senders) { + if (!state.pending || !isOpen || state.resuming) return false; + const req = state.pending; + const sent = + req.kind === 'create' + ? senders.create() + : req.kind === 'quickMatch' + ? senders.quickMatch() + : senders.join(req.code); + if (sent) state.pending = null; + return sent; + }, + + /** Marks a resume attempt as starting, optionally behind a held join. */ + startResume() { + state.resuming = true; + }, + + /** + * The resume worked, so nothing that happens from here is its fault — + * and an invite code held behind it has been answered by arriving in a + * room. + */ + noteRoom() { + state.resuming = false; + state.pending = null; + state.resumeFailed = false; + }, + + /** + * A resume that the server refused, or that ran out the client's own + * patience for. Neither is something the player did, so the token is + * dropped and an invite code held behind it is either spent (the + * player already has a name) or turned into asking for one. + * @param {boolean} named + */ + noteResumeFailed(named) { + if (!state.resuming) return; + state.resuming = false; + state.resumeFailed = true; + if (state.pending && !named) { + state.needName = true; + state.pending = null; + } + }, + + /** @param {boolean} value */ + setStalled(value) { + state.stalled = value; + }, + + resetQueued() { + state.queuedForS = 0; + }, + + tickQueued() { + state.queuedForS += 1; + }, + + /** + * Holds a request the socket refused, replacing whatever this screen + * was already waiting to retry: a later action is a later statement of + * intent, and every action here is safe to resend regardless of order. + * @param {RoomAction} action + */ + holdAction(action) { + state.heldAction = action; + }, + + /** Drops a held action without sending it, for a screen being torn down. */ + clearAction() { + state.heldAction = null; + }, + + /** + * Resends a held action once the socket can carry it. + * @param {boolean} isOpen + * @param {(action: RoomAction) => boolean} dispatch + * @returns {boolean} whether the held action was sent + */ + flushAction(isOpen, dispatch) { + if (!state.heldAction || !isOpen) return false; + const sent = dispatch(state.heldAction); + if (sent) state.heldAction = null; + return sent; + } + }; +} diff --git a/web/src/routes/online/+page.svelte b/web/src/routes/online/+page.svelte index d67c541..731361d 100644 --- a/web/src/routes/online/+page.svelte +++ b/web/src/routes/online/+page.svelte @@ -9,9 +9,10 @@ import Lobby from '$lib/components/Lobby.svelte'; import NicknameInput from '$lib/components/NicknameInput.svelte'; import PlayerStatus from '$lib/components/PlayerStatus.svelte'; - import { fill, t } from '$lib/i18n/vi.js'; + import { errorMessage, fill, t } from '$lib/i18n/vi.js'; import { scrollBehavior } from '$lib/motion.js'; import { isRoomCode, normalizeRoomCode, ROOM_CODE_LENGTH } from '$lib/room-code.js'; + import { createRoomSession } from '$lib/stores/room-session.svelte.js'; import { game } from '$lib/stores/game.svelte.js'; import { settings } from '$lib/stores/settings.svelte.js'; import { @@ -40,20 +41,11 @@ } from '$lib/ws/connection.svelte.js'; /** - * What the player asked for, held until the socket can carry it. Same shape - * as the bot screen's request latch and for the same reason: a request is - * something the player did, not a condition to be re-derived from the board. - * @type {{ kind: 'create' } | { kind: 'join', code: string } | { kind: 'quickMatch' } | null} + * The join/resume/quick-match/leave machine. Extracted to its own store — + * see room-session.svelte.js — so the resume time-box below is something + * Vitest can drive directly instead of only through a mounted page. */ - let pending = $state(null); - - /** - * How long a quick match has been waiting, in whole seconds. Client-only - * and approximate on purpose — this is a "still looking" indicator, not - * the turn clock, so it is timed off the device rather than the server's - * estimated time. - */ - let queuedForS = $state(0); + const session = createRoomSession(); /** * How long the wait runs before the screen offers the bot instead. A @@ -70,6 +62,17 @@ */ const STALL_MS = 5000; + /** + * How long `resuming` waits, once the socket is actually open, before the + * join form comes back on its own. The server answering an unknown resume + * token used to mean silence — no Welcome, no error — which left every + * button on this screen reading "Đang kết nối…" forever, recoverable only + * by a reload that reproduced the same dead end. A server new enough to + * answer with `session_not_resumable` clears this sooner, through the + * ordinary error path below; this is the backstop for one that cannot. + */ + const RESUME_TIMEOUT_MS = 5000; + /** * Where the two-column layout starts. The media queries in the styles * below are the same decision expressed in CSS, so the two must agree: the @@ -105,8 +108,15 @@ /** @type {HTMLElement | undefined} */ let talkPane = $state(); + // Folded going into a game, on a narrow screen where the two would crowd + // each other; unfolded again once it ends, since the compact lobby that + // appears beneath the result is the same "waiting in a room" situation + // the chat is open for everywhere else. Phase never actually revisits + // 'lobby' after the first game — the room goes over → (next start) → + // playing directly — so folding was permanent after one game without this. $effect(() => { if (game.state.phase === 'playing') chatFolded = true; + else if (game.state.phase === 'over') chatFolded = false; }); function openChat() { @@ -116,14 +126,6 @@ let codeInput = $state(normalizeRoomCode(page.url.searchParams.get('code') ?? '')); let codeError = $state(''); - // True while the only reason this screen has a socket is to reclaim a game - // it might no longer be able to reclaim. - let resuming = $state(false); - // An invite link arrived before this player had a name. Asking is one extra - // tap, and the alternative is being seated as "Người chơi" with no way to - // fix it from inside the room. - let needName = $state(false); - let stalled = $state(false); const inviteCode = $derived(normalizeRoomCode(page.url.searchParams.get('code') ?? '')); const playing = $derived(game.state.phase === 'playing' || game.state.phase === 'over'); @@ -135,11 +137,7 @@ // The resume worked, so nothing that happens from here is its fault — and // an invite code held behind it has been answered by arriving in a room. $effect(() => { - if (inRoom) - untrack(() => { - resuming = false; - pending = null; - }); + if (inRoom) untrack(() => session.noteRoom()); }); // Owns the socket while this screen is on, exactly as the bot screen does. @@ -168,8 +166,8 @@ // given their old one back to — a red banner on a board that had // in fact been restored correctly. The code is held instead, and // only spent if the resume is refused. - resuming = true; - if (isRoomCode(code)) pending = { kind: 'join', code }; + session.startResume(); + if (isRoomCode(code)) session.holdPendingJoin({ kind: 'join', code }); connect(); } else if (isRoomCode(code)) { if (named) { @@ -177,7 +175,7 @@ } else { // Held, not sent. The name field is already on this screen and // the code is already in its field, so this is one button. - needName = true; + session.setNeedName(true); } } }); @@ -205,17 +203,23 @@ // Leaving mid-wait is leaving the queue too: nobody is left to pair // with a tab that has gone. if (game.state.queued) send(cancelQuickMatch()); - pending = null; + session.clearPending(); + session.clearAction(); disconnect(); game.reset(); game.clearChat(); }; }); - // Held requests go out once the handshake has landed. + // Held requests go out once the handshake has landed — both the one that + // opens or joins a room, and any lobby action the socket refused while it + // was down. $effect(() => { const open = connection.status === Status.OPEN; - untrack(() => flush(open)); + untrack(() => { + flush(open); + session.flushAction(open, dispatchAction); + }); }); // Counts up while queued, for the waiting panel's elapsed time and the @@ -224,11 +228,11 @@ // clock. $effect(() => { if (!game.state.queued) { - queuedForS = 0; + session.resetQueued(); return; } - queuedForS = 0; - const id = setInterval(() => (queuedForS += 1), 1000); + session.resetQueued(); + const id = setInterval(() => session.tickQueued(), 1000); return () => clearInterval(id); }); @@ -237,44 +241,55 @@ // backoff cycles between "reconnecting" and "connecting" indefinitely and // neither of them is news. $effect(() => { - const waiting = !!pending && connection.status !== Status.OPEN; + const waiting = !!session.state.pending && connection.status !== Status.OPEN; if (!waiting) { - stalled = false; + session.setStalled(false); return; } - const timer = setTimeout(() => (stalled = true), STALL_MS); + const timer = setTimeout(() => session.setStalled(true), STALL_MS); + return () => clearTimeout(timer); + }); + + // The bound this screen puts on how long a resume may run once the socket + // is actually open. An older server answers a stale token with silence + // rather than an error, which without this left `resuming` stuck true + // forever — every button on the join form disabled, and the `stalled` + // banner never firing because it only watches a socket that never opened, + // not a handshake that opened and then went quiet. + $effect(() => { + if (!(session.state.resuming && connection.status === Status.OPEN)) return; + const timer = setTimeout(() => { + untrack(() => { + session.noteResumeFailed(named); + forgetSession(); + if (!session.state.needName) flush(connection.status === Status.OPEN); + }); + }, RESUME_TIMEOUT_MS); return () => clearTimeout(timer); }); // A resume that the server cannot honour is not something the player did. // Reporting it would open the lobby with a red banner about a game they // have already left behind, so the token is dropped quietly instead — and - // an invite code held behind the resume is spent now. + // an invite code held behind the resume is spent now. A server new enough + // to answer a stale token with `session_not_resumable` lands here, ahead + // of the time-box above. $effect(() => { - const failed = resuming && !!game.state.error; + const failed = session.state.resuming && !!game.state.error; untrack(() => { if (!failed) return; - resuming = false; game.clearError(); forgetSession(); - if (pending && !named) { - // The link was for somebody who has still not given a name. - needName = true; - pending = null; - return; - } - flush(connection.status === Status.OPEN); + session.noteResumeFailed(named); + if (!session.state.needName) flush(connection.status === Status.OPEN); }); }); - /** @param {{ kind: 'create' } | { kind: 'join', code: string } | { kind: 'quickMatch' }} req */ + /** @param {import('$lib/stores/room-session.svelte.js').RoomRequest} req */ function request(req) { codeError = ''; - resuming = false; - needName = false; - stalled = false; game.clearError(); - pending = req; + session.request(req); // The handshake carries the nickname as it stands now, which is why the // connection waits until the player has actually asked for a room. connect(); @@ -283,23 +298,35 @@ /** @param {boolean} isOpen */ function flush(isOpen) { - // Nothing goes out while a resume is in flight: the seat this tab is - // reclaiming may be in the very room the held code names. - if (!pending || !isOpen || resuming) return; - // Cleared only once the socket has taken it, so a request made during a - // reconnect is carried by the next open connection rather than lost. - let sent; - switch (pending.kind) { - case 'create': - sent = send(createRoom()); - break; - case 'quickMatch': - sent = send(quickMatch()); - break; + session.flush(isOpen, { + create: () => send(createRoom()), + join: (code) => send(joinRoom(code)), + quickMatch: () => send(quickMatch()) + }); + } + + /** + * Resends a lobby action the socket refused the first time. Every one of + * these is safe to resend regardless of what happened in between: the + * server refuses whichever no longer apply rather than misapplying them. + * @param {import('$lib/stores/room-session.svelte.js').RoomAction} action + * @returns {boolean} + */ + function dispatchAction(action) { + switch (action.kind) { + case 'cancelQueue': + return send(cancelQuickMatch()); + case 'leaveRoom': + return send(leaveRoom()); + case 'setReady': + return send(setReady(action.ready)); + case 'startGame': + return send(startGame()); + case 'kickPlayer': + return send(kickPlayer(action.playerId)); default: - sent = send(joinRoom(pending.code)); + return false; } - if (sent) pending = null; } function join() { @@ -309,7 +336,7 @@ return; } if (!named) { - needName = true; + session.setNeedName(true); return; } request({ kind: 'join', code }); @@ -325,8 +352,9 @@ /** Withdraws from the pairing queue without leaving the page. */ function cancelQueue() { - send(cancelQuickMatch()); - pending = null; + const sent = send(cancelQuickMatch()); + if (!sent) session.holdAction({ kind: 'cancelQueue' }); + session.clearPending(); } function goHome() { @@ -347,12 +375,16 @@ * @returns {boolean} */ function ready(ready) { - return send(setReady(ready)); + const sent = send(setReady(ready)); + if (!sent) session.holdAction({ kind: 'setReady', ready }); + return sent; } /** @returns {boolean} */ function start() { - return send(startGame()); + const sent = send(startGame()); + if (!sent) session.holdAction({ kind: 'startGame' }); + return sent; } /** @@ -362,13 +394,20 @@ function kick(playerId) { // The lobby arms this with a second press of the same button; a native // confirm() would block the frame loop the countdown runs on. - return send(kickPlayer(playerId)); + const sent = send(kickPlayer(playerId)); + if (!sent) session.holdAction({ kind: 'kickPlayer', playerId }); + return sent; } function leave() { - send(leaveRoom()); + const sent = send(leaveRoom()); + if (!sent) session.holdAction({ kind: 'leaveRoom' }); game.leave(); - pending = null; + session.clearPending(); + // Matches the page-teardown path: leaving deliberately must not leave + // a token behind for the next load of /online to resume with — the + // player just walked out of this room on purpose. + forgetSession(); } /** @param {string} text */ @@ -439,11 +478,11 @@ the next game is agreed in the lobby below exactly as the last one was. --> - + {/snippet} {:else} - + {/if} @@ -475,7 +514,7 @@ - {#if needName} + {#if session.state.needName} {/if} @@ -483,7 +522,17 @@ {/if} - {#if stalled} + {#if session.state.resumeFailed} + + + {/if} + + {#if session.state.stalled} {/if} @@ -492,9 +541,9 @@ branch the moment a match is found — so the only button here is the way out. -->
-

{fill(t.quickMatchWaiting, { n: queuedForS })}

+

{fill(t.quickMatchWaiting, { n: session.state.queuedForS })}

- {#if queuedForS >= QUICK_MATCH_NUDGE_S} + {#if session.state.queuedForS >= QUICK_MATCH_NUDGE_S}

{t.quickMatchNudge} {t.quickMatchNudgeLink} @@ -506,12 +555,17 @@ send a real CreateRoom, and the fifth one came back as "you are creating rooms too quickly" to a player who thought they had tapped nothing at all. --> - -

(codeError = '')} /> -

{codeError || t.roomCodeHint}