mirror of
				https://github.com/OpenMW/openmw.git
				synced 2025-11-04 13:26:44 +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
 |