Carry scene materials through Load Build - #729
Open
alxbouchard wants to merge 1 commit into
Open
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
validateBuildJsondrops the top-levelmaterialstable: everyscene:<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:handleConfirmImportpasses onlyinstalledPluginstosetScene, whoseextra.materialssupport already exists.The fix
ParsedBuildJsongainsmaterials?: 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.safeParse().datainjects defaults and drops unknown keys — the opposite ofapiGraphSchema'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.handleConfirmImportnow takesParsedBuildJsondirectly (no widened record + cast) and handsparsed.materialstosetScene.bun:test(bun test src/validation: 8 pass);tsc --noEmitandbiome checkclean; 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
setScenematerial support is reused.Overview
Load Build no longer drops the top-level
materialstable, soscene:<id>slot refs keep their custom finishes instead of reverting to defaults.validateBuildJsonnow includes optionalmaterialsonParsedBuildJson, validating each entry withSceneMaterial.safeParseand normalizing successful parses (defaults applied, unknown keys stripped) for the live scene store. Invalid entries are skipped withinvalid_materialswarnings; a non-objectmaterialsfield is ignored without failing import. The settings panel passesparsed.materialsintosetSceneon confirm and types the handler withParsedBuildJson.New unit tests cover valid carry-through, partial invalid entries, and malformed
materialsshapes.Reviewed by Cursor Bugbot for commit 2f9803f. Bugbot is set up for automated code reviews on this repo. Configure here.