Skip to content

nodes: let hosts set the wall materials resolver - #1031

Open
atyrode wants to merge 2 commits into
pascalorg:mainfrom
atyrode:nodes-wall-materials-resolver
Open

atyrode wants to merge 2 commits into
pascalorg:mainfrom
atyrode:nodes-wall-materials-resolver

Conversation

@atyrode

@atyrode atyrode commented Oct 12, 2026 •

Copy link
Copy Markdown

WallCutout already accepts a materialResolver, but WallSystems hard-wires getCurtainAwareWallMaterials, so a host cannot restyle cut-away walls without forking the wall kind (plugins cannot re-register wall).

This adds setWallMaterialsResolver(resolver | null), read by WallSystems through useSyncExternalStore (null restores the default), and exports getCurtainAwareWallMaterials so a host can wrap the default and replace one variant — for example a low stub in place of the translucent film for cut walls.

No behaviour change by default.

  • bun run check-types --filter=@pascal-app/nodes: passes
  • Biome on the three files: clean

Note

Low Risk
Optional host hook with default unchanged; batch re-sew on resolver change is covered by a new test and mirrors existing appearance invalidation.

Overview
Hosts can now override how WallCutout picks wall material sets (visible, cut-away, translucent, delete) without forking the built-in wall kind. setWallMaterialsResolver(resolver | null) swaps the resolver globally (null restores getCurtainAwareWallMaterials), with getCurtainAwareWallMaterials exported so hosts can wrap the default and tweak one variant (e.g. cut-away).

WallSystems subscribes via useSyncExternalStore and passes the active resolver into WallCutout instead of hard-wiring the curtain-aware helper. Wall batch merging treats resolver changes like other appearance inputs, so merged floor meshes re-sew after a swap instead of keeping stale materials. Default behavior is unchanged when no host override is set.

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

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 747292e. Configure here.

Comment thread packages/nodes/src/wall/system.tsx

Copy link
Copy Markdown
Member

Hi @atyrode, thanks for this, and sorry it sat for a bit. The opt-in resolver is a small, clean way to let hosts restyle cut-away walls without forking the wall kind.

One thing to sort out before it can go in: Bugbot's finding on system.tsx holds up. WallBatchSystem doesn't know about the resolver, so swapping it after a floor has been batched leaves the merged meshes on the old materials. Either have a resolver change invalidate the batch (for example by adding a resolver version to whatever key WallBatchSystem rebuilds on), or document that hosts must call setWallMaterialsResolver before the scene mounts. A rebase on the latest main would also let the full CI run. Are you still up for landing it?

@atyrode
atyrode force-pushed the nodes-wall-materials-resolver branch from 747292e to b773bdf Compare October 27, 2026 20:46
@atyrode

atyrode commented Oct 27, 2026

Copy link
Copy Markdown
Author

Yes, thanks for the review. Rebased on main and pushed b773bdf:

  • WallBatchSystem now counts the active resolver among its appearance inputs (appearanceChanged), alongside shading, textures, theme and the material library. A resolver swap disposes the batches and re-sews every level from the walls' new materials, the same way a theme switch does.
  • New test in wall-batch-system.test.ts: on a batched level, swapping the resolver re-sews a new batch holding the new materials. It fails without the change.

Locally: turbo run check-types --filter=@pascal-app/nodes passes; Biome is clean on the touched files. packages/nodes bun test gives 4966 pass and 9 fail, and the same 9 fail on main in my environment, so none are introduced here.

This branch has not been deployed

No deployments
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.

2 participants