Unify the UI IDs for actions between the menus and toolbars

By unifying the IDs so that an action only has one UI ID,
it will receive only one update event that will work for
all controls using the action.

common/tool/action_menu.cpp

@@ -162,17 +162,17 @@ wxMenuItem* ACTION_MENU::Add( const wxString& aLabel, const wxString& aTooltip,
 
 wxMenuItem* ACTION_MENU::Add( const TOOL_ACTION& aAction, bool aIsCheckmarkEntry )
 {
-    /// ID numbers for tool actions need to have a value higher than ACTION_ID
+    /// ID numbers for tool actions are assigned above ACTION_BASE_UI_ID inside TOOL_EVENT
     const BITMAP_OPAQUE* icon = aAction.GetIcon();
 
-    wxMenuItem* item = new wxMenuItem( this, getMenuId( aAction ), aAction.GetMenuItem(),
+    wxMenuItem* item = new wxMenuItem( this, aAction.GetUIId(), aAction.GetMenuItem(),
                                        aAction.GetDescription(),
                                        aIsCheckmarkEntry ? wxITEM_CHECK : wxITEM_NORMAL );
 
     if( icon )
         AddBitmapToMenuItem( item, KiBitmap( icon ) );
 
-    m_toolActions[getMenuId( aAction )] = &aAction;
+    m_toolActions[aAction.GetUIId()] = &aAction;
 
     return Append( item );
 }
@@ -436,7 +436,7 @@ void ACTION_MENU::OnMenuEvent( wxMenuEvent& aEvent )
         }
 
         // Check if there is a TOOL_ACTION for the given ID
-        if( m_selected >= ACTION_ID )
+        if( m_selected >= TOOL_ACTION::GetBaseUIId() )
             evt = findToolAction( m_selected );
 
         if( !evt )

common/tool/action_toolbar.cpp

@@ -60,7 +60,7 @@ ACTION_TOOLBAR::~ACTION_TOOLBAR()
 void ACTION_TOOLBAR::Add( const TOOL_ACTION& aAction, bool aIsToggleEntry )
 {
     wxWindow* parent = dynamic_cast<wxWindow*>( m_toolManager->GetToolHolder() );
-    int       toolId = aAction.GetId() + ACTION_ID;
+    int       toolId = aAction.GetUIId();
 
     AddTool( toolId, wxEmptyString, KiScaledBitmap( aAction.GetIcon(), parent ),
              aAction.GetDescription(), aIsToggleEntry ? wxITEM_CHECK : wxITEM_NORMAL );
@@ -73,7 +73,7 @@ void ACTION_TOOLBAR::Add( const TOOL_ACTION& aAction, bool aIsToggleEntry )
 void ACTION_TOOLBAR::AddButton( const TOOL_ACTION& aAction )
 {
     wxWindow* parent = dynamic_cast<wxWindow*>( m_toolManager->GetToolHolder() );
-    int       toolId = aAction.GetId() + ACTION_ID;
+    int       toolId = aAction.GetUIId();
 
     AddTool( toolId, wxEmptyString, KiScaledBitmap( aAction.GetIcon(), parent ),
              aAction.GetName(), wxITEM_NORMAL );
@@ -102,7 +102,7 @@ void ACTION_TOOLBAR::AddScaledSeparator( wxWindow* aWindow )
 
 void ACTION_TOOLBAR::AddToolContextMenu( const TOOL_ACTION& aAction, CONDITIONAL_MENU* aMenu )
 {
-    int toolId = aAction.GetId() + ACTION_ID;
+    int toolId = aAction.GetUIId();
 
     // If this is replacing an existing menu, delete the existing menu before adding the new one
     const auto it = m_toolMenus.find( toolId );
@@ -134,7 +134,7 @@ void ACTION_TOOLBAR::ClearToolbar()
 
 void ACTION_TOOLBAR::SetToolBitmap( const TOOL_ACTION& aAction, const wxBitmap& aBitmap )
 {
-    int toolId = aAction.GetId() + ACTION_ID;
+    int toolId = aAction.GetUIId();
     wxAuiToolBar::SetToolBitmap( toolId, aBitmap );
 
     // Set the disabled bitmap: we use the disabled bitmap version
@@ -148,7 +148,7 @@ void ACTION_TOOLBAR::SetToolBitmap( const TOOL_ACTION& aAction, const wxBitmap&
 
 void ACTION_TOOLBAR::Toggle( const TOOL_ACTION& aAction, bool aState )
 {
-    int toolId = aAction.GetId() + ACTION_ID;
+    int toolId = aAction.GetUIId();
 
     if( m_toolKinds[ toolId ] )
         ToggleTool( toolId, aState );
@@ -159,7 +159,7 @@ void ACTION_TOOLBAR::Toggle( const TOOL_ACTION& aAction, bool aState )
 
 void ACTION_TOOLBAR::Toggle( const TOOL_ACTION& aAction, bool aEnabled, bool aChecked )
 {
-    int toolId = aAction.GetId() + ACTION_ID;
+    int toolId = aAction.GetUIId();
 
     EnableTool( toolId, aEnabled );
     ToggleTool( toolId, aEnabled && aChecked );
@@ -173,7 +173,7 @@ void ACTION_TOOLBAR::onToolEvent( wxAuiToolBarEvent& aEvent )
 
     wxEventType type = aEvent.GetEventType();
 
-    if( type == wxEVT_COMMAND_TOOL_CLICKED && aEvent.GetId() >= ACTION_ID )
+    if( type == wxEVT_COMMAND_TOOL_CLICKED && aEvent.GetId() >= TOOL_ACTION::GetBaseUIId() )
     {
         const auto it = m_toolActions.find( aEvent.GetId() );
 

include/tool/action_menu.h

@@ -37,8 +37,6 @@
 class TOOL_INTERACTIVE;
 
 /**
- * ACTION_MENU
- *
  * Defines the structure of a menu based on ACTIONs.
  */
 class ACTION_MENU : public wxMenu
@@ -53,28 +51,26 @@ public:
     ACTION_MENU& operator=( const ACTION_MENU& aMenu ) = delete;
 
     /**
-     * Function SetTitle()
      * Sets title for the menu. The title is shown as a text label shown on the top of
      * the menu.
+     *
      * @param aTitle is the new title.
      */
     void SetTitle( const wxString& aTitle ) override;
 
     /**
-     * Function DisplayTitle()
      * Decides whether a title for a pop up menu should be displayed.
      */
     void DisplayTitle( bool aDisplay = true );
 
     /**
-     * Function SetIcon()
      * Assigns an icon for the entry.
+     *
      * @param aIcon is the icon to be assigned. NULL is used to remove icon.
      */
     void SetIcon( const BITMAP_OPAQUE* aIcon );
 
     /**
-     * Function Add()
      * Adds a wxWidgets-style entry to the menu. After highlighting/selecting the entry,
      * a wxWidgets event is generated.
      */
@@ -83,39 +79,36 @@ public:
                      const BITMAP_OPAQUE* aIcon );
 
     /**
-     * Function Add()
      * Adds an entry to the menu, basing on the TOOL_ACTION object. After selecting the entry,
      * a TOOL_EVENT command containing name of the action is sent.
+     *
      * @param aAction is the action to be added to menu entry.
      */
     wxMenuItem* Add( const TOOL_ACTION& aAction, bool aIsCheckmarkEntry = false );
 
     /**
-     * Function Add()
      * Adds an action menu as a submenu. The difference between this function and
      * wxMenu::AppendSubMenu() is the capability to handle icons.
+     *
      * @param aMenu is the submenu to be added.
      */
     wxMenuItem* Add( ACTION_MENU* aMenu );
 
     /**
-     * Function Clear()
      * Removes all the entries from the menu (as well as its title). It leaves the menu in the
      * initial state.
      */
     void Clear();
 
     /**
-     * Function HasEnabledItems();
-     *
      * Returns true if the menu has any enabled items
      */
     bool HasEnabledItems() const;
 
     /**
-     * Function GetSelected()
      * Returns the position of selected item. If the returned value is negative, that means that
      * menu was dismissed.
+     *
      * @return The position of selected item in the action menu.
      */
     inline int GetSelected() const
@@ -124,21 +117,19 @@ public:
     }
 
     /**
-     * Function UpdateAll()
      * Runs update handlers for the menu and its submenus.
      */
     void UpdateAll();
 
     /**
-     * Function ClearDirty()
      * Clears the dirty flag on the menu and all descendants.
      */
     void ClearDirty();
     void SetDirty();
 
     /**
-     * Function SetTool()
      * Sets a tool that is the creator of the menu.
+     *
      * @param aTool is the tool that created the menu.
      */
     void SetTool( TOOL_INTERACTIVE* aTool );
@@ -158,12 +149,6 @@ protected:
     ///> Returns an instance of TOOL_MANAGER class.
     TOOL_MANAGER* getToolManager() const;
 
-    ///> Returns the corresponding wxMenuItem identifier for a TOOL_ACTION object.
-    static inline int getMenuId( const TOOL_ACTION& aAction )
-    {
-        return aAction.GetId() + ACTION_ID;
-    }
-
     /**
      * Update menu state stub. It is called before a menu is shown, in order to update its state.
      * Here you can tick current settings, enable/disable entries, etc.
@@ -230,9 +215,6 @@ protected:
     ///> Creator of the menu
     TOOL_INTERACTIVE* m_tool;
 
-    ///> Menu items with ID higher than that are considered TOOL_ACTIONs
-    static const int ACTION_ID = 20000;
-
     ///> Associates tool actions with menu item IDs. Non-owning.
     std::map<int, const TOOL_ACTION*> m_toolActions;
 

include/tool/action_toolbar.h

@@ -49,15 +49,14 @@ public:
     virtual ~ACTION_TOOLBAR();
 
     /**
-     * Function Add()
      * Adds a TOOL_ACTION-based button to the toolbar. After selecting the entry,
      * a TOOL_EVENT command containing name of the action is sent.
      */
     void Add( const TOOL_ACTION& aAction, bool aIsToggleEntry = false );
 
     /**
-     * Function AddButton()
      * Adds a large button such as used in the Kicad Manager Frame's launch bar.
+     *
      * @param aAction
      */
     void AddButton( const TOOL_ACTION& aAction );
@@ -86,7 +85,6 @@ public:
     void ClearToolbar();
 
     /**
-     * Function SetToolBitmap()
      * Updates the bitmap of a particular tool.  Not icon-based because we use it
      * for the custom-drawn layer pair bitmap.
      */
@@ -110,9 +108,6 @@ protected:
     void onToolRightClick( wxAuiToolBarEvent& aEvent );
 
 protected:
-    ///> Tool items with ID higher than that are considered TOOL_ACTIONs
-    static const int ACTION_ID = 10000;
-
     TOOL_MANAGER* m_toolManager;
     std::map<int, bool>               m_toolKinds;
     std::map<int, const TOOL_ACTION*> m_toolActions;

include/tool/tool_action.h

@@ -34,8 +34,6 @@
 struct BITMAP_OPAQUE;
 
 /**
- * TOOL_ACTION
- *
  * Represents a single user action. For instance:
  * - changing layer to top by pressing PgUp
  * - running the DRC from the menu
@@ -47,9 +45,9 @@ class TOOL_ACTION
 {
 public:
     TOOL_ACTION( const std::string& aName, TOOL_ACTION_SCOPE aScope = AS_CONTEXT,
-                 int aDefaultHotKey = 0, const std::string& aLegacyHotKeyName = "", 
+                 int aDefaultHotKey = 0, const std::string& aLegacyHotKeyName = "",
-                 const wxString& aMenuText = wxEmptyString, const wxString& aTooltip = wxEmptyString, 
+                 const wxString& aMenuText = wxEmptyString, const wxString& aTooltip = wxEmptyString,
-                 const BITMAP_OPAQUE* aIcon = nullptr, TOOL_ACTION_FLAGS aFlags = AF_NONE, 
+                 const BITMAP_OPAQUE* aIcon = nullptr, TOOL_ACTION_FLAGS aFlags = AF_NONE,
                  void* aParam = nullptr );
 
     ~TOOL_ACTION();
@@ -69,29 +67,25 @@ public:
     }
 
     /**
-     * Function GetName()
      * Returns name of the action. It is the same one that is contained in TOOL_EVENT that is
      * sent by activating the TOOL_ACTION.  Convention is "app.tool.actionName".
      *
      * @return Name of the action.
      */
     const std::string& GetName() const { return m_name; }
-    
+
     /**
-     * Function GetDefaultHotKey()
      * Returns the default hotkey (if any) for the action.
      */
     int GetDefaultHotKey() const { return m_defaultHotKey; }
-    
+
     /**
-     * Function GetHotKey()
      * Returns the hotkey keycode which initiates the action.
      */
     int GetHotKey() const { return m_hotKey; }
     void SetHotKey( int aKeycode );
-    
+
     /**
-     * Function GetId()
      * Returns the unique id of the TOOL_ACTION object. It is valid only after registering the
      * TOOL_ACTION by ACTION_MANAGER.
      *
@@ -99,8 +93,20 @@ public:
      */
     int GetId() const { return m_id; }
 
+    /*
+     * Get the unique ID for this action in the user interface system. This is simply
+     * the action ID offset by @c ACTION_BASE_UI_ID.
+     *
+     * @return The unique ID number for use in the user interface system.
+     */
+    int GetUIId() const { return m_id + ACTION_BASE_UI_ID; }
+
+    /*
+     * Get the base value used to offset the user interface IDs for the actions.
+     */
+    static int GetBaseUIId() { return ACTION_BASE_UI_ID; }
+
     /**
-     * Function MakeEvent()
      * Returns the event associated with the action (i.e. the event that will be sent after
      * activating the action).
      */
@@ -156,11 +162,14 @@ protected:
 
     friend class ACTION_MANAGER;
 
+    ///> Base ID to use inside the user interface system to offset the action IDs.
+    static constexpr int ACTION_BASE_UI_ID = 20000;
+
     /// Name of the action (convention is "app.tool.actionName")
     std::string          m_name;
     TOOL_ACTION_SCOPE    m_scope;
 
-    const int            m_defaultHotKey;  // Default hot key  
+    const int            m_defaultHotKey;  // Default hot key
     int                  m_hotKey;         // The curret hotkey (post-user-settings-application)
     const std::string    m_legacyName;     // Name for reading legacy hotkey settings
 

pcbnew/tools/pcb_editor_control.cpp

@@ -105,15 +105,15 @@ private:
                                           SELECTION_CONDITIONS::OnlyTypes( GENERAL_COLLECTOR::Zones )
                                         )( selTool->GetSelection() );
 
-        Enable( getMenuId( PCB_ACTIONS::zoneDuplicate ), singleZoneActionsEnabled );
+        Enable( PCB_ACTIONS::zoneDuplicate.GetUIId(),   singleZoneActionsEnabled );
-        Enable( getMenuId( PCB_ACTIONS::drawZoneCutout ), singleZoneActionsEnabled );
+        Enable( PCB_ACTIONS::drawZoneCutout.GetUIId(),  singleZoneActionsEnabled );
-        Enable( getMenuId( PCB_ACTIONS::drawSimilarZone ), singleZoneActionsEnabled );
+        Enable( PCB_ACTIONS::drawSimilarZone.GetUIId(), singleZoneActionsEnabled );
 
         // enable zone actions that ably to a specific set of zones (as opposed to all of them)
         bool nonGlobalActionsEnabled = ( SELECTION_CONDITIONS::MoreThan( 0 ) )( selTool->GetSelection() );
 
-        Enable( getMenuId( PCB_ACTIONS::zoneFill ), nonGlobalActionsEnabled );
+        Enable( PCB_ACTIONS::zoneFill.GetUIId(),   nonGlobalActionsEnabled );
-        Enable( getMenuId( PCB_ACTIONS::zoneUnfill ), nonGlobalActionsEnabled );
+        Enable( PCB_ACTIONS::zoneUnfill.GetUIId(), nonGlobalActionsEnabled );
 
         // lines like this make me really think about a better name for SELECTION_CONDITIONS class
         bool mergeEnabled = ( SELECTION_CONDITIONS::MoreThan( 1 ) &&
@@ -121,7 +121,7 @@ private:
                               PCB_SELECTION_CONDITIONS::SameNet( true ) &&
                               PCB_SELECTION_CONDITIONS::SameLayer() )( selTool->GetSelection() );
 
-        Enable( getMenuId( PCB_ACTIONS::zoneMerge ), mergeEnabled );
+        Enable( PCB_ACTIONS::zoneMerge.GetUIId(), mergeEnabled );
     }
 };
 
@@ -181,12 +181,12 @@ private:
 
         BOARD::GroupLegalOpsField legalOps = board->GroupLegalOps( selection );
 
-        Enable( getMenuId( PCB_ACTIONS::groupCreate ), legalOps.create );
+        Enable( PCB_ACTIONS::groupCreate.GetUIId(),      legalOps.create );
-        Enable( getMenuId( PCB_ACTIONS::groupMerge ), legalOps.merge );
+        Enable( PCB_ACTIONS::groupMerge.GetUIId(),       legalOps.merge );
-        Enable( getMenuId( PCB_ACTIONS::groupUngroup ), legalOps.ungroup );
+        Enable( PCB_ACTIONS::groupUngroup.GetUIId(),     legalOps.ungroup );
-        Enable( getMenuId( PCB_ACTIONS::groupRemoveItems ), legalOps.removeItems );
+        Enable( PCB_ACTIONS::groupRemoveItems.GetUIId(), legalOps.removeItems );
-        Enable( getMenuId( PCB_ACTIONS::groupFlatten ), legalOps.flatten );
+        Enable( PCB_ACTIONS::groupFlatten.GetUIId(),     legalOps.flatten );
-        Enable( getMenuId( PCB_ACTIONS::groupEnter ), legalOps.enter );
+        Enable( PCB_ACTIONS::groupEnter.GetUIId(),       legalOps.enter );
     }
 };
 

pcbnew/tools/pcb_inspection_tool.cpp

@@ -76,9 +76,9 @@ private:
             }
         }
 
-        Enable( getMenuId( PCB_ACTIONS::showNet ), haveNetCode );
+        Enable( PCB_ACTIONS::showNet.GetUIId(), haveNetCode );
-        Enable( getMenuId( PCB_ACTIONS::hideNet ), haveNetCode );
+        Enable( PCB_ACTIONS::hideNet.GetUIId(), haveNetCode );
-        // Enable( getMenuId( PCB_ACTIONS::highlightNet ), haveNetCode );
+        // Enable( PCB_ACTIONS::highlightNet.GetUIId(), haveNetCode );
     }
 
     ACTION_MENU* create() const override

pcbnew/tools/selection_tool.cpp

@@ -87,9 +87,9 @@ private:
         bool connItem = S_C::OnlyTypes( GENERAL_COLLECTOR::Tracks )( selection );
         bool sheetSelEnabled = ( S_C::OnlyType( PCB_MODULE_T ) )( selection );
 
-        Enable( getMenuId( PCB_ACTIONS::selectNet ), connItem );
+        Enable( PCB_ACTIONS::selectNet.GetUIId(),        connItem );
-        Enable( getMenuId( PCB_ACTIONS::selectConnection ), connItem );
+        Enable( PCB_ACTIONS::selectConnection.GetUIId(), connItem );
-        Enable( getMenuId( PCB_ACTIONS::selectSameSheet ), sheetSelEnabled );
+        Enable( PCB_ACTIONS::selectSameSheet.GetUIId(),  sheetSelEnabled );
     }
 
     ACTION_MENU* create() const override