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

Less tedious private commands #205

Closed
Hackmastr opened this Issue Jun 19, 2017 · 10 comments

Comments

Projects
None yet
4 participants
@Hackmastr
Copy link
Contributor

Hackmastr commented Jun 19, 2017

Hello,

Writing commands and checking whether the user typed a slash('/') or an exclamation mark('!') to privatize the command, starts to annoy after some time.

I was talking with iPlayer, and I believe he had few nice ideas.

For example, We use 2 decorators, but tell SP that one of them is private:

@SayCommand("!cmd")
@SayCommand("/cmd", suppress_output=True)

Or use only one decorator, and tell SP to automatically register the private command:

@SayCommand("!cmd", register_private_command=True)

Thanks.

@Mahi

This comment has been minimized.

Copy link
Contributor

Mahi commented Jun 19, 2017

If something like this were to be implemented, I'd go with something like:

@SayCommand('!cmd', private_command='/cmd')

Or even add support for multiple private commands (provide an iterable instead of a single string)

@KirillMysnik

This comment has been minimized.

Copy link
Member

KirillMysnik commented Jun 19, 2017

@Mahi that's actually the third thing I thought of. I also think that SayCommand should be... low-level, as it is now. And all that kind of stuff should go to typed commands.

But I personally like the suppress_output kwarg more.

@Ayuto

This comment has been minimized.

Copy link
Member

Ayuto commented Jun 19, 2017

I don't see a great benefit in doing this. It's rather more confusing as it conflicts with the return value of the callback. If you have reccuring tasks, you can simply create a function or your own decorator.

from commands.say import SayCommand
from commands import CommandReturn

def get_command_return(command):
    if command[0].startswith('!'):
        return CommandReturn.BLOCK
        
    return CommandReturn.CONTINUE

@SayCommand(['/cmd', '!cmd'])
def cmd(command, index, team_only):
    print(tuple(command))
    return get_command_return(command)

This way you can also easily change the prefix that determines whether the command should be supressed or not.

@KirillMysnik

This comment has been minimized.

Copy link
Member

KirillMysnik commented Jun 19, 2017

Just noticed that

@TypedSayCommand(['/cmd', '!cmd'])

registers the /cmd !cmd (subcommand) instead of registering two separate commands, while

@SayCommand(['/cmd', '!cmd'])

registers the callback to two command aliases, like in your example.

I didn't get how it's going to conflict with the return value of the callback... Whenever CommandReturn.CONTINUE is returned from the command built with suppress_output, it's overridden with CommandReturn.BLOCK.

I.e. it will remove the difference between returning CONTINUE and returning BLOCK, as it will get set to BLOCK anyways - that's confusing to some extend, but the scripter probably expects this kind of behavior when setting suppress_output to True.

@Ayuto

This comment has been minimized.

Copy link
Member

Ayuto commented Jun 19, 2017

I.e. it will remove the difference between returning CONTINUE and returning BLOCK, as it will get set to BLOCK anyways - that's confusing to some extend, but the scripter probably expects this kind of behavior when setting suppress_output to True.

That's exactly the conflict I meant. It makes returning something redundant.

@KirillMysnik

This comment has been minimized.

Copy link
Member

KirillMysnik commented Jun 19, 2017

Oh yeah, I forgot that the BLOCK and CONTINUE are the only possible command return types. Then indeed the returning will become redundant.

Also. Sometimes it might turn out useful to register the client command as well.

I'd throw in an idea of the UniversalCommand decorator that will register all 3 commands at once.

# Register !my_command and /my_command chat commands as well as the my_command client command
@UniversalCommand('my_command')
def callback(command_info):
    ...

With supported kwargs like public_chat_command, private_chat_command and client_command (all defaulting to True).

The command_info in that case might also contain a .message_factory property to use like this:

msg = command_info.message_factory("Command executed")
# `msg` is either a SayText2 or TextMsg (built with HudDestination.CONSOLE) instance - depends on the source of the command

msg.send(command_info.index)

In case we wanted to enforce the same command design as on Sourcemod (public chat command !, private chat command /, client command), the UniversalCommand class might be useful.
Otherwise, if it's up to the scripters to decide their command patterns, it's not worth the space.

@Ayuto

This comment has been minimized.

Copy link
Member

Ayuto commented Jul 4, 2017

I have been thinking about this a little bit more and I think I would be fine with a UniversalCommand and/or TypedUniversalCommand decorator.

The factory thing is a nice idea, although I would implement it differently. What about just having a respond(<str or _TranslationString>) method? It would probably mainly used to send an answer to the command issuer.

@TypedUniversalCommand('multiply')
def on_multiply(info, x:float, y:float):
    info.respond('{} x {} = {}'.format(x, y, x*y))
@KirillMysnik

This comment has been minimized.

Copy link
Member

KirillMysnik commented Jul 4, 2017

Yep, respond looks reasonable.
Especially considering that my message_factory approach would probably modify the arguments passed to TextMsg constructor anyways, in order to set HudDestination.CONSOLE.
So yeah, respond looks better.

@Ayuto Ayuto added the enhancement label Jul 5, 2017

@KirillMysnik

This comment has been minimized.

Copy link
Member

KirillMysnik commented Jul 15, 2017

Just found out that TypedSayCommand, TypedClientCommand and TypedServerCommand include send_message method.
So CommandInfo.respond might be just an alias for that.

Ayuto added a commit that referenced this issue Dec 15, 2018

@Ayuto

This comment has been minimized.

Copy link
Member

Ayuto commented Dec 15, 2018

I just extended the CommandInfo class, so you can now use the following:

from commands.typed import TypedSayCommand

@TypedSayCommand('/test')
def on_test(info):
    info.reply('This is a private command.')
    
    # This line is optional. If None is being returned, it will fallback
    # to info.auto_command_return
    return info.auto_command_return

auto_command_return will determine the probably most desired CommandReturn value for a say, client or server command. See also:
f711ebe#diff-d983e9bdd74d707769ff532a5b6aae38R510

@Ayuto Ayuto closed this Dec 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment