From d9680ae0a800cd3d25b969c8bd5f761ca8c67df0 Mon Sep 17 00:00:00 2001 From: Tanish Manku Date: Fri, 1 May 2026 02:54:19 -0700 Subject: [PATCH] PageScene: route lock undo through gizmo side effects SelectionBridge.lockUnlockObject toggled userData.locked AND attached/detached the gizmo, but HistoryContext.setLocked only did the userData write. After Ctrl+Z the lock flag reverted but the gizmo state stayed wrong. Extract the side-effect chain (setObjectLocked + gizmo attach/detach + updateSelectedUI) into a private SelectionBridge.applyLockState helper. Both lockUnlockObject (toggle path) and the new setLockState (history-replay path) call it. Add a SceneUtils direct-set helper (setObjectLocked) so applyLockState can target a specific state instead of toggling. Wire HistoryContext.setLocked through selection.setLockState so undo of a lock toggle re-runs the gizmo attach/detach. The recordLockChange dep stays for now; it moves to the call site in a later commit when the action layer migrates to LockAction. --- .../app/src/pages/PageScene/engine/editor.ts | 8 +++-- .../engine/editor/SelectionBridge.ts | 32 ++++++++++++++----- .../app/src/pages/PageScene/engine/helper.ts | 7 ++++ 3 files changed, 36 insertions(+), 11 deletions(-) diff --git a/frontend/apps/artcraft/app/src/pages/PageScene/engine/editor.ts b/frontend/apps/artcraft/app/src/pages/PageScene/engine/editor.ts index 51d7584655..3f0e2d1d18 100644 --- a/frontend/apps/artcraft/app/src/pages/PageScene/engine/editor.ts +++ b/frontend/apps/artcraft/app/src/pages/PageScene/engine/editor.ts @@ -163,6 +163,8 @@ class Editor { cameraName: this.cameraController.camera_name, version: this.version, toggleObjectLocked: (uuid) => this.utils.toggleObjectLocked(uuid), + setObjectLocked: (uuid, locked) => + this.utils.setObjectLocked(uuid, locked), isObjectLocked: (uuid) => this.utils.isObjectLocked(uuid), removeTransformControls: () => this.utils.removeTransformControls(false), @@ -256,9 +258,9 @@ class Editor { this.activeScene.setColor(uuid, color); }, setLocked: (uuid, locked) => { - const obj = this.activeScene.scene.getObjectByProperty("uuid", uuid); - if (!obj) return; - obj.userData.locked = locked; + // Routes through SelectionBridge so the gizmo attach/detach + // side effects fire (matches the original lock toggle path). + this.selection.setLockState(uuid, locked); }, setVisible: (uuid, visible) => { const obj = this.activeScene.scene.getObjectByProperty("uuid", uuid); diff --git a/frontend/apps/artcraft/app/src/pages/PageScene/engine/editor/SelectionBridge.ts b/frontend/apps/artcraft/app/src/pages/PageScene/engine/editor/SelectionBridge.ts index 2526df1a77..01c6879289 100644 --- a/frontend/apps/artcraft/app/src/pages/PageScene/engine/editor/SelectionBridge.ts +++ b/frontend/apps/artcraft/app/src/pages/PageScene/engine/editor/SelectionBridge.ts @@ -26,6 +26,7 @@ export type SelectionBridgeDeps = { // and the gizmo attach/detach lives on GizmoController. The bridge // calls them through these callbacks instead of importing either. toggleObjectLocked: (uuid: string) => boolean; // returns new locked state + setObjectLocked: (uuid: string, locked: boolean) => void; // direct set (history replay) isObjectLocked: (uuid: string) => boolean; removeTransformControls: () => void; attachGizmoToCurrentSelection: () => void; @@ -45,20 +46,35 @@ export class SelectionBridge { constructor(private readonly engine: SelectionBridgeDeps) {} - // Toggle the locked flag on an object's userData, then either yank - // the gizmo (lock) or re-attach it to the current selection (unlock). - // Returns the new locked state. - lockUnlockObject(object_uuid: string): boolean { - const before = this.engine.isObjectLocked(object_uuid); - const locked = this.engine.toggleObjectLocked(object_uuid); + // Centralized side-effect chain shared by the user-toggle path and + // history replay. Sets userData.locked, attaches/detaches the gizmo, + // refreshes the inspector. No recording — the user-facing entry + // points decide whether to push to the undo stack. + private applyLockState(object_uuid: string, locked: boolean) { + this.engine.setObjectLocked(object_uuid, locked); if (locked) { this.engine.removeTransformControls(); } else { this.engine.attachGizmoToCurrentSelection(); } this.updateSelectedUI(); - this.engine.recordLockChange?.(object_uuid, before, locked); - return locked; + } + + // Toggle the locked flag on an object's userData, then either yank + // the gizmo (lock) or re-attach it to the current selection (unlock). + // Returns the new locked state. + lockUnlockObject(object_uuid: string): boolean { + const before = this.engine.isObjectLocked(object_uuid); + const after = !before; + this.applyLockState(object_uuid, after); + this.engine.recordLockChange?.(object_uuid, before, after); + return after; + } + + // For history replay: set the locked state directly without recording. + // Runs the same gizmo attach/detach side effects as lockUnlockObject. + setLockState(object_uuid: string, locked: boolean) { + this.applyLockState(object_uuid, locked); } // True if the object is currently locked. Pure passthrough. diff --git a/frontend/apps/artcraft/app/src/pages/PageScene/engine/helper.ts b/frontend/apps/artcraft/app/src/pages/PageScene/engine/helper.ts index b3311a5617..fd78e69e6f 100644 --- a/frontend/apps/artcraft/app/src/pages/PageScene/engine/helper.ts +++ b/frontend/apps/artcraft/app/src/pages/PageScene/engine/helper.ts @@ -43,6 +43,13 @@ export class SceneUtils { return object.userData["locked"]; } + // Direct setter used by history replay — sets userData.locked without + // toggling. Side effects (gizmo attach/detach) live on SelectionBridge. + setObjectLocked(object_uuid: string, locked: boolean) { + const object = this.scene.get_object_by_uuid(object_uuid); + if (object) object.userData["locked"] = locked; + } + // Removes transform controls and publishes selected. removeTransformControls(remove_outline: boolean = true) { if (this.editor.gizmo.control == undefined) {