#5613: Key value merge actions should be applying their target values…

… to the affected entity node to allow for proper preview and bounds calculation during selection tests.
codereader · Oct 22, 2021 · faee6e7 · faee6e7
1 parent 5a9d89b
commit faee6e7
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 9 deletions.
diff --git a/libs/scene/merge/MergeAction.h b/libs/scene/merge/MergeAction.h
@@ -225,6 +225,7 @@ class SetEntityKeyValueAction :
     INodePtr _node;
     std::string _key;
     std::string _value;
+    std::string _existingValue;
 
 public:
     // Will call setKeyValue(key, value) on the targetnode when applyChanges() is called
@@ -237,21 +238,16 @@ class SetEntityKeyValueAction :
         assert(_node);
         assert(Node_isEntity(_node));
         assert(!_key.empty());
+
+        // Store the existing value, it's reverted when deactivating this action
+        _existingValue = Node_getEntity(node)->getKeyValue(key);
     }
 
     void applyChanges() override
     {
         if (!isActive()) return;
 
-        // No post-clone callback since we don't care about selection groups right now
-        auto entity = Node_getEntity(_node);
-
-        if (!entity)
-        {
-            throw std::runtime_error("Node " + _node->name() + " is not an entity");
-        }
-
-        entity->setKeyValue(_key, _value);
+        applyValue(_value);
     }
 
     const INodePtr& getEntityNode() const
@@ -273,6 +269,35 @@ class SetEntityKeyValueAction :
     {
         return getEntityNode();
     }
+
+    virtual void activate() override
+    {
+        MergeAction::activate();
+
+        // Key Value Actions apply their values to the target node
+        // on activation, to allow for proper preview and bounds calculations
+        applyValue(_value);
+    }
+
+    virtual void deactivate() override
+    {
+        MergeAction::deactivate();
+
+        applyValue(_existingValue);
+    }
+
+private:
+    void applyValue(const std::string& value)
+    {
+        auto entity = Node_getEntity(_node);
+
+        if (!entity)
+        {
+            throw std::runtime_error("Node " + _node->name() + " is not an entity");
+        }
+
+        entity->setKeyValue(_key, value);
+    }
 };
 
 class AddEntityKeyValueAction :

diff --git a/libs/scene/merge/MergeActionNode.cpp b/libs/scene/merge/MergeActionNode.cpp
@@ -36,6 +36,9 @@ void MergeActionNodeBase::onInsertIntoScene(IMapRootNode& rootNode)
         {
             action->activate();
         });
+
+        // The activated actions might have changed the node bounds in some way
+        boundsChanged();
     }
 
     hideAffectedNodes();