mirror of
				https://github.com/OpenMW/openmw.git
				synced 2025-10-31 19:56:38 +00:00 
			
		
		
		
	This PR aims to spark the retirement of a questionable pattern I have found all over our code base. I will illustrate how this pattern encourages code duplication, lacks type safety, requires documentation and can be prone to bugs. ``` std::map<std::string, Object> mMap; // Stored in all lowercase for a case-insensitive lookup std::string lowerKey = Misc::StringUtils::lowerCase(key); mMap.emplace(lowerKey, object); std::string lowerKey = Misc::StringUtils::lowerCase(key); mMap.find(lowerKey); mMap.find(key); // Not found. Oops! ``` An alternative approach produces no such issues. ``` std::unordered_map<std::string, Object, Misc::StringUtils::CiHash, Misc::StringUtils::CiEqual> mMap; mMap.emplace(key, object); mMap.find(key); ``` Of course, such an alternative will work for a `map` as well, but an `unordered_map` is generally preferable over a `map` with these changes because we have moved `lowerCase` into the comparison operator. In this PR I have refactored `Animation::mNodeMap` accordingly. I have reviewed and adapted all direct and indirect usage of `Animation::mNodeMap` to ensure we do not change behaviour with this PR.
		
			
				
	
	
		
			113 lines
		
	
	
	
		
			3 KiB
		
	
	
	
		
			C++
		
	
	
	
	
	
			
		
		
	
	
			113 lines
		
	
	
	
		
			3 KiB
		
	
	
	
		
			C++
		
	
	
	
	
	
| #ifndef OPENMW_COMPONENTS_SCENEUTIL_VISITOR_H
 | |
| #define OPENMW_COMPONENTS_SCENEUTIL_VISITOR_H
 | |
| 
 | |
| #include <osg/MatrixTransform>
 | |
| #include <osg/NodeVisitor>
 | |
| 
 | |
| #include <unordered_map>
 | |
| 
 | |
| #include <components/misc/stringops.hpp>
 | |
| 
 | |
| // Commonly used scene graph visitors
 | |
| namespace SceneUtil
 | |
| {
 | |
| 
 | |
|     // Find a Group by name, case-insensitive
 | |
|     // If not found, mFoundNode will be nullptr
 | |
|     class FindByNameVisitor : public osg::NodeVisitor
 | |
|     {
 | |
|     public:
 | |
|         FindByNameVisitor(const std::string& nameToFind)
 | |
|             : osg::NodeVisitor(TRAVERSE_ALL_CHILDREN)
 | |
|             , mNameToFind(nameToFind)
 | |
|             , mFoundNode(nullptr)
 | |
|         {
 | |
|         }
 | |
| 
 | |
|         void apply(osg::Group& group) override;
 | |
|         void apply(osg::MatrixTransform& node) override;
 | |
|         void apply(osg::Geometry& node) override;
 | |
| 
 | |
|         bool checkGroup(osg::Group& group);
 | |
| 
 | |
|         std::string mNameToFind;
 | |
|         osg::Group* mFoundNode;
 | |
|     };
 | |
| 
 | |
|     class FindByClassVisitor : public osg::NodeVisitor
 | |
|     {
 | |
|     public:
 | |
|         FindByClassVisitor(const std::string& nameToFind)
 | |
|             : osg::NodeVisitor(TRAVERSE_ALL_CHILDREN)
 | |
|             , mNameToFind(nameToFind)
 | |
|         {
 | |
|         }
 | |
| 
 | |
|         void apply(osg::Node &node) override;
 | |
| 
 | |
|         std::string mNameToFind;
 | |
|         std::vector<osg::Node *> mFoundNodes;
 | |
|     };
 | |
| 
 | |
|     /// Maps names to nodes
 | |
|     class NodeMapVisitor : public osg::NodeVisitor
 | |
|     {
 | |
|     public:
 | |
|         typedef std::unordered_map<std::string, osg::ref_ptr<osg::MatrixTransform>, Misc::StringUtils::CiHash, Misc::StringUtils::CiEqual> NodeMap;
 | |
| 
 | |
|         NodeMapVisitor(NodeMap& map)
 | |
|             : osg::NodeVisitor(osg::NodeVisitor::TRAVERSE_ALL_CHILDREN)
 | |
|             , mMap(map)
 | |
|         {
 | |
|         }
 | |
| 
 | |
|         void apply(osg::MatrixTransform& trans) override;
 | |
| 
 | |
|     private:
 | |
|         NodeMap& mMap;
 | |
|     };
 | |
| 
 | |
|     /// @brief Base class for visitors that remove nodes from a scene graph.
 | |
|     /// Subclasses need to fill the mToRemove vector.
 | |
|     /// To use, node->accept(removeVisitor); removeVisitor.remove();
 | |
|     class RemoveVisitor : public osg::NodeVisitor
 | |
|     {
 | |
|     public:
 | |
|         RemoveVisitor()
 | |
|             : osg::NodeVisitor(TRAVERSE_ALL_CHILDREN)
 | |
|         {
 | |
|         }
 | |
| 
 | |
|         void remove();
 | |
| 
 | |
|     protected:
 | |
|         // <node to remove, parent node to remove it from>
 | |
|         typedef std::vector<std::pair<osg::Node*, osg::Group*> > RemoveVec;
 | |
|         std::vector<std::pair<osg::Node*, osg::Group*> > mToRemove;
 | |
|     };
 | |
| 
 | |
|     // Removes all drawables from a graph.
 | |
|     class CleanObjectRootVisitor : public RemoveVisitor
 | |
|     {
 | |
|     public:
 | |
|         void apply(osg::Drawable& drw) override;
 | |
|         void apply(osg::Group& node) override;
 | |
|         void apply(osg::MatrixTransform& node) override;
 | |
|         void apply(osg::Node& node) override;
 | |
| 
 | |
|         void applyNode(osg::Node& node);
 | |
|         void applyDrawable(osg::Node& node);
 | |
|     };
 | |
| 
 | |
|     class RemoveTriBipVisitor : public RemoveVisitor
 | |
|     {
 | |
|     public:
 | |
|         void apply(osg::Drawable& drw) override;
 | |
|         void apply(osg::Group& node) override;
 | |
|         void apply(osg::MatrixTransform& node) override;
 | |
| 
 | |
|         void applyImpl(osg::Node& node);
 | |
|     };
 | |
| }
 | |
| 
 | |
| #endif
 |