Move filter event management to separate helper class

radiant/filters/BasicFilterSystem.cpp

@@ -111,7 +111,6 @@ void BasicFilterSystem::setObjectSelectionByFilter(const std::string& filterName
 	GlobalSceneGraph().root()->traverse(walker);
 }
 
-// Initialise the filter system
 void BasicFilterSystem::initialiseModule(const ApplicationContext& ctx)
 {
 	game::IGamePtr game = GlobalGameManager().currentGame();
@@ -201,13 +200,18 @@ void BasicFilterSystem::addFiltersFromXML(const xml::NodeList& nodes, bool readO
 			std::make_pair(filterName, filter)
 		).first->second;
 
+		bool filterShouldBeActive = activeFilterNames.find(filterName) != activeFilterNames.end();
+
+		auto adapter = ensureEventAdapter(inserted);
+
 		// Add the according toggle command to the eventmanager
-		auto fEvent = createEventToggle(inserted);
+		//auto fEvent = createEventToggle(inserted);
 
 		// If this filter is in our active set, enable it
-		if (activeFilterNames.find(filterName) != activeFilterNames.end())
+		if (filterShouldBeActive)
 		{
-			fEvent->setToggled(true);
+			//fEvent->setToggled(true);
+			adapter->setFilterState(true);
 			_activeFilters.insert(std::make_pair(filterName, inserted));
 		}
 
@@ -216,6 +220,21 @@ void BasicFilterSystem::addFiltersFromXML(const xml::NodeList& nodes, bool readO
 	}
 }
 
+XmlFilterEventAdapter::Ptr BasicFilterSystem::ensureEventAdapter(XMLFilter& filter)
+{
+	auto existing = _eventAdapters.find(filter.getName());
+
+	if (existing != _eventAdapters.end())
+	{
+		return existing->second;
+	}
+
+	auto result = _eventAdapters.emplace(std::make_pair(filter.getName(), 
+		std::make_shared<XmlFilterEventAdapter>(filter)));
+
+	return result.first->second;
+}
+
 // Shut down the Filters module, saving active filters to registry
 void BasicFilterSystem::shutdownModule() 
 {
@@ -270,6 +289,8 @@ void BasicFilterSystem::shutdownModule()
 			criterion.setAttributeValue("action", rule.show ? "show" : "hide");
 		}
 	}
+
+	_eventAdapters.clear();
 }
 
 sigc::signal<void> BasicFilterSystem::filtersChangedSignal() const
@@ -324,6 +345,12 @@ void BasicFilterSystem::setFilterState(const std::string& filter, bool state)
 		_activeFilters.erase(filter);
 	}
 
+	auto adapter = _eventAdapters.find(filter);
+	if (adapter != _eventAdapters.end())
+	{
+		adapter->second->setFilterState(state);
+	}
+
 	// Invalidate the visibility cache to force new values to be
 	// loaded from the filters themselves
 	_visibilityCache.clear();
@@ -341,11 +368,19 @@ void BasicFilterSystem::updateEvents()
 {
 	for (const auto& pair : _availableFilters)
 	{
+		auto adapter = _eventAdapters.find(pair.second.getName());
+
+		if (adapter != _eventAdapters.end())
+		{
+			adapter->second->setFilterState(getFilterState(pair.first));
+		}
+		/*
 		auto toggle = GlobalEventManager().findEvent(pair.second.getEventName());
 
 		if (!toggle) continue;
 
 		toggle->setToggled(getFilterState(pair.first));
+		*/
 	}
 }
 
@@ -373,8 +408,11 @@ bool BasicFilterSystem::addFilter(const std::string& filterName, const FilterRul
 	// Apply the ruleset
 	result.first->second.setRules(ruleSet);
 
+	// Create the event adapter
+	ensureEventAdapter(result.first->second);
+
 	// Add the according toggle command to the eventmanager
-	createEventToggle(result.first->second);
+	//createEventToggle(result.first->second);
 
 	// Clear the cache, the rules have changed
 	_visibilityCache.clear();
@@ -384,13 +422,15 @@ bool BasicFilterSystem::addFilter(const std::string& filterName, const FilterRul
 	return true;
 }
 
+#if 0
 IEventPtr BasicFilterSystem::createEventToggle(XMLFilter& filter)
 {
 	return GlobalEventManager().addToggle(
 		filter.getEventName(),
 		std::bind(&XMLFilter::toggle, &filter, std::placeholders::_1)
 	);
 }
+#endif
 
 bool BasicFilterSystem::removeFilter(const std::string& filter) 
 {
@@ -402,11 +442,15 @@ bool BasicFilterSystem::removeFilter(const std::string& filter)
 		return false;
 	}
 
+	_eventAdapters.erase(f->second.getName());
+
+#if 0
 	// Remove all accelerators from that event before removal
 	GlobalEventManager().disconnectAccelerator(f->second.getEventName());
 
 	// Disable the event in the EventManager, to avoid crashes when calling the menu items
 	GlobalEventManager().disableEvent(f->second.getEventName());
+#endif
 
 	// Check if the filter was active
 	auto found = _activeFilters.find(f->first);
@@ -459,17 +503,28 @@ bool BasicFilterSystem::renameFilter(const std::string& oldFilterName, const std
 
 	std::string oldEventName = f->second.getEventName();
 
+#if 0
 	auto oldEvent = GlobalEventManager().findEvent(oldEventName);
 
 	// Get the accelerator associated to the old event, if appropriate
 	IAccelerator& oldAccel = GlobalEventManager().findAccelerator(oldEvent);
+#endif
 
 	// Perform the actual rename procedure
 	f->second.setName(newFilterName);
 
+	// Find the adapter to update the event bindings
+	auto adapter = _eventAdapters.find(oldFilterName);
+
+	if (adapter != _eventAdapters.end())
+	{
+		adapter->second->onEventNameChanged(oldEventName, f->second.getEventName());
+	}
+
 	// Insert the new filter into the table
 	auto result = _availableFilters.insert(std::make_pair(newFilterName, f->second));
 
+#if 0
 	// Add the toggle command to the eventmanager
 	auto fEvent = createEventToggle(result.first->second);
 
@@ -481,25 +536,35 @@ bool BasicFilterSystem::renameFilter(const std::string& oldFilterName, const std
 	{
 		rWarning() << "Can't register event after rename, the new event name is already registered!" << std::endl;
 	}
+#endif
+
+	if (adapter != _eventAdapters.end())
+	{
+		adapter->second->setFilterState(wasActive);
+	}
 
 	// If this filter is in our active set, enable it
 	if (wasActive)
 	{
+#if 0
 		fEvent->setToggled(true);
-
+#endif
 		_activeFilters.insert(std::make_pair(newFilterName, f->second));
 	}
 	else 
 	{
+#if 0
 		fEvent->setToggled(false);
+#endif
 	}
 
 	// Remove the old filter from the filtertable
 	_availableFilters.erase(oldFilterName);
 
+#if 0
 	// Remove the old event from the EventManager
 	GlobalEventManager().removeEvent(oldEventName);
-
+#endif
 	return true;
 }
 

radiant/filters/BasicFilterSystem.h

@@ -1,6 +1,5 @@
 #pragma once
 
-#include "XMLFilter.h"
 #include "imodule.h"
 #include "ifilter.h"
 #include "icommandsystem.h"
@@ -12,6 +11,9 @@
 #include <string>
 #include <iostream>
 
+#include "XMLFilter.h"
+#include "XmlFilterEventAdapter.h"
+
 namespace filters
 {
 
@@ -38,6 +40,9 @@ class BasicFilterSystem :
 
     sigc::signal<void> _filtersChangedSignal;
 
+	typedef std::map<std::string, XmlFilterEventAdapter::Ptr> FilterAdapters;
+	FilterAdapters _eventAdapters;
+
 private:
 
 	// Perform a traversal of the scenegraph, setting or clearing the filtered
@@ -50,7 +55,8 @@ class BasicFilterSystem :
 
 	void addFiltersFromXML(const xml::NodeList& nodes, bool readOnly);
 
-	IEventPtr createEventToggle(XMLFilter& filter);
+	//IEventPtr createEventToggle(XMLFilter& filter);
+	XmlFilterEventAdapter::Ptr ensureEventAdapter(XMLFilter& filter);
 
 	void setAllFilterStatesCmd(const cmd::ArgumentList& args);
 

radiant/filters/XMLFilter.cpp

@@ -88,14 +88,13 @@ bool XMLFilter::isEntityVisible(const FilterRule::Type type, const Entity& entit
 	return visible;
 }
 
-// The command target
-void XMLFilter::toggle(bool newState)
-{
-	GlobalFilterSystem().setFilterState(_name, newState);
+const std::string& XMLFilter::getEventName() const {
+	return _eventName;
 }
 
-std::string XMLFilter::getEventName() const {
-	return _eventName;
+const std::string& XMLFilter::getName() const
+{
+	return _name;
 }
 
 void XMLFilter::setName(const std::string& newName) {

radiant/filters/XMLFilter.h

@@ -1,5 +1,4 @@
-#ifndef XMLFILTER_H_
-#define XMLFILTER_H_
+#pragma once
 
 #include <string>
 #include <vector>
@@ -92,19 +91,20 @@ class XMLFilter
 	 */
 	bool isEntityVisible(const FilterRule::Type type, const Entity& entity) const;
 
-	/** greebo: Returns the name of the toggle event associated to this filter
+	/** greebo: Returns the name of the toggle event associated to this filter.
+	* It's lacking any spaces or other incompatible characters, compared to the actual
+	* name returned in getName().
 	 */
-	std::string getEventName() const;
+	const std::string& getEventName() const;
+
+	// The name of this Filter
+	const std::string& getName() const;
 
 	/**
 	 * greebo: Renames the filter to <newName>. This also updates the event name.
 	 */
 	void setName(const std::string& newName);
 
-	/** greebo: Gets called when the associated Event is fired.
-	 */
-	void toggle(bool newState);
-
 	// Whether this filter is read-only
 	bool isReadOnly() const;
 
@@ -118,7 +118,4 @@ class XMLFilter
 	void updateEventName();
 };
 
-
 }
-
-#endif /*XMLFILTER_H_*/

radiant/filters/XmlFilterEventAdapter.h

@@ -0,0 +1,91 @@
+#pragma once
+
+#include "ieventmanager.h"
+#include "itextstream.h"
+#include "ifilter.h"
+
+#include "XMLFilter.h"
+
+namespace filters
+{
+
+/**
+ * An object responsible for managing the commands and events 
+ * bound to a single XMLFilter object.
+ */
+class XmlFilterEventAdapter
+{
+private:
+    XMLFilter& _filter;
+
+    IEventPtr _toggle;
+
+public:
+    typedef std::shared_ptr<XmlFilterEventAdapter> Ptr;
+
+    XmlFilterEventAdapter(XMLFilter& filter) :
+        _filter(filter)
+    {
+        // Add the corresponding toggle command to the eventmanager
+        _toggle = createEventToggle();
+    }
+
+    ~XmlFilterEventAdapter()
+    {
+        // Remove all accelerators from that event before removal
+        GlobalEventManager().disconnectAccelerator(_filter.getEventName());
+
+        // Disable the event in the EventManager, to avoid crashes when calling the menu items
+        GlobalEventManager().disableEvent(_filter.getEventName());
+    }
+
+    // Synchronisation routine to notify this class once the filter has been activated
+    void setFilterState(bool isActive)
+    {
+        if (_toggle)
+        {
+            _toggle->setToggled(isActive);
+        }
+    }
+
+    // Post-filter-rename event, to be invoked by the FilterSystem after a rename operation
+    void onEventNameChanged(const std::string& oldEventName, const std::string& newEventName)
+    {
+        auto oldEvent = GlobalEventManager().findEvent(oldEventName);
+
+        // Get the accelerator associated to the old event, if appropriate
+        IAccelerator& oldAccel = GlobalEventManager().findAccelerator(oldEvent);
+
+        // Add the toggle command to the eventmanager
+        auto newToggle = createEventToggle();
+
+        if (!newToggle->empty())
+        {
+            GlobalEventManager().connectAccelerator(oldAccel, newEventName);
+        }
+        else
+        {
+            rWarning() << "Can't register event after rename, the new event name is already registered!" << std::endl;
+        }
+
+        // Remove the old event from the EventManager
+        GlobalEventManager().removeEvent(oldEventName);
+    }
+
+private:
+    // The command target
+    void toggle(bool newState)
+    {
+        GlobalFilterSystem().setFilterState(_filter.getName(), newState);
+    }
+
+    IEventPtr createEventToggle()
+    {
+        return GlobalEventManager().addToggle(
+            _filter.getEventName(),
+            std::bind(&XmlFilterEventAdapter::toggle, this, std::placeholders::_1)
+        );
+    }
+};
+
+}