Fix spawn flag inlining + shelf cursor frame + simpler renderer

Three concrete bugs surfaced when first-running the spike in community:

1) NEXT_PUBLIC_USE_REGISTRY_FOR_SPAWN flag never detected:
   The previous readEnvFlag used dynamic bracket access
   (`env?.[name]`), which Next.js / Turbopack does NOT substitute at
   build time. Only literal `process.env.NEXT_PUBLIC_FOO` references
   get inlined into the client bundle. Switched to literal access
   plus a `typeof process` guard. Spawn now toggles via the flag as
   designed.

2) Shelf cursor appeared offset from the mouse:
   The cursor mesh lives inside the ToolManager's building-local
   group, but the tool was setting `cursorRef.current.position` to
   level-local coordinates (computed via `worldToLocal(level)`).
   Result: cursor shifted by (building-pos − level-pos) in worst
   case. Switched cursor display to use `event.localPosition`
   (already building-local) with grid snap — matches the legacy
   spawn-tool pattern. The commit path keeps the worldToLocal(level)
   conversion since the shelf node's `position` field is stored
   relative to its level parent.

3) Shelf rendered invisibly after click (suspected):
   The renderer used a useEffect-swap pattern where it mounted an
   empty <group> and imperatively added Three.js children from a
   buildShelfGeometry() Group. Plausibly fragile under StrictMode
   double-invoke or fast HMR. Switched to inline R3F JSX — top
   board + brackets as plain <mesh> primitives. The pure geometry
   function still exists in geometry.ts for tests and AI-authored
   consumers; renderer just doesn't go through it.

Diagnostics added (dev-only; removed once spawn parity ships):
- `[shelf] placed <id> level-local <pos> parent <levelId>` on click
- `[shelf] rendered <id> at <pos>` on mount

Also: types: ["node"] in nodes/tsconfig.json so the typeof process
guard typechecks cleanly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Wassim SAMAD
2026-05-14 14:27:08 -04:00
co-authored by Claude Opus 4.7
parent ac71c1a83b
commit a89a1efccf
4 changed files with 83 additions and 52 deletions
+8 -8
View File
@@ -10,15 +10,15 @@ import { spawnDefinition } from './spawn'
* Removed in the PR that signs off parity (legacy spawn files deleted in the * Removed in the PR that signs off parity (legacy spawn files deleted in the
* same commit). All other built-in node migrations follow the same pattern. * same commit). All other built-in node migrations follow the same pattern.
*/ */
function readEnvFlag(name: string): boolean {
const env = (globalThis as { process?: { env?: Record<string, string | undefined> } }).process
?.env
const flag = env?.[name]
return flag === '1' || flag === 'true'
}
function isSpawnRegistryEnabled(): boolean { function isSpawnRegistryEnabled(): boolean {
return readEnvFlag('NEXT_PUBLIC_USE_REGISTRY_FOR_SPAWN') // Next.js / Turbopack inlines `process.env.NEXT_PUBLIC_*` references at
// build time, but ONLY when the access is a literal property — dynamic
// bracket access (`env[name]`) is not substituted and resolves to
// undefined in the browser. Keep this as a literal so the value is baked
// into the client bundle.
if (typeof process === 'undefined') return false
const flag = process.env.NEXT_PUBLIC_USE_REGISTRY_FOR_SPAWN
return flag === '1' || flag === 'true'
} }
function getBuiltinNodes(): AnyNodeDefinition[] { function getBuiltinNodes(): AnyNodeDefinition[] {
+54 -33
View File
@@ -2,24 +2,23 @@
import { useLiveTransforms, useRegistry } from '@pascal-app/core' import { useLiveTransforms, useRegistry } from '@pascal-app/core'
import { useEffect, useMemo, useRef } from 'react' import { useEffect, useMemo, useRef } from 'react'
import type { Group } from 'three' import { Color, type Group } from 'three'
import { buildShelfGeometry } from './geometry'
import type { ShelfNode } from './schema' import type { ShelfNode } from './schema'
// Note: `useNodeEvents` from @pascal-app/viewer has a hardcoded kind list and // Note: useNodeEvents from @pascal-app/viewer has a hardcoded kind list and
// doesn't yet know about 'shelf'. Phase 4 generalizes it to consume the // doesn't yet know about 'shelf'. Phase 4 generalizes it via the registry —
// registry — until then, shelf selection works via R3F's default raycast // until then, shelf selection works via R3F's default raycast (clicks bubble
// (clicks bubble through the scene; the editor's selection manager handles // through; the editor's selection manager hit-tests the registered Object3D).
// them by hit-testing the registered Object3D).
/** /**
* Registry-driven shelf renderer. * Registry-driven shelf renderer. Renders top board + brackets as inline R3F
* primitives so React owns the scene graph end-to-end — no imperative
* children swap.
* *
* The pure `buildShelfGeometry` function returns a Group of meshes. We mount * The pure `buildShelfGeometry` function in `./geometry.ts` produces the same
* an empty group, attach event handlers, register with `sceneRegistry`, and * shape outside of React (used by tests + reachable by AI-authored consumers
* imperatively swap in the built geometry whenever the schema-relevant fields * that want a Three.js Group). Keeping both costs nothing because the shape
* change. This pattern keeps the JSX trivial and centralizes parametric work * primitives are tiny.
* in the pure function — better for AI authoring and easier to swap out.
*/ */
const ShelfRenderer = ({ node }: { node: ShelfNode }) => { const ShelfRenderer = ({ node }: { node: ShelfNode }) => {
const ref = useRef<Group>(null!) const ref = useRef<Group>(null!)
@@ -27,27 +26,23 @@ const ShelfRenderer = ({ node }: { node: ShelfNode }) => {
useRegistry(node.id, 'shelf', ref) useRegistry(node.id, 'shelf', ref)
// Build a fresh Group each time the parametric fields change. const color = useMemo(() => new Color(node.color), [node.color])
const built = useMemo( const topY = node.height + node.thickness / 2
() => buildShelfGeometry(node),
[node.width, node.depth, node.thickness, node.height, node.bracketStyle, node.color], // Bracket dimensions mirror buildShelfGeometry — keep these in sync if the
) // geometry function evolves. Phase 4 may consolidate.
const inset = Math.min(0.12, node.width / 6)
const bracketHeight = Math.max(0.01, node.height)
const bracketWidth =
node.bracketStyle === 'industrial'
? Math.max(0.04, node.depth * 0.2)
: Math.max(0.02, node.depth * 0.12)
const bracketDepth = node.bracketStyle === 'industrial' ? node.depth * 0.95 : node.depth * 0.7
// Mount the built children under our group ref. Re-runs when `built`
// changes (parametric edit) or when the parent ref mounts.
useEffect(() => { useEffect(() => {
const root = ref.current // biome-ignore lint/suspicious/noConsole: dev-only verification log
if (!root) return console.info('[shelf] rendered', node.id, 'at', node.position)
// Clear previous children. We don't dispose the buffer geometries here }, [node.id, node.position])
// because they're owned by the previous `built` and were already
// discarded by React's reconciler when useMemo recomputed.
while (root.children.length > 0) {
root.remove(root.children[0]!)
}
for (const child of [...built.children]) {
root.add(child)
}
}, [built])
return ( return (
<group <group
@@ -55,7 +50,33 @@ const ShelfRenderer = ({ node }: { node: ShelfNode }) => {
ref={ref} ref={ref}
rotation={liveTransform?.rotation ? [0, liveTransform.rotation, 0] : node.rotation} rotation={liveTransform?.rotation ? [0, liveTransform.rotation, 0] : node.rotation}
visible={node.visible} visible={node.visible}
/> >
{/* Top board */}
<mesh position={[0, topY, 0]} name="shelf-top">
<boxGeometry args={[node.width, node.thickness, node.depth]} />
<meshStandardMaterial color={color} roughness={0.65} metalness={0.05} />
</mesh>
{/* Brackets (skipped for 'hidden' style) */}
{node.bracketStyle !== 'hidden' && (
<>
<mesh
position={[-(node.width / 2 - inset), bracketHeight / 2, 0]}
name="shelf-bracket-left"
>
<boxGeometry args={[bracketWidth, bracketHeight, bracketDepth]} />
<meshStandardMaterial color={color} roughness={0.65} metalness={0.05} />
</mesh>
<mesh
position={[node.width / 2 - inset, bracketHeight / 2, 0]}
name="shelf-bracket-right"
>
<boxGeometry args={[bracketWidth, bracketHeight, bracketDepth]} />
<meshStandardMaterial color={color} roughness={0.65} metalness={0.05} />
</mesh>
</>
)}
</group>
) )
} }
+19 -10
View File
@@ -3,8 +3,8 @@
import { import {
emitter, emitter,
type GridEvent, type GridEvent,
sceneRegistry,
ShelfNode, ShelfNode,
sceneRegistry,
snapPointToGrid, snapPointToGrid,
useScene, useScene,
} from '@pascal-app/core' } from '@pascal-app/core'
@@ -13,8 +13,17 @@ import { useEffect, useRef } from 'react'
import { type Group, Vector3 } from 'three' import { type Group, Vector3 } from 'three'
const worldVector = new Vector3() const worldVector = new Vector3()
const GRID_STEP = 0.5 // match the editor's default placement grid const GRID_STEP = 0.5
/**
* Convert a click event into the shelf's commit position (level-local). The
* shelf node's `position` field is stored relative to its level parent, so
* we project the click point into the level's local frame before storing.
*
* Different from the cursor preview path: the cursor lives inside the
* ToolManager's building-local group and snaps to `event.localPosition`
* directly. This conversion only applies to the *committed* data.
*/
function getLevelLocalPosition(levelId: string, event: GridEvent): [number, number, number] { function getLevelLocalPosition(levelId: string, event: GridEvent): [number, number, number] {
const levelObject = sceneRegistry.nodes.get(levelId) const levelObject = sceneRegistry.nodes.get(levelId)
if (!levelObject) { if (!levelObject) {
@@ -36,11 +45,12 @@ const ShelfTool = () => {
if (!activeLevelId) return if (!activeLevelId) return
const onGridMove = (event: GridEvent) => { const onGridMove = (event: GridEvent) => {
// Imperative position update — no React state, so the component // Cursor lives in the ToolManager's building-local group. Use
// doesn't re-render. R3F-applied props would otherwise clobber the // `event.localPosition` (already building-local) so the visual cursor
// imperative `position.set` on the next render. // sits where the mouse hits the floor. Legacy spawn-tool does the
const next = getLevelLocalPosition(activeLevelId, event) // same — don't apply worldToLocal here.
cursorRef.current?.position.set(next[0], next[1], next[2]) const [sx, sz] = snapPointToGrid([event.localPosition[0], event.localPosition[2]], GRID_STEP)
cursorRef.current?.position.set(sx, event.localPosition[1], sz)
} }
const onGridClick = (event: GridEvent) => { const onGridClick = (event: GridEvent) => {
@@ -52,6 +62,8 @@ const ShelfTool = () => {
}) })
useScene.getState().createNode(shelf, activeLevelId) useScene.getState().createNode(shelf, activeLevelId)
useViewer.getState().setSelection({ selectedIds: [shelf.id] }) useViewer.getState().setSelection({ selectedIds: [shelf.id] })
// biome-ignore lint/suspicious/noConsole: dev-only verification log
console.info('[shelf] placed', shelf.id, 'level-local', position, 'parent', activeLevelId)
} }
emitter.on('grid:move', onGridMove) emitter.on('grid:move', onGridMove)
@@ -65,9 +77,6 @@ const ShelfTool = () => {
if (!activeLevelId) return null if (!activeLevelId) return null
// Cursor preview — a translucent shelf-shaped slab. No `position` prop on
// the group; we move it imperatively via the ref so React re-renders don't
// reset it to the origin.
return ( return (
<group ref={cursorRef}> <group ref={cursorRef}>
<mesh position={[0, 0.9, 0]}> <mesh position={[0, 0.9, 0]}>
+2 -1
View File
@@ -7,7 +7,8 @@
"composite": true, "composite": true,
"incremental": true, "incremental": true,
"jsx": "react-jsx", "jsx": "react-jsx",
"lib": ["DOM", "ES2022"] "lib": ["DOM", "ES2022"],
"types": ["node"]
}, },
"include": ["src"], "include": ["src"],
"exclude": ["node_modules", "dist", "**/*.test.ts", "**/*.test.tsx"], "exclude": ["node_modules", "dist", "**/*.test.ts", "**/*.test.tsx"],