diff --git a/CLAUDE.md b/CLAUDE.md index 0056a14..ac28ab5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -74,13 +74,15 @@ inconsistency here without checking the client first. `{ error, success, value: null }` — e.g. `clubs` `PUT /club/:id/clubhouse` left the old clubhouse on screen until it answered the full details envelope. - Every subroom mutation (`rooms`: create, delete, `/subrooms/:sid/clone`, - `/subrooms/:sid/accessibility`, `/subrooms/:sid/data`, `/subrooms/:sid/publish_save`) - answers `{ success, error, value }` with the whole updated ROOM — the client re-renders - the room from `value`. Notably `value` is the room even for `clone`, whose product is a - new SUBROOM, and even for `data`, whose product is a SAVE; only the room-level - `POST /rooms/:id/clone` returns the thing it created. A bare subroom from `data` leaves - the old scene on screen even though the save landed — the symptom is "saved fine in the - DB, didn't update visually". + `/subrooms/:sid/accessibility`, `/subrooms/:sid/publish_save`) answers + `{ success, error, value }` with the whole updated ROOM — the client re-renders the room + from `value`. Notably `value` is the room even for `clone`, whose product is a new + SUBROOM; only the room-level `POST /rooms/:id/clone` returns the thing it created. +- The room save (`rooms`: `POST /subrooms/:sid/data`) is the ONE exception to that shape: + `value` is `{ room, subRoomDataSave }`, and `error` is NULL rather than `""`. The + `subRoomDataSave` is camelCase with a different field set from the PascalCase + `CurrentSave` embedded in the room (no persistence/OM/UGC versions, no moderation state, + 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. The key must be present (null before the first publish); read it via diff --git a/apps/rooms/src/openapi.ts b/apps/rooms/src/openapi.ts index 5b013e4..3d82d4b 100644 --- a/apps/rooms/src/openapi.ts +++ b/apps/rooms/src/openapi.ts @@ -138,6 +138,31 @@ export const LoadScreenDto = z.object({ Subtitle: z.string(), }) +/** + * The save as the room-save RESPONSE renders it — camelCase, and a different field set + * from the PascalCase `CurrentSave` embedded in a room (no persistence/OM/UGC versions, + * no moderation state, no asset arrays; but `unityAsset`/`unityAssetHash`/`dataBlobHash` + * that `CurrentSave` doesn't show). The two are deliberately not unified. + */ +export const SubRoomDataSaveResponseDto = z.object({ + subRoomDataSaveId: z.int(), + subRoomId: z.int(), + unityAssetId: z.string().nullable().describe('Null unless the save carried one'), + unityAsset: z.string().nullable().describe('Always null — we resolve no baked assets'), + unityAssetHash: z.string().nullable().describe('Always null — we resolve no baked assets'), + dataBlob: z.string(), + dataBlobHash: z.string().nullable().describe('Echoed from the request’s `SubRoomData.Hash`'), + savedByAccountId: z.int().nullable(), + savedOnPlatform: z + .int() + .describe( + 'Steam=0 Oculus=1 PlayStation=2 Xbox=3 RecNet=4 IOS=5 GooglePlay=6 Standalone=7 Pico=8' + ), + savedOnDeviceClass: z.int().describe('Unknown=0 VR=1 Screen=2 Mobile=3 VRLow=4 Quest2=5'), + description: z.string().nullable(), + createdAt: z.string(), +}) + /** * A subroom's most recent room save — the `SubRoomDataSave` the client reads to find the * scene-data blob to download. This is the ONLY place the loader looks for it, so a @@ -351,6 +376,20 @@ export const RoomEnvelope = z.object({ value: RoomDto.nullable(), }) +/** + * What `POST /rooms/{roomId}/subrooms/{subRoomId}/data` answers: `value` carries BOTH the + * updated room and the save that was just created. Note `error` is NULL here, not the + * empty string the other room envelopes use. + */ +export const RoomSaveEnvelope = z.object({ + success: z.boolean(), + error: z.string().nullable().describe('Null on success'), + value: z + .object({ room: RoomDto, subRoomDataSave: SubRoomDataSaveResponseDto }) + .nullable() + .describe('Null on a rejection'), +}) + /** The 401 the envelope-returning routes answer with — the only one that isn’t HTTP 200. */ export const UNAUTHORIZED_ENVELOPE = json( z.object({ success: z.literal(false), error: z.literal('Unauthorized'), value: z.null() }), diff --git a/apps/rooms/src/rooms.app.ts b/apps/rooms/src/rooms.app.ts index 92ce6f1..4d51b74 100644 --- a/apps/rooms/src/rooms.app.ts +++ b/apps/rooms/src/rooms.app.ts @@ -75,6 +75,7 @@ import { roomIdParam, RoomLookup, RoomResultEnvelope, + RoomSaveEnvelope, SaveSubRoomDataRequest, ServiceStatus, stringQuery, @@ -271,6 +272,33 @@ function roomResult( }) } +/** + * The room save's `value.subRoomDataSave` — a camelCase projection with a DIFFERENT + * field set from the PascalCase `CurrentSave` embedded in a room (no persistence/OM/UGC + * versions, no moderation state, no asset arrays; but `unityAsset`/`unityAssetHash` + * that `CurrentSave` never shows). Don't unify the two without checking the client. + * + * `unityAsset`/`unityAssetHash` are always null: we resolve no baked Unity assets. + */ +function toSaveResponse(save: Record) { + const str = (v: unknown) => (typeof v === 'string' ? v : null) + const num = (v: unknown) => (typeof v === 'number' ? v : null) + return { + subRoomDataSaveId: num(save.SubRoomDataSaveId), + subRoomId: num(save.SubRoomId), + unityAssetId: str(save.UnityAssetId), + unityAsset: null, + unityAssetHash: null, + dataBlob: str(save.DataBlob) ?? '', + dataBlobHash: str(save.DataBlobHash), + savedByAccountId: num(save.SavedByAccountId), + savedOnPlatform: num(save.SavedOnPlatform) ?? 0, + savedOnDeviceClass: num(save.SavedOnDeviceClass) ?? 0, + description: str(save.Description), + createdAt: str(save.CreatedAt) ?? '', + } +} + /** Client envelope for room mutations: `{ success, error, value }` (lowercase). */ function roomEnvelope(c: Context, value: unknown, error = '') { return c.json({ success: error === '', error, value }) @@ -1504,9 +1532,10 @@ const app = new Hono() '`POST …/subrooms/{subRoomId}/publish_save`. DORMS always publish — they have no', 'publish step in the client, so staging one would hide the player’s own edits.', '', - 'Answers the whole updated ROOM in the `{ success, error, value }` envelope, like', - 'every other subroom mutation — the client re-renders from `value`, and a bare', - 'subroom leaves the old scene on screen even though the save landed.', + '`value` carries BOTH the updated `room` and the `subRoomDataSave` just created,', + 'and `error` is NULL here rather than the empty string the other room envelopes', + 'use. The save is projected in camelCase with a different field set from the', + 'PascalCase `CurrentSave` embedded in the room — the two are not the same shape.', 'A subroom with no `CreatorAccountId` yet (the seeded rooms start null) gets the', 'saver’s id here, because the client NREs on a null one.', ].join('\n'), @@ -1514,7 +1543,7 @@ const app = new Hono() parameters: [roomIdParam, subRoomIdParam], requestBody: jsonBody(SaveSubRoomDataRequest, 'The uploaded blob keys and save fields'), responses: { - 200: json(RoomEnvelope, 'The updated room, or a rejection with `success: false`'), + 200: json(RoomSaveEnvelope, 'The updated room + the new save, or a rejection'), 401: UNAUTHORIZED_EMPTY, 403: FORBIDDEN_RESPONSE, }, @@ -1527,7 +1556,9 @@ const app = new Hono() const subRoomId = Number.parseInt(c.req.param('subRoomId'), 10) const room = await getRoomById(c.env.DB, roomId) - if (!room) return roomEnvelope(c, null, 'This room does not exist!') + if (!room) { + return c.json({ success: false, error: 'This room does not exist!', value: null }) + } // A valid token but not the room's owner/co-owner → 403 (the auth gate above // already returned 401 for a missing/invalid token). if (!canManageRoom(room, accountId)) return c.body(null, 403) @@ -1537,7 +1568,7 @@ const app = new Hono() // metadata blob. `OwnershipProof` is accepted and ignored. const body = (await c.req.json().catch(() => ({}))) as { RoomData?: { Filename?: string } - SubRoomData?: { Filename?: string } + SubRoomData?: { Filename?: string; Hash?: string | null } UnityAssetId?: string | null Description?: string PersistenceVersion?: number @@ -1545,8 +1576,10 @@ const app = new Hono() AutoPublish?: boolean } - const updated = await saveSubRoomData(c.env.DB, roomId, subRoomId, accountId, { + const result = await saveSubRoomData(c.env.DB, roomId, subRoomId, accountId, { subRoomDataFilename: body.SubRoomData?.Filename, + subRoomDataHash: + typeof body.SubRoomData?.Hash === 'string' ? body.SubRoomData.Hash : undefined, roomDataFilename: body.RoomData?.Filename, unityAssetId: typeof body.UnityAssetId === 'string' ? body.UnityAssetId : undefined, autoPublish: body.AutoPublish === true, @@ -1555,14 +1588,18 @@ const app = new Hono() typeof body.PersistenceVersion === 'number' ? body.PersistenceVersion : undefined, inventionUsage: typeof body.InventionUsage === 'string' ? body.InventionUsage : undefined, }) - if (!updated) return roomEnvelope(c, null, 'This subroom does not exist!') + if (!result) { + return c.json({ success: false, error: 'This subroom does not exist!', value: null }) + } - // The whole updated ROOM, like every other subroom mutation — the client - // re-renders the room from `value` and does NOT pick up a bare subroom, so - // answering with just the saved subroom leaves the old scene on screen even - // though the save landed. - await pushRoomUpdate(c, accountId, updated) - return roomEnvelope(c, updated) + // `value` carries BOTH the updated room and the save just created — and `error` + // is null here, not the empty string the other room envelopes use. + await pushRoomUpdate(c, accountId, result.room) + return c.json({ + success: true, + error: null, + value: { room: result.room, subRoomDataSave: toSaveResponse(result.save) }, + }) } ) diff --git a/apps/rooms/src/test/integration/api.test.ts b/apps/rooms/src/test/integration/api.test.ts index e01f3e8..7da49c6 100644 --- a/apps/rooms/src/test/integration/api.test.ts +++ b/apps/rooms/src/test/integration/api.test.ts @@ -989,16 +989,40 @@ describe('rooms endpoints', () => { }) expect(await envOf(await authed(2, 9999, '1'))).toMatchObject({ success: false }) - // Owner saves → 200 with the whole updated ROOM in the envelope. A bare subroom - // here leaves the client showing the old scene even though the save landed. This - // fixture sends `AutoPublish: true`, so the save goes live immediately. + // Owner saves → 200. `value` carries BOTH the updated room and the new save, and + // `error` is null (not ''). This fixture sends `AutoPublish: true`, so it goes live. const ok = await authed(2, 2, '1') expect(ok.status).toBe(200) - const saved = await envOf(ok) + const saved = (await ok.json()) as { + success: boolean + error: string | null + value: { + room: Record + subRoomDataSave: Record + } + } expect(saved.success).toBe(true) - expect(saved.value).toMatchObject({ RoomId: 2, Description: 'mydescription here' }) + expect(saved.error).toBeNull() + expect(saved.value.room).toMatchObject({ RoomId: 2, Description: 'mydescription here' }) + + // The save is a camelCase projection, NOT the PascalCase CurrentSave shape. + expect(saved.value.subRoomDataSave).toEqual({ + subRoomDataSaveId: expect.any(Number), + subRoomId: 2, + unityAssetId: null, + unityAsset: null, + unityAssetHash: null, + dataBlob: 'a84167b16796452ab70ee8a6a5b1dc5f', + dataBlobHash: null, + savedByAccountId: 1, + savedOnPlatform: 0, + savedOnDeviceClass: 0, + description: 'mydescription here', + createdAt: expect.any(String), + }) + // The saved subroom rides along inside the room's SubRooms, carrying the new save. - const savedSub = (saved.value!.SubRooms as Array>).find( + const savedSub = (saved.value.room.SubRooms as Array>).find( (s) => s.SubRoomId === 2 )! expect(savedSub).toMatchObject({ @@ -1047,11 +1071,16 @@ describe('rooms endpoints', () => { // with the room envelope. The creator stays account 1 (not clobbered). const coOwner = await authed(2, 2, '2') expect(coOwner.status).toBe(200) - const coOwnerEnv = await envOf(coOwner) + const coOwnerEnv = (await coOwner.json()) as { + success: boolean + value: { room: { SubRooms: Array> }; subRoomDataSave: unknown } + } expect(coOwnerEnv.success).toBe(true) - expect( - (coOwnerEnv.value!.SubRooms as Array>).find((s) => s.SubRoomId === 2) - ).toMatchObject({ CreatorAccountId: 1 }) + expect(coOwnerEnv.value.room.SubRooms.find((s) => s.SubRoomId === 2)).toMatchObject({ + CreatorAccountId: 1, + }) + // The save records who actually saved it, not the room's creator. + expect(coOwnerEnv.value.subRoomDataSave).toMatchObject({ savedByAccountId: 2 }) }) it('GET /rooms/:id gives every subroom a CurrentSave key (null before the first save)', async () => { @@ -1125,9 +1154,9 @@ describe('rooms endpoints', () => { UgcSubVersion: 0, ModerationState: 0, }) - // No DataBlobHash — it is commented out of the reference DTO — and no UnityAssetId - // key at all, since the client sent null. - expect('DataBlobHash' in live.CurrentSave!).toBe(false) + // DataBlobHash rides along (null — the client sent `Hash: null`); UnityAssetId is + // omitted entirely rather than nulled, since the save carried none. + expect(live.CurrentSave!.DataBlobHash).toBeNull() expect('UnityAssetId' in live.CurrentSave!).toBe(false) // It's on the room read too — that's what the loader actually fetches. diff --git a/packages/domain/src/rooms-db.ts b/packages/domain/src/rooms-db.ts index 2082e70..6799ebf 100644 --- a/packages/domain/src/rooms-db.ts +++ b/packages/domain/src/rooms-db.ts @@ -312,6 +312,8 @@ export function findSubRoom(room: Room, subRoomId: number): SubRoom | undefined export interface SaveSubRoomDataInput { /** Uploaded blob key for this subroom's scene data (becomes `CurrentSave.DataBlob`). */ subRoomDataFilename?: string + /** `SubRoomData.Hash` — echoed back as the save response's `dataBlobHash`. */ + subRoomDataHash?: string /** Uploaded blob key for the room-level METADATA blob (a separate upload). */ roomDataFilename?: string description?: string @@ -354,6 +356,7 @@ export function subRoomDataBlob(sub: SubRoom | undefined | null): string { interface BuildSaveInput { subRoomId: unknown dataBlob: string + dataBlobHash: string | null persistenceVersion: number savedByAccountId: unknown description: string @@ -379,6 +382,10 @@ function buildSubRoomSave(input: BuildSaveInput): SubRoomDataSave { ReferencedUnityAssets: [], SubRoomId: input.subRoomId, DataBlob: input.dataBlob, + // The client sends `SubRoomData.Hash` (usually null); the room-save response echoes + // it as `dataBlobHash`. One observed room payload carries it on `CurrentSave` and + // another omits it, so storing it and letting it ride along is the safe reading. + DataBlobHash: input.dataBlobHash, ReferencedUnityAssetIds: [], PersistenceVersion: input.persistenceVersion, OMVersion: 0, @@ -412,6 +419,7 @@ function legacySubRoomSave(sub: SubRoom): SubRoomDataSave | null { return buildSubRoomSave({ subRoomId: sub.SubRoomId, dataBlob: blob, + dataBlobHash: null, persistenceVersion: typeof sub.PersistenceVersion === 'number' ? sub.PersistenceVersion : 0, // The legacy shape never recorded who saved; the subroom's creator is the best // available answer (the save path is owner/co-owner gated). @@ -423,8 +431,8 @@ function legacySubRoomSave(sub: SubRoom): SubRoomDataSave | null { /** * Persist a room-save against a specific subroom and record the room-level fields the - * save carries. Returns the updated (hydrated) room, or null when the room or subroom - * doesn't exist. + * save carries. Returns the updated (hydrated) room AND the save that was just created — + * the route answers with both — or null when the room or subroom doesn't exist. * * Whether the save goes live is the client's call: `AutoPublish: true` publishes it * outright, otherwise it becomes the subroom's `staged_save_id` with the live @@ -438,7 +446,7 @@ export async function saveSubRoomData( subRoomId: number, accountId: number, input: SaveSubRoomDataInput -): Promise { +): Promise<{ room: Room; save: SubRoomDataSave } | null> { const room = await getRoomById(db, roomId) if (!room) return null // Read off the already-hydrated room rather than re-querying the subroom and its @@ -475,6 +483,7 @@ export async function saveSubRoomData( // A save that carries no new blob (e.g. a description-only save) keeps the one // the subroom already loads from. dataBlob: input.subRoomDataFilename ?? (typeof priorBlob === 'string' ? priorBlob : ''), + dataBlobHash: input.subRoomDataHash ?? null, persistenceVersion: input.persistenceVersion ?? (typeof priorVersion === 'number' ? priorVersion : 0), savedByAccountId: accountId, @@ -517,7 +526,8 @@ export async function saveSubRoomData( ]) // Re-hydrate so the returned room reflects the just-saved subroom. - return hydrateRoom(db, room) + await attachSubRooms(db, [room]) + return { room, save } } /**