From 7bd34e5ff031ef64db134f2d44a456963c150c19 Mon Sep 17 00:00:00 2001 From: Wassim SAMAD Date: Thu, 14 May 2026 16:12:42 -0400 Subject: [PATCH] Move: update node.position directly per tick (matches item pattern) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The useLiveTransforms + sceneRegistry.position.set approach used by MoveColumnTool is broken — column ALSO disappears during move, per user observation. The mesh doesn't visibly follow the cursor. Items work because their move tool directly updates the scene store's node.position on every grid:move tick (with history paused), and the renderer reads node.position. Matching that pattern here. MoveRegistryNodeTool now: - Snapshots the original position at mount (for cancel / commit revert path). - Pauses scene history so per-tick updateNode calls don't fill undo. - On grid:move: `useScene.updateNode(id, { position })`. The kind's registered renderer reads node.position and re-renders, so the actual mesh visibly follows the cursor. - On commit: revert to original while still paused → resume → final update (single tracked action) → re-pause. Undo replays one step, not the per-tick spam. - On cancel / unmount-without-commit: restore original position with history still paused (won't enter undo), then resume. The cursor sphere stays as the aim indicator alongside the moving mesh. No translucent ghost — the actual mesh IS the preview now. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../registry/move-registry-node-tool.tsx | 86 +++++++++++-------- 1 file changed, 48 insertions(+), 38 deletions(-) diff --git a/packages/editor/src/components/tools/registry/move-registry-node-tool.tsx b/packages/editor/src/components/tools/registry/move-registry-node-tool.tsx index fae5355b..ba3e75bf 100644 --- a/packages/editor/src/components/tools/registry/move-registry-node-tool.tsx +++ b/packages/editor/src/components/tools/registry/move-registry-node-tool.tsx @@ -9,7 +9,6 @@ import { type GridEvent, nodeRegistry, sceneRegistry, - useLiveTransforms, useScene, } from '@pascal-app/core' import { useCallback, useEffect, useMemo, useRef, useState } from 'react' @@ -23,30 +22,31 @@ const roundToHalf = (value: number) => Math.round(value * 2) / 2 /** * Generic move tool for any registry-backed kind. * - * The node's actual mesh — registered with `sceneRegistry` via the kind's - * renderer — follows the cursor via: - * - `useLiveTransforms.set(node.id, { position, rotation })` triggers a - * re-render of the renderer, which applies the new position via R3F. - * - `sceneRegistry.nodes.get(node.id).position.set(...)` is a defensive - * imperative update so the move feels snappy even if the React render - * tick is delayed. + * Pattern: matches the item move tool (which works). The + * `useLiveTransforms` + `sceneRegistry.position.set` approach used by + * MoveColumnTool is broken — the renderer doesn't visibly follow. * - * No separate translucent ghost — the actual rendered mesh IS the preview. - * The cursor sphere is just a visual aim point (ring + line on the floor). + * Instead, on every `grid:move` we directly `useScene.updateNode(id, { position })` + * while history is paused. The kind's renderer already reads + * `node.position` from the store, so the mesh visibly follows the cursor. * - * Re-creation path: if the node was somehow orphaned (no entry in - * `useScene.nodes`), the registry's schema parses a fresh node at the - * committed position. Mirrors MoveColumnTool's behavior. + * Commit: pause-revert-resume-update sequence so undo replays one + * coherent action (revert → final position) instead of the per-tick + * spam that the move generated. + * + * Cancel: restore the original position before unmounting. */ export function MoveRegistryNodeTool({ node }: { node: AnyNode }) { - const initialPosition: [number, number, number] = useMemo( + // Snapshot the original position once at mount — used for cancel revert + // and for the pause-revert-resume sequence at commit. + const originalPosition: [number, number, number] = useMemo( () => 'position' in node && Array.isArray((node as { position?: unknown }).position) ? ((node as { position: [number, number, number] }).position ?? [0, 0, 0]) : [0, 0, 0], [node], ) - const [previewPosition, setPreviewPosition] = useState<[number, number, number]>(initialPosition) + const [cursorPosition, setCursorPosition] = useState<[number, number, number]>(originalPosition) const previousSnapRef = useRef<[number, number] | null>(null) const exitMoveMode = useCallback(() => { @@ -54,17 +54,17 @@ export function MoveRegistryNodeTool({ node }: { node: AnyNode }) { }, []) useEffect(() => { + // Pause history so per-tick updateNode calls don't fill the undo stack. useScene.temporal.getState().pause() previousSnapRef.current = null let committed = false const applyPreview = (position: [number, number, number]) => { - setPreviewPosition(position) - useLiveTransforms.getState().set(node.id, { - position, - rotation: 'rotation' in node ? ((node as { rotation?: number }).rotation ?? 0) : 0, - }) - sceneRegistry.nodes.get(node.id)?.position.set(position[0], position[1], position[2]) + setCursorPosition(position) + // Update the actual scene node — the renderer reads node.position and + // re-renders. The mesh visibly follows. Same pattern as the item move + // tool (useDraftNode.adopt). + useScene.getState().updateNode(node.id, { position } as Partial) } const onGridMove = (event: GridEvent) => { @@ -86,14 +86,20 @@ export function MoveRegistryNodeTool({ node }: { node: AnyNode }) { 0, roundToHalf(event.localPosition[2]), ] - const nodeId = node.id - if (nodeId && useScene.getState().nodes[nodeId]) { - committed = true - useLiveTransforms.getState().clear(nodeId) + if (useScene.getState().nodes[node.id]) { + // Restore original position while still paused, then resume and + // do a single tracked update. Undo replays the (original → final) + // single step, not every grid-move tick. + useScene.getState().updateNode(node.id, { + position: originalPosition, + } as Partial) useScene.temporal.getState().resume() - useScene.getState().updateNode(nodeId, { position } as Partial) + useScene.getState().updateNode(node.id, { position } as Partial) + useScene.temporal.getState().pause() + committed = true } else if (node.parentId) { + // Orphan re-create path: re-parse via the registry's schema. const def = nodeRegistry.get(node.type) if (def) { const reparsed = def.schema.parse({ @@ -102,23 +108,27 @@ export function MoveRegistryNodeTool({ node }: { node: AnyNode }) { metadata: {}, position, }) - committed = true useScene.temporal.getState().resume() useScene.getState().createNode(reparsed as AnyNode, node.parentId as AnyNodeId) + useScene.temporal.getState().pause() + committed = true } } - useLiveTransforms.getState().clear(node.id) sfxEmitter.emit('sfx:item-place') exitMoveMode() event.nativeEvent?.stopPropagation?.() } const onCancel = () => { - useLiveTransforms.getState().clear(node.id) + // Restore original position while paused — this won't enter undo. + useScene.getState().updateNode(node.id, { + position: originalPosition, + } as Partial) + // Defensive Three.js reset in case React render lags. sceneRegistry.nodes .get(node.id) - ?.position.set(initialPosition[0], initialPosition[1], initialPosition[2]) + ?.position.set(originalPosition[0], originalPosition[1], originalPosition[2]) useScene.temporal.getState().resume() markToolCancelConsumed() exitMoveMode() @@ -132,17 +142,17 @@ export function MoveRegistryNodeTool({ node }: { node: AnyNode }) { emitter.off('grid:move', onGridMove) emitter.off('grid:click', onGridClick) emitter.off('tool:cancel', onCancel) - useLiveTransforms.getState().clear(node.id) + // If we unmount without committing (e.g., user picks a different + // tool), restore original position so the scene doesn't show the + // stale preview state, and resume history. if (!committed) { - sceneRegistry.nodes - .get(node.id) - ?.position.set(initialPosition[0], initialPosition[1], initialPosition[2]) + useScene.getState().updateNode(node.id, { + position: originalPosition, + } as Partial) useScene.temporal.getState().resume() } } - }, [exitMoveMode, initialPosition, node]) + }, [exitMoveMode, node, originalPosition]) - // Cursor sphere is just the aim point — the actual node's rendered mesh - // is what follows via live transforms. Visible alongside. - return + return }