Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add menu_getprop() function #507

Open
wants to merge 21 commits into
base: master
Choose a base branch
from

Conversation

OciXCrom
Copy link
Contributor

@OciXCrom OciXCrom commented Jul 27, 2018

This native is something that was requested many times, even by myself. The goal is to be able to get all values that are set with the menu_setprop function.

Discussed here - https://forums.alliedmods.net/showthread.php?t=303560

/* example usage */

new szTitle[128]
menu_getprop(iMenu, MPROP_TITLE, szTitle, charsmax(szTitle))

new iPerPage = menu_getprop(iMenu, MPROP_PERPAGE)

@Arkshine
Copy link
Member

Thanks for contributin, but just a word when you create a PR:

  • Provide a complete description. Reference links if necessary. You need to explain things.
  • Remove unrelated changes (Don't try to merge from master, update master -if necessary- first and create a branch from it, Here we have unrelated lang.inc changes )
  • Add meaningful commit messages. Here you could have said for example: "Newmenu: Add menu_getprop() native", "Newmenu: Update documentation". You see what I mean.

@OciXCrom
Copy link
Contributor Author

Thanks. I'll have this in mind for my next PR. I added the required description, but I'm not sure if I can remove the unrelated changes. Guess it's happening because I was doing it directly from Github rather than using Git on Desktop.

@Arkshine
Copy link
Member

Arkshine commented Aug 5, 2018

Do you have any usage/situation/context example in mind?

@OciXCrom
Copy link
Contributor Author

OciXCrom commented Aug 6, 2018

@Arkshine A good example would be when using the player_menu_info function to get the menu that the client is currently viewing. Right now there's no way to find any information about the menu if the menu itself is created in a different plugin. By adding menu_getprop scripters would be able to check if a player is viewing a specific menu by checking its title or other properties.

@Arkshine
Copy link
Member

Arkshine commented Aug 20, 2018

Checking by title or others properties doesn't look reliable for me. Not the best example I would say. Do you have another concrete & useful usage?

@justgo97
Copy link

an usefull use is when you need to remember the players current page, somthing like this :

#include <amxmodx>

new g_LastPage[MAX_PLAYERS+1]

public plugin_init()
{
    register_clcmd( "say /mymenu", "cmd_mymenu" )
}

public cmd_mymenu(id)
{
    new Menu = menu_create( "My menu", "mymenu_handler" )
    new ItemName[32]
    
    for(new i = 0; i <= MAX_PLAYERS; i++) 
    {
        formatex( ItemName, charsmax( ItemName ), "My item:%d", i )
        menu_additem( Menu, ItemName )
    }

    // If remembered page is greater than number of pages, clamp down the value
    g_LastPage[id] = min(g_LastPage[id], menu_pages(Menu)-1)
    
    menu_display(id, Menu, g_LastPage[id])
}

public mymenu_handler(id, Menu, Item)
{
    // Menu was closed
    if (Item == MENU_EXIT)
    {
        g_LastPage[id] = 0
        menu_destroy(Menu)
        return PLUGIN_HANDLED;
    }

    g_LastPage[id] = Item / menu_getprop( Menu, MPROP_PERPAGE)

    menu_destroy(Menu)
    return PLUGIN_HANDLED;
}

public client_disconnected(id)
{
    // Reset remembered menu pages
    g_LastPage[id] = 0
}

{
GETMENU(params[1]);

int len = params[0] / sizeof(cell);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

len is not really a good variable name. It could be numParams or count or something like that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Arkshine Done.

Copy link
Member

@Arkshine Arkshine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to put the native header as comment above and using an enum as you did in others PRs.

Also, using brace everywhere.

@OciXCrom
Copy link
Contributor Author

OciXCrom commented Sep 3, 2018

@Arkshine Done.

@Arkshine
Copy link
Member

Arkshine commented Sep 5, 2018

We should avoid natives which are not type-safe.
I would create menu_getprop_int (or without _int) and menu_getprop_string versions.

@OciXCrom
Copy link
Contributor Author

OciXCrom commented Sep 5, 2018

@Arkshine Since menu_setprop isn't type-safe either, IMO it's better to have menu_getprop mimic the same style for consistency.

@voed
Copy link
Contributor

voed commented May 29, 2020

@Arkshine BUMP

@Arkshine
Copy link
Member

Arkshine commented Jun 3, 2020

Did you test properly?

@OciXCrom
Copy link
Contributor Author

OciXCrom commented Jun 4, 2020

@Arkshine - pretty sure I did, if I recall correctly. The code is very straight forward.

Not sure how to compile the modules anymore so I can test again. The tutorial from the forum doesn't seem to work after the 1.10 updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants