[xswiftbus] Fix destroying of removed menu items

src/xswiftbus/menus.cpp

@@ -13,6 +13,7 @@
 #include <type_traits>
 #include <cassert>
 #include <string>
+#include <algorithm>
 
 namespace XSwiftBus
 {
@@ -53,20 +54,40 @@ namespace XSwiftBus
     {
         assert(! name.empty());
         m_data->items->emplace_back(
-            CMenuItem { m_data->id, XPLMAppendMenuItem(m_data->id, name.c_str(), voidptr_cast(m_data->items->size() + 1), false), false, false },
+        CMenuItem { m_data->id, 0, false, false, [callback](bool) { callback(); } }
-            [callback](bool){ callback(); }
         );
-        return m_data->items->back().first;
+        auto &menuItem = m_data->items->back();
+        menuItem.setIndex(XPLMAppendMenuItem(m_data->id, name.c_str(), &menuItem, false));
+        return menuItem;
     }
 
     CMenuItem CMenu::checkableItem(const std::string &name, bool checked, std::function<void(bool)> callback)
     {
-        assert(! name.empty());
+        assert(!name.empty());
         m_data->items->emplace_back(
-            CMenuItem { m_data->id, XPLMAppendMenuItem(m_data->id, name.c_str(), voidptr_cast(m_data->items->size() + 1), false), true, checked },
+            CMenuItem{ m_data->id, 0, true, checked, callback }
-            callback
         );
-        return m_data->items->back().first;
+        auto &menuItem = m_data->items->back();
+        menuItem.setIndex(XPLMAppendMenuItem(m_data->id, name.c_str(), voidptr_cast(m_data->items->size() + 1), false));
+        return menuItem;
+    }
+
+    void CMenu::removeItem(const CMenuItem &item)
+    {
+        auto it = std::find_if(m_data->items->begin(), m_data->items->end(), [ = ](const auto & i)
+        {
+            return i.m_data->index == item.m_data->index;
+        });
+
+        XPLMRemoveMenuItem(m_data->id, it->m_data->index);
+        it = m_data->items->erase(it);
+
+        // Decrement the index of all below menu items
+        while (it != m_data->items->end())
+        {
+            it->m_data->index--;
+            ++it;
+        }
     }
 
     void CMenu::sep()
@@ -77,6 +98,7 @@ namespace XSwiftBus
     CMenu CMenu::subMenu(const std::string &name)
     {
         assert(! name.empty());
+        // auto items = std::make_unique<ItemList>();
         auto items = std::make_unique<ItemList>();
         auto itemsVoidPtr = static_cast<void *>(&*items);
         return { XPLMCreateMenu(name.c_str(), m_data->id, XPLMAppendMenuItem(m_data->id, name.c_str(), nullptr, false), handler, itemsVoidPtr), false, std::move(items) };
@@ -86,16 +108,13 @@ namespace XSwiftBus
     {
         if (menuRef && itemRef)
         {
-            auto items = static_cast<ItemList *>(menuRef);
+            CMenuItem *menuItem = static_cast<CMenuItem *>(itemRef);
-            auto itemIdx = intptr_cast<intptr_t>(itemRef) - 1;
+            menuItem->m_data->callback(menuItem->getChecked());
-            assert(itemIdx >= 0);
-
-            (*items)[itemIdx].second((*items)[itemIdx].first.getChecked());
         }
     }
 
-    CMenuItem::CMenuItem(XPLMMenuID parent, int item, bool checkable, bool checked)
+    CMenuItem::CMenuItem(XPLMMenuID parent, int item, bool checkable, bool checked, std::function<void(bool)> callback)
-        : m_data(std::make_shared<Data>(parent, item, checkable))
+        : m_data(std::make_shared<Data>(parent, item, checkable, callback))
     {
         if (checkable)
         {
@@ -106,18 +125,18 @@ namespace XSwiftBus
     bool CMenuItem::getChecked() const
     {
         XPLMMenuCheck check = xplm_Menu_NoCheck;
-        XPLMCheckMenuItemState(m_data->parent, m_data->item, &check);
+        XPLMCheckMenuItemState(m_data->parent, m_data->index, &check);
         return check == xplm_Menu_Checked;
     }
 
     void CMenuItem::setChecked(bool checked)
     {
-        XPLMCheckMenuItem(m_data->parent, m_data->item, checked);
+        XPLMCheckMenuItem(m_data->parent, m_data->index, checked);
     }
 
     void CMenuItem::setEnabled(bool enabled)
     {
-        XPLMEnableMenuItem(m_data->parent, m_data->item, enabled);
+        XPLMEnableMenuItem(m_data->parent, m_data->index, enabled);
     }
 
 }

src/xswiftbus/menus.h

@@ -14,7 +14,7 @@
 
 #include <XPLM/XPLMMenus.h>
 #include <functional>
-#include <vector>
+#include <list>
 #include <memory>
 
 namespace XSwiftBus
@@ -43,14 +43,17 @@ namespace XSwiftBus
     private:
         friend class CMenu;
 
-        CMenuItem(XPLMMenuID parent, int item, bool checkable, bool checked);
+        CMenuItem(XPLMMenuID parent, int item, bool checkable, bool checked, std::function<void(bool)> callback);
+
+        void setIndex(int index) { m_data->index = index; }
 
         struct Data
         {
-            Data(XPLMMenuID parent_, int item_, bool checkable_) : parent(parent_), item(item_), checkable(checkable_) {}
+            Data(XPLMMenuID parent_, int index_, bool checkable_, std::function<void(bool)> callback_) : parent(parent_), index(index_), checkable(checkable_), callback(callback_) {}
             XPLMMenuID parent;
-            int item;
+            int index;
             bool checkable;
+            std::function<void(bool)> callback;
         };
         std::shared_ptr<Data> m_data;
     };
@@ -73,6 +76,9 @@ namespace XSwiftBus
         //! Appends a checkbox item to the menu and returns it.
         CMenuItem checkableItem(const std::string &name, bool checked, std::function<void(bool)> callback);
 
+        //! Removes item from the menu
+        void removeItem(const CMenuItem &item);
+
         //! Appends a separator to the menu.
         void sep();
 
@@ -80,9 +86,11 @@ namespace XSwiftBus
         CMenu subMenu(const std::string &name);
 
     private:
-        typedef std::vector<std::pair<CMenuItem, std::function<void(bool)>>> ItemList;
+        // Using std::list, since it does not invalidate pointers.
+        using ItemList = std::list<CMenuItem>;
 
-        CMenu(XPLMMenuID id, bool isMainMenu, std::unique_ptr<ItemList> callbacks);
+        // CMenu(XPLMMenuID id, bool isMainMenu, std::unique_ptr<ItemList> callbacks);
+        CMenu(XPLMMenuID id, bool isMainMenu, std::unique_ptr<ItemList> items);
 
         static void handler(void *menuRef, void *itemRef);
 
@@ -93,7 +101,6 @@ namespace XSwiftBus
             XPLMMenuID id;
             bool isMainMenu;
             std::unique_ptr<ItemList> items;
-            std::vector<CMenu> subMenus;
             ~Data();
             Data(const Data &) = delete;
             Data &operator =(const Data &) = delete;

src/xswiftbus/traffic.cpp

@@ -233,7 +233,12 @@ namespace XSwiftBus
 
     void CTraffic::removePlane(const std::string &callsign)
     {
-        m_planeViewMenuItems.erase(callsign);
+        auto menuItemIt = m_planeViewMenuItems.find(callsign);
+        if (menuItemIt != m_planeViewMenuItems.end())
+        {
+            m_planeViewSubMenu.removeItem(menuItemIt->second);
+            m_planeViewMenuItems.erase(menuItemIt);
+        }
 
         auto planeIt = m_planesByCallsign.find(callsign);
         if (planeIt == m_planesByCallsign.end()) { return; }
@@ -254,6 +259,13 @@ namespace XSwiftBus
             XPMPDestroyPlane(plane->id);
             delete plane;
         }
+
+        for (const auto &kv : m_planeViewMenuItems)
+        {
+            CMenuItem item = kv.second;
+            m_planeViewSubMenu.removeItem(item);
+        }
+
         m_planesByCallsign.clear();
         m_planesById.clear();
         m_planeViewMenuItems.clear();