Remove variableOwner() and listOwner() from IEngine

Author: adazem009

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;
 

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;

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;

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()));

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");

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));
 

test/script/script_test.cpp

@@ -49,12 +49,12 @@ TEST_F(ScriptTest, Start)
     static std::vector<BlockFunc> functions = { &testFunction };
     static std::vector<Value> constValues = { "test" };
 
-    std::unique_ptr<Variable> var1 = std::make_unique<Variable>("a", "", Value());
-    std::unique_ptr<Variable> var2 = std::make_unique<Variable>("b", "", Value());
+    std::shared_ptr<Variable> var1 = std::make_unique<Variable>("a", "", Value());
+    std::shared_ptr<Variable> var2 = std::make_unique<Variable>("b", "", Value());
     static std::vector<Variable *> variables = { var1.get(), var2.get() };
 
-    std::unique_ptr<List> list1 = std::make_unique<List>("c", "");
-    std::unique_ptr<List> list2 = std::make_unique<List>("d", "");
+    std::shared_ptr<List> list1 = std::make_unique<List>("c", "");
+    std::shared_ptr<List> list2 = std::make_unique<List>("d", "");
     static std::vector<List *> lists = { list1.get(), list2.get() };
 
     Script script1(nullptr, nullptr);
@@ -95,6 +95,8 @@ TEST_F(ScriptTest, Start)
     ASSERT_EQ(vm->lists()[0], lists[0]);
 
     Target target;
+    target.addVariable(var1);
+    target.addList(list1);
     vm = script3.start(&target);
     ASSERT_TRUE(vm);
     ASSERT_EQ(vm->target(), &target);
@@ -107,8 +109,8 @@ TEST_F(ScriptTest, Start)
 
     Sprite root;
     root.setEngine(&m_engine);
-    root.addVariable(std::make_shared<Variable>("b", "", Value()));
-    root.addList(std::make_shared<List>("d", ""));
+    root.addVariable(var2);
+    root.addList(list2);
 
     EXPECT_CALL(m_engine, initClone).Times(1);
     auto clone = root.clone();
@@ -121,10 +123,6 @@ TEST_F(ScriptTest, Start)
     script4.setVariables(variables);
     script4.setLists(lists);
 
-    EXPECT_CALL(m_engine, variableOwner(var1.get())).WillOnce(Return(&target));
-    EXPECT_CALL(m_engine, variableOwner(var2.get())).WillOnce(Return(&root));
-    EXPECT_CALL(m_engine, listOwner(list1.get())).WillOnce(Return(&target));
-    EXPECT_CALL(m_engine, listOwner(list2.get())).WillOnce(Return(&root));
     vm = script4.start(clone.get());
 
     ASSERT_TRUE(vm);