diff --git a/frontend/apps/artcraft/app/src/pages/PageScene/actions/setObjectColor.ts b/frontend/apps/artcraft/app/src/pages/PageScene/actions/setObjectColor.ts index 330df13029..76263612b0 100644 --- a/frontend/apps/artcraft/app/src/pages/PageScene/actions/setObjectColor.ts +++ b/frontend/apps/artcraft/app/src/pages/PageScene/actions/setObjectColor.ts @@ -1,17 +1,14 @@ import type Editor from "../engine/editor"; import { ColorAction } from "../engine/editor/actions/ColorAction"; -// Apply a color change and record it for undo. +// One-shot apply: use when the change is atomic (e.g. a button click +// or programmatic setter). Records a single ColorAction. // -// Burst inputs (the native dialog spams onChange -// per slider pixel) call this on every onChange. HistoryManager.record -// coalesces consecutive ColorActions for the same uuid into one undo -// entry via ColorAction.tryMerge — so the caller doesn't need -// session-state. -// -// Reads the before-state from the live engine object so the helper is -// self-contained and stays correct even after undo/redo (where the -// engine state diverges from any local React state the view may hold). +// For interactive flows where a single user gesture fires many +// intermediate values (the native dialog spams +// onChange per slider pixel), use beginColorSession instead — it +// applies each intermediate value to the engine for visual feedback +// but records only ONE ColorAction on commit. export function setObjectColor( editor: Editor, uuid: string, @@ -24,3 +21,38 @@ export function setObjectColor( editor.activeScene.setColor(uuid, color); editor.history.record(new ColorAction(editor, uuid, before, color)); } + +// Session-style color edit: snapshots the before-state at construction +// (focus / picker open), applies each intermediate value to the engine +// (visual feedback during slider drag), and records exactly one +// ColorAction on commit (focus loss / picker close). No-op on commit +// if the final color matches the before-state. +export type ColorSession = { + apply(color: string): void; + commit(): void; +}; + +export function beginColorSession( + editor: Editor, + uuid: string, +): ColorSession { + const startObj = editor.activeScene.scene.getObjectByProperty( + "uuid", + uuid, + ); + const before = (startObj?.userData.color as string) ?? "#ffffff"; + return { + apply(color: string) { + editor.activeScene.setColor(uuid, color); + }, + commit() { + const endObj = editor.activeScene.scene.getObjectByProperty( + "uuid", + uuid, + ); + const after = (endObj?.userData.color as string) ?? before; + if (before === after) return; + editor.history.record(new ColorAction(editor, uuid, before, after)); + }, + }; +} diff --git a/frontend/apps/artcraft/app/src/pages/PageScene/actions/transformObject.ts b/frontend/apps/artcraft/app/src/pages/PageScene/actions/transformObject.ts index ca0316723e..4cdf449645 100644 --- a/frontend/apps/artcraft/app/src/pages/PageScene/actions/transformObject.ts +++ b/frontend/apps/artcraft/app/src/pages/PageScene/actions/transformObject.ts @@ -1,39 +1,39 @@ import type Editor from "../engine/editor"; import { TransformAction } from "../engine/editor/actions/TransformAction"; -import { - snapshotTransform, - transformsEqual, -} from "../engine/editor/actions/snapshots"; -import type { XYZ } from "../datastructures/common"; -// Apply a transform change and record it for undo. +// A transform "session" — opened on the first edit of a multi-input +// editing flow (e.g. Object Panel position/rotation/scale text fields), +// committed at the end of the session (selection change, panel +// unmount, focus moving away from the panel). // -// Burst inputs (panel keystrokes, scrub drags) call this on every -// engine update. HistoryManager.record coalesces consecutive -// TransformActions for the same uuid into one undo entry, so the -// caller doesn't need session-state — `tryMerge` on TransformAction -// folds successive entries into the most recent one. +// The session captures the before-state at construction. On commit it +// captures the after-state, pushes a single TransformAction onto the +// undo stack only if the transform actually changed, and re-syncs the +// Zustand `objectPanel` slice from the engine so the panel's view of +// the selection matches the engine state. // -// Snapshots before and after so the recorded action holds the actual -// engine state delta (not the panel's local React state). Also -// re-syncs the Zustand objectPanel slice so the panel's -// currentSceneObject reflects the post-update transform. -export function transformObject( +// Engine-level concerns (capture / apply / revert) stay in +// `engine/editor/actions/TransformAction.ts`; this file is the +// orchestration wrapper that the action layer (UI code) uses so the +// view never has to construct an UndoableAction directly. +export type TransformSession = { + commit(): void; +}; + +export function beginTransformSession( editor: Editor, uuid: string, - position: XYZ, - rotation: XYZ, - scale: XYZ, -): void { - const obj = editor.activeScene.scene.getObjectByProperty("uuid", uuid); - if (!obj) return; - const before = snapshotTransform(obj); - - editor.sceneManager?.updateSelectedTransform(position, rotation, scale); - - const after = snapshotTransform(obj); - if (!transformsEqual(before, after)) { - editor.history.record(new TransformAction(editor, uuid, before, after)); - } - editor.selection.updateSelectedUI(); +): TransformSession { + const action = new TransformAction(editor, uuid); + return { + commit() { + if (action.commit()) { + editor.history.record(action); + } + // Push the engine's post-edit transform back into the Zustand + // objectPanel slice so the panel's currentSceneObject vectors + // are in sync with what the user just committed. Idempotent. + editor.selection.updateSelectedUI(); + }, + }; } diff --git a/frontend/apps/artcraft/app/src/pages/PageScene/comps/ControlPanelSceneObject/ControlPanelSceneObject.tsx b/frontend/apps/artcraft/app/src/pages/PageScene/comps/ControlPanelSceneObject/ControlPanelSceneObject.tsx index 2cbd148a48..cab0d6c34b 100644 --- a/frontend/apps/artcraft/app/src/pages/PageScene/comps/ControlPanelSceneObject/ControlPanelSceneObject.tsx +++ b/frontend/apps/artcraft/app/src/pages/PageScene/comps/ControlPanelSceneObject/ControlPanelSceneObject.tsx @@ -3,6 +3,7 @@ import { useContext, useEffect, useId, + useRef, useState, } from "react"; import { Transition } from "@headlessui/react"; @@ -24,8 +25,14 @@ import { objectMismatch } from "~/pages/PageScene/comps/ControlPanelSceneObject/ import { XYZ } from "~/pages/PageScene/datastructures/common"; import { pageHeight } from "~/signals"; import { DraggablePrecisionMutator } from "./DraggablePrecisionMutator"; -import { setObjectColor } from "~/pages/PageScene/actions/setObjectColor"; -import { transformObject } from "~/pages/PageScene/actions/transformObject"; +import { + beginColorSession, + ColorSession, +} from "~/pages/PageScene/actions/setObjectColor"; +import { + beginTransformSession, + TransformSession, +} from "~/pages/PageScene/actions/transformObject"; // TODO this will be useful later to fix the bug on leading zeros // const formatNumber = (input: string): number => { @@ -65,11 +72,34 @@ export const ControlPanelSceneObject = () => { const [color, setColor] = useState("#ffffff"); - // No session machinery here — burst inputs (keystrokes, scrub drags, - // native color-picker per-pixel onChange) record per engine update. - // HistoryManager coalesces consecutive same-target same-kind actions - // into one undo entry via tryMerge, so each editing run collapses to - // a single Ctrl+Z step automatically. + // Pending transform session — opened on first panel edit, committed + // when the *selection's uuid* actually changes (NOT on every per-frame + // updateSelectedUI re-push that just creates a new currentSceneObject + // reference for the same uuid). lastUuidRef tracks the previous uuid + // so the commit effect can distinguish a real selection change from + // a same-uuid resync. + const transformSessionRef = useRef(null); + const lastUuidRef = useRef(null); + + const beginPanelTransform = () => { + if (transformSessionRef.current) return; + if (!editorEngine || !currentSceneObject) return; + transformSessionRef.current = beginTransformSession( + editorEngine, + currentSceneObject.object_uuid, + ); + }; + + const commitPanelTransform = () => { + transformSessionRef.current?.commit(); + transformSessionRef.current = null; + }; + + // Color picker session — opened on input focus (picker dialog opens), + // committed on input blur (picker dialog closes). Records exactly one + // ColorAction per pick regardless of how many onChange events the + // native picker fires while the user drags the slider. + const colorSessionRef = useRef(null); const colorInputId = useId(); @@ -93,32 +123,37 @@ export const ControlPanelSceneObject = () => { } useEffect(() => { - if (!inputsUpdated || !editorEngine || !currentSceneObject) { + if (!inputsUpdated || !editorEngine) { return; } setInputsUpdated(false); - transformObject( - editorEngine, - currentSceneObject.object_uuid, + editorEngine.sceneManager?.updateSelectedTransform( localToEngine(localPosition), localToEngine(localRotation), localToEngine(localScale), ); - }, [ - inputsUpdated, - localPosition, - localRotation, - localScale, - editorEngine, - currentSceneObject, - ]); + }, [inputsUpdated, localPosition, localRotation, localScale, editorEngine]); useEffect(() => { if (!editorEngine || !currentSceneObject) { return; } + // The per-frame `updateSelectedUI` in the engine pushes a fresh + // `currentSceneObject` object into the store every frame the + // selected obj's transform changes. That re-fires this effect with + // the *same* uuid but new vectors. Only an actual selection change + // (different uuid) is a real session boundary — commit the pending + // session for the previous uuid before we resync local state. + if ( + lastUuidRef.current !== null && + lastUuidRef.current !== currentSceneObject.object_uuid + ) { + commitPanelTransform(); + } + lastUuidRef.current = currentSceneObject.object_uuid; + const vectors = currentSceneObject.objectVectors; // local state relies on strings @@ -128,8 +163,15 @@ export const ControlPanelSceneObject = () => { setLocked(editorEngine.selection.isObjectLocked(editorEngine?.selected?.uuid || "")); setColor(editorEngine?.selected?.userData.color); + // No cleanup function — uuid-change commit lives in the body + // above. Unmount commit is handled by the separate effect below. }, [currentSceneObject, editorEngine]); + // Final commit on panel unmount. + useEffect(() => { + return () => commitPanelTransform(); + }, []); + if (!currentSceneObject) { return null; } @@ -149,6 +191,7 @@ export const ControlPanelSceneObject = () => { } const cleanXyz = sanitize(xyz); if (objectMismatch(localPosition, cleanXyz)) { + beginPanelTransform(); setInputsUpdated(true); } setLocalPosition(xyz); @@ -161,6 +204,7 @@ export const ControlPanelSceneObject = () => { } const cleanXyz = sanitize(xyz); if (objectMismatch(localRotation, cleanXyz)) { + beginPanelTransform(); setInputsUpdated(true); } setLocalRotation(xyz); @@ -182,6 +226,7 @@ export const ControlPanelSceneObject = () => { } const cleanXyz = sanitize(xyz); if (objectMismatch(localScale, cleanXyz)) { + beginPanelTransform(); setInputsUpdated(true); } setLocalScale(xyz); @@ -245,18 +290,31 @@ export const ControlPanelSceneObject = () => { type="color" value={color} disabled={locked} - onChange={(e: ChangeEvent) => { - // The native picker fires onChange per slider pixel. - // setObjectColor records each one as a small ColorAction; - // HistoryManager.record uses ColorAction.tryMerge to - // collapse the burst into a single undo entry. - const after = e.target.value; + onFocus={() => { + // Picker is opening — open a session that captures the + // before-state once. apply()/commit() handle visual + // feedback during the drag and the final undo entry on + // close. const uuid = editorEngine?.selected?.uuid; if (uuid && editorEngine) { - setObjectColor(editorEngine, uuid, after); + colorSessionRef.current = beginColorSession( + editorEngine, + uuid, + ); } + }} + onChange={(e: ChangeEvent) => { + // Native color picker fires per-pixel during slider + // drag. apply() updates the engine for visual feedback + // but does NOT record. Recording happens once on blur. + const after = e.target.value; + colorSessionRef.current?.apply(after); setColor(after); }} + onBlur={() => { + colorSessionRef.current?.commit(); + colorSessionRef.current = null; + }} />