From 7371d9a44b0efdab8590026dcfe0c91f706a8e9b Mon Sep 17 00:00:00 2001
From: codereader <greebo@angua.at>
Date: Mon, 15 Aug 2022 13:53:53 +0200
Subject: [PATCH] #6030: First rough implementation of
 Material::updateFromSourceText

---
 include/ishaders.h              |  1 +
 libs/decl/DeclarationBase.h     | 10 ++++++++
 radiantcore/shaders/CShader.cpp | 20 ++++++++++------
 test/Materials.cpp              | 41 +++++++++++++++++++++++++++++++++
 4 files changed, 65 insertions(+), 7 deletions(-)

diff --git a/include/ishaders.h b/include/ishaders.h
index 9bbea30d5c..acc265ba15 100644
--- a/include/ishaders.h
+++ b/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
diff --git a/libs/decl/DeclarationBase.h b/libs/decl/DeclarationBase.h
index 24f7b4fa80..cd9b5f5909 100644
--- a/libs/decl/DeclarationBase.h
+++ b/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;
         }
diff --git a/radiantcore/shaders/CShader.cpp b/radiantcore/shaders/CShader.cpp
index 7d2c28906a..3fb24274ce 100644
--- a/radiantcore/shaders/CShader.cpp
+++ b/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
diff --git a/test/Materials.cpp b/test/Materials.cpp
index 755db67e1f..d0d6e94a71 100644
--- a/test/Materials.cpp
+++ b/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";
+
+
+}
+
 }