#5897: Fix mouse tool mapping loading code. It should not consider ma…

…ppings from other groups. Simplify the loading algorithm a bit.
codereader · Mar 1, 2022 · 91e75cb · 91e75cb
1 parent 81dd9d6
commit 91e75cb
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 34 deletions.
diff --git a/radiant/eventmanager/MouseToolManager.cpp b/radiant/eventmanager/MouseToolManager.cpp
@@ -6,13 +6,15 @@
 #include "itextstream.h"
 #include "ui/imainframe.h"
 
+#include "xmlutil/Node.h"
 #include "string/convert.h"
 #include "string/join.h"
 #include "wxutil/MouseButton.h"
 #include "wxutil/Modifier.h"
 #include "module/StaticModule.h"
 #include "ModifierHintPopup.h"
 
+#include <fmt/format.h>
 #include <wx/frame.h>
 
 namespace ui
@@ -33,6 +35,12 @@ namespace
             throw std::logic_error("Tool group name not resolvable: " + string::to_string(static_cast<int>(group)));
         }
     }
+
+    inline std::string getMappingPath(bool userMappings, MouseToolGroup::Type group)
+    {
+        return fmt::format("user/ui/input/mouseToolMappings[@name='{0}']//mouseToolMapping[@id={1}]//tool",
+            userMappings ? "user" : "default", static_cast<int>(group));
+    }
 }
 
 MouseToolManager::MouseToolManager() :
@@ -69,54 +77,43 @@ void MouseToolManager::initialiseModule(const IApplicationContext& ctx)
         sigc::mem_fun(this, &MouseToolManager::onMainFrameConstructed));
 }
 
-void MouseToolManager::loadGroupMapping(MouseToolGroup::Type type, const xml::NodeList& userMappings, const xml::NodeList& defaultMappings)
+void MouseToolManager::loadGroupMapping(MouseToolGroup::Type type)
 {
 	MouseToolGroup& group = getGroup(type);
 
+    // User-defined mapping take precedence, add them first
+    auto mappings = GlobalRegistry().findXPath(getMappingPath(true, type));
+
+    // Append the default mappings at the end of the list
+    auto defaultMappings = GlobalRegistry().findXPath(getMappingPath(false, type));
+    mappings.insert(mappings.end(), defaultMappings.begin(), defaultMappings.end());
+
 	group.clearToolMappings();
 
 	group.foreachMouseTool([&] (const MouseToolPtr& tool)
 	{
-		// First, look in the userMappings if we have a user-defined setting
-		for (const xml::Node& node : userMappings)
+		for (const auto& node : mappings)
 		{
-			if (node.getAttributeValue("name") == tool->getName())
-			{
-				// Load the condition
-				unsigned int state = wxutil::MouseButton::LoadFromNode(node) | wxutil::Modifier::LoadFromNode(node);
-				group.addToolMapping(state, tool);
-
-				return; // done here
-			}
-		}
+            if (node.getAttributeValue("name") != tool->getName())
+            {
+                continue;
+            }
 
-		// nothing found in the user mapping, fall back to default
-		for (const xml::Node& node : defaultMappings)
-		{
-			if (node.getAttributeValue("name") == tool->getName())
-			{
-				// Load the condition
-				unsigned int state = wxutil::MouseButton::LoadFromNode(node) | wxutil::Modifier::LoadFromNode(node);
-				group.addToolMapping(state, tool);
-
-				return; // done here
-			}
-		}
+			// Found it, load the modifier+button combination
+			auto state = wxutil::MouseButton::LoadFromNode(node) | wxutil::Modifier::LoadFromNode(node);
+			group.addToolMapping(state, tool);
 
-		// No mapping for this tool
+			break; // done here
+		}
 	});
 }
 
 void MouseToolManager::loadToolMappings()
 {
     // All modules have registered their stuff, now load the mapping
-    // Try the user-defined mapping first
-    xml::NodeList userMappings = GlobalRegistry().findXPath("user/ui/input/mouseToolMappings[@name='user']//mouseToolMapping//tool");
-	xml::NodeList defaultMappings = GlobalRegistry().findXPath("user/ui/input/mouseToolMappings[@name='default']//mouseToolMapping//tool");
-
-	loadGroupMapping(MouseToolGroup::Type::CameraView, userMappings, defaultMappings);
-	loadGroupMapping(MouseToolGroup::Type::OrthoView, userMappings, defaultMappings);
-	loadGroupMapping(MouseToolGroup::Type::TextureTool, userMappings, defaultMappings);
+    loadGroupMapping(MouseToolGroup::Type::CameraView);
+	loadGroupMapping(MouseToolGroup::Type::OrthoView);
+	loadGroupMapping(MouseToolGroup::Type::TextureTool);
 }
 
 void MouseToolManager::resetBindingsToDefault()

diff --git a/radiant/eventmanager/MouseToolManager.h b/radiant/eventmanager/MouseToolManager.h
@@ -3,7 +3,6 @@
 #include <map>
 #include "imousetool.h"
 #include "imousetoolmanager.h"
-#include "xmlutil/Node.h"
 #include "MouseToolGroup.h"
 #include <wx/event.h>
 #include <wx/timer.h>
@@ -62,7 +61,7 @@ class MouseToolManager :
     void onMainFrameConstructed();
 
     void loadToolMappings();
-	void loadGroupMapping(MouseToolGroup::Type type, const xml::NodeList& userMappings, const xml::NodeList& defaultMappings);
+	void loadGroupMapping(MouseToolGroup::Type type);
 
     void saveToolMappings();