Menu items indexes can change while a menu is open

[asherkin]

Which can cause the wrong item to be selected, or a failure if the last item in the menu is selected.

menu_slots_t holds a mapping of display positions to menu items while the menu is open, but the menu items are just indexes into the menu's m_items vector, which will shift when items are added or removed.

I think the best option here is to add a new ITEMDRAW flag for removed items, overwrite the draw style on RemoveItem instead of removing, treat them as ITEMDRAW_IGNORE when rendering, and only actually purge them from the items vector when the last display for a menu has finished. I'm not sure if we're currently tracking how many panels a menu has live, but that'll probably be important for #1047 as well.

Minimal test case:

#pragma semicolon 1
#pragma newdecls required

public void OnPluginStart()
{
    RegConsoleCmd("sm_m", Command_Menu);
}

public Action Command_Menu(int client, int args)
{
    Menu menu = new Menu(MenuHandler, MenuAction_Display | MENU_ACTIONS_DEFAULT);
    menu.SetTitle("Test Menu");
    menu.AddItem("Item 1", "Item 1");
    menu.AddItem("Item 2", "Item 2");
    menu.AddItem("Item 3", "Item 3");
    menu.Display(client, MENU_TIME_FOREVER);
    
    return Plugin_Handled;
}

int MenuHandler(Menu menu, MenuAction action, int param1, int param2)
{
    switch(action) {
        case MenuAction_Display: {
            // Remove the 2nd menu item to trigger the bug.
            menu.RemoveItem(1);

            return 0;
        }
        case MenuAction_Select: {
            int client = param1;
            int item = param2;
            
            char info[32];
            if (!menu.GetItem(item, info, sizeof(info))) {
                PrintToChat(client, "|| Menu item %d doesn't exist", item);

                return 0;
            }

            PrintToChat(client, "|| Selected menu item %d - '%s'", item, info);

            return 0;
        }
        case MenuAction_Cancel: {
            int client = param1;
            int reason = param2;

            PrintToChat(client, "|| Menu canceled - %d", reason);

            return 0;
        }
        case MenuAction_End: {
            int endReason = param1;
            int cancelReason = param2;

            PrintToChatAll("|| Menu ended - %d (%d)", endReason, cancelReason);

            delete menu;

            return 0;
        }
        default: {
            ThrowError("Unhandled menu action %d", action);
            return 0;
        }
    }
}

Selecting item 1 works fine, selecting item 2 prints that item 3 was selected, and selecting item 3 displays an error.

[asherkin]

The suggested solution in the description isn't immediately workable as a number of plugins rely on the item indexes shifting when removing items from a menu, so we'll need to keep separate internal (synchronised with display cycles) and visible-to-the-API lists of items. Putting this up for grabs as I'm not currently working on it any more.