From f349ecc5e862f24747cd3c4012bf7c3919910541 Mon Sep 17 00:00:00 2001 From: codereader Date: Sun, 17 Oct 2021 17:14:48 +0200 Subject: [PATCH] #5613: Extend EntityInspector tests to prove that undo/redo is notifying the CollectiveSpawnargs values --- test/Entity.cpp | 23 ++---- test/EntityInspector.cpp | 108 +++++++++++++++++++++++++ test/algorithm/Entity.h | 26 ++++++ tools/msvc/Tests/Tests.vcxproj | 1 + tools/msvc/Tests/Tests.vcxproj.filters | 3 + 5 files changed, 144 insertions(+), 17 deletions(-) create mode 100644 test/algorithm/Entity.h diff --git a/test/Entity.cpp b/test/Entity.cpp index ac67292929..ed210eaf4b 100644 --- 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> getAllKeyValuePairs(Entity* entity) -{ - std::vector> existingKeyValues; - - entity->forEachKeyValue([&](const std::string& key, const std::string& value) - { - existingKeyValues.emplace_back(key, value); - }); - - return existingKeyValues; -} - inline void expectKeyValuesAreEquivalent(const std::vector>& stack1, const std::vector>& 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,7 +1250,7 @@ TEST_F(EntityTest, EntityObserverUndoRedo) guard->setKeyValue(NewKey, ""); } - auto keyValuesAfterChange = getAllKeyValuePairs(guard); + auto keyValuesAfterChange = algorithm::getAllKeyValuePairs(guard); observer.reset(); @@ -1269,7 +1258,7 @@ TEST_F(EntityTest, EntityObserverUndoRedo) 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 index 052243bdf8..d6a52cf287 100644 --- 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 new file mode 100644 index 0000000000..15553c7462 --- /dev/null +++ b/test/algorithm/Entity.h @@ -0,0 +1,26 @@ +#pragma once + +#include +#include "ientity.h" + +namespace test +{ + +namespace algorithm +{ + +inline std::vector> getAllKeyValuePairs(Entity* entity) +{ + std::vector> 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 index 98d9f9348a..997adb194f 100644 --- a/tools/msvc/Tests/Tests.vcxproj +++ b/tools/msvc/Tests/Tests.vcxproj @@ -60,6 +60,7 @@ + diff --git a/tools/msvc/Tests/Tests.vcxproj.filters b/tools/msvc/Tests/Tests.vcxproj.filters index 40ff4ca2d6..11f7d7cd3f 100644 --- a/tools/msvc/Tests/Tests.vcxproj.filters +++ b/tools/msvc/Tests/Tests.vcxproj.filters @@ -76,6 +76,9 @@ testutil + + algorithm +