Merge pull request #531 from scratchcpp/fix_undefined_variable

Fix #446: Create missing variables and lists used by inputs and fields

Author: adazem009

src/engine/compiler.cpp

@@ -289,7 +289,9 @@ std::shared_ptr<Block> Compiler::inputBlock(int id) const
 /*! Returns the index of the given variable. */
 unsigned int Compiler::variableIndex(std::shared_ptr<Entity> varEntity)
 {
+    assert(varEntity);
     auto var = dynamic_cast<Variable *>(varEntity.get());
+    assert(var);
     auto it = std::find(impl->variables.begin(), impl->variables.end(), var);
     if (it != impl->variables.end())
         return it - impl->variables.begin();

src/engine/internal/engine.cpp

@@ -113,20 +113,62 @@ void Engine::resolveIds()
             const auto &inputs = block->inputs();
             for (const auto &input : inputs) {
                 input->setValueBlock(getBlock(input->valueBlockId()));
+
                 if (container)
                     input->setInputId(container->resolveInput(input->name()));
-                input->primaryValue()->setValuePtr(getEntity(input->primaryValue()->valueId()));
-                input->secondaryValue()->setValuePtr(getEntity(input->primaryValue()->valueId()));
+
+                InputValue *value = input->primaryValue();
+                std::string id = value->valueId(); // no reference!
+                value->setValuePtr(getEntity(id));
+                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()));
+
+                // Add missing variables and lists
+                if (!value->valuePtr()) {
+                    if (value->type() == InputValue::Type::Variable) {
+                        assert(!id.empty());
+                        std::cout << "warning: variable " << value->value().toString() << " referenced by an input missing, creating in stage" << std::endl;
+                        auto var = std::make_shared<Variable>(id, value->value().toString());
+                        value->setValuePtr(var);
+                        stage()->addVariable(var);
+                    } else if (value->type() == InputValue::Type::List) {
+                        assert(!id.empty());
+                        std::cout << "warning: list " << value->value().toString() << " referenced by an input missing, creating in stage" << std::endl;
+                        std::shared_ptr<List> list = std::make_shared<List>(id, value->value().toString());
+                        value->setValuePtr(list);
+                        stage()->addList(list);
+                    }
+                }
             }
 
             const auto &fields = block->fields();
             for (auto field : fields) {
-                field->setValuePtr(getEntity(field->valueId()));
+                std::string id = field->valueId(); // no reference!
+                field->setValuePtr(getEntity(id));
+
                 if (container) {
                     field->setFieldId(container->resolveField(field->name()));
+
                     if (!field->valuePtr())
                         field->setSpecialValueId(container->resolveFieldValue(field->value().toString()));
                 }
+
+                // TODO: Move field information out of Engine
+                if (!field->valuePtr()) {
+                    if (field->name() == "VARIABLE") {
+                        assert(!id.empty());
+                        std::cout << "warning: variable " << field->value().toString() << " referenced by a field missing, creating in stage" << std::endl;
+                        auto var = std::make_shared<Variable>(id, field->value().toString());
+                        field->setValuePtr(var);
+                        stage()->addVariable(var);
+                    } else if (field->name() == "LIST") {
+                        assert(!id.empty());
+                        std::cout << "warning: list " << field->value().toString() << " referenced by a field missing, creating in stage" << std::endl;
+                        std::shared_ptr<List> list = std::make_shared<List>(id, field->value().toString());
+                        field->setValuePtr(list);
+                        stage()->addList(list);
+                    }
+                }
             }
 
             block->updateInputMap();

src/scratch/inputvalue.cpp

@@ -34,10 +34,12 @@ void InputValue::compile(Compiler *compiler)
             break;
 
         case Type::Variable:
+            assert(impl->valuePtr);
             compiler->addInstruction(vm::OP_READ_VAR, { compiler->variableIndex(impl->valuePtr) });
             break;
 
         case Type::List:
+            assert(impl->valuePtr);
             compiler->addInstruction(vm::OP_READ_LIST, { compiler->listIndex(impl->valuePtr) });
             break;
 

test/engine/engine_test.cpp

@@ -2201,3 +2201,44 @@ TEST(EngineTest, NoCrashWhenLoadingUndefinedVariableOrListMonitor)
     ASSERT_EQ(list->id(), "7a5rAs|X2_[1APT7@B1V");
     ASSERT_TRUE(list->empty());
 }
+
+TEST(EngineTest, NoCrashWithUndefinedVariableOrList)
+{
+    // Regtest for #446
+    Project p("regtest_projects/446_undefined_variable_or_list_input_crash.sb3");
+    ASSERT_TRUE(p.load());
+
+    auto engine = p.engine();
+
+    // The undefined variable and list should now be defined
+    Stage *stage = engine->stage();
+    ASSERT_TRUE(stage);
+    ASSERT_VAR(stage, "test var");
+    auto var = GET_VAR(stage, "test var");
+    ASSERT_EQ(var->id(), "J3iHmLt:dwaR}^-fnGmG");
+    ASSERT_EQ(var->value(), Value());
+
+    ASSERT_LIST(stage, "test list");
+    auto list = GET_LIST(stage, "test list");
+    ASSERT_EQ(list->id(), "}l+w#Er5y!:*gh~5!3t%");
+    ASSERT_TRUE(list->empty());
+
+    // Test with fields
+    p.setFileName("regtest_projects/446_undefined_variable_or_list_field_crash.sb3");
+    ASSERT_TRUE(p.load());
+
+    engine = p.engine();
+
+    // The undefined variable and list should now be defined
+    stage = engine->stage();
+    ASSERT_TRUE(stage);
+    ASSERT_VAR(stage, "test var");
+    var = GET_VAR(stage, "test var");
+    ASSERT_EQ(var->id(), "J3iHmLt:dwaR}^-fnGmG");
+    ASSERT_EQ(var->value(), Value());
+
+    ASSERT_LIST(stage, "test list");
+    list = GET_LIST(stage, "test list");
+    ASSERT_EQ(list->id(), "}l+w#Er5y!:*gh~5!3t%");
+    ASSERT_TRUE(list->empty());
+}