#5853: Improve ParticleLoader such that it can deal with duplicate de…

…fs. They don't overwrite previously parsed ones in the same pass.

include/iparticles.h

@@ -89,7 +89,7 @@ class IParticleDef
 	virtual void swapParticleStages(std::size_t index, std::size_t index2) = 0;
 
     /// Signal emitted when some aspect of the particle def has changed
-    virtual sigc::signal<void> signal_changed() const = 0;
+    virtual sigc::signal<void>& signal_changed() = 0;
 
 	// Comparison operators - particle defs are considered equal if all properties (except the name!),
 	// number of stages and stage contents are the equal
@@ -176,7 +176,7 @@ class IParticlesManager :
 public:
 
     /// Signal emitted when particle definitions are reloaded
-    virtual sigc::signal<void> signal_particlesReloaded() const = 0;
+    virtual sigc::signal<void>& signal_particlesReloaded() = 0;
 
 	/// Enumerate each particle def.
 	virtual void forEachParticleDef(const ParticleDefVisitor&) = 0;

radiantcore/particles/ParticleDef.h

@@ -35,6 +35,9 @@ class ParticleDef
     // Changed signal
     sigc::signal<void> _changedSignal;
 
+    // A unique parse pass identifier
+    std::size_t _parseStamp;
+
 public:
 
 	/**
@@ -71,7 +74,7 @@ class ParticleDef
 	}
 
     // IParticleDef implementation
-    sigc::signal<void> signal_changed() const override 
+    sigc::signal<void>& signal_changed() override 
 	{ 
 		return _changedSignal;
 	}
@@ -136,6 +139,16 @@ class ParticleDef
 
 	void parseFromTokens(parser::DefTokeniser& tok);
 
+    std::size_t getParseStamp() const
+    {
+        return _parseStamp;
+    }
+
+    void setParseStamp(std::size_t stamp)
+    {
+        _parseStamp = stamp;
+    }
+
 	// Stream insertion operator, writing the entire particle def to the given stream
 	friend std::ostream& operator<< (std::ostream& stream, const ParticleDef& def);
 };

radiantcore/particles/ParticleLoader.cpp

@@ -40,13 +40,30 @@ void ParticleLoader::parseParticleDef(parser::DefTokeniser& tok, const std::stri
     auto name = tok.nextToken();
     tok.assertNextToken("{");
 
-    // Find the particle def (use the non-blocking, internal lookup)
-    auto def = _findOrInsert(name);
+    // Find any existing particle def
+    auto existing = _particles.try_emplace(name, std::make_shared<ParticleDef>(name));
 
-    def->setFilename(filename);
+    if (!existing.second && existing.first->second->getParseStamp() == _curParseStamp)
+    {
+        rWarning() << "Particle " << name << " already defined in " <<
+            existing.first->second->getFilename() <<
+            ", ignoring definition in " << filename << std::endl;
+
+        // Use a dummy particle def to consume the rest of this block
+        ParticleDef("").parseFromTokens(tok);
+        return;
+    }
+
+    existing.first->second->setParseStamp(_curParseStamp);
+    existing.first->second->setFilename(filename);
 
     // Let the particle construct itself from the token stream
-    def->parseFromTokens(tok);
+    existing.first->second->parseFromTokens(tok);
+}
+
+void ParticleLoader::onBeginParsing()
+{
+    ++_curParseStamp;
 }
 
 void ParticleLoader::parse(std::istream& stream, const vfs::FileInfo& fileInfo, const std::string& modDir)
@@ -60,4 +77,9 @@ void ParticleLoader::parse(std::istream& stream, const vfs::FileInfo& fileInfo,
     }
 }
 
+void ParticleLoader::onFinishParsing()
+{
+    rMessage() << "Found " << _particles.size() << " particle definitions." << std::endl;
+}
+
 }

radiantcore/particles/ParticleLoader.h

@@ -9,24 +9,31 @@
 namespace particles
 {
 
+using ParticleDefMap = std::map<std::string, ParticleDefPtr>;
+
 class ParticleLoader :
     public parser::ThreadedDeclParser<void>
 {
 private:
-    std::function<ParticleDefPtr(const std::string&)> _findOrInsert;
+    ParticleDefMap& _particles;
+
+    // A unique parse pass identifier, used to check when existing
+    // definitions have been parsed
+    std::size_t _curParseStamp;
 
 public:
-    ParticleLoader(const std::function<ParticleDefPtr(const std::string&)>& findOrInsert) :
+    ParticleLoader(ParticleDefMap& particles) :
         parser::ThreadedDeclParser<void>(decl::Type::Particle, PARTICLES_DIR, PARTICLES_EXT, 1),
-        _findOrInsert(findOrInsert)
+        _particles(particles),
+        _curParseStamp(0)
     {}
 
 protected:
+    void onBeginParsing() override;
     void parse(std::istream& stream, const vfs::FileInfo& fileInfo, const std::string& modDir) override;
+    void onFinishParsing() override;
 
 private:
-    // Accept a stream containing particle definitions to parse and add to the list.
-    void parseStream(std::istream& contents, const std::string& filename);
     void parseParticleDef(parser::DefTokeniser& tok, const std::string& filename);
 };
 

radiantcore/particles/ParticlesManager.cpp

@@ -43,13 +43,13 @@ namespace
 }
 
 ParticlesManager::ParticlesManager() :
-    _defLoader(std::bind(&ParticlesManager::findOrInsertParticleDefInternal, this, std::placeholders::_1))
+    _defLoader(_particleDefs)
 {
     _defLoader.signal_finished().connect(
         sigc::mem_fun(this, &ParticlesManager::onParticlesLoaded));
 }
 
-sigc::signal<void> ParticlesManager::signal_particlesReloaded() const
+sigc::signal<void>& ParticlesManager::signal_particlesReloaded()
 {
     return _particlesReloadedSignal;
 }
@@ -199,8 +199,6 @@ void ParticlesManager::reloadParticleDefs()
 
 void ParticlesManager::onParticlesLoaded()
 {
-    rMessage() << "Found " << _particleDefs.size() << " particle definitions." << std::endl;
-
     // Notify observers about this event
     _particlesReloadedSignal.emit();
 }

radiantcore/particles/ParticlesManager.h

@@ -15,9 +15,7 @@ namespace particles
 class ParticlesManager :
 	public IParticlesManager
 {
-	// Map of named particle defs
-	typedef std::map<std::string, ParticleDefPtr> ParticleDefMap;
-
+private:
 	ParticleDefMap _particleDefs;
 
     ParticleLoader _defLoader;
@@ -29,7 +27,7 @@ class ParticlesManager :
     ParticlesManager();
 
 	// IParticlesManager implementation
-    sigc::signal<void> signal_particlesReloaded() const override;
+    sigc::signal<void>& signal_particlesReloaded() override;
 
     void forEachParticleDef(const ParticleDefVisitor& visitor) override;
 

test/Particles.cpp

@@ -26,4 +26,14 @@ TEST_F(ParticlesTest, ParticleFilenamePrecedence)
         << "The particle using the caulk texture should have taken precedence";
 }
 
+// Prove that the ignored, duplicate particleDef is not stopping the parser from processing the rest of the file
+TEST_F(ParticlesTest, ParticleBelowDuplicatedIsParsed)
+{
+    auto decl = GlobalParticlesManager().getDefByName("particle_after_precedencecheck");
+
+    EXPECT_TRUE(decl) << "Could not locate the particleDef that should go below the precedencecheck in z_precedence.prt";
+    EXPECT_EQ(decl->getStage(0).getMaterialName(), "textures/common/nodraw")
+        << "The particle using the caulk texture should have taken precedence";
+}
+
 }

test/resources/tdm/particles/z_precedence.prt

@@ -24,4 +24,31 @@ particle precedencecheck {
 		offset 				0.000 0.000 0.000
 		gravity 			world 0.000
 	}
-}
+}
+
+// Just to prove that particleDefs after a duplicate definition are not ignored
+particle particle_after_precedencecheck
+{
+	{
+		count				1
+		material			textures/common/nodraw
+		time				10.000
+		cycles				1.000
+		bunching			1.000
+		distribution		rect 128.000 128.000 32.000 
+		direction			outward "0.000" 
+		orientation			view 
+		speed				 "1.000"  to "10.000"
+		size				 "200.000" 
+		aspect				 "1.000" 
+		randomDistribution				1
+		boundsExpansion				0.000
+		fadeIn				0.700
+		fadeOut				0.800
+		fadeIndex				0.000
+		color 				1.000 1.000 0.900 0.250
+		fadeColor 			1.000 1.000 1.000 0.000
+		offset 				0.000 0.000 0.000
+		gravity 			world 0.000
+	}
+}