From 2f9803f9e50f9ce2d648b91c1a3dc647fdbea409 Mon Sep 17 00:00:00 2001 From: ALX Date: Fri, 28 Aug 2026 11:29:36 -0400 Subject: [PATCH 1/2] Carry scene materials through Load Build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit validateBuildJson dropped the top-level materials table, so every scene: slot ref in an imported file pointed at a material that no longer existed — custom finishes silently reverted to defaults on Load Build. ParsedBuildJson now carries materials, each entry validated individually (a bad material never takes the import down, it is skipped with a warning), and handleConfirmImport hands them to setScene, whose extra.materials support already existed. Normalization here is DELIBERATE and documented in-line: safeParse().data injects defaults and drops unknown keys — the opposite of apiGraphSchema's preserve-unknowns stance — because import feeds the live scene store, which only understands schema-shaped materials. Split out of #720 at the maintainer's request. --- .../validation/validate-build-json.test.ts | 45 +++++++++++++++++++ .../src/validation/validate-build-json.ts | 45 +++++++++++++++++++ .../sidebar/panels/settings-panel/index.tsx | 11 ++--- 3 files changed, 96 insertions(+), 5 deletions(-) diff --git a/packages/core/src/validation/validate-build-json.test.ts b/packages/core/src/validation/validate-build-json.test.ts index 64e3d0972e..e7d20ebb25 100644 --- a/packages/core/src/validation/validate-build-json.test.ts +++ b/packages/core/src/validation/validate-build-json.test.ts @@ -126,3 +126,48 @@ describe('validateBuildJson with registered plugin kinds', () => { expect(result.schemaIssues[0]?.nodeType).toBe('trees:tree') }) }) + +describe('scene materials', () => { + const minimalGraph = () => ({ + nodes: { + building_1: { id: 'building_1', type: 'building', children: ['level_1'] }, + level_1: { id: 'level_1', type: 'level', children: [] }, + }, + rootNodeIds: ['building_1'], + }) + + test('carries valid materials through to parsed', () => { + const result = validateBuildJson({ + ...minimalGraph(), + materials: { + mat_a: { + id: 'mat_a', + name: 'Measured cabinet', + material: { properties: { color: '#595c5a' } }, + }, + }, + }) + expect(result.ok).toBe(true) + expect(result.parsed?.materials?.mat_a?.name).toBe('Measured cabinet') + }) + + test('skips invalid material entries with a warning, keeps the rest', () => { + const result = validateBuildJson({ + ...minimalGraph(), + materials: { + mat_ok: { id: 'mat_ok', name: 'Fine', material: {} }, + mat_bad: { name: 42 }, + }, + }) + expect(result.ok).toBe(true) + expect(Object.keys(result.parsed?.materials ?? {})).toEqual(['mat_ok']) + expect(result.warnings.some((w) => w.code === 'invalid_materials')).toBe(true) + }) + + test('warns when materials is not an object', () => { + const result = validateBuildJson({ ...minimalGraph(), materials: 'nope' }) + expect(result.ok).toBe(true) + expect(result.parsed?.materials).toBeUndefined() + expect(result.warnings.some((w) => w.code === 'invalid_materials')).toBe(true) + }) +}) diff --git a/packages/core/src/validation/validate-build-json.ts b/packages/core/src/validation/validate-build-json.ts index 1257a43ef2..19615386c8 100644 --- a/packages/core/src/validation/validate-build-json.ts +++ b/packages/core/src/validation/validate-build-json.ts @@ -1,4 +1,5 @@ import { nodeRegistry } from '../registry' +import { SceneMaterial } from '../schema/scene-material' import { AnyNode, type AnyNodeType } from '../schema/types' import { healSceneNodes } from '../utils/heal-scene-graph' @@ -24,6 +25,8 @@ export type ParsedBuildJson = { nodes: Record rootNodeIds: string[] installedPlugins?: string[] + /** Scene materials referenced by node `slots` (`scene:`). */ + materials?: Record } export type SchemaIssue = { @@ -111,6 +114,7 @@ export function validateBuildJson(input: unknown): ValidateBuildJsonResult { const nodesRaw = input.nodes const rootNodeIdsRaw = input.rootNodeIds const installedPluginsRaw = input.installedPlugins + const materialsRaw = input.materials if (!isPlainObject(nodesRaw)) { errors.push({ @@ -160,6 +164,46 @@ export function validateBuildJson(input: unknown): ValidateBuildJsonResult { }) } + // Scene materials ride along with the graph: nodes reference them by + // `scene:` slot refs, so dropping the table here silently strips + // every custom finish from the imported scene. Invalid entries are + // skipped one by one — a bad material must not take the import down. + // + // DELIBERATE: `safeParse().data` NORMALIZES — defaults are injected + // and unknown keys dropped. That is the opposite of the API boundary + // (`apiGraphSchema` preserves unknown fields on purpose), and it is + // chosen here because import feeds the live scene store, which only + // understands schema-shaped materials; a hand-edited file with a + // half-formed material should land as something the renderer can + // draw, not round-trip garbage. + let materials: Record | undefined + if (isPlainObject(materialsRaw)) { + let skipped = 0 + const kept: Record = {} + for (const [id, value] of Object.entries(materialsRaw)) { + const result = SceneMaterial.safeParse(value) + if (result.success) { + kept[id] = result.data + } else { + skipped += 1 + } + } + if (Object.keys(kept).length > 0) materials = kept + if (skipped > 0) { + warnings.push({ + severity: 'warning', + code: 'invalid_materials', + message: `Ignored ${skipped} invalid scene material${skipped === 1 ? '' : 's'}.`, + }) + } + } else if (materialsRaw !== undefined) { + warnings.push({ + severity: 'warning', + code: 'invalid_materials', + message: 'Ignored invalid "materials" — expected an object of id → material.', + }) + } + if (strippedChildRefs > 0 || droppedWallIds.length > 0) { warnings.push({ severity: 'warning', @@ -373,6 +417,7 @@ export function validateBuildJson(input: unknown): ValidateBuildJsonResult { nodes, rootNodeIds, ...(installedPlugins ? { installedPlugins } : {}), + ...(materials ? { materials } : {}), } : null, stats, diff --git a/packages/editor/src/components/ui/sidebar/panels/settings-panel/index.tsx b/packages/editor/src/components/ui/sidebar/panels/settings-panel/index.tsx index fecc75978c..4b466466fa 100644 --- a/packages/editor/src/components/ui/sidebar/panels/settings-panel/index.tsx +++ b/packages/editor/src/components/ui/sidebar/panels/settings-panel/index.tsx @@ -2,6 +2,7 @@ import { clearSceneHistory, emitter, useScene, + type ParsedBuildJson, validateBuildJson, } from '@pascal-app/core' import { useViewer } from '@pascal-app/viewer' @@ -287,16 +288,16 @@ export function SettingsPanel({ e.target.value = '' } - const handleConfirmImport = (parsed: { - nodes: Record - rootNodeIds: string[] - installedPlugins?: string[] - }) => { + const handleConfirmImport = (parsed: ParsedBuildJson) => { const currentScene = useScene.getState() setScene( parsed.nodes as Parameters[0], parsed.rootNodeIds as Parameters[1], { + // Without this, every `scene:` slot ref in the imported file + // pointed at a material that no longer existed — custom finishes + // silently reverted to defaults on import. + materials: parsed.materials, installedPlugins: parsed.installedPlugins ?? currentScene.installedPlugins, hasExplicitPluginInstallState: parsed.installedPlugins !== undefined || currentScene.hasExplicitPluginInstallState, From 2996a1190dadb9b7e31e084463758598827afc05 Mon Sep 17 00:00:00 2001 From: ALX Date: Sun, 30 Aug 2026 23:48:27 -0400 Subject: [PATCH 2/2] Save Build exports the materials table it now imports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up (#729): paint a finish, Save Build, Load Build that file — the finish reverted to default because handleSaveBuild still exported only { nodes, rootNodeIds, installedPlugins }. Materials ride along now, closing the round-trip this PR opened on the import side. Also names the skipped ids in the invalid_materials warning: the audience is hand-edited files, and a bare count leaves nothing to repair by. Co-Authored-By: Claude Fable 5 --- .../core/src/validation/validate-build-json.test.ts | 5 ++++- packages/core/src/validation/validate-build-json.ts | 12 ++++++++---- .../ui/sidebar/panels/settings-panel/index.tsx | 6 +++++- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/packages/core/src/validation/validate-build-json.test.ts b/packages/core/src/validation/validate-build-json.test.ts index e7d20ebb25..0c7836fc9d 100644 --- a/packages/core/src/validation/validate-build-json.test.ts +++ b/packages/core/src/validation/validate-build-json.test.ts @@ -161,7 +161,10 @@ describe('scene materials', () => { }) expect(result.ok).toBe(true) expect(Object.keys(result.parsed?.materials ?? {})).toEqual(['mat_ok']) - expect(result.warnings.some((w) => w.code === 'invalid_materials')).toBe(true) + const warning = result.warnings.find((w) => w.code === 'invalid_materials') + expect(warning).toBeDefined() + // The skipped ids are named so a hand-edited file can be repaired. + expect(warning?.message).toContain('mat_bad') }) test('warns when materials is not an object', () => { diff --git a/packages/core/src/validation/validate-build-json.ts b/packages/core/src/validation/validate-build-json.ts index 19615386c8..a31e1726c4 100644 --- a/packages/core/src/validation/validate-build-json.ts +++ b/packages/core/src/validation/validate-build-json.ts @@ -178,22 +178,26 @@ export function validateBuildJson(input: unknown): ValidateBuildJsonResult { // draw, not round-trip garbage. let materials: Record | undefined if (isPlainObject(materialsRaw)) { - let skipped = 0 + const skippedIds: string[] = [] const kept: Record = {} for (const [id, value] of Object.entries(materialsRaw)) { const result = SceneMaterial.safeParse(value) if (result.success) { kept[id] = result.data } else { - skipped += 1 + skippedIds.push(id) } } if (Object.keys(kept).length > 0) materials = kept - if (skipped > 0) { + if (skippedIds.length > 0) { + // Name the ids: the audience is hand-edited files, and a count + // alone leaves nothing to repair by. warnings.push({ severity: 'warning', code: 'invalid_materials', - message: `Ignored ${skipped} invalid scene material${skipped === 1 ? '' : 's'}.`, + message: `Ignored ${skippedIds.length} invalid scene material${ + skippedIds.length === 1 ? '' : 's' + }: ${skippedIds.join(', ')}.`, }) } } else if (materialsRaw !== undefined) { diff --git a/packages/editor/src/components/ui/sidebar/panels/settings-panel/index.tsx b/packages/editor/src/components/ui/sidebar/panels/settings-panel/index.tsx index 4b466466fa..3baefe5a6f 100644 --- a/packages/editor/src/components/ui/sidebar/panels/settings-panel/index.tsx +++ b/packages/editor/src/components/ui/sidebar/panels/settings-panel/index.tsx @@ -192,6 +192,7 @@ export function SettingsPanel({ const nodes = useScene((state) => state.nodes) const rootNodeIds = useScene((state) => state.rootNodeIds) const installedPlugins = useScene((state) => state.installedPlugins) + const materials = useScene((state) => state.materials) const setScene = useScene((state) => state.setScene) const clearScene = useScene((state) => state.clearScene) const resetSelection = useViewer((state) => state.resetSelection) @@ -232,7 +233,10 @@ export function SettingsPanel({ const isLocalProject = false // Props-based; only show cloud sections when projectId provided const handleSaveBuild = () => { - const sceneData = { nodes, rootNodeIds, installedPlugins } + // Materials ride along: nodes reference them by `scene:` slot + // refs, so a save without the table produces a file whose custom + // finishes revert to defaults on the very Load Build path below. + const sceneData = { nodes, rootNodeIds, installedPlugins, materials } const json = JSON.stringify(sceneData, null, 2) const blob = new Blob([json], { type: 'application/json' }) const url = URL.createObjectURL(blob)