From f7cb3e116f14e3f13bb72e35a54273c9a5584fc5 Mon Sep 17 00:00:00 2001 From: codereader Date: Wed, 2 Mar 2022 18:35:46 +0100 Subject: [PATCH] #5853: Improve ParticleLoader such that it can deal with duplicate defs. They don't overwrite previously parsed ones in the same pass. --- include/iparticles.h | 4 +-- radiantcore/particles/ParticleDef.h | 15 +++++++++- radiantcore/particles/ParticleLoader.cpp | 30 ++++++++++++++++--- radiantcore/particles/ParticleLoader.h | 17 +++++++---- radiantcore/particles/ParticlesManager.cpp | 6 ++-- radiantcore/particles/ParticlesManager.h | 6 ++-- test/Particles.cpp | 10 +++++++ test/resources/tdm/particles/z_precedence.prt | 29 +++++++++++++++++- 8 files changed, 96 insertions(+), 21 deletions(-) diff --git a/include/iparticles.h b/include/iparticles.h index ba082945fb..0eff484634 100644 --- a/include/iparticles.h +++ b/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 signal_changed() const = 0; + virtual sigc::signal& 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 signal_particlesReloaded() const = 0; + virtual sigc::signal& signal_particlesReloaded() = 0; /// Enumerate each particle def. virtual void forEachParticleDef(const ParticleDefVisitor&) = 0; diff --git a/radiantcore/particles/ParticleDef.h b/radiantcore/particles/ParticleDef.h index 740caddbe2..e3a628a63b 100644 --- a/radiantcore/particles/ParticleDef.h +++ b/radiantcore/particles/ParticleDef.h @@ -35,6 +35,9 @@ class ParticleDef // Changed signal sigc::signal _changedSignal; + // A unique parse pass identifier + std::size_t _parseStamp; + public: /** @@ -71,7 +74,7 @@ class ParticleDef } // IParticleDef implementation - sigc::signal signal_changed() const override + sigc::signal& 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); }; diff --git a/radiantcore/particles/ParticleLoader.cpp b/radiantcore/particles/ParticleLoader.cpp index 950dfcf15d..2672cf00d1 100644 --- a/radiantcore/particles/ParticleLoader.cpp +++ b/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(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; +} + } diff --git a/radiantcore/particles/ParticleLoader.h b/radiantcore/particles/ParticleLoader.h index ff007fa98c..463b088b13 100644 --- a/radiantcore/particles/ParticleLoader.h +++ b/radiantcore/particles/ParticleLoader.h @@ -9,24 +9,31 @@ namespace particles { +using ParticleDefMap = std::map; + class ParticleLoader : public parser::ThreadedDeclParser { private: - std::function _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& findOrInsert) : + ParticleLoader(ParticleDefMap& particles) : parser::ThreadedDeclParser(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); }; diff --git a/radiantcore/particles/ParticlesManager.cpp b/radiantcore/particles/ParticlesManager.cpp index 62d55463b2..7ecde920d1 100644 --- a/radiantcore/particles/ParticlesManager.cpp +++ b/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 ParticlesManager::signal_particlesReloaded() const +sigc::signal& 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(); } diff --git a/radiantcore/particles/ParticlesManager.h b/radiantcore/particles/ParticlesManager.h index bc7d513faf..aaeb4b9b31 100644 --- a/radiantcore/particles/ParticlesManager.h +++ b/radiantcore/particles/ParticlesManager.h @@ -15,9 +15,7 @@ namespace particles class ParticlesManager : public IParticlesManager { - // Map of named particle defs - typedef std::map ParticleDefMap; - +private: ParticleDefMap _particleDefs; ParticleLoader _defLoader; @@ -29,7 +27,7 @@ class ParticlesManager : ParticlesManager(); // IParticlesManager implementation - sigc::signal signal_particlesReloaded() const override; + sigc::signal& signal_particlesReloaded() override; void forEachParticleDef(const ParticleDefVisitor& visitor) override; diff --git a/test/Particles.cpp b/test/Particles.cpp index 32986ba2c4..29dc2aeb2d 100644 --- a/test/Particles.cpp +++ b/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"; +} + } diff --git a/test/resources/tdm/particles/z_precedence.prt b/test/resources/tdm/particles/z_precedence.prt index 0059a6cbbd..0a02febd66 100644 --- a/test/resources/tdm/particles/z_precedence.prt +++ b/test/resources/tdm/particles/z_precedence.prt @@ -24,4 +24,31 @@ particle precedencecheck { offset 0.000 0.000 0.000 gravity world 0.000 } -} \ No newline at end of file +} + +// 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 + } +}