#6030: First rough implementation of Material::updateFromSourceText

include/ishaders.h

@@ -447,6 +447,7 @@ class Material
     struct ParseResult
     {
         bool success;           // whether the update was successful
+        std::string parseError; // if success == false, this contains the error message
     };
 
     // Attempts to redefine this material from the given source text, which is

libs/decl/DeclarationBase.h

@@ -37,6 +37,7 @@ class DeclarationBase :
     DeclarationBlockSyntax _declBlock;
 
     bool _parsed;
+    std::string _parseErrors;
 
     sigc::signal<void> _changedSignal;
 
@@ -125,6 +126,12 @@ class DeclarationBase :
         return _changedSignal;
     }
 
+    const std::string& getParseErrors()
+    {
+        ensureParsed();
+        return _parseErrors;
+    }
+
 protected:
     // Defines the whitespace characters used by the DefTokeniser to separate tokens
     virtual const char* getWhitespaceDelimiters() const
@@ -148,6 +155,7 @@ class DeclarationBase :
 
         // Set the flag to true before parsing, to avoid infinite loops
         _parsed = true;
+        _parseErrors.clear();
 
         onBeginParsing();
 
@@ -160,6 +168,8 @@ class DeclarationBase :
         }
         catch (const parser::ParseException& ex)
         {
+            _parseErrors = ex.what();
+
             rError() << "[DeclParser]: Error parsing " << getTypeName(getDeclType()) << " " << getDeclName()
                 << ": " << ex.what() << std::endl;
         }

radiantcore/shaders/CShader.cpp

@@ -678,16 +678,22 @@ Material::ParseResult CShader::updateFromSourceText(const std::string& sourceTex
 {
     ensureTemplateCopy();
 
-    if (_template->getMaterialFlags() & Material::FLAG_NOSHADOWS)
-    {
-        _template->clearMaterialFlag(Material::FLAG_NOSHADOWS);
-    }
-    else
+    // Attempt to parse the template (separately from the active one)
+    auto newTemplate= std::make_shared<ShaderTemplate>(getName());
+
+    auto syntax = _template->getBlockSyntax();
+    syntax.contents = sourceText;
+    newTemplate->setBlockSyntax(syntax);
+
+    const auto& error = newTemplate->getParseErrors();
+
+    if (error.empty())
     {
-        _template->setMaterialFlag(Material::FLAG_NOSHADOWS);
+        // Parse seems to be successful, assign the text to the actual template
+        _template->setBlockSyntax(syntax);
     }
 
-    return ParseResult{ true };
+    return ParseResult{ error.empty(), error };
 }
 
 } // namespace shaders

test/Materials.cpp

@@ -1707,4 +1707,45 @@ TEST_F(MaterialsTest, ShaderExpressionEvaluation)
     }
 }
 
+TEST_F(MaterialsTest, UpdateFromValidSourceText)
+{
+    auto material = GlobalMaterialManager().getMaterial("textures/exporttest/empty");
+    EXPECT_TRUE(material) << "Could not find the material textures/exporttest/empty";
+
+    auto sourceText = R"(
+    diffusemap _white
+    {
+        blend blend
+        map _flat
+        rgb 0.5
+    }
+)";
+
+    auto result = material->updateFromSourceText(sourceText);
+
+    EXPECT_TRUE(result.success) << "Update from source text should have been succeeded";
+
+    EXPECT_EQ(material->getNumLayers(), 2);
+    EXPECT_EQ(material->getLayer(0)->getMapExpression()->getExpressionString(), "_white");
+}
+
+TEST_F(MaterialsTest, UpdateFromValidSourceTextEmitsSignal)
+{
+    auto material = GlobalMaterialManager().getMaterial("textures/exporttest/empty");
+    EXPECT_TRUE(material) << "Could not find the material textures/exporttest/empty";
+
+    auto changedSignalCount = 0;
+
+    material->sig_materialChanged().connect(
+        [&]() { changedSignalCount++; }
+    );
+
+    auto result = material->updateFromSourceText(R"(diffusemap _white)");
+
+    EXPECT_TRUE(result.success) << "Update from source text should have been succeeded";
+    EXPECT_EQ(changedSignalCount, 1) << "Changed signal should have been fired exactly once";
+
+
+}
+
 }