Optimize target processing

src/engine/internal/engine.cpp

@@ -107,8 +107,8 @@ void Engine::resolveIds()
         const auto &blocks = target->blocks();
         for (auto block : blocks) {
             auto container = blockSectionContainer(block->opcode());
-            block->setNext(getBlock(block->nextId()));
-            block->setParent(getBlock(block->parentId()));
+            block->setNext(getBlock(block->nextId(), target.get()));
+            block->setParent(getBlock(block->parentId(), target.get()));
 
             if (container) {
                 block->setCompileFunction(container->resolveBlockCompileFunc(block->opcode()));
@@ -117,16 +117,16 @@ void Engine::resolveIds()
 
             const auto &inputs = block->inputs();
             for (const auto &input : inputs) {
-                input->setValueBlock(getBlock(input->valueBlockId()));
+                input->setValueBlock(getBlock(input->valueBlockId(), target.get()));
 
                 if (container)
                     input->setInputId(container->resolveInput(input->name()));
 
                 InputValue *value = input->primaryValue();
                 std::string id = value->valueId(); // no reference!
-                value->setValuePtr(getEntity(id));
+                value->setValuePtr(getEntity(id, target.get()));
                 assert(input->secondaryValue()->type() != InputValue::Type::Variable && input->secondaryValue()->type() != InputValue::Type::List); // secondary values never have a variable or list
-                input->secondaryValue()->setValuePtr(getEntity(input->secondaryValue()->valueId()));
+                input->secondaryValue()->setValuePtr(getEntity(input->secondaryValue()->valueId(), target.get()));
 
                 // Add missing variables and lists
                 if (!value->valuePtr()) {
@@ -149,7 +149,7 @@ void Engine::resolveIds()
             const auto &fields = block->fields();
             for (auto field : fields) {
                 std::string id = field->valueId(); // no reference!
-                field->setValuePtr(getEntity(id));
+                field->setValuePtr(getEntity(id, target.get()));
 
                 if (container) {
                     field->setFieldId(container->resolveField(field->name()));
@@ -179,7 +179,7 @@ void Engine::resolveIds()
             block->updateInputMap();
             block->updateFieldMap();
 
-            auto comment = getComment(block->commentId());
+            auto comment = getComment(block->commentId(), target.get());
             block->setComment(comment);
 
             if (comment) {
@@ -209,7 +209,7 @@ void Engine::resolveIds()
         assert(target);
 
         for (auto field : fields) {
-            field->setValuePtr(getEntity(field->valueId()));
+            field->setValuePtr(getEntity(field->valueId(), target));
 
             if (container) {
                 field->setFieldId(container->resolveField(field->name()));
@@ -1411,47 +1411,99 @@ const std::unordered_map<std::shared_ptr<Block>, std::shared_ptr<Script>> &Engin
 }
 
 // Returns the block with the given ID.
-std::shared_ptr<Block> Engine::getBlock(const std::string &id)
+std::shared_ptr<Block> Engine::getBlock(const std::string &id, Target *target)
 {
     if (id.empty())
         return nullptr;
 
-    for (auto target : m_targets) {
-        int index = target->findBlock(id);
+    int index;
+
+    if (target) {
+        index = target->findBlock(id);
+
         if (index != -1)
             return target->blockAt(index);
     }
 
+    for (auto t : m_targets) {
+        index = t->findBlock(id);
+
+        if (index != -1)
+            return t->blockAt(index);
+    }
+
     return nullptr;
 }
 
 // Returns the variable with the given ID.
-std::shared_ptr<Variable> Engine::getVariable(const std::string &id)
+std::shared_ptr<Variable> Engine::getVariable(const std::string &id, Target *target)
 {
     if (id.empty())
         return nullptr;
 
-    for (auto target : m_targets) {
-        int index = target->findVariableById(id);
+    Stage *stage = this->stage();
+    int index;
+
+    // Check stage
+    index = stage->findVariableById(id);
+
+    if (index != -1)
+        return stage->variableAt(index);
+
+    // Check currently compiled target
+    if (target != stage) {
+        index = target->findVariableById(id);
+
         if (index != -1)
             return target->variableAt(index);
     }
 
+    // Fall back to checking all the other targets
+    for (auto t : m_targets) {
+        if (t.get() != stage && t.get() != target) {
+            int index = t->findVariableById(id);
+
+            if (index != -1)
+                return t->variableAt(index);
+        }
+    }
+
     return nullptr;
 }
 
 // Returns the Scratch list with the given ID.
-std::shared_ptr<List> Engine::getList(const std::string &id)
+std::shared_ptr<List> Engine::getList(const std::string &id, Target *target)
 {
     if (id.empty())
         return nullptr;
 
-    for (auto target : m_targets) {
-        int index = target->findListById(id);
+    Stage *stage = this->stage();
+    int index;
+
+    // Check stage
+    index = stage->findListById(id);
+
+    if (index != -1)
+        return stage->listAt(index);
+
+    // Check currently compiled target
+    if (target != stage) {
+        index = target->findListById(id);
+
         if (index != -1)
             return target->listAt(index);
     }
 
+    // Fall back to checking all the other targets
+    for (auto t : m_targets) {
+        if (t.get() != stage && t.get() != target) {
+            int index = t->findListById(id);
+
+            if (index != -1)
+                return t->listAt(index);
+        }
+    }
+
     return nullptr;
 }
 
@@ -1469,35 +1521,40 @@ std::shared_ptr<Broadcast> Engine::getBroadcast(const std::string &id)
 }
 
 // Returns the comment with the given ID.
-std::shared_ptr<Comment> Engine::getComment(const std::string &id)
+std::shared_ptr<Comment> Engine::getComment(const std::string &id, Target *target)
 {
     if (id.empty())
         return nullptr;
 
-    for (auto target : m_targets) {
-        int index = target->findComment(id);
+    int index;
+
+    if (target) {
+        index = target->findComment(id);
+
         if (index != -1)
             return target->commentAt(index);
     }
 
+    for (auto t : m_targets) {
+        index = t->findComment(id);
+
+        if (index != -1)
+            return t->commentAt(index);
+    }
+
     return nullptr;
 }
 
 // Returns the entity with the given ID. \see IEntity
-std::shared_ptr<Entity> Engine::getEntity(const std::string &id)
+std::shared_ptr<Entity> Engine::getEntity(const std::string &id, Target *target)
 {
-    // Blocks
-    auto block = getBlock(id);
-    if (block)
-        return std::static_pointer_cast<Entity>(block);
-
     // Variables
-    auto variable = getVariable(id);
+    auto variable = getVariable(id, target);
     if (variable)
         return std::static_pointer_cast<Entity>(variable);
 
     // Lists
-    auto list = getList(id);
+    auto list = getList(id, target);
     if (list)
         return std::static_pointer_cast<Entity>(list);
 
@@ -1506,6 +1563,16 @@ std::shared_ptr<Entity> Engine::getEntity(const std::string &id)
     if (broadcast)
         return std::static_pointer_cast<Entity>(broadcast);
 
+    // Blocks
+    auto block = getBlock(id, target);
+    if (block)
+        return std::static_pointer_cast<Entity>(block);
+
+    // Comments
+    auto comment = getComment(id, target);
+    if (comment)
+        return std::static_pointer_cast<Entity>(comment);
+
     return nullptr;
 }
 

src/engine/internal/engine.h

@@ -202,12 +202,12 @@ class Engine : public IEngine
         void removeExecutableClones();
         void createMissingMonitors();
         void addVarOrListMonitor(std::shared_ptr<Monitor> monitor, Target *target);
-        std::shared_ptr<Block> getBlock(const std::string &id);
-        std::shared_ptr<Variable> getVariable(const std::string &id);
-        std::shared_ptr<List> getList(const std::string &id);
+        std::shared_ptr<Block> getBlock(const std::string &id, Target *target);
+        std::shared_ptr<Variable> getVariable(const std::string &id, Target *target);
+        std::shared_ptr<List> getList(const std::string &id, Target *target);
         std::shared_ptr<Broadcast> getBroadcast(const std::string &id);
-        std::shared_ptr<Comment> getComment(const std::string &id);
-        std::shared_ptr<Entity> getEntity(const std::string &id);
+        std::shared_ptr<Comment> getComment(const std::string &id, Target *target);
+        std::shared_ptr<Entity> getEntity(const std::string &id, Target *target);
         std::shared_ptr<IBlockSection> blockSection(const std::string &opcode) const;
 
         void addHatToMap(std::unordered_map<Target *, std::vector<Script *>> &map, Script *script);

test/engine/engine_test.cpp

@@ -2333,3 +2333,20 @@ TEST(EngineTest, AlwaysStopCloneThreads)
     ASSERT_VAR(stage, "test");
     ASSERT_EQ(GET_VAR(stage, "test")->value().toInt(), 0);
 }
+
+TEST(EngineTest, DuplicateVariableOrListIDs)
+{
+    // Regtest for #567
+    Project p("regtest_projects/567_duplicate_variable_list_id.sb3");
+    ASSERT_TRUE(p.load());
+
+    auto engine = p.engine();
+
+    Stage *stage = engine->stage();
+    ASSERT_TRUE(stage);
+
+    engine->run();
+
+    ASSERT_VAR(stage, "passed");
+    ASSERT_TRUE(GET_VAR(stage, "passed")->value().toBool());
+}