Merge pull request #708 from 3DStreet/fix-undo-unselected-entity

Fix several issues with entityupdate undo
3DStreet · Jul 15, 2024 · 58da709 · 58da709
2 parents fb8ddf0 + 8c36077
commit 58da709
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 61 deletions.
diff --git a/src/editor/components/scenegraph/Toolbar.js b/src/editor/components/scenegraph/Toolbar.js
@@ -343,21 +343,6 @@ export default class Toolbar extends Component {
     Events.emit('entitycreate', { element: 'a-entity', components: {} });
   }
 
-  /**
-   * Try to write changes with aframe-inspector-watcher.
-   */
-  writeChanges = () => {
-    const xhr = new XMLHttpRequest();
-    xhr.open('POST', 'http://localhost:51234/save');
-    xhr.onerror = () => {
-      alert(
-        'aframe-watcher not running. This feature requires a companion service running locally. npm install aframe-watcher to save changes back to file. Read more at supermedium.com/aframe-watcher'
-      );
-    };
-    xhr.setRequestHeader('Content-Type', 'application/json');
-    xhr.send(JSON.stringify(AFRAME.INSPECTOR.history.updates));
-  };
-
   toggleScenePlaying = () => {
     if (this.state.isPlaying) {
       AFRAME.scenes[0].pause();
@@ -387,12 +372,6 @@ export default class Toolbar extends Component {
   };
 
   render() {
-    // const watcherClassNames = classNames({
-    //   button: true,
-    //   fa: true,
-    //   'fa-save': true
-    // });
-    // const watcherTitle = 'Write changes with aframe-watcher.';
     return (
       <div id="toolbar">
         <div className="toolbarActions">

diff --git a/src/editor/lib/commands/EntityUpdateCommand.js b/src/editor/lib/commands/EntityUpdateCommand.js
@@ -32,28 +32,34 @@ export class EntityUpdateCommand extends Command {
 
     this.type = 'EntityUpdateCommand';
     this.name = 'Update Entity';
-    this.updatable =
-      payload.component === 'position' ||
-      payload.component === 'rotation' ||
-      payload.component === 'scale';
+    this.updatable = true;
 
     this.entity = payload.entity;
     this.component = payload.component;
     this.property = payload.property;
 
-    const component = AFRAME.components[payload.component];
+    const component = this.entity.components[payload.component];
+    // Don't use AFRAME.components[payload.component] here, but use this.entity.components[payload.component] so we have the dynamic schema,
+    // important for material or geometry components like for example modifying material metalness,
+    // otherwise component.schema[payload.property] would be undefined.
     if (component) {
       if (payload.property) {
         if (component.schema[payload.property]) {
           this.newValue = component.schema[payload.property].stringify(
             payload.value
           );
           this.oldValue = component.schema[payload.property].stringify(
-            payload.entity.getAttribute(payload.component, payload.property)
+            payload.entity.getAttribute(payload.component)[payload.property]
           );
-          if (this.editor.debugUndoRedo) {
-            console.log(this.component, this.oldValue, this.newValue);
-          }
+        } else {
+          // Just in case dynamic schema is not properly updated and we set an unknown property. I don't think this should happen.
+          this.newValue = payload.value;
+          this.oldValue = payload.entity.getAttribute(payload.component)[
+            payload.property
+          ];
+        }
+        if (this.editor.debugUndoRedo) {
+          console.log(this.component, this.oldValue, this.newValue);
         }
       } else {
         this.newValue = component.schema.stringify(payload.value);
@@ -87,6 +93,13 @@ export class EntityUpdateCommand extends Command {
   }
 
   undo() {
+    if (
+      this.editor.selectedEntity &&
+      this.editor.selectedEntity !== this.entity
+    ) {
+      // If the selected entity is not the entity we are undoing, select the entity.
+      this.editor.selectEntity(this.entity);
+    }
     updateEntity(this.entity, this.component, this.property, this.oldValue);
     Events.emit('entityupdate', {
       entity: this.entity,

diff --git a/src/editor/lib/history.js b/src/editor/lib/history.js
@@ -1,35 +1,5 @@
 import Events from './Events';
 
-// export const updates = {};
-
-/**
- * Store change to export.
- *
- * payload: entity, component, property, value.
- */
-// This code was used for aframe-watcher
-// Events.on('entityupdate', (payload) => {
-//   let value = payload.value;
-
-//   const entity = payload.entity;
-//   updates[entity.id] = updates[entity.id] || {};
-
-//   const component = AFRAME.components[payload.component];
-//   if (component) {
-//     if (payload.property) {
-//       updates[entity.id][payload.component] =
-//         updates[entity.id][payload.component] || {};
-//       if (component.schema[payload.property]) {
-//         value = component.schema[payload.property].stringify(payload.value);
-//       }
-//       updates[entity.id][payload.component][payload.property] = value;
-//     } else {
-//       value = component.schema.stringify(payload.value);
-//       updates[entity.id][payload.component] = value;
-//     }
-//   }
-// });
-
 export class History {
   constructor(editor) {
     this.editor = editor;

diff --git a/src/editor/lib/viewport.js b/src/editor/lib/viewport.js
@@ -198,7 +198,11 @@ export function Viewport(inspector) {
   sceneHelpers.add(transformControls);
 
   Events.on('entityupdate', (detail) => {
-    if (inspector.selectedEntity.object3DMap.mesh) {
+    const object = detail.entity.object3D;
+    if (
+      inspector.selected === object &&
+      inspector.selectedEntity.object3DMap.mesh
+    ) {
       selectionBox.setFromObject(inspector.selected);
       hoverBox.visible = false;
     }