Skip to content

Carry scene materials through Load Build - #729

Open
alxbouchard wants to merge 1 commit into
pascalorg:mainfrom
alxbouchard:materials-through-import
Open

Carry scene materials through Load Build#729
alxbouchard wants to merge 1 commit into
pascalorg:mainfrom
alxbouchard:materials-through-import

Conversation

@alxbouchard

@alxbouchard alxbouchard commented Aug 28, 2026

Copy link
Copy Markdown

Split out of #720 as requested in the review — the materials fix on its own, so it doesn't wait on the import-page discussion.

The bug

validateBuildJson drops the top-level materials table: every scene:<id> slot ref in an imported file points at a material that no longer exists, so custom finishes silently revert to defaults on Load Build. Confirmed against main: handleConfirmImport passes only installedPlugins to setScene, whose extra.materials support already exists.

The fix

  • ParsedBuildJson gains materials?: Record<string, SceneMaterial>; entries are validated one by one (SceneMaterial.safeParse), invalid ones skipped with a warning so a bad material never takes the import down.
  • Normalization is a deliberate, documented choice (per the review): 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. The in-line comment states this and why.
  • handleConfirmImport now takes ParsedBuildJson directly (no widened record + cast) and hands parsed.materials to setScene.
  • Tests are bun:test (bun test src/validation: 8 pass); tsc --noEmit and biome check clean; no lockfile changes.

The review's other points (vitest→bun:test in the page test, byte-accurate size cap, capture-protocol seam) are being addressed on #720.

🤖 Generated with Claude Code


Note

Low Risk
Scoped to build JSON validation and import wiring; invalid materials are non-blocking warnings and existing setScene material support is reused.

Overview
Load Build no longer drops the top-level materials table, so scene:<id> slot refs keep their custom finishes instead of reverting to defaults.

validateBuildJson now includes optional materials on ParsedBuildJson, validating each entry with SceneMaterial.safeParse and normalizing successful parses (defaults applied, unknown keys stripped) for the live scene store. Invalid entries are skipped with invalid_materials warnings; a non-object materials field is ignored without failing import. The settings panel passes parsed.materials into setScene on confirm and types the handler with ParsedBuildJson.

New unit tests cover valid carry-through, partial invalid entries, and malformed materials shapes.

Reviewed by Cursor Bugbot for commit 2f9803f. Bugbot is set up for automated code reviews on this repo. Configure here.

validateBuildJson dropped the top-level materials table, so every
scene:<id> 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 pascalorg#720 at the maintainer's request.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant