From ffb6fe61b02fead0ec064700a39808962b37da9a Mon Sep 17 00:00:00 2001 From: James Ketrenos Date: Wed, 8 Oct 2025 16:28:58 -0700 Subject: [PATCH] Fixing game logic for save/restore --- client/src/Activities.tsx | 2 - client/src/Board.tsx | 5 +- client/src/PlayerList.tsx | 13 +- client/src/RoomView.tsx | 5 + client/src/index.tsx | 10 +- server/package.json | 2 +- server/routes/games.ts | 187 ++++++++++++++----------- server/routes/games/sessionState.ts | 107 ++++++++++++++ server/routes/games/transientSchema.ts | 52 +++++++ server/routes/games/types.ts | 37 ++--- 10 files changed, 309 insertions(+), 111 deletions(-) create mode 100644 server/routes/games/sessionState.ts create mode 100644 server/routes/games/transientSchema.ts diff --git a/client/src/Activities.tsx b/client/src/Activities.tsx index 447bf4a..af1834f 100644 --- a/client/src/Activities.tsx +++ b/client/src/Activities.tsx @@ -180,8 +180,6 @@ const Activities: React.FC = () => { rollForOrder = state === "game-order", selectResources = turn && turn.actions && turn.actions.indexOf("select-resources") !== -1; - console.log(`activities - `, state, turn, activities); - const discarders: React.ReactElement[] = []; let mustDiscard = false; for (const key in players) { diff --git a/client/src/Board.tsx b/client/src/Board.tsx index 085bce5..e87e84a 100644 --- a/client/src/Board.tsx +++ b/client/src/Board.tsx @@ -933,6 +933,10 @@ const Board: React.FC = ({ animations }) => { } }); + useEffect(() => { + console.log(`board - tile elements`, tileElements); + }, [tileElements]); + const canAction = (action) => { return turn && Array.isArray(turn.actions) && turn.actions.indexOf(action) !== -1; }; @@ -948,7 +952,6 @@ const Board: React.FC = ({ animations }) => { const canPip = canAction("place-robber") && turn.color === color && (state === "initial-placement" || state === "normal"); - console.log(`board - tile elements`, tileElements); return (
tooltip
diff --git a/client/src/PlayerList.tsx b/client/src/PlayerList.tsx index 45bd525..57f4c45 100644 --- a/client/src/PlayerList.tsx +++ b/client/src/PlayerList.tsx @@ -27,7 +27,7 @@ const PlayerList: React.FC = () => { const [players, setPlayers] = useState(null); const [peers, setPeers] = useState>({}); useEffect(() => { - console.log("PlayerList - Mounted - requesting fields"); + console.log("player-list - Mounted - requesting fields"); if (sendJsonMessage) { sendJsonMessage({ type: "get", @@ -38,7 +38,7 @@ const PlayerList: React.FC = () => { // Debug logging useEffect(() => { - console.log("PlayerList - Debug state:", { + console.log("player-list - Debug state:", { session_id: session?.id, session_name: session?.name, players_count: players?.length, @@ -84,17 +84,18 @@ const PlayerList: React.FC = () => { const data: any = lastJsonMessage; switch (data.type) { case "game-update": { - console.log(`PlayerList - game-update:`, data.update); + console.log(`player-list - game-update:`, data.update); // Handle participants list if ("participants" in data.update && data.update.participants) { const participantsList: Player[] = data.update.participants; - console.log(`PlayerList - participants:`, participantsList); + console.log(`player-list - participants:`, participantsList); participantsList.forEach((player) => { player.local = player.session_id === session?.id; }); participantsList.sort(sortPlayers); + console.log(`player-list - sorted participants:`, participantsList); setPlayers(participantsList); // Initialize peers with remote mute/video state @@ -149,7 +150,7 @@ const PlayerList: React.FC = () => { // Debug logging useEffect(() => { - console.log("PlayerList - Debug state:", { + console.log("player-list - Debug state:", { session_id: session?.id, session_name: session?.name, players_count: players?.length, @@ -162,7 +163,7 @@ const PlayerList: React.FC = () => { return ( { } const data: any = lastJsonMessage; switch (data.type) { + case "ping": + // Respond to server ping immediately to maintain connection + console.log("App - Received ping from server, sending pong"); + sendJsonMessage({ type: "pong" }); + break; case "error": console.error(`App - error`, data.error); setError(data.error); diff --git a/client/src/index.tsx b/client/src/index.tsx index 83bbbe9..7b054d0 100644 --- a/client/src/index.tsx +++ b/client/src/index.tsx @@ -13,11 +13,11 @@ const rootEl = document.getElementById("root"); if (rootEl) { const root = ReactDOM.createRoot(rootEl); root.render( - - - - - + // + + + + // ); } diff --git a/server/package.json b/server/package.json index c29f04a..f25e9a9 100644 --- a/server/package.json +++ b/server/package.json @@ -6,7 +6,7 @@ "start": "export $(cat ../.env | xargs) && node dist/src/app.js", "start:legacy": "export $(cat ../.env | xargs) && node app.js", "build": "tsc -p tsconfig.json", - "start:dev": "ts-node-dev --respawn --transpile-only src/app.ts", + "start:dev": "ts-node-dev --respawn --transpile-only --watch routes src/app.ts", "list-games": "ts-node-dev --transpile-only tools/list-games.ts", "import-games": "ts-node-dev --transpile-only tools/import-games-to-db.ts", "test": "jest", diff --git a/server/routes/games.ts b/server/routes/games.ts index ad0de53..aa5e802 100755 --- a/server/routes/games.ts +++ b/server/routes/games.ts @@ -46,6 +46,7 @@ import { adjustResources, } from "./games/helpers"; import type { GameDB } from "./games/store"; +import { transientState } from "./games/sessionState"; let gameDB: GameDB | undefined; initGameDB() @@ -624,7 +625,7 @@ const processRoll = (game: Game, session: Session, dice: number[]): any => { // newPlayer is provided by ./games/playerFactory -const getSession = (game: Game, id: string) => { +const getSession = (game: Game, id: string): Session => { if (!game.sessions) { game.sessions = {}; } @@ -637,7 +638,7 @@ const getSession = (game: Game, id: string) => { color: "", lastActive: Date.now(), live: true, - } as unknown as Session; + }; } const session = game.sessions[id]!; @@ -665,13 +666,11 @@ const getSession = (game: Game, id: string) => { if (age > 60 * 60 * 1000) { console.log(`${_session.id}: Expiring old session ${_id}: ${age / (60 * 1000)} minutes`); delete game.sessions[_id]; - if (_id in game.sessions) { - console.log("delete DID NOT WORK!"); - } + transientState.clearSession(game.id, _id); } } - return game.sessions[id]; + return game.sessions[id]!; }; const loadGame = async (id: string) => { @@ -680,17 +679,7 @@ const loadGame = async (id: string) => { } if (id in games) { - // If we have a cached game in memory, ensure any ephemeral flags that - // control per-session lifecycle (like _initialSnapshotSent) are cleared - // so that a newly attached websocket will receive the consolidated - // initial snapshot. This is important for long-running dev servers - // where the in-memory cache may persist between reconnects. const cached = games[id]!; - for (let sid in cached.sessions) { - if (cached.sessions[sid] && cached.sessions[sid]._initialSnapshotSent) { - delete cached.sessions[sid]._initialSnapshotSent; - } - } return cached; } @@ -719,6 +708,16 @@ const loadGame = async (id: string) => { game = null; } + if (game) { + // After loading, restore transient state + transientState.restoreGame(id, game); + for (let sid in game.sessions) { + if (game.sessions[sid]) { + transientState.restoreSession(id, sid, game.sessions[sid]); + } + } + } + if (!game) { game = await createGame(id); // Persist the newly-created game immediately @@ -751,11 +750,6 @@ const loadGame = async (id: string) => { } session.live = false; - // Ensure we treat initial snapshot as unsent on (re)load so new socket - // attachments will get a fresh 'initial-game' message. - if (session._initialSnapshotSent) { - delete session._initialSnapshotSent; - } /* Populate the 'unselected' list from the session table */ if (!game.sessions[id].color && game.sessions[id].name) { @@ -1107,6 +1101,7 @@ const setPlayerName = (game: Game, session: Session, name: string): string | und Object.assign(session, tmp, { ws: session.ws, id: session.id }); console.log(`${info}: ${name} has been reallocated to a new session.`); delete game.sessions[id]; + transientState.clearSession(game.id, id); } else { return `${name} is already taken and has been active in the last minute.`; } @@ -3346,30 +3341,53 @@ const ping = (session: Session) => { return; } - (session as any)["ping"] = Date.now(); - // console.log(`Sending ping to ${session.name}`); + session.ping = Date.now(); + console.log(`${session.id}: Sending ping to ${session.name}`); + try { - session.ws.send(JSON.stringify({ type: "ping", ping: (session as any)["ping"] })); + session.ws.send(JSON.stringify({ type: "ping", ping: session.ping })); } catch (e) { - // ignore send errors + console.error(`${session.id}: Failed to send ping:`, e); + // If send fails, the socket is likely dead - clean up + if (session.keepAlive) { + clearTimeout(session.keepAlive); + session.keepAlive = undefined; + } + return; } + // Clear any existing timeout if (session.keepAlive) { clearTimeout(session.keepAlive); } + + // Set timeout to disconnect if no pong received within 20 seconds session.keepAlive = setTimeout(() => { - // mark the session as inactive if the keepAlive fires - try { - if (session.ws) { - session.ws.close?.(); + console.warn(`${session.id}: No pong received from ${session.name} within 20s, closing connection`); + if (session.ws) { + try { + session.ws.close(); + } catch (e) { + console.error(`${session.id}: Error closing socket:`, e); } - } catch (e) { - /* ignore */ } session.ws = undefined; + session.keepAlive = undefined; }, 20000); }; +// Add new function to schedule recurring pings +const schedulePing = (session: Session) => { + if (session.pingInterval) { + clearInterval(session.pingInterval); + } + + // Send ping every 10 seconds + session.pingInterval = setInterval(() => { + ping(session); + }, 10000); +}; + // wsInactive not present in this refactor; no-op placeholder removed const setGameState = (game: any, session: any, state: any): string | undefined => { @@ -3437,24 +3455,9 @@ const saveGame = async (game: any): Promise => { for (let id in game.sessions) { const reduced = Object.assign({}, game.sessions[id]); - // Remove private or non-serializable fields from the session copy - if (reduced.player) delete reduced.player; - if (reduced.ws) delete reduced.ws; - if (reduced.keepAlive) delete reduced.keepAlive; - // Remove any internal helper fields (prefixed with '_') and any - // non-primitive values such as functions or timers which may cause - // JSON.stringify to throw due to circular structures. - Object.keys(reduced).forEach((k) => { - if (k.startsWith("_")) { - delete reduced[k]; - } else if (typeof reduced[k] === "function") { - delete reduced[k]; - } - }); - // Do not persist ephemeral test/runtime-only flags - if (reduced._initialSnapshotSent) { - delete reduced._initialSnapshotSent; - } + + // Automatically remove all transient fields (uses TRANSIENT_SESSION_SCHEMA as source of truth) + transientState.stripSessionTransients(reduced); reducedGame.sessions[id] = reduced; @@ -3462,8 +3465,8 @@ const saveGame = async (game: any): Promise => { reducedSessions.push(reduced); } - delete reducedGame.turnTimer; - delete reducedGame.unselected; + // Automatically remove all game-level transient fields (uses TRANSIENT_GAME_SCHEMA) + transientState.stripGameTransients(reducedGame); /* Save per turn while debugging... */ game.step = game.step ? game.step : 0; @@ -3507,6 +3510,7 @@ const departLobby = (game: any, session: any, _color?: string): void => { for (let id in game.sessions) { if (game.sessions[id] === session) { delete game.sessions[id]; + transientState.clearSession(game.id, id); break; } } @@ -4099,6 +4103,8 @@ router.ws("/ws/:id", async (ws, req) => { session.player.live = false; } session.live = false; + session.initialSnapshotSent = false; + // Only cleanup the session.ws if it references the same socket object try { console.log(`${short}: ws.on('close') - session.ws === ws? ${session.ws === ws}`); @@ -4106,6 +4112,18 @@ router.ws("/ws/:id", async (ws, req) => { `${short}: ws.on('close') - session.id=${session && session.id}, lastActive=${session && session.lastActive}` ); if (session.ws && session.ws === ws) { + // Clear ping interval + if (session.pingInterval) { + clearInterval(session.pingInterval); + session.pingInterval = undefined; + } + + // Clear keepAlive timeout + if (session.keepAlive) { + clearTimeout(session.keepAlive); + session.keepAlive = undefined; + } + /* Cleanup any voice channels */ if (gameId in audio) { try { @@ -4153,10 +4171,12 @@ router.ws("/ws/:id", async (ws, req) => { console.warn(`${short}: error closing session socket during game removal:`, e); } delete game.sessions[id]; + transientState.clearSession(game.id, id); } } delete audio[gameId]; delete games[gameId]; + transientState.clearGame(gameId); try { if (!gameDB || !gameDB.deleteGame) { console.error(`${session.id}: gameDB.deleteGame is not available; cannot remove ${id}`); @@ -4230,14 +4250,7 @@ router.ws("/ws/:id", async (ws, req) => { // websocket was just replaced (reconnect), send an initial consolidated // snapshot so clients can render deterministically without needing to // wait for a flurry of incremental game-update events. - if (!session._initialSnapshotSent) { - try { - sendInitialGameSnapshot(game, session); - session._initialSnapshotSent = true; - } catch (e) { - console.error(`${session.id}: error sending initial snapshot`, e); - } - } + sendInitialGameSnapshot(game, session); switch (incoming.type) { case "join": @@ -4266,7 +4279,30 @@ router.ws("/ws/:id", async (ws, req) => { break; case "pong": - resetDisconnectCheck(game, req); + console.log(`${short}: Received pong from ${getName(session)}`); + + // Clear the keepAlive timeout since we got a response + if (session.keepAlive) { + clearTimeout(session.keepAlive); + session.keepAlive = undefined; + } + + // Calculate latency if ping timestamp was sent + if (session.ping) { + session.lastPong = Date.now(); + const latency = session.lastPong - session.ping; + // Only accept latency values that are within a reasonable window + // (e.g. 0 - 60s). Ignore stale or absurdly large stored ping + // timestamps which can occur if session state was persisted or + // restored with an old ping value. + if (latency >= 0 && latency < 60000) { + console.log(`${short}: Latency: ${latency}ms`); + } else { + console.warn(`${short}: Ignoring stale ping value; computed latency ${latency}ms`); + } + } + + // No need to resetDisconnectCheck since it's non-functional break; case "game-update": @@ -4675,14 +4711,7 @@ router.ws("/ws/:id", async (ws, req) => { // Ensure we only attempt to send the consolidated initial snapshot once // per session lifecycle. Tests and clients expect a single 'initial-game' // message when a socket first attaches. - if (!session._initialSnapshotSent) { - try { - sendInitialGameSnapshot(game, session); - session._initialSnapshotSent = true; - } catch (e) { - console.error(`${session.id}: error sending initial snapshot on connect`, e); - } - } + sendInitialGameSnapshot(game, session); if (session.name) { sendUpdateToPlayers(game, { players: getFilteredPlayers(game), @@ -4708,16 +4737,9 @@ router.ws("/ws/:id", async (ws, req) => { resetDisconnectCheck(game, req); console.log(`${short}: Game ${id} - WebSocket connect from ${getName(session)}`); - /* Send initial ping to initiate communication with client */ - if (!session.keepAlive) { - console.log(`${short}: Sending initial ping`); - ping(session); - } else { - clearTimeout(session.keepAlive); - session.keepAlive = setTimeout(() => { - ping(session); - }, 2500); - } + /* Start recurring ping mechanism */ + console.log(`${short}: Starting ping interval for ${getName(session)}`); + schedulePing(session); }); const debugChat = (game: any, preamble: any) => { @@ -4837,7 +4859,11 @@ const getFilteredGameForPlayer = (game: any, session: any) => { * game state deterministically on first attach instead of having * to wait for a flurry of incremental game-update events. */ -const sendInitialGameSnapshot = (game: any, session: any) => { +const sendInitialGameSnapshot = (game: Game, session: Session) => { + if (session.initialSnapshotSent) { + return; + } + try { const snapshot = getFilteredGameForPlayer(game, session); const message = JSON.stringify({ type: "initial-game", snapshot }); @@ -4853,6 +4879,7 @@ const sendInitialGameSnapshot = (game: any, session: any) => { } if (session && session.ws && session.ws.send) { session.ws.send(message); + session.initialSnapshotSent = true; } else { console.warn(`${session.id}: Unable to send initial snapshot - no websocket available`); } @@ -5247,7 +5274,7 @@ router.get("/", (req, res /*, next*/) => { player: playerId, name: null, lobbies: [], - has_media: true // Default to true for regular users + has_media: true, // Default to true for regular users }); }); diff --git a/server/routes/games/sessionState.ts b/server/routes/games/sessionState.ts new file mode 100644 index 0000000..ac66929 --- /dev/null +++ b/server/routes/games/sessionState.ts @@ -0,0 +1,107 @@ +// server/routes/games/sessionState.ts + +import { + TransientGameState, + TransientSessionState, + TRANSIENT_SESSION_KEYS, + TRANSIENT_GAME_KEYS +} from "./transientSchema"; + +class TransientStateManager { + private sessions = new Map(); + private games = new Map(); + + // Session transient state + preserveSession(gameId: string, sessionId: string, session: any): void { + const key = `${gameId}:${sessionId}`; + const transient: any = {}; + + // Automatically preserve all transient fields from schema + TRANSIENT_SESSION_KEYS.forEach(k => { + if (k in session) { + transient[k] = session[k]; + } + }); + + this.sessions.set(key, transient); + } + + restoreSession(gameId: string, sessionId: string, session: any): void { + const key = `${gameId}:${sessionId}`; + const transient = this.sessions.get(key); + if (transient) { + Object.assign(session, transient); + // Don't delete - keep for future loads + } + } + + clearSession(gameId: string, sessionId: string): void { + const key = `${gameId}:${sessionId}`; + const transient = this.sessions.get(key); + if (transient) { + // Clean up timers + if (transient.keepAlive) clearTimeout(transient.keepAlive); + if (transient.pingInterval) clearTimeout(transient.pingInterval); + if (transient._getBatch?.timer) clearTimeout(transient._getBatch.timer); + if (transient._pendingTimeout) clearTimeout(transient._pendingTimeout); + } + this.sessions.delete(key); + } + + // Game transient state + preserveGame(gameId: string, game: any): void { + const transient: any = {}; + + // Automatically preserve all transient fields from schema + TRANSIENT_GAME_KEYS.forEach(k => { + if (k in game) { + transient[k] = game[k]; + } + }); + + this.games.set(gameId, transient); + } + + restoreGame(gameId: string, game: any): void { + const transient = this.games.get(gameId); + if (transient) { + Object.assign(game, transient); + } + } + + clearGame(gameId: string): void { + const transient = this.games.get(gameId); + if (transient?.turnTimer) { + clearTimeout(transient.turnTimer); + } + this.games.delete(gameId); + } + + /** + * Remove all transient fields from a session object (for serialization) + * Automatically uses all keys from TRANSIENT_SESSION_SCHEMA + */ + stripSessionTransients(session: any): void { + // Remove all transient fields automatically + TRANSIENT_SESSION_KEYS.forEach(key => delete session[key]); + + // Remove player reference (runtime only) + delete session.player; + + // Catch-all: remove any underscore-prefixed fields and functions + Object.keys(session).forEach((k) => { + if (k.startsWith("_")) delete session[k]; + else if (typeof session[k] === "function") delete session[k]; + }); + } + + /** + * Remove all transient fields from a game object (for serialization) + * Automatically uses all keys from TRANSIENT_GAME_SCHEMA + */ + stripGameTransients(game: any): void { + TRANSIENT_GAME_KEYS.forEach(key => delete game[key]); + } +} + +export const transientState = new TransientStateManager(); \ No newline at end of file diff --git a/server/routes/games/transientSchema.ts b/server/routes/games/transientSchema.ts new file mode 100644 index 0000000..81ba9c5 --- /dev/null +++ b/server/routes/games/transientSchema.ts @@ -0,0 +1,52 @@ +/** + * Transient State Schemas - SINGLE SOURCE OF TRUTH + * + * Define transient fields here ONCE. Both TypeScript types and runtime operations + * derive from these schemas, ensuring DRY compliance. + * + * To add a new transient field: + * 1. Add it to the appropriate schema below + * 2. That's it! All preserve/restore/strip operations automatically include it + */ + +/** + * Transient Session Fields Schema + * These fields are never persisted to the database + */ +export const TRANSIENT_SESSION_SCHEMA = { + ws: undefined as any, + keepAlive: undefined as NodeJS.Timeout | undefined, + pingInterval: undefined as NodeJS.Timeout | undefined, + lastPong: undefined as number | undefined, + initialSnapshotSent: undefined as boolean | undefined, + _getBatch: undefined as { fields: Set; timer?: any } | undefined, + _pendingMessage: undefined as any, + _pendingTimeout: undefined as any, + live: false as boolean, + hasAudio: undefined as boolean | undefined, + audio: undefined as any, + video: undefined as any, + ping: undefined as number | undefined, +}; + +/** + * Transient Game Fields Schema + * These fields are never persisted to the database + */ +export const TRANSIENT_GAME_SCHEMA = { + turnTimer: undefined as any, + unselected: undefined as any[] | undefined, +}; + +// Derive runtime key arrays from schemas +export const TRANSIENT_SESSION_KEYS = Object.keys(TRANSIENT_SESSION_SCHEMA) as (keyof typeof TRANSIENT_SESSION_SCHEMA)[]; +export const TRANSIENT_GAME_KEYS = Object.keys(TRANSIENT_GAME_SCHEMA) as (keyof typeof TRANSIENT_GAME_SCHEMA)[]; + +// Export TypeScript types derived from schemas +export type TransientSessionState = { + [K in keyof typeof TRANSIENT_SESSION_SCHEMA]?: typeof TRANSIENT_SESSION_SCHEMA[K]; +}; + +export type TransientGameState = { + [K in keyof typeof TRANSIENT_GAME_SCHEMA]?: typeof TRANSIENT_GAME_SCHEMA[K]; +}; diff --git a/server/routes/games/types.ts b/server/routes/games/types.ts index e974887..990d251 100644 --- a/server/routes/games/types.ts +++ b/server/routes/games/types.ts @@ -2,6 +2,11 @@ export type ResourceKey = "wood" | "brick" | "sheep" | "wheat" | "stone"; export type ResourceMap = Partial>; +export interface TransientGameState { + turnTimer?: any; + unselected?: any[]; +} + export interface Player { name?: string; color?: string; @@ -75,29 +80,29 @@ export interface DevelopmentCard { [key: string]: any; } -export interface Session { +// Import from schema for DRY compliance +import { TransientSessionState } from './transientSchema'; + +/** + * Persistent Session data (saved to DB) + */ +export interface PersistentSessionData { id: string; + name: string; + color: string; + lastActive: number; userId?: number; - name?: string; - color?: string; - ws?: any; // WebSocket instance; keep as any to avoid dependency on ws types player?: Player; - live?: boolean; - lastActive?: number; - keepAlive?: any; connected?: boolean; - hasAudio?: boolean; - audio?: any; - video?: any; - ping?: number; - _initialSnapshotSent?: boolean; - _getBatch?: { fields: Set; timer?: any }; - _pendingMessage?: any; - _pendingTimeout?: any; resources?: number; - [key: string]: any; } +/** + * Runtime Session type = Persistent + Transient + * At runtime, sessions have both persistent and transient fields + */ +export type Session = PersistentSessionData & TransientSessionState; + export interface OfferItem { type: string; // 'bank' or resource key or other count: number;