Expand GetClientMenu() to include Menu handle

[Scags]

There currently isn't a way to get a client's exact menu, so I wanted to implement that for my use case.

I wanted to close a certain menu when a round ended, since there's no way to get an exact menu, I had to settle for checking if GetClientMenu(client) != MenuSource_None. Works, but what if there's a vote right before the end of the round?

I wasn't sure if it would be better to create a new native or expand on GetClientMenu, so if that's preferred, please let me know.

Test script:

public void OnPluginStart()
{
    RegConsoleCmd("sm_makemenu", CmdMakeMenu);
    RegConsoleCmd("sm_mymenu", CmdMyMenu);
}

public Action CmdMakeMenu(int client, int args)
{
    Menu menu = new Menu(MakeMenuHandler);
    char buffer[64]; FormatEx(buffer, sizeof(buffer), "Menu: %x", menu);
    menu.SetTitle(buffer);
    menu.AddItem("", "Bananas");
    menu.AddItem("", "Grapes");
    menu.Display(client, MENU_TIME_FOREVER);
    return Plugin_Handled;
}

public int MakeMenuHandler(Menu menu, MenuAction action, int client, int select)
{
    if (action == MenuAction_End)
        delete menu;
}

public Action CmdMyMenu(int client, int args)
{
    Menu menu;
    MenuSource src = GetClientMenu(client, .hMenu = menu);
    if (src == MenuSource_Normal)
        ReplyToCommand(client, "%x", menu);
    else
        ReplyToCommand(client, "No menu");
}

[asherkin]

This is a reasonable request and the change looks good, but I'm a little worried about the interaction with Handle permissions. I'm not sure the existing Menu handle type restrictions are going to be strict enough to prevent other plugins from doing surprising things here.

Could you have a test and see what interactions are possible when you're using this to nab another plugin's open menu? Particularly re-displaying it and making changes to the items / configuration - which I think we really don't want other plugins to be doing.

[Scags]

Yes, looks like other plugins are capable of modifying a players current menu regardless of handle ownership. I can freely remove from, add to, and redisplay a menu from a separate plugin.

Owner Plugin Other Plugin

One could argue that this feature would be useful for various subplugins, though I can't think of a good use case off of the top of my head. A problem that could arise would be other plugin's messing with callbacks, in particular VoteResultCallback.

What would be the best course of action here? Restricting other plugins from getting a menu that isn't owned by itself?

[asherkin]

I think that is probably the easiest and safest route - as long as we avoid this function leaking them the open permissions aren't really a problem. We could instead tweak the handle permissions to only be readable by the owner for most functions, but as you say that could break some legitimate use cases where menu handles are being explicitly passed around via other mechanisms today. It's easy enough to make the permission changes and open up other plugin handles here in the future if that ends up being desired as well.

[Wend4r]

I feel that there are huge changes behind this in the system of handles for controlling write/change access. Combining the transfer of descriptors with the API concept (natives and forwards) in SourcePawn, it will come to this sooner or later, but this problem is now being solved by cloning the handle.

[KyleSanderson]

@Scags are you OK to make this change? If you need/want help I can spend some time on Discord with you.

[Scags]

Discussed in Discord. Closing as no longer needed.