diff --git a/CLAUDE.md b/CLAUDE.md index f7434f0..334318c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -90,29 +90,20 @@ inconsistency here without checking the client first. no asset arrays; but `unityAsset`/`unityAssetHash`). Don't unify the two projections. - A subroom's saved scene loads from `CurrentSave.DataBlob` (`rooms`: `GET /rooms/:id`), NOT the flat `DataBlob` on the subroom — a subroom with no `CurrentSave` silently loads - nothing. Read it via `subRoomDataBlob()` so `match`/`auth` instance payloads resolve it - the same way. It resolves in `attachCurrentSaves`, which every room read goes through: - the PUBLISHED save (`current_save_id`), falling back to the STAGED one - (`staged_save_id`) when nothing has been published yet, and with NEITHER the key is - omitted entirely — a `CurrentSave: null` breaks the client on every endpoint that serves - rooms (not just the save flow — `/rooms/ownedby/me` and friends too). Don't reintroduce - a null in a hand-built subroom payload. + nothing. The key must be present (null before the first publish); read it via + `subRoomDataBlob()` so `match`/`auth` instance payloads resolve it the same way. - A room save (`rooms`: `POST …/subrooms/:sid/data`) publishes only when the body says `AutoPublish: true`; otherwise it STAGES onto `StagedSubRoomDataSaveId` and leaves - `current_save_id` alone, so players keep loading the last published version until the - owner posts `…/subrooms/:sid/publish_save` with `subRoomDataSaveId=` — except before - the FIRST publish, where `CurrentSave` falls back to the staged save (there is no older - version to keep serving, and a subroom that has been saved shouldn't read as empty). - DORMS always + `CurrentSave` alone, so players keep loading the last published version until the owner + posts `…/subrooms/:sid/publish_save` with `subRoomDataSaveId=`. DORMS always publish: no publish step exists in the client for them. Saves live in the `subroom_save` table with globally-unique ids (a bare id has to resolve — `StagedSubRoomDataSaveId` carries no subroom context), and nothing is overwritten, so `…/saves` is real history and `publish_save` doubles as restore-a-save. `…/saves` is auth-gated and CREATOR-only (not co-owners) — it lists unpublished staged saves. There is no `GET …/subrooms/:sid/data`; only the POST (the room save) exists on that path. -- Matchmaking (`match`: `/matchmake/room/:roomId/:subRoomId`) serves whatever - `CurrentSave` resolves to — the published save once there is one — to everyone alike, - creator included. Joining a private instance, the client itself asks +- Matchmaking (`match`: `/matchmake/room/:roomId/:subRoomId`) always serves the PUBLISHED + `CurrentSave` blob, creator included. Joining a private instance, the client itself asks the owner whether to load the latest or the published version and resolves it from the `/subrooms/:sid/saves` list — the matchmake call is identical either way. Don't make this server-side: it would put two people in one instance on different versions. diff --git a/apps/rooms/src/openapi.ts b/apps/rooms/src/openapi.ts index a3e38da..5bbcf79 100644 --- a/apps/rooms/src/openapi.ts +++ b/apps/rooms/src/openapi.ts @@ -213,10 +213,9 @@ export const SubRoomDataSaveDto = z.object({ * sequence, not per-room); a room's `SubRooms` array is reconstructed on read. * * `CreatorAccountId` starts null on the seeded rooms and is filled in on the first save — - * the client NREs on a null one. `CurrentSave` is the published save, or the staged one - * when nothing has been published; with neither it is ABSENT rather than null (a null one - * breaks the client's parser). The flat `DataBlob`/`RoomDataBlob`/`DataSavedAt` fields are - * legacy and are NOT what the client loads from. + * the client NREs on a null one. `CurrentSave` is null until the first save; the flat + * `DataBlob`/`RoomDataBlob`/`DataSavedAt` fields are legacy and are NOT what the client + * loads from. */ export const SubRoomDto = z.object({ SubRoomId: z.int(), @@ -232,8 +231,8 @@ export const SubRoomDto = z.object({ .describe('0 Private, 1 Public, 2 Unlisted, 3 Dev_only, 4 Dev_Unlisted — set independently'), ShouldAutoStageSaves: z.boolean(), StagedSubRoomDataSaveId: z.int().nullable(), - CurrentSave: SubRoomDataSaveDto.optional().describe( - 'The room save the client loads the scene blob from: the published one, falling back to the STAGED one when nothing has been published yet. Omitted entirely (never null) when the subroom has neither' + CurrentSave: SubRoomDataSaveDto.nullable().describe( + 'The latest room save — where the client finds the scene blob. Null until first save' ), DataBlob: z.string().optional().describe('Legacy flat key; the client reads `CurrentSave`'), RoomDataBlob: z.string().optional().describe('Uploaded room-data key; absent until first save'), diff --git a/apps/rooms/src/test/integration/api.test.ts b/apps/rooms/src/test/integration/api.test.ts index e012491..953a7a6 100644 --- a/apps/rooms/src/test/integration/api.test.ts +++ b/apps/rooms/src/test/integration/api.test.ts @@ -1470,22 +1470,19 @@ describe('rooms endpoints', () => { expect(coOwnerEnv.value.subRoomDataSave).toMatchObject({ savedByAccountId: 2 }) }) - it('GET /rooms/:id omits CurrentSave entirely on a never-saved subroom', async () => { - // A null CurrentSave breaks the client's room parser, so an unsaved subroom has to - // carry no such key at all. `in` rather than a value check: absent and null differ - // here, and null is exactly what this must not be. + it('GET /rooms/:id gives every subroom a CurrentSave key (null before the first save)', async () => { + // The client loads a subroom's scene data from CurrentSave and nothing else, so + // the key must be PRESENT — the seeded rooms predate it and have no such field in + // their stored blob. `in` rather than a value check: absent and null differ here. // Room 3 is seeded and never saved by another test (room 2 is the save fixture). const room = (await (await SELF.fetch(`${ORIGIN}/rooms/3`)).json()) as { SubRooms: Array> } expect(room.SubRooms.length).toBeGreaterThan(0) for (const sub of room.SubRooms) { - expect('CurrentSave' in sub).toBe(false) + expect('CurrentSave' in sub).toBe(true) + expect(sub.CurrentSave).toBeNull() } - // Not just missing from the parsed object — absent from the wire bytes too, which is - // what the client's parser actually reads. - const raw = await (await SELF.fetch(`${ORIGIN}/rooms/3`)).text() - expect(raw).not.toContain('"CurrentSave"') }) it('a real client room-save body stages, and publish_save makes it live', async () => { @@ -1496,7 +1493,7 @@ describe('rooms endpoints', () => { body: JSON.stringify(body), }) type Sub = { - CurrentSave?: Record + CurrentSave: Record | null StagedSubRoomDataSaveId: number | null } const subOf = async () => (await subRoomOf(5, 5)) as unknown as Sub @@ -1523,16 +1520,11 @@ describe('rooms endpoints', () => { }) expect(res.status).toBe(200) - // Room 5 is not a dorm → the save is STAGED, not published. Nothing has ever been - // published here, so there is no older version to keep serving: CurrentSave falls - // back to the staged save rather than leaving the subroom reading as never-saved. + // Room 5 is not a dorm → staged, nothing live yet. const stagedSub = await subOf() + expect(stagedSub.CurrentSave).toBeNull() const firstId = stagedSub.StagedSubRoomDataSaveId! expect(firstId).toBeGreaterThan(0) - expect(stagedSub.CurrentSave).toMatchObject({ - SubRoomDataSaveId: firstId, - DataBlob: '2026-07-28/f176fc3b-scene', - }) // Publishing makes it what the loader fetches, and clears the staging slot. expect((await publish(firstId)).status).toBe(200) @@ -1556,7 +1548,7 @@ describe('rooms endpoints', () => { // It's on the room read too — that's what the loader actually fetches. const room = (await (await SELF.fetch(`${ORIGIN}/rooms/5`)).json()) as { - SubRooms: Array<{ SubRoomId: number; CurrentSave?: { SubRoomDataSaveId: number } }> + SubRooms: Array<{ SubRoomId: number; CurrentSave: { SubRoomDataSaveId: number } | null }> } expect(room.SubRooms.find((s) => s.SubRoomId === 5)!.CurrentSave!.SubRoomDataSaveId).toBe( firstId @@ -1701,9 +1693,9 @@ describe('rooms endpoints', () => { headers: await bearer('1'), }) const body = (await res.json()) as { - value: { SubRooms: Array<{ SubRoomId: number; CurrentSave?: { SubRoomId: number } }> } + value: { SubRooms: Array<{ SubRoomId: number; CurrentSave: { SubRoomId: number } | null }> } } - const clone = body.value.SubRooms.find((s) => s.SubRoomId !== 2 && s.CurrentSave !== undefined)! + const clone = body.value.SubRooms.find((s) => s.SubRoomId !== 2 && s.CurrentSave !== null)! expect(clone).toBeDefined() // The copy's save must claim the COPY, or the client resolves it against the source. expect(clone.CurrentSave!.SubRoomId).toBe(clone.SubRoomId) diff --git a/packages/domain/src/rooms-db.ts b/packages/domain/src/rooms-db.ts index 6b3d771..c78d8e1 100644 --- a/packages/domain/src/rooms-db.ts +++ b/packages/domain/src/rooms-db.ts @@ -797,7 +797,7 @@ export async function createSubRoom( LastModeratedSaveModerationState: 0, ShouldAutoStageSaves: true, // Nothing saved yet — the first room save mints one and points current_save_id - // at it. Until then the subroom reads with no `CurrentSave` key at all. + // at it. Until then the subroom reads with `CurrentSave: null`. }) // Refresh the hydrated SubRooms so the returned room includes the one just inserted. await attachSubRooms(db, [room]) @@ -863,11 +863,10 @@ const SUBROOM_COLUMNS = 'sub_room_id, room_id, data, current_save_id, staged_sav /** * Materialize a subroom row into its client shape, with the columns authoritative. - * `CurrentSave` is left alone here and resolved by {@link attachCurrentSaves} — it lives - * in `subroom_save`, and resolving it per row would be a query each. Callers must go - * through the helpers below rather than this: the client reads the scene blob from - * `CurrentSave` and nowhere else, so a subroom served without one loads nothing, and a - * stale one spread out of the stored blob would point at another subroom's save. + * `CurrentSave` is left undefined here and filled in by {@link attachCurrentSaves} — it + * lives in `subroom_save`, and resolving it per row would be a query each. Callers must + * go through the helpers below so the key is never missing: the client reads the scene + * blob from `CurrentSave` and nowhere else, so a subroom without one loads nothing. */ const parseSubRoomRow = (row: SubRoomRow): SubRoom => ({ ...(JSON.parse(row.data) as SubRoom), @@ -904,31 +903,12 @@ const serializeRoom = (room: Room): string => { return JSON.stringify({ ...rest, Stats: storedStats(stats) }) } -/** - * The save a subroom actually serves as its `CurrentSave`: the published one, or the - * staged one when nothing has been published yet. Null when it has neither. - */ -const liveSaveId = (row: SubRoomRow): number | null => row.current_save_id ?? row.staged_save_id - /** * Fill in each subroom's `CurrentSave` from `subroom_save`, in ONE query for the whole - * batch. + * batch. Every subroom ends up with the key present — null when it points at no save + * (never saved) or the pointer dangles — because the client's loader reads it directly. * - * `current_save_id` (the published save) wins; a subroom that has none FALLS BACK to - * `staged_save_id`, so a subroom whose only save is unpublished still serves that save as - * its `CurrentSave` rather than reading as never-saved. Note what this means for staging: - * before the first publish there is nothing older to keep serving, so the staged work is - * what everyone loads — but once a save has been published, staging behaves as it did and - * players keep loading the published version. - * - * A subroom with neither pointer (never saved, or both dangling) has the key REMOVED - * rather than set to null: a null `CurrentSave` breaks the client's room parser, so an - * unsaved subroom has to look like it has no such field at all. The key is deleted rather - * than merely left unset because the stored blob of a subroom written before saves moved - * to their own table can still carry a `CurrentSave: null` of its own, which - * {@link parseSubRoomRow} would spread straight through. - * - * `rows` must line up with `subs` positionally; the pointers live on the row, not the + * `rows` must line up with `subs` positionally; the pointer lives on the row, not the * parsed blob. */ async function attachCurrentSaves( @@ -936,7 +916,7 @@ async function attachCurrentSaves( subs: SubRoom[], rows: SubRoomRow[] ): Promise { - const saveIds = [...new Set(rows.map(liveSaveId).filter((id) => id != null))] + const saveIds = [...new Set(rows.map((r) => r.current_save_id).filter((id) => id != null))] const byId = new Map() if (saveIds.length > 0) { const placeholders = saveIds.map((_, i) => `?${i + 1}`).join(',') @@ -950,10 +930,8 @@ async function attachCurrentSaves( for (const r of results) byId.set(r.sub_room_data_save_id, parseSubRoomSaveRow(r)) } subs.forEach((sub, i) => { - const id = liveSaveId(rows[i]!) - const save = id == null ? undefined : byId.get(id) - if (save) sub.CurrentSave = save - else delete sub.CurrentSave + const id = rows[i]!.current_save_id + sub.CurrentSave = id == null ? null : (byId.get(id) ?? null) }) } @@ -1324,12 +1302,9 @@ export async function insertSubRoom( ...sub, SubRoomId: subRoomId, RoomId: roomId, + CurrentSave: null, StagedSubRoomDataSaveId: null, } - // A fresh subroom has no save, so the key is absent rather than null — same rule the - // read path applies (see {@link attachCurrentSaves}). The delete also drops the SOURCE's - // save that the spread carried on a clone; the copy below re-points it at its own row. - delete created.CurrentSave // The permission overrides follow the copy too — they live in their own table (keyed by // the id the caller is cloning FROM), so unlike the rest of the settings they aren't // carried by the blob. A fresh subroom (`createSubRoom`) passes no id and copies nothing.