IContextMenu Tweaks and Cleanup (#1753)

* Tweaks and cleanup

* Fix backwards logic

Dalamud/Game/Gui/ContextMenu/ContextMenu.cs

@@ -47,14 +47,14 @@ internal sealed unsafe class ContextMenu : IInternalDisposableService, IContextM
         this.addonContextMenuOnMenuSelectedHook.Enable();
     }
 
-    private unsafe delegate ushort RaptureAtkModuleOpenAddonByAgentDelegate(RaptureAtkModule* module, byte* addonName, AtkUnitBase* addon, int valueCount, AtkValue* values, AgentInterface* agent, nint a7, ushort parentAddonId);
+    private delegate ushort RaptureAtkModuleOpenAddonByAgentDelegate(RaptureAtkModule* module, byte* addonName, AtkUnitBase* addon, int valueCount, AtkValue* values, AgentInterface* agent, nint a7, ushort parentAddonId);
     
-    private unsafe delegate bool AddonContextMenuOnMenuSelectedDelegate(AddonContextMenu* addon, int selectedIdx, byte a3);
+    private delegate bool AddonContextMenuOnMenuSelectedDelegate(AddonContextMenu* addon, int selectedIdx, byte a3);
     
-    private unsafe delegate ushort RaptureAtkModuleOpenAddonDelegate(RaptureAtkModule* a1, uint addonNameId, uint valueCount, AtkValue* values, AgentInterface* parentAgent, ulong unk, ushort parentAddonId, int unk2);
+    private delegate ushort RaptureAtkModuleOpenAddonDelegate(RaptureAtkModule* a1, uint addonNameId, uint valueCount, AtkValue* values, AgentInterface* parentAgent, ulong unk, ushort parentAddonId, int unk2);
 
     /// <inheritdoc/>
-    public event IContextMenu.OnMenuOpenedDelegate OnMenuOpened;
+    public event IContextMenu.OnMenuOpenedDelegate? OnMenuOpened;
 
     private Dictionary<ContextMenuType, List<MenuItem>> MenuItems { get; } = new();
 
@@ -171,7 +171,7 @@ internal sealed unsafe class ContextMenu : IInternalDisposableService, IContextM
 
     private void SetupGenericMenu(int headerCount, int sizeHeaderIdx, int returnHeaderIdx, int submenuHeaderIdx, IReadOnlyList<MenuItem> items, ref int valueCount, ref AtkValue* values)
     {
-        var itemsWithIdx = items.Select((item, idx) => (item, idx)).OrderBy(i => i.item.Priority);
+        var itemsWithIdx = items.Select((item, idx) => (item, idx)).OrderBy(i => i.item.Priority).ToArray();
         var prefixItems = itemsWithIdx.Where(i => i.item.Priority < 0).ToArray();
         var suffixItems = itemsWithIdx.Where(i => i.item.Priority >= 0).ToArray();
 
@@ -268,10 +268,10 @@ internal sealed unsafe class ContextMenu : IInternalDisposableService, IContextM
 
         foreach (var item in items)
         {
-            if (!item.Prefix.HasValue)
+            if (!item.Prefix.HasValue && !item.UseDefaultPrefix)
             {
-                item.PrefixChar = 'D';
+                item.PrefixChar = MenuItem.DalamudDefaultPrefix.ToIconChar();
-                item.PrefixColor = 539;
+                item.PrefixColor = MenuItem.DalamudDefaultPrefixColor;
                 Log.Warning($"Menu item \"{item.Name}\" has no prefix, defaulting to Dalamud's. Menu items outside of a submenu must have a prefix.");
             }
         }
@@ -378,13 +378,13 @@ internal sealed unsafe class ContextMenu : IInternalDisposableService, IContextM
     {
         // The in game menu actually supports 32 items, but the last item can't have a visible submenu arrow.
         // As such, we'll only work with 31 items.
-        const int MaxMenuItems = 31;
+        const int maxMenuItems = 31;
-        if (items.Count + nativeMenuSize > MaxMenuItems)
+        if (items.Count + nativeMenuSize > maxMenuItems)
         {
-            Log.Warning($"Menu size exceeds {MaxMenuItems} items, truncating.");
+            Log.Warning($"Menu size exceeds {maxMenuItems} items, truncating.");
             var orderedItems = items.OrderBy(i => i.Priority).ToArray();
-            var newItems = orderedItems[..(MaxMenuItems - nativeMenuSize - 1)];
+            var newItems = orderedItems[..(maxMenuItems - nativeMenuSize - 1)];
-            var submenuItems = orderedItems[(MaxMenuItems - nativeMenuSize - 1)..];
+            var submenuItems = orderedItems[(maxMenuItems - nativeMenuSize - 1)..];
             return newItems.Append(new MenuItem
             {
                 Prefix = SeIconChar.BoxedLetterD,
@@ -450,7 +450,6 @@ internal sealed unsafe class ContextMenu : IInternalDisposableService, IContextM
         if (callbackId < 0)
         {
             selectedIdx = -callbackId - 1;
-            goto original;
         }
         else
         {
@@ -461,17 +460,17 @@ internal sealed unsafe class ContextMenu : IInternalDisposableService, IContextM
             {
                 if (item.OnClicked == null)
                     throw new InvalidOperationException("Item has no OnClicked handler");
-                item.OnClicked.InvokeSafely(new(
+                item.OnClicked.InvokeSafely(new MenuItemClickedArgs(
-                    (name, items) =>
+                    (name, submenuItems) =>
                     {
                         short x, y;
                         addon->AtkUnitBase.GetPosition(&x, &y);
-                        this.OpenSubmenu(name ?? item.Name, items, x, y);
+                        this.OpenSubmenu(name ?? item.Name, submenuItems, x, y);
                         openedSubmenu = true;
                     },
                     this.SelectedParentAddon,
                     this.SelectedAgent,
-                    this.SelectedMenuType.Value,
+                    this.SelectedMenuType ?? default,
                     this.SelectedEventInterfaces));
             }
             catch (Exception e)
@@ -479,14 +478,14 @@ internal sealed unsafe class ContextMenu : IInternalDisposableService, IContextM
                 Log.Error(e, "Error while handling context menu click");
             }
 
-            // Close with clicky sound
+            // Close with click sound
             if (!openedSubmenu)
                 addon->AtkUnitBase.FireCallbackInt(-2);
             return false;
         }
 
 original:
-        // Eventually handled by inventorycontext here: 14022BBD0 (6.51)
+        // Eventually handled by inventory context here: 14022BBD0 (6.51)
         return this.addonContextMenuOnMenuSelectedHook.Original(addon, selectedIdx, a3);
     }
 }
@@ -511,7 +510,7 @@ internal class ContextMenuPluginScoped : IInternalDisposableService, IContextMen
     }
 
     /// <inheritdoc/>
-    public event IContextMenu.OnMenuOpenedDelegate OnMenuOpened;
+    public event IContextMenu.OnMenuOpenedDelegate? OnMenuOpened;
 
     private Dictionary<ContextMenuType, List<MenuItem>> MenuItems { get; } = new();
 

Dalamud/Game/Gui/ContextMenu/MenuArgs.cs

@@ -70,8 +70,18 @@ public abstract unsafe class MenuArgs
     /// Almost always an agent pointer. You can use this to find out what type of context menu it is.
     /// </summary>
     /// <exception cref="InvalidOperationException">Thrown when the context menu is not a <see cref="ContextMenuType.Default"/>.</exception>
-    public IReadOnlySet<nint> EventInterfaces =>
+    public IReadOnlySet<nint> EventInterfaces 
-        this.MenuType != ContextMenuType.Default ?
+    {
-            this.eventInterfaces :
+        get
-            throw new InvalidOperationException("Not a default context menu");
+        {
+            if (this.MenuType is ContextMenuType.Default)
+            {
+                return this.eventInterfaces ?? new HashSet<nint>();
+            }
+            else
+            {
+                throw new InvalidOperationException("Not a default context menu");
+            }
+        }
+    }
 }

Dalamud/Game/Gui/ContextMenu/MenuItem.cs

@@ -10,6 +10,16 @@ namespace Dalamud.Game.Gui.ContextMenu;
 /// </summary>
 public sealed record MenuItem
 {
+    /// <summary>
+    /// The default prefix used if no specific preset is specified.
+    /// </summary>
+    public const SeIconChar DalamudDefaultPrefix = SeIconChar.BoxedLetterD;
+
+    /// <summary>
+    /// The default prefix color used if no specific preset is specified.
+    /// </summary>
+    public const ushort DalamudDefaultPrefixColor = 539;
+    
     /// <summary>
     /// Gets or sets the display name of the menu item.
     /// </summary>
@@ -46,6 +56,11 @@ public sealed record MenuItem
     /// Gets or sets the color of the <see cref="Prefix"/>. Specifies a <see cref="UIColor"/> row id.
     /// </summary>
     public ushort PrefixColor { get; set; }
+    
+    /// <summary>
+    /// Gets or sets a value indicating whether the dev wishes to intentionally use the default prefix symbol and color.
+    /// </summary>
+    public bool UseDefaultPrefix { get; set; }
 
     /// <summary>
     /// Gets or sets the callback to be invoked when the menu item is clicked.

Dalamud/Plugin/Services/IContextMenu.cs

@@ -1,6 +1,4 @@
 using Dalamud.Game.Gui.ContextMenu;
-using Dalamud.Game.Text;
-using Dalamud.Game.Text.SeStringHandling;
 
 namespace Dalamud.Plugin.Services;
 
@@ -16,8 +14,9 @@ public interface IContextMenu
     public delegate void OnMenuOpenedDelegate(MenuOpenedArgs args);
 
     /// <summary>
-    /// Event that gets fired every time the game framework updates.
+    /// Event that gets fired whenever any context menu is opened.
     /// </summary>
+    /// <remarks>Use this event and then check if the triggering addon is the desired addon, then add custom context menu items to the provided args.</remarks>
     event OnMenuOpenedDelegate OnMenuOpened;
 
     /// <summary>
@@ -25,6 +24,7 @@ public interface IContextMenu
     /// </summary>
     /// <param name="menuType">The type of context menu to add the item to.</param>
     /// <param name="item">The item to add.</param>
+    /// <remarks>Used to add a context menu entry to <em>all</em> context menus.</remarks>
     void AddMenuItem(ContextMenuType menuType, MenuItem item);
 
     /// <summary>
@@ -32,6 +32,7 @@ public interface IContextMenu
     /// </summary>
     /// <param name="menuType">The type of context menu to remove the item from.</param>
     /// <param name="item">The item to add.</param>
+    /// <remarks>Used to remove a context menu entry from <em>all</em> context menus.</remarks>
     /// <returns><see langword="true"/> if the item was removed, <see langword="false"/> if it was not found.</returns>
     bool RemoveMenuItem(ContextMenuType menuType, MenuItem item);
 }