Newmenus: Add menu timeouts (+ extra bugfix)

[Nextra]

This series of patches adds timeouts to newmenus, working similar to those of show_menu. Fixes bug 2974.

When a menu is acted upon after the specified timeout has run out it will pass a new MENU_TIMEOUT status code into the menu handler. Actions are: player disconnect, selecting an item, cancelling/destroying the menu in the plugin, sending a different menu or using get_user_menu on a player with an open menu.

Just like with the old menus timeouts are not actively polled, and MENU_TIMEOUT is not guaranteed to fire directly after the time has run out. Also get_user_menu will detect timeouts and reset the menu flags, which is why it also has to trigger the callback.

I discovered another issue when playing around with newmenus. Using the old show_menu when a newmenu was opened just reset CPlayer::newmenu without actually triggering the menu callback. This could result in menu handle leaks and is theoretically even client exploitable. This is also fixed.

This is the plugin I used to test the various possible occurances of MENU_TIMEOUT and verify that old behavior is not broken.

#include <amxmodx>

new last[33] = {-1, ...};

public plugin_init() {
    register_clcmd("menu_display", "menudisplay");
    register_clcmd("menu_destroy", "menudestroy");
    register_clcmd("menu_cancel", "menucancel");
    register_clcmd("show_menu", "menushow");
    register_clcmd("get_user_menu", "menuget");
}

public menudisplay(const client) {
    if (read_argc() != 2 || client == 0) {
        return PLUGIN_HANDLED;
    }

    new arg[32];
    read_argv(1, arg, charsmax(arg));
    new time = max(strtol(arg), -1);

    new menu = menu_create("Menu Test", "_handler");
    menu_additem(menu, "Test");
    menu_display(client, menu, _, time);

    last[client] = menu;

    return PLUGIN_HANDLED;
}

public _handler(const client, const menu, const item) {
    server_print("_handler(%d, %d, %d)", client, menu, item);
    client_print(client, print_console, "_handler(%d, %d, %d)", client, menu, item);

    last[client] = -1;
    menu_destroy(menu);
}

public menushow(const client) {
    show_menu(client, MENU_KEY_1, "Test");
    return PLUGIN_HANDLED;
}

public menudestroy(const client) {
    if (last[client] != -1) {
        menu_destroy(last[client]);
    }

    return PLUGIN_HANDLED;
}

public menucancel(const client) {
    if (last[client] != -1) {
        menu_cancel(client);
    }

    return PLUGIN_HANDLED;
}

public menuget(const client) {
    new menu, keys;
    get_user_menu(client, menu, keys);
    return PLUGIN_HANDLED;
}

[Arkshine]

This looks good for me.

@dvander , if you have some thoughts on it before I merge this, please let us hear (because of your answer in bug 2974, in case we may missed something as Nextra's implementation is fairly light (if you remember!))

[dvander]

It looks good at a glance - thank you for abstracting a lot of that previous gunk into Close(), that was a good idea. If you wanted to take another step you could factor out another antipattern that was present:

Menu *get_menu_by_id(int id)
{
    if (id < 0 || size_t(id) >= g_NewMenus.size() || !g_NewMenus[id])
        return NULL;
    return g_NewMenus[id];
}

...
if (Menu *pMenu = get_menu_by_id(pPlayer->menu)) {
    ...
}

[Nextra]

Good idea. Done.