#5180: Move most of the toolbar logic to the ToolbarManager.

include/ieventmanager.h

@@ -164,15 +164,13 @@ class IEventManager :
 	// Disconnects the given command from any accelerators
 	virtual void disconnectAccelerator(const std::string& command) = 0;
 
-	// Before destruction, it's advisable to disconnect any events from a toolbar's items
-	virtual void disconnectToolbar(wxToolBar* toolbar) = 0;
-
 	// Register the given menu item with the given command. The event manager updates this item
 	// when the accelerator association changes
 	virtual void registerMenuItem(const std::string& eventName, const ui::IMenuElementPtr& item) = 0;
 	virtual void unregisterMenuItem(const std::string& eventName, const ui::IMenuElementPtr& item) = 0;
 
 	virtual void registerToolItem(const std::string& eventName, wxToolBarToolBase* item) = 0;
+	virtual void unregisterToolItem(const std::string& eventName, wxToolBarToolBase* item) = 0;
 
 	// Loads the shortcut->command associations from the XMLRegistry
 	virtual void loadAccelerators() = 0;

radiant/eventmanager/EventManager.cpp

@@ -391,6 +391,34 @@ void EventManager::registerToolItem(const std::string& eventName, wxToolBarToolB
 	}
 }
 
+void EventManager::unregisterToolItem(const std::string& eventName, wxToolBarToolBase* item)
+{
+	for (auto it = _toolItems.begin(); it != _toolItems.end(); ++it)
+	{
+		if (it->second == item)
+		{
+			auto evt = findEvent(it->first);
+
+			if (!evt->empty())
+			{
+				evt->disconnectToolItem(item);
+			}
+			else
+			{
+				item->GetToolBar()->Unbind(wxEVT_TOOL, &EventManager::onToolItemClicked, this, item->GetId());
+			}
+
+			_toolItems.erase(it);
+			break;
+		}
+	}
+
+	for (EventMap::value_type& pair : _events)
+	{
+		pair.second->disconnectToolItem(item);
+	}
+}
+
 void EventManager::onToolItemClicked(wxCommandEvent& ev)
 {
 	for (const auto & pair : _toolItems)
@@ -548,42 +576,6 @@ void EventManager::removeEvent(const std::string& eventName)
 	}
 }
 
-void EventManager::disconnectToolbar(wxToolBar* toolbar)
-{
-	for (std::size_t tool = 0; tool < toolbar->GetToolsCount(); tool++)
-	{
-		for (auto it = _toolItems.begin(); it != _toolItems.end(); ++it)
-		{
-			auto toolItem = toolbar->GetToolByPos(tool);
-
-			if (it->second == toolItem)
-			{
-				auto evt = findEvent(it->first);
-
-				if (!evt->empty())
-				{
-					evt->disconnectToolItem(toolItem);
-				}
-				else
-				{
-					toolbar->Unbind(wxEVT_TOOL, &EventManager::onToolItemClicked, this, toolItem->GetId());
-				}
-
-				_toolItems.erase(it);
-				break;
-			}
-		}
-	}
-
-	for (EventMap::value_type& pair : _events)
-	{
-		for (std::size_t tool = 0; tool < toolbar->GetToolsCount(); tool++)
-		{
-			pair.second->disconnectToolItem(const_cast<wxToolBarToolBase*>(toolbar->GetToolByPos(static_cast<int>(tool))));
-		}
-	}
-}
-
 // Loads the default shortcuts from the registry
 void EventManager::loadAccelerators()
 {

radiant/eventmanager/EventManager.h

@@ -79,6 +79,7 @@ class EventManager :
 	void unregisterMenuItem(const std::string& eventName, const ui::IMenuElementPtr& item) override;
 
 	void registerToolItem(const std::string& eventName, wxToolBarToolBase* item) override;
+	void unregisterToolItem(const std::string& eventName, wxToolBarToolBase* item) override;
 
 	// Connects the given accelerator to the given command (identified by the string)
 	void connectAccelerator(wxKeyEvent& keyEvent, const std::string& command) override;
@@ -90,8 +91,6 @@ class EventManager :
 	void renameEvent(const std::string& oldEventName, const std::string& newEventName) override;
 	void removeEvent(const std::string& eventName) override;
 
-	void disconnectToolbar(wxToolBar* toolbar) override;
-
 	// Loads the default shortcuts from the registry
 	void loadAccelerators() override;
 

radiant/uimanager/ToolbarManager.cpp

@@ -12,6 +12,11 @@
 namespace ui
 {
 
+namespace
+{
+	const char* const CMD_VARIANT_NAME = "command";
+}
+
 int ToolbarManager::_nextToolItemId = 100;
 
 void ToolbarManager::initialise()
@@ -55,11 +60,11 @@ wxToolBar* ToolbarManager::getToolbar(const std::string& toolbarName, wxWindow*
 	}
 }
 
-wxToolBarToolBase* ToolbarManager::createToolItem(wxToolBar* toolbar, xml::Node& node)
+wxToolBarToolBase* ToolbarManager::createToolItem(wxToolBar* toolbar, const xml::Node& node)
 {
 	const std::string nodeName = node.getName();
 
-	wxToolBarToolBase* toolItem = NULL;
+	wxToolBarToolBase* toolItem = nullptr;
 
 	if (nodeName == "separator")
 	{
@@ -95,6 +100,8 @@ wxToolBarToolBase* ToolbarManager::createToolItem(wxToolBar* toolbar, xml::Node&
 				tooltip, wxITEM_CHECK);
 		}
 
+		toolItem->SetClientData(new wxVariant(action, CMD_VARIANT_NAME));
+
 		GlobalEventManager().registerToolItem(action, toolItem);
 #if 0
 		IEventPtr ev = GlobalEventManager().findEvent(action);
@@ -136,10 +143,10 @@ wxToolBar* ToolbarManager::createToolbar(xml::Node& node, wxWindow* parent)
         // this will not resize the actual icons, just the buttons
         toolbar->SetToolBitmapSize(wxSize(20, 20));
 
-		for (std::size_t i = 0; i < toolItemList.size(); ++i)
+		for (const auto& toolNode : toolItemList)
 		{
 			// Create and get the toolItem with the parsing
-			createToolItem(toolbar, toolItemList[i]);
+			createToolItem(toolbar, toolNode);
 		}
 
 		toolbar->Realize();
@@ -158,9 +165,25 @@ void ToolbarManager::onToolbarDestroy(wxWindowDestroyEvent& ev)
 {
 	auto toolbar = wxDynamicCast(ev.GetEventObject(), wxToolBar);
 
-	if (toolbar != nullptr)
+	if (toolbar == nullptr)
+	{
+		return;
+	}
+
+	for (std::size_t tool = 0; tool < toolbar->GetToolsCount(); tool++)
 	{
-		GlobalEventManager().disconnectToolbar(toolbar);
+		auto toolItem = toolbar->GetToolByPos(tool);
+
+		auto cmdData = wxDynamicCast(toolItem->GetClientData(), wxVariant);
+
+		if (cmdData != nullptr && cmdData->GetName() == CMD_VARIANT_NAME)
+		{
+			GlobalEventManager().unregisterToolItem(cmdData->GetString().ToStdString(), toolItem);
+
+			// free the client data, the toolbar item doesn't delete it
+			toolItem->SetClientData(nullptr);
+			delete cmdData;
+		}
 	}
 }
 

radiant/uimanager/ToolbarManager.h

@@ -51,7 +51,7 @@ class ToolbarManager :
 	 * Checks the passed xmlNode for a recognized item (ToolButton, ToggleToolButton, Separator)
 	 * Returns the widget or NULL if nothing useful is found.
 	 */
-	wxToolBarToolBase* createToolItem(wxToolBar* toolbar, xml::Node& node);
+	wxToolBarToolBase* createToolItem(wxToolBar* toolbar, const xml::Node& node);
 
 	bool toolbarExists(const std::string& toolbarName);