#5613: Extend EntityInspector tests to prove that undo/redo is notify…

…ing the CollectiveSpawnargs values
codereader · Oct 17, 2021 · f349ecc · f349ecc
1 parent b268207
commit f349ecc
Show file tree

Hide file tree

Showing 5 changed files with 144 additions and 17 deletions.
diff --git a/test/Entity.cpp b/test/Entity.cpp
@@ -17,6 +17,7 @@
 #include "eclass.h"
 #include "string/join.h"
 #include "scenelib.h"
+#include "algorithm/Entity.h"
 
 namespace test
 {
@@ -1037,18 +1038,6 @@ inline EntityKeyValue* findKeyValue(Entity* entity, const std::string& keyToFind
     return keyValue;
 }
 
-inline std::vector<std::pair<std::string, std::string>> getAllKeyValuePairs(Entity* entity)
-{
-    std::vector<std::pair<std::string, std::string>> existingKeyValues;
-
-    entity->forEachKeyValue([&](const std::string& key, const std::string& value)
-    {
-        existingKeyValues.emplace_back(key, value);
-    });
-
-    return existingKeyValues;
-}
-
 inline void expectKeyValuesAreEquivalent(const std::vector<std::pair<std::string, std::string>>& stack1, 
     const std::vector<std::pair<std::string, std::string>>& stack2)
 {
@@ -1071,7 +1060,7 @@ TEST_F(EntityTest, EntityObserverAttachDetach)
     TestEntityObserver observer;
 
     // Collect all existing key values of this entity
-    auto existingKeyValues = getAllKeyValuePairs(guard);
+    auto existingKeyValues = algorithm::getAllKeyValuePairs(guard);
     EXPECT_FALSE(existingKeyValues.empty()) << "Entity doesn't have any keys";
 
     // On attachment, the observer gets notified about all existing keys (insert)
@@ -1239,7 +1228,7 @@ TEST_F(EntityTest, EntityObserverUndoRedo)
     TestEntityObserver observer;
 
     // Collect all existing key values of this entity
-    auto keyValuesBeforeChange = getAllKeyValuePairs(guard);
+    auto keyValuesBeforeChange = algorithm::getAllKeyValuePairs(guard);
     EXPECT_FALSE(keyValuesBeforeChange.empty()) << "Entity doesn't have any keys";
 
     // On attachment, the observer gets notified about all existing keys (insert)
@@ -1261,15 +1250,15 @@ TEST_F(EntityTest, EntityObserverUndoRedo)
         guard->setKeyValue(NewKey, "");
     }
 
-    auto keyValuesAfterChange = getAllKeyValuePairs(guard);
+    auto keyValuesAfterChange = algorithm::getAllKeyValuePairs(guard);
 
     observer.reset();
 
     // UNDO
     GlobalUndoSystem().undo();
 
     // Check that the entity has now the same state as before the change
-    expectKeyValuesAreEquivalent(getAllKeyValuePairs(guard), keyValuesBeforeChange);
+    expectKeyValuesAreEquivalent(algorithm::getAllKeyValuePairs(guard), keyValuesBeforeChange);
 
     // The Undo operation spams the observer with an erase() for each existing pair, 
     // and a subsequent insert() for each one imported from the undo stack
@@ -1304,7 +1293,7 @@ TEST_F(EntityTest, EntityObserverUndoRedo)
     GlobalUndoSystem().redo();
 
     // Check that the entity has now the same state as before the undo
-    expectKeyValuesAreEquivalent(getAllKeyValuePairs(guard), keyValuesAfterChange);
+    expectKeyValuesAreEquivalent(algorithm::getAllKeyValuePairs(guard), keyValuesAfterChange);
 
     // The Redo operation should behave analogous to the undo, report all key values before the change as erased
     EXPECT_EQ(observer.eraseStack.size(), keyValuesBeforeChange.size()) << "All keys before redo should have been reported";

diff --git a/test/EntityInspector.cpp b/test/EntityInspector.cpp
@@ -6,6 +6,7 @@
 #include "algorithm/Scene.h"
 #include "iselectable.h"
 #include "selection/EntitySelection.h"
+#include "algorithm/Entity.h"
 
 namespace test
 {
@@ -386,4 +387,111 @@ TEST_F(EntityInspectorTest, DeselectOneEntity)
     expectUnique(keyValueStore, "classname", "light_torchflame");
 }
 
+TEST_F(EntityInspectorTest, UndoRedoKeyValueChange)
+{
+    KeyValueStore keyValueStore;
+    GlobalCommandSystem().executeCommand("OpenMap", cmd::Argument("maps/entityinspector.map"));
+
+    auto entity1Node = selectEntity("light_torchflame_1");
+    auto entity1 = Node_getEntity(entity1Node);
+    keyValueStore.rescanSelection();
+
+    // All of the entity's spawnargs should be present and showing their value
+    assumeLightTorchflame1Spawnargs(keyValueStore);
+
+    auto keyValuesBeforeChange = algorithm::getAllKeyValuePairs(entity1);
+    constexpr const char* ChangedValue = "changed_value";
+    {
+        UndoableCommand cmd("ChangeKeyValue");
+        entity1->setKeyValue("unique_to_1", ChangedValue);
+    }
+
+    for (const auto& pair : keyValuesBeforeChange)
+    {
+        EXPECT_EQ(keyValueStore.store[pair.first], pair.first == "unique_to_1" ? ChangedValue : pair.second) 
+            << "Keyvalues not matching up after change";
+    }
+
+    // Undo, this should revert the change and immediately update the store
+    GlobalUndoSystem().undo();
+
+    for (const auto& pair : keyValuesBeforeChange)
+    {
+        EXPECT_EQ(keyValueStore.store[pair.first], pair.second) << "Keyvalues not matching up after undo";
+    }
+
+    // Redo and check again
+    GlobalUndoSystem().redo();
+
+    for (const auto& pair : keyValuesBeforeChange)
+    {
+        EXPECT_EQ(keyValueStore.store[pair.first], pair.first == "unique_to_1" ? ChangedValue : pair.second)
+            << "Keyvalues not matching up after redo";
+    }
+}
+
+TEST_F(EntityInspectorTest, UndoRedoKeyValueAdditionRemoval)
+{
+    KeyValueStore keyValueStore;
+    GlobalCommandSystem().executeCommand("OpenMap", cmd::Argument("maps/entityinspector.map"));
+
+    auto entity1Node = selectEntity("light_torchflame_1");
+    auto entity1 = Node_getEntity(entity1Node);
+    keyValueStore.rescanSelection();
+
+    // All of the entity's spawnargs should be present and showing their value
+    assumeLightTorchflame1Spawnargs(keyValueStore);
+
+    constexpr const char* NewKey = "NewKey";
+    constexpr const char* NewValue = "NewValue";
+
+    auto keyValuesBeforeChange = algorithm::getAllKeyValuePairs(entity1);
+    {
+        UndoableCommand cmd("AddAndRemoveKeyValues");
+
+        // Add a new key
+        entity1->setKeyValue(NewKey, NewValue);
+
+        // Remove the unique key
+        entity1->setKeyValue("unique_to_1", "");
+    }
+
+    // Check that the change got reflected
+    expectNotListed(keyValueStore, "unique_to_1");
+    expectUnique(keyValueStore, NewKey, NewValue);
+
+    for (const auto& pair : keyValuesBeforeChange)
+    {
+        if (pair.first != "unique_to_1")
+        {
+            EXPECT_EQ(keyValueStore.store[pair.first], pair.second) << "Keyvalues not matching up after change";
+        }
+    }
+
+    // Undo, this should revert the two changes
+    GlobalUndoSystem().undo();
+
+    expectNotListed(keyValueStore, NewKey);
+
+    // All other values should be restored again
+    for (const auto& pair : keyValuesBeforeChange)
+    {
+        EXPECT_EQ(keyValueStore.store[pair.first], pair.second) << "Keyvalues not matching up after undo";
+    }
+
+    // Redo and check the opposite
+    GlobalUndoSystem().redo();
+
+    expectNotListed(keyValueStore, "unique_to_1");
+    expectUnique(keyValueStore, NewKey, NewValue);
+
+    for (const auto& pair : keyValuesBeforeChange)
+    {
+        if (pair.first != "unique_to_1")
+        {
+            EXPECT_EQ(keyValueStore.store[pair.first], pair.second) << "Keyvalues not matching up after redo";
+        }
+    }
+}
+
 }
diff --git a/test/algorithm/Entity.h b/test/algorithm/Entity.h
@@ -0,0 +1,26 @@
+#pragma once
+
+#include <vector>
+#include "ientity.h"
+
+namespace test
+{
+
+namespace algorithm
+{
+
+inline std::vector<std::pair<std::string, std::string>> getAllKeyValuePairs(Entity* entity)
+{
+    std::vector<std::pair<std::string, std::string>> existingKeyValues;
+
+    entity->forEachKeyValue([&](const std::string& key, const std::string& value)
+    {
+        existingKeyValues.emplace_back(key, value);
+    });
+
+    return existingKeyValues;
+}
+
+}
+
+}
diff --git a/tools/msvc/Tests/Tests.vcxproj b/tools/msvc/Tests/Tests.vcxproj
@@ -60,6 +60,7 @@
   <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|x64'" />
   <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|x64'" />
   <ItemGroup>
+    <ClInclude Include="..\..\..\test\algorithm\Entity.h" />
     <ClInclude Include="..\..\..\test\algorithm\Primitives.h" />
     <ClInclude Include="..\..\..\test\algorithm\Scene.h" />
     <ClInclude Include="..\..\..\test\algorithm\View.h" />

diff --git a/tools/msvc/Tests/Tests.vcxproj.filters b/tools/msvc/Tests/Tests.vcxproj.filters
@@ -76,6 +76,9 @@
     <ClInclude Include="..\..\..\test\testutil\FileSelectionHelper.h">
       <Filter>testutil</Filter>
     </ClInclude>
+    <ClInclude Include="..\..\..\test\algorithm\Entity.h">
+      <Filter>algorithm</Filter>
+    </ClInclude>
   </ItemGroup>
   <ItemGroup>
     <Filter Include="algorithm">