Merge pull request #295 from scratchcpp/variable_list_target

adazem009 · web-flow · commit 284c0977a6a1 · 2023-10-30T09:00:29.000+01:00
Add target property to Variable and List
diff --git a/include/scratchcpp/iengine.h b/include/scratchcpp/iengine.h
@@ -247,12 +247,6 @@ class LIBSCRATCHCPP_EXPORT IEngine
         /*! Returns the Stage. */
         virtual Stage *stage() const = 0;
 
-        /*! Returns the target which owns the given variable. If it is the stage, the variable is global. */
-        virtual Target *variableOwner(Variable *variable) const = 0;
-
-        /*! Returns the target which owns the given list. If it is the stage, the list is global. */
-        virtual Target *listOwner(List *list) const = 0;
-
         /*! Returns the list of extension names. */
         virtual const std::vector<std::string> &extensions() const = 0;
 
diff --git a/include/scratchcpp/list.h b/include/scratchcpp/list.h
@@ -11,6 +11,7 @@
 namespace libscratchcpp
 {
 
+class Target;
 class ListPrivate;
 
 /*! \brief The List class represents a Scratch list. */
@@ -25,6 +26,9 @@ class LIBSCRATCHCPP_EXPORT List
         const std::string &name();
         void setName(const std::string &name);
 
+        Target *target() const;
+        void setTarget(Target *target);
+
         long indexOf(const Value &value) const;
         bool contains(const Value &value) const;
 
diff --git a/include/scratchcpp/variable.h b/include/scratchcpp/variable.h
@@ -9,6 +9,7 @@
 namespace libscratchcpp
 {
 
+class Target;
 class VariablePrivate;
 
 /*! \brief The Variable class represents a Scratch variable. */
@@ -29,6 +30,9 @@ class LIBSCRATCHCPP_EXPORT Variable : public Entity
         bool isCloudVariable() const;
         void setIsCloudVariable(bool isCloudVariable);
 
+        Target *target() const;
+        void setTarget(Target *target);
+
         std::shared_ptr<Variable> clone();
 
     private:
diff --git a/src/engine/internal/engine.cpp b/src/engine/internal/engine.cpp
@@ -689,8 +689,6 @@ const std::vector<std::shared_ptr<Target>> &Engine::targets() const
 void Engine::setTargets(const std::vector<std::shared_ptr<Target>> &newTargets)
 {
     m_targets = newTargets;
-    m_variableOwners.clear();
-    m_listOwners.clear();
 
     for (auto target : m_targets) {
         // Set engine in the target
@@ -702,18 +700,6 @@ void Engine::setTargets(const std::vector<std::shared_ptr<Target>> &newTargets)
             block->setEngine(this);
             block->setTarget(target.get());
         }
-
-        // Add variables to owner map
-        const auto &variables = target->variables();
-
-        for (auto variable : variables)
-            m_variableOwners[variable.get()] = target.get();
-
-        // Add lists to owner map
-        const auto &lists = target->lists();
-
-        for (auto list : lists)
-            m_listOwners[list.get()] = target.get();
     }
 }
 
@@ -746,26 +732,6 @@ Stage *Engine::stage() const
         return dynamic_cast<Stage *>((*it).get());
 }
 
-Target *Engine::variableOwner(Variable *variable) const
-{
-    auto it = m_variableOwners.find(variable);
-
-    if (it == m_variableOwners.cend())
-        return nullptr;
-
-    return it->second;
-}
-
-Target *Engine::listOwner(List *list) const
-{
-    auto it = m_listOwners.find(list);
-
-    if (it == m_listOwners.cend())
-        return nullptr;
-
-    return it->second;
-}
-
 const std::vector<std::string> &Engine::extensions() const
 {
     return m_extensions;
diff --git a/src/engine/internal/engine.h b/src/engine/internal/engine.h
@@ -100,9 +100,6 @@ class Engine : public IEngine
 
         Stage *stage() const override;
 
-        Target *variableOwner(Variable *variable) const override;
-        Target *listOwner(List *list) const override;
-
         const std::vector<std::string> &extensions() const override;
         void setExtensions(const std::vector<std::string> &newExtensions) override;
 
@@ -133,8 +130,6 @@ class Engine : public IEngine
         std::vector<VirtualMachine *> m_scriptsToRemove;
         std::unordered_map<std::shared_ptr<Block>, std::shared_ptr<Script>> m_scripts;
         std::vector<BlockFunc> m_functions;
-        std::unordered_map<Variable *, Target *> m_variableOwners;
-        std::unordered_map<List *, Target *> m_listOwners;
 
         std::unique_ptr<ITimer> m_defaultTimer;
         ITimer *m_timer = nullptr;
diff --git a/src/engine/script.cpp b/src/engine/script.cpp
@@ -77,7 +77,7 @@ std::shared_ptr<VirtualMachine> Script::start(Target *target)
         std::vector<List *> lists;
 
         for (const auto &var : impl->variables) {
-            Target *owner = impl->engine->variableOwner(var);
+            Target *owner = var->target();
 
             if (owner && (owner == root)) {
                 auto cloneVar = sprite->variableAt(sprite->findVariableById(var->id()));
@@ -90,7 +90,7 @@ std::shared_ptr<VirtualMachine> Script::start(Target *target)
         }
 
         for (const auto &list : impl->lists) {
-            Target *owner = impl->engine->listOwner(list);
+            Target *owner = list->target();
 
             if (owner && (owner == root)) {
                 auto cloneList = sprite->listAt(sprite->findListById(list->id()));
diff --git a/src/scratch/list.cpp b/src/scratch/list.cpp
@@ -26,6 +26,18 @@ void List::setName(const std::string &name)
     impl->name = name;
 }
 
+/*! Returns the sprite or stage this list belongs to. */
+Target *List::target() const
+{
+    return impl->target;
+}
+
+/*! Sets the sprite or stage this list belongs to. */
+void List::setTarget(Target *target)
+{
+    impl->target = target;
+}
+
 /*! Returns the index of the given item. */
 long List::indexOf(const Value &value) const
 {
diff --git a/src/scratch/list_p.h b/src/scratch/list_p.h
@@ -7,11 +7,14 @@
 namespace libscratchcpp
 {
 
+class Target;
+
 struct ListPrivate
 {
         ListPrivate(const std::string &name);
 
         std::string name;
+        Target *target = nullptr;
 };
 
 } // namespace libscratchcpp
diff --git a/src/scratch/target.cpp b/src/scratch/target.cpp
@@ -42,6 +42,8 @@ int Target::addVariable(std::shared_ptr<Variable> variable)
         return it - impl->variables.begin();
 
     impl->variables.push_back(variable);
+    variable->setTarget(this);
+
     return impl->variables.size() - 1;
 }
 
@@ -93,6 +95,8 @@ int Target::addList(std::shared_ptr<List> list)
         return it - impl->lists.begin();
 
     impl->lists.push_back(list);
+    list->setTarget(this);
+
     return impl->lists.size() - 1;
 }
 
diff --git a/src/scratch/variable.cpp b/src/scratch/variable.cpp
@@ -49,6 +49,18 @@ void Variable::setIsCloudVariable(bool isCloudVariable)
     impl->isCloudVariable = isCloudVariable;
 }
 
+/*! Returns the sprite or stage this variable belongs to. */
+Target *Variable::target() const
+{
+    return impl->target;
+}
+
+/*! Sets the sprite or stage this variable belongs to. */
+void Variable::setTarget(Target *target)
+{
+    impl->target = target;
+}
+
 /*! Creates a copy of the variable. */
 std::shared_ptr<Variable> Variable::clone()
 {
diff --git a/src/scratch/variable_p.h b/src/scratch/variable_p.h
@@ -7,6 +7,8 @@
 namespace libscratchcpp
 {
 
+class Target;
+
 struct VariablePrivate
 {
         VariablePrivate(const std::string &name, const Value &value = Value(), bool isCloudVariable = false);
@@ -15,6 +17,7 @@ struct VariablePrivate
         std::string name;
         Value value;
         bool isCloudVariable;
+        Target *target = nullptr;
 };
 
 } // namespace libscratchcpp
diff --git a/test/engine/engine_test.cpp b/test/engine/engine_test.cpp
@@ -502,52 +502,6 @@ TEST(EngineTest, Stage)
     ASSERT_EQ(engine.stage(), nullptr);
 }
 
-TEST(EngineTest, VariableOwner)
-{
-    Engine engine;
-    auto t1 = std::make_shared<Target>();
-    auto t2 = std::make_shared<Target>();
-    auto t3 = std::make_shared<Target>();
-
-    auto var1 = std::make_shared<Variable>("", "", Value());
-    auto var2 = std::make_shared<Variable>("", "", Value());
-    auto var3 = std::make_shared<Variable>("", "", Value());
-    auto var4 = std::make_shared<Variable>("", "", Value());
-    t1->addVariable(var1);
-    t1->addVariable(var4);
-    t3->addVariable(var3);
-
-    engine.setTargets({ t1, t2, t3 });
-
-    ASSERT_EQ(engine.variableOwner(var1.get()), t1.get());
-    ASSERT_EQ(engine.variableOwner(var2.get()), nullptr);
-    ASSERT_EQ(engine.variableOwner(var3.get()), t3.get());
-    ASSERT_EQ(engine.variableOwner(var4.get()), t1.get());
-}
-
-TEST(EngineTest, ListOwner)
-{
-    Engine engine;
-    auto t1 = std::make_shared<Target>();
-    auto t2 = std::make_shared<Target>();
-    auto t3 = std::make_shared<Target>();
-
-    auto list1 = std::make_shared<List>("", "");
-    auto list2 = std::make_shared<List>("", "");
-    auto list3 = std::make_shared<List>("", "");
-    auto list4 = std::make_shared<List>("", "");
-    t1->addList(list1);
-    t1->addList(list4);
-    t3->addList(list3);
-
-    engine.setTargets({ t1, t2, t3 });
-
-    ASSERT_EQ(engine.listOwner(list1.get()), t1.get());
-    ASSERT_EQ(engine.listOwner(list2.get()), nullptr);
-    ASSERT_EQ(engine.listOwner(list3.get()), t3.get());
-    ASSERT_EQ(engine.listOwner(list4.get()), t1.get());
-}
-
 TEST(EngineTest, Clones)
 {
     Project p("clones.sb3");
diff --git a/test/mocks/enginemock.h b/test/mocks/enginemock.h
@@ -86,9 +86,6 @@ class EngineMock : public IEngine
 
         MOCK_METHOD(Stage *, stage, (), (const, override));
 
-        MOCK_METHOD(Target *, variableOwner, (Variable *), (const, override));
-        MOCK_METHOD(Target *, listOwner, (List *), (const, override));
-
         MOCK_METHOD(std::vector<std::string> &, extensions, (), (const, override));
         MOCK_METHOD(void, setExtensions, (const std::vector<std::string> &), (override));
 
diff --git a/test/scratch_classes/list_test.cpp b/test/scratch_classes/list_test.cpp
@@ -1,4 +1,5 @@
 #include <scratchcpp/list.h>
+#include <scratchcpp/target.h>
 
 #include "../common.h"
 
@@ -18,6 +19,16 @@ TEST(ListTest, Name)
     ASSERT_EQ(list.name(), "test list");
 }
 
+TEST(ListTest, Target)
+{
+    List list("", "");
+    ASSERT_EQ(list.target(), nullptr);
+
+    Target target;
+    list.setTarget(&target);
+    ASSERT_EQ(list.target(), &target);
+}
+
 TEST(ListTest, IndexOf)
 {
     List list("", "test list");
diff --git a/test/scratch_classes/sprite_test.cpp b/test/scratch_classes/sprite_test.cpp
@@ -71,6 +71,7 @@ TEST(SpriteTest, Clone)
         ASSERT_EQ(clone->variableAt(1)->id(), "b");
         ASSERT_EQ(clone->variableAt(1)->name(), "var2");
         ASSERT_EQ(clone->variableAt(1)->value().toString(), "world");
+        ASSERT_EQ(clone->variableAt(1)->target(), clone);
 
         ASSERT_EQ(clone->lists().size(), 2);
         ASSERT_NE(clone->lists(), root->lists());
@@ -80,6 +81,7 @@ TEST(SpriteTest, Clone)
         ASSERT_EQ(clone->listAt(1)->id(), "d");
         ASSERT_EQ(clone->listAt(1)->name(), "list2");
         ASSERT_EQ(*clone->listAt(1), std::deque<Value>({ "test" }));
+        ASSERT_EQ(clone->listAt(1)->target(), clone);
 
         ASSERT_EQ(clone->currentCostume(), 2);
         ASSERT_EQ(clone->layerOrder(), 5);
diff --git a/test/scratch_classes/target_test.cpp b/test/scratch_classes/target_test.cpp
@@ -38,6 +38,10 @@ TEST(TargetTest, Variables)
     ASSERT_EQ(target.addVariable(v3), 2);
     ASSERT_EQ(target.addVariable(v2), 1); // add existing variable
 
+    ASSERT_EQ(v1->target(), &target);
+    ASSERT_EQ(v2->target(), &target);
+    ASSERT_EQ(v3->target(), &target);
+
     ASSERT_EQ(target.variables(), std::vector<std::shared_ptr<Variable>>({ v1, v2, v3 }));
     ASSERT_EQ(target.variableAt(0), v1);
     ASSERT_EQ(target.variableAt(1), v2);
@@ -68,6 +72,10 @@ TEST(TargetTest, Lists)
     ASSERT_EQ(target.addList(l3), 2);
     ASSERT_EQ(target.addList(l2), 1); // add existing list
 
+    ASSERT_EQ(l1->target(), &target);
+    ASSERT_EQ(l2->target(), &target);
+    ASSERT_EQ(l3->target(), &target);
+
     ASSERT_EQ(target.lists(), std::vector<std::shared_ptr<List>>({ l1, l2, l3 }));
     ASSERT_EQ(target.listAt(0), l1);
     ASSERT_EQ(target.listAt(1), l2);
diff --git a/test/scratch_classes/variable_test.cpp b/test/scratch_classes/variable_test.cpp
@@ -1,4 +1,5 @@
 #include <scratchcpp/variable.h>
+#include <scratchcpp/target.h>
 
 #include "../common.h"
 
@@ -57,6 +58,16 @@ TEST(VariableTest, IsCloudVariable)
     ASSERT_TRUE(var.isCloudVariable());
 }
 
+TEST(VariableTest, Target)
+{
+    Variable var("", "");
+    ASSERT_EQ(var.target(), nullptr);
+
+    Target target;
+    var.setTarget(&target);
+    ASSERT_EQ(var.target(), &target);
+}
+
 TEST(VariableTest, Clone)
 {
     std::shared_ptr<Variable> clone;
diff --git a/test/script/script_test.cpp b/test/script/script_test.cpp

Original file line number	Diff line number	Diff line change
`@@ -7,11 +7,14 @@`
`7`	`7`	`namespace libscratchcpp`
`8`	`8`	`{`
`9`	`9`
	`10`	`+class Target;`
	`11`	`+`
`10`	`12`	`struct ListPrivate`
`11`	`13`	`{`
`12`	`14`	`ListPrivate(const std::string &name);`
`13`	`15`
`14`	`16`	`std::string name;`
	`17`	`+ Target *target = nullptr;`
`15`	`18`	`};`
`16`	`19`
`17`	`20`	`} // namespace libscratchcpp`
Original file line number	Diff line number	Diff line change
`@@ -42,6 +42,8 @@ int Target::addVariable(std::shared_ptr<Variable> variable)`
`42`	`42`	`return it - impl->variables.begin();`
`43`	`43`
`44`	`44`	`impl->variables.push_back(variable);`
	`45`	`+ variable->setTarget(this);`
	`46`	`+`
`45`	`47`	`return impl->variables.size() - 1;`
`46`	`48`	`}`
`47`	`49`
`@@ -93,6 +95,8 @@ int Target::addList(std::shared_ptr<List> list)`
`93`	`95`	`return it - impl->lists.begin();`
`94`	`96`
`95`	`97`	`impl->lists.push_back(list);`
	`98`	`+ list->setTarget(this);`
	`99`	`+`
`96`	`100`	`return impl->lists.size() - 1;`
`97`	`101`	`}`
`98`	`102`