#5977: SoundManager no longer maintains its own dictionary of shaders…

… as member
codereader · Jun 19, 2022 · e6c400e · e6c400e
1 parent 30e8b3f
commit e6c400e
Show file tree

Hide file tree

Showing 9 changed files with 67 additions and 92 deletions.
diff --git a/include/ideclmanager.h b/include/ideclmanager.h
@@ -96,6 +96,10 @@ class IDeclarationManager :
     // Registering a folder will immediately trigger parsing of all contained files matching the criteria.
     virtual void registerDeclFolder(Type defaultType, const std::string& vfsFolder, const std::string& extension) = 0;
 
+    // Find the declaration with the given type and name
+    // Returns an empty reference if no declaration with that name could be found
+    virtual IDeclaration::Ptr findDeclaration(Type type, const std::string& name) = 0;
+
     // Iterate over all known declarations, using the given visitor
     virtual void foreachDeclaration(Type type, const std::function<void(const IDeclaration&)>& functor) = 0;
 

diff --git a/plugins/sound/SoundFileLoader.h b/plugins/sound/SoundFileLoader.h
@@ -1,87 +1,27 @@
 #pragma once
 
-#include <map>
 #include "ideclmanager.h"
-#include "SoundManager.h"
-
-#include "parser/DefBlockTokeniser.h"
-#include "parser/DefTokeniser.h"
-#include "parser/ThreadedDeclParser.h"
-#include "ifilesystem.h"
-#include "iarchive.h"
-#include "ui/imainframe.h"
-
-#include <iostream>
+#include "SoundShader.h"
 
 namespace sound
 {
 
-namespace
-{
-    /// Sound directory name
-    constexpr const char* const SOUND_FOLDER = "sound/";
-    constexpr const char* const SOUND_FILE_EXTENSION = ".sndshd";
-}
-
-using ShaderMap = std::map<std::string, SoundShader::Ptr>;
-
 /**
  * Declaration parser capable of dealing with sound shader blocks
  */
 class SoundFileLoader final :
     public decl::IDeclarationParser
 {
-private:
-    // Shader map to populate
-    ShaderMap _shaders;
-
 public:
     decl::Type getDeclType() const override
     {
         return decl::Type::SoundShader;
     }
 
-    // Create a new declaration instance from the given block
     decl::IDeclaration::Ptr parseFromBlock(const decl::DeclarationBlockSyntax& block) override
     {
-        return std::make_shared<SoundShader>(block.name, block.contents, block.fileInfo, block.getModName());
-    }
-#if 0
-    void onBeginParsing() override
-    {
-        _shaders.clear();
-    }
-
-    void parse(std::istream& stream, const vfs::FileInfo& fileInfo, const std::string& modDir) override
-    {
-        // Construct a DefTokeniser to tokenise the string into sound shader decls
-        parser::BasicDefBlockTokeniser<std::istream> tok(stream);
-
-        while (tok.hasMoreBlocks())
-        {
-            // Retrieve a named definition block from the parser
-            parser::BlockTokeniser::Block block = tok.nextBlock();
-
-            // Create a new shader with this name
-            auto result = _shaders.emplace(block.name,
-                std::make_shared<SoundShader>(block.name, block.contents, fileInfo, modDir)
-            );
-
-            if (!result.second)
-            {
-                rError() << "[SoundManager]: SoundShader with name "
-                    << block.name << " already exists." << std::endl;
-            }
-        }
-    }
-
-    ShaderMap onFinishParsing() override
-    {
-        rMessage() << _shaders.size() << " sound shaders found." << std::endl;
-
-        return std::move(_shaders);
+        return std::make_shared<SoundShader>(block);
     }
-#endif
 };
 
 }
diff --git a/plugins/sound/SoundManager.cpp b/plugins/sound/SoundManager.cpp
@@ -4,7 +4,6 @@
 #include "icommandsystem.h"
 #include "ideclmanager.h"
 
-#include "debugging/ScopedDebugTimer.h"
 #include "os/path.h"
 #include "string/case_conv.h"
 
@@ -20,6 +19,10 @@ namespace sound
 namespace
 {
 
+/// Sound directory name
+constexpr const char* const SOUND_FOLDER = "sound/";
+constexpr const char* const SOUND_FILE_EXTENSION = ".sndshd";
+
 // Load the given file, trying different extensions (first OGG, then WAV) as fallback
 ArchiveFilePtr openSoundFile(const std::string& fileName)
 {
@@ -61,11 +64,13 @@ ArchiveFilePtr openSoundFile(const std::string& fileName)
 
 }
 
-// Constructor
-SoundManager::SoundManager() :
-	_emptyShader(new SoundShader("", "", 
-        vfs::FileInfo("sounds/", "_autogenerated_by_darkradiant_.sndshd", vfs::Visibility::HIDDEN), ""))
-{}
+SoundManager::SoundManager()
+{
+    decl::DeclarationBlockSyntax defaultBlock;
+    defaultBlock.fileInfo = vfs::FileInfo("sounds/", "_autogenerated_by_darkradiant_.sndshd", vfs::Visibility::HIDDEN);
+
+    _emptyShader = std::make_unique<SoundShader>(defaultBlock);
+}
 
 void SoundManager::forEachShader(std::function<void(const ISoundShader&)> functor)
 {
@@ -106,10 +111,9 @@ sigc::signal<void>& SoundManager::signal_soundShadersReloaded()
 
 ISoundShaderPtr SoundManager::getSoundShader(const std::string& shaderName)
 {
-	auto found = _shaders.find(shaderName);
-
-    // If the name was found, return it, otherwise return an empty shader object
-	return found != _shaders.end() ? found->second : _emptyShader;
+    return std::static_pointer_cast<ISoundShader>(
+        GlobalDeclarationManager().findDeclaration(decl::Type::SoundShader, shaderName)
+    );
 }
 
 const std::string& SoundManager::getName() const

diff --git a/plugins/sound/SoundManager.h b/plugins/sound/SoundManager.h
@@ -18,9 +18,6 @@ class SoundManager final :
     public ISoundManager
 {
 private:
-    // Master map of shaders
-	ShaderMap _shaders;
-
 	SoundShader::Ptr _emptyShader;
 
 	// The helper class for playing the sounds

diff --git a/plugins/sound/SoundShader.cpp b/plugins/sound/SoundShader.cpp
@@ -20,14 +20,11 @@ struct SoundShader::ParsedContents
 	std::string displayFolder;
 };
 
-SoundShader::SoundShader(const std::string& name,
-                         const std::string& blockContents,
-                         const vfs::FileInfo& fileInfo,
-                         const std::string& modName)
-:	_name(name),
-    _blockContents(blockContents),
-	_fileInfo(fileInfo),
-    _modName(modName)
+SoundShader::SoundShader(const decl::DeclarationBlockSyntax& block)
+:	_name(block.name),
+    _blockContents(block.contents),
+	_fileInfo(block.fileInfo),
+    _modName(block.getModName())
 { }
 
 // Destructor must be defined with ParsedContents definition visible, otherwise

diff --git a/plugins/sound/SoundShader.h b/plugins/sound/SoundShader.h
@@ -34,11 +34,7 @@ class SoundShader :
 public:
 	using Ptr = std::shared_ptr<SoundShader>;
 
-	/// Constructor.
-	SoundShader(const std::string& name,
-				const std::string& blockContents,
-				const vfs::FileInfo& fileInfo,
-				const std::string& modName = "base");
+	SoundShader(const decl::DeclarationBlockSyntax& block);
 
     virtual ~SoundShader();
 

diff --git a/radiantcore/decl/DeclarationManager.cpp b/radiantcore/decl/DeclarationManager.cpp
@@ -51,8 +51,37 @@ void DeclarationManager::registerDeclFolder(Type defaultType, const std::string&
     decls.parser->start();
 }
 
+IDeclaration::Ptr DeclarationManager::findDeclaration(Type type, const std::string& name)
+{
+    IDeclaration::Ptr returnValue;
+
+    doWithDeclarations(type, [&](const NamedDeclarations& decls)
+    {
+        auto decl = decls.find(name);
+
+        if (decl != decls.end())
+        {
+            returnValue = decl->second;
+        }
+    });
+
+    return returnValue;
+}
+
 void DeclarationManager::foreachDeclaration(Type type, const std::function<void(const IDeclaration&)>& functor)
 {
+    doWithDeclarations(type, [&](const NamedDeclarations& decls)
+    {
+        for (const auto& [_, decl] : decls)
+        {
+            functor(*decl);
+        }
+    });
+}
+
+void DeclarationManager::doWithDeclarations(Type type, const std::function<void(const NamedDeclarations&)>& action)
+{
+    // Find type dictionary
     auto declLock = std::make_unique<std::lock_guard<std::mutex>>(_declarationLock);
 
     auto decls = _declarationsByType.find(type);
@@ -71,10 +100,7 @@ void DeclarationManager::foreachDeclaration(Type type, const std::function<void(
         declLock = std::make_unique<std::lock_guard<std::mutex>>(_declarationLock);
     }
 
-    for (const auto& [_, decl] : decls->second.decls)
-    {
-        functor(*decl);
-    }
+    action(decls->second.decls);
 }
 
 sigc::signal<void>& DeclarationManager::signal_DeclsReloaded(Type type)

diff --git a/radiantcore/decl/DeclarationManager.h b/radiantcore/decl/DeclarationManager.h
@@ -47,6 +47,7 @@ class DeclarationManager :
     void registerDeclType(const std::string& typeName, const IDeclarationParser::Ptr& parser) override;
     void unregisterDeclType(const std::string& typeName) override;
     void registerDeclFolder(Type defaultType, const std::string& inputFolder, const std::string& inputExtension) override;
+    IDeclaration::Ptr findDeclaration(Type type, const std::string& name) override;
     void foreachDeclaration(Type type, const std::function<void(const IDeclaration&)>& functor) override;
     sigc::signal<void>& signal_DeclsReloaded(Type type) override;
 
@@ -62,6 +63,7 @@ class DeclarationManager :
     static void InsertDeclaration(NamedDeclarations& map, IDeclaration::Ptr&& declaration);
 
 private:
+    void doWithDeclarations(Type type, const std::function<void(const NamedDeclarations&)>& action);
     void handleUnrecognisedBlocks();
 };
 

diff --git a/test/DeclManager.cpp b/test/DeclManager.cpp
@@ -264,4 +264,13 @@ TEST_F(DeclManagerTest, DeclsReloadedSignal)
     EXPECT_FALSE(modelSignalFired) << "Model-type Signal should not have been fired";
 }
 
+TEST_F(DeclManagerTest, FindDeclaration)
+{
+    GlobalDeclarationManager().registerDeclType("testdecl", std::make_shared<TestDeclarationParser>());
+    GlobalDeclarationManager().registerDeclFolder(decl::Type::Material, "testdecls", ".decl");
+
+    EXPECT_TRUE(GlobalDeclarationManager().findDeclaration(decl::Type::Material, "decl/exporttest/guisurf1"));
+    EXPECT_FALSE(GlobalDeclarationManager().findDeclaration(decl::Type::Material, "decl/nonexistent"));
+}
+
 }