#5977: Add unit test checking that reloadDecls re-parses the file

include/ideclmanager.h

@@ -49,6 +49,9 @@ class IDeclaration
 
     // The type of this declaration
     virtual Type getDeclType() const = 0;
+
+    // The raw syntax block (without the outer curly braces) used to construct this decl
+    virtual const DeclarationBlockSyntax& getBlockSyntax() const = 0;
 };
 
 using NamedDeclarations = std::map<std::string, IDeclaration::Ptr>;

plugins/sound/SoundShader.cpp

@@ -22,7 +22,7 @@ struct SoundShader::ParsedContents
 
 SoundShader::SoundShader(const decl::DeclarationBlockSyntax& block)
 :	_name(block.name),
-    _blockContents(block.contents),
+    _declBlock(block),
 	_fileInfo(block.fileInfo),
     _modName(block.getModName())
 { }
@@ -37,7 +37,7 @@ void SoundShader::parseDefinition() const
 	_contents.reset(new ParsedContents);
 
 	// Get a new tokeniser and parse the block
-	parser::BasicDefTokeniser<std::string> tok(_blockContents);
+	parser::BasicDefTokeniser<std::string> tok(_declBlock.contents);
 
 	while (tok.hasMoreTokens())
     {
@@ -97,7 +97,12 @@ std::string SoundShader::getShaderFilePath() const
 
 std::string SoundShader::getDefinition() const
 {
-	return _blockContents;
+	return _declBlock.contents;
+}
+
+const decl::DeclarationBlockSyntax& SoundShader::getBlockSyntax() const
+{
+    return _declBlock;
 }
 
 } // namespace sound

plugins/sound/SoundShader.h

@@ -9,14 +9,14 @@ namespace sound
 {
 
 /// Representation of a single sound shader.
-class SoundShader :
+class SoundShader final :
 	public ISoundShader
 {
 	// Name of the shader
 	std::string _name;
 
 	// The raw unparsed definition block
-	std::string _blockContents;
+    decl::DeclarationBlockSyntax _declBlock;
 
     // Information we have parsed on demand
     struct ParsedContents;
@@ -36,7 +36,7 @@ class SoundShader :
 
 	SoundShader(const decl::DeclarationBlockSyntax& block);
 
-    virtual ~SoundShader();
+    ~SoundShader();
 
     // ISoundShader implementation
 	SoundRadii getRadii() const override;
@@ -47,6 +47,8 @@ class SoundShader :
 	const std::string& getDisplayFolder() const override;
 	std::string getShaderFilePath() const override;
 	std::string getDefinition() const override;
+
+    const decl::DeclarationBlockSyntax& getBlockSyntax() const override;
 };
 
 }

radiantcore/decl/DeclarationManager.cpp

@@ -124,7 +124,10 @@ void DeclarationManager::onParserFinished(Type parserType, std::map<Type, NamedD
         {
             auto& map = _declarationsByType.try_emplace(pair.first, Declarations()).first->second.decls;
 
-            map.merge(pair.second);
+            for (auto& parsedDecl : pair.second)
+            {
+                InsertDeclaration(map, std::move(parsedDecl.second));
+            }
         }
     }
 
@@ -139,7 +142,7 @@ void DeclarationManager::onParserFinished(Type parserType, std::map<Type, NamedD
     // We might have received a parser in the meantime
     handleUnrecognisedBlocks();
 
-    // Invoke the declsReloaded singal for this type
+    // Invoke the declsReloaded signal for this type
     signal_DeclsReloaded(parserType).emit();
 }
 

test/DeclManager.cpp

@@ -1,6 +1,7 @@
 #include "RadiantTest.h"
 
 #include "ideclmanager.h"
+#include "testutil/TemporaryFile.h"
 
 namespace test
 {
@@ -13,11 +14,13 @@ class TestDeclaration :
 private:
     decl::Type _type;
     std::string _name;
+    decl::DeclarationBlockSyntax _block;
 
 public:
-    TestDeclaration(decl::Type type, const std::string& name) :
+    TestDeclaration(decl::Type type, const decl::DeclarationBlockSyntax& block) :
         _type(type),
-        _name(name)
+        _block(block),
+        _name(block.name)
     {}
 
     const std::string& getDeclName() const override
@@ -29,6 +32,11 @@ class TestDeclaration :
     {
         return _type;
     }
+
+    const decl::DeclarationBlockSyntax& getBlockSyntax() const override
+    {
+        return _block;
+    }
 };
 
 class TestDeclarationParser :
@@ -52,7 +60,7 @@ class TestDeclarationParser :
             std::this_thread::sleep_for(20ms);
         }
 
-        return std::make_shared<TestDeclaration>(getDeclType(), block.name);
+        return std::make_shared<TestDeclaration>(getDeclType(), block);
     }
 };
 
@@ -69,7 +77,7 @@ class TestDeclaration2Parser :
     // Create a new declaration instance from the given block
     decl::IDeclaration::Ptr parseFromBlock(const decl::DeclarationBlockSyntax& block) override
     {
-        return std::make_shared<TestDeclaration>(getDeclType(), block.name);
+        return std::make_shared<TestDeclaration>(getDeclType(), block);
     }
 };
 
@@ -273,4 +281,64 @@ TEST_F(DeclManagerTest, FindDeclaration)
     EXPECT_FALSE(GlobalDeclarationManager().findDeclaration(decl::Type::Material, "decl/nonexistent"));
 }
 
+TEST_F(DeclManagerTest, ReloadDeclarationWithChangedFile)
+{
+    TemporaryFile tempFile(_context.getTestProjectPath() + "testdecls/temp_file.decl");
+    tempFile.setContents(R"(
+
+decl/temporary/11
+{
+    diffusemap textures/temporary/11
+}
+
+decl/temporary/12
+{
+    diffusemap textures/temporary/12
+}
+
+)");
+
+    GlobalDeclarationManager().registerDeclType("testdecl", std::make_shared<TestDeclarationParser>());
+    GlobalDeclarationManager().registerDeclFolder(decl::Type::Material, "testdecls", ".decl");
+
+    auto temp12 = GlobalDeclarationManager().findDeclaration(decl::Type::Material, "decl/temporary/12");
+    EXPECT_TRUE(temp12) << "Couldn't find the declaration decl/temporary/12";
+
+    auto temp11 = GlobalDeclarationManager().findDeclaration(decl::Type::Material, "decl/temporary/11");
+    EXPECT_TRUE(temp11) << "Couldn't find the declaration decl/temporary/11";
+
+    EXPECT_FALSE(GlobalDeclarationManager().findDeclaration(decl::Type::Material, "decl/temporary/13"))
+        << "decl/temporary/13 should not be present";
+
+    EXPECT_NE(temp12->getBlockSyntax().contents.find("diffusemap textures/temporary/12"), std::string::npos) <<
+        "Didn't find the expected contents in the decl block";
+
+    // Change the file, change temp12, remove temp11 and add temp13 instead
+    tempFile.setContents(R"(
+
+decl/temporary/12
+{
+    diffusemap textures/changed_temporary/12
+}
+
+decl/temporary/13
+{
+    diffusemap textures/temporary/13
+}
+
+)");
+
+    // Check the change sin temp12
+    temp12 = GlobalDeclarationManager().findDeclaration(decl::Type::Material, "decl/temporary/12");
+    EXPECT_TRUE(temp12) << "Couldn't find the declaration decl/temporary/12";
+
+    EXPECT_NE(temp12->getBlockSyntax().contents.find("diffusemap textures/changed_temporary/12"), std::string::npos) <<
+        "Couldn't find the changed contents in the decl block";
+
+    EXPECT_TRUE(GlobalDeclarationManager().findDeclaration(decl::Type::Material, "decl/temporary/13"))
+        << "decl/temporary/13 should be present now";
+    EXPECT_FALSE(GlobalDeclarationManager().findDeclaration(decl::Type::Material, "decl/temporary/11"))
+        << "decl/temporary/11 should be gone now";
+}
+
 }

test/testutil/TemporaryFile.h

@@ -0,0 +1,37 @@
+#pragma once
+
+#include <string>
+#include <fstream>
+#include "os/file.h"
+
+namespace test
+{
+
+// File object removing itself when going out of scope
+class TemporaryFile
+{
+private:
+    std::string _path;
+
+public:
+    TemporaryFile(const std::string& path, const std::string& contents = "") :
+        _path(path)
+    {
+        setContents(contents);
+    }
+
+    void setContents(const std::string& contents)
+    {
+        std::ofstream stream(_path, std::ofstream::out);
+        stream << contents;
+        stream.flush();
+        stream.close();
+    }
+
+    ~TemporaryFile()
+    {
+        fs::remove(_path);
+    }
+};
+
+}

tools/msvc/Tests/Tests.vcxproj

@@ -75,6 +75,7 @@
     <ClInclude Include="..\..\..\test\testutil\CommandFailureHelper.h" />
     <ClInclude Include="..\..\..\test\testutil\MapOperationMonitor.h" />
     <ClInclude Include="..\..\..\test\testutil\RenderUtils.h" />
+    <ClInclude Include="..\..\..\test\testutil\TemporaryFile.h" />
     <ClInclude Include="..\..\..\test\testutil\TestBufferObjectProvider.h" />
     <ClInclude Include="..\..\..\test\testutil\FileSelectionHelper.h" />
     <ClInclude Include="..\..\..\test\testutil\TestObjectRenderer.h" />

tools/msvc/Tests/Tests.vcxproj.filters

@@ -107,6 +107,9 @@
     <ClInclude Include="..\..\..\test\testutil\RenderUtils.h">
       <Filter>testutil</Filter>
     </ClInclude>
+    <ClInclude Include="..\..\..\test\testutil\TemporaryFile.h">
+      <Filter>testutil</Filter>
+    </ClInclude>
   </ItemGroup>
   <ItemGroup>
     <Filter Include="algorithm">