Update to sm_ban, sm_kick, & sm_map where args == 0 to display menu by JoinedSenses · Pull Request #838 · alliedmodders/sourcemod

JoinedSenses · 2018-06-27T03:15:21Z

This change makes it so /kick, /ban, and /map open the already created methods for displaying their menus when there are no args.

The reason for the feature is to take advantage of menus that already exist and to make the commands easier to use.

The client == 0 check prevents them from opening if it was ran via rcon, sm_rcon, or server command. Client auth is also checked because its a registered admin command.

Usage params will display if client == 0 and args < min

For example, a moderator wants to change a map, instead of running through the admin menu, they can instead type just /map to display available maps and choose one.

If a mod wants to quickly ban or kick someone without having to either run through the admin menu or type it out, they could then type the corresponding commands with no args to open the menus.

Headline · 2018-06-28T00:19:57Z

plugins/basebans/ban.sp

+		{
+			DisplayBanTargetMenu(client);
+		}
 		ReplyToCommand(client, "[SM] Usage: sm_ban <#userid|name> <minutes|0> [reason]");


Is it intended to error w/ usage when an argument-less sm_ban would now be considered valid?

ditto for the other commands, as well

I left in the replytocommand as a reminder of the usage just in case it's needed and also as a fallback where client ==0.

I don't think I'm alone in thinking that we probably shouldn't print usage errors if it's gonna be valid. Other than that this lgtm

Maybe/maybe not. Either way, there are going to be people who either like it or not. I also suggested this idea also to sbpp, but it was an "annoying change", though I think the opposite. This feature is implemented on my servers and the moderators/admins like it, especially the menu display for /map, considering it's already a feature for /nominate.

If sm_map and sm_kick were to at least be consistent with sm_nominate, then I suppose the usage parameter return could be removed *unless client = 0

*edit

I think that's best. It'd be annoying if users typed !ban Headline 0 "lemon" and got hit with usage errors every time.

KyleSanderson · 2018-06-28T20:21:47Z

At the very least if we're even going to consider merging something similar to this (it should be a convar) the cmdsrc needs to be evaluated.

Kenzzer · 2018-06-28T21:08:45Z

And what if I'm a newbie to SM and wanna know the params of sm_ban how do I check that since you will literally slam a menu I don't want in my face. I suggest making plural version of these commands (sm_bans, sm_maps, sm_kicks ect...) doing what you suggested.

JoinedSenses · 2018-06-28T21:22:35Z

Haha "slam a menu"? That's a bit dramatic. The method I used to add the menus will still post the usage parameters to chat/console, and will also display the menu. I'm not sure what cmdsrc is, and by cvar, do you mean adding an option to enable/disable the menus when args == 0?

JoinedSenses · 2018-06-28T21:34:10Z

Also, see above conversation with Headline regarding consistency with sm_nominate

JoinedSenses · 2018-06-30T00:03:13Z

~~On top of everything else, I have now also renamed the translation file for basecommands to be consistent with every other translation file (plugin.basecommands -> basecommands.phrases)~~

asherkin · 2018-08-11T12:42:14Z

I'd be moderately OK (but still not a fan) of doing this for chat commands, but console usage of these should definitely not be opening a menu.

Is there a driving force behind this? PRs with no description are rather annoying (at the very least, every feature addition needs a use case.)

JoinedSenses · 2018-08-11T13:05:12Z

This change makes it so /kick, /ban, and /map open the already created methods for displaying their menus when there are no args.

The reason for the feature is to take advantage of menus that already exist and to make the commands easier to use.

The client == 0 check prevents them from opening if it was ran via rcon, sm_rcon, or server command. Client auth is also checked because its a registered admin command.

Usage params will still display for each method when given incorrect args.

For example, a moderator wants to change a map, instead of running through the admin menu, they can instead type just /map to display available maps and choose one.

If a mod wants to quickly ban or kick someone without having to either run through the admin menu or type it out, they could then type the corresponding commands with no args to open the menus.

Headline

Just the comment from before, otherwise I'm okay with taking this.

Headline · 2018-09-03T00:02:26Z

plugins/basebans/ban.sp

+		{
+			DisplayBanTargetMenu(client);
+		}
 		ReplyToCommand(client, "[SM] Usage: sm_ban <#userid|name> <minutes|0> [reason]");


I think that's best. It'd be annoying if users typed !ban Headline 0 "lemon" and got hit with usage errors every time.

JoinedSenses · 2018-09-03T02:35:08Z

The latest change requested from Headline will now only display usage params if client == 0 && args == 0, or if args is greater than 0 and less than 2 for sm_ban

asherkin

I feel strongly enough that this should be checking the ReplySource rather than the client index to r-, it at least needs to be discussed. I also think the menu should only show on 0 args.

JoinedSenses · 2018-09-03T07:04:56Z

With these changes, the menu does only show on 0 args. No idea what you mean by the reply source? What sources should be checked for and how? It currently runs a check against client == 0, which would occur on rcon, sm_rcon, and servercommand.

My previous comment was regarding usage params reply, not the menu.

Edit: I see - console vs chat. Ill look into replysource. Not a bad idea.

Tested and worked as expected.

JoinedSenses · 2018-09-03T07:36:01Z

New commit, which now checks both reply source and client. Tested - usage params always display when issued via console and menu would not display.

Menu will only display if there are 0 arguments, client != 0, and the command was sent in chat.

asherkin · 2018-09-03T23:41:45Z

Thanks! Nice work.

JoinedSenses added 3 commits June 26, 2018 22:58

Update sm_ban where args == 0 to display menu

a6c0059

Update sm_kick where args == 0 to display menu

0db253f

Update sm_map where args == 0 to display menu

7424e7d

Headline reviewed Jun 28, 2018

View reviewed changes

JoinedSenses and others added 2 commits June 28, 2018 17:46

Merge remote-tracking branch 'upstream/master'

be6616f

Update map.sp

47086bb

JoinedSenses changed the title ~~Update to sm_ban, sm_kick, & sm_map where args == 0 to display menu~~ Update to /ban, /kick, /map where args == 0 to display menu (and translation file) Jun 30, 2018

JoinedSenses changed the title ~~Update to /ban, /kick, /map where args == 0 to display menu (and translation file)~~ Update to sm_ban, sm_kick, & sm_map where args == 0 to display menu Jun 30, 2018

Edit to continue displaying usage params

dd9cd37

Headline requested changes Sep 3, 2018

View reviewed changes

As per Headline's comment and formatting for consistency

c539e49

Headline approved these changes Sep 3, 2018

View reviewed changes

asherkin requested changes Sep 3, 2018

View reviewed changes

CmdReplySource Checking

73b6f47

Tested and worked as expected.

asherkin approved these changes Sep 3, 2018

View reviewed changes

asherkin merged commit a5b2249 into alliedmodders:master Sep 3, 2018

Uh oh!

Conversation

JoinedSenses commented Jun 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Headline Jun 28, 2018

Choose a reason for hiding this comment

Uh oh!

JoinedSenses Jun 28, 2018

Choose a reason for hiding this comment

Uh oh!

Headline Jun 28, 2018

Choose a reason for hiding this comment

Uh oh!

JoinedSenses Jun 28, 2018

Choose a reason for hiding this comment

Uh oh!

JoinedSenses Jun 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Headline Sep 3, 2018

Choose a reason for hiding this comment

Uh oh!

KyleSanderson commented Jun 28, 2018

Uh oh!

Kenzzer commented Jun 28, 2018

Uh oh!

JoinedSenses commented Jun 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JoinedSenses commented Jun 28, 2018

Uh oh!

JoinedSenses commented Jun 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asherkin commented Aug 11, 2018

Uh oh!

JoinedSenses commented Aug 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Headline left a comment

Choose a reason for hiding this comment

Uh oh!

Headline Sep 3, 2018

Choose a reason for hiding this comment

Uh oh!

JoinedSenses commented Sep 3, 2018

Uh oh!

asherkin left a comment

Choose a reason for hiding this comment

Uh oh!

JoinedSenses commented Sep 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JoinedSenses commented Sep 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asherkin commented Sep 3, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

JoinedSenses commented Jun 27, 2018 •

edited

Loading

JoinedSenses Jun 28, 2018 •

edited

Loading

JoinedSenses commented Jun 28, 2018 •

edited

Loading

JoinedSenses commented Jun 30, 2018 •

edited

Loading

JoinedSenses commented Aug 11, 2018 •

edited

Loading

JoinedSenses commented Sep 3, 2018 •

edited

Loading

JoinedSenses commented Sep 3, 2018 •

edited

Loading