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

on_named_slashcommand event for bot #1261

Closed
ruslan-ilesik opened this issue Oct 7, 2024 · 8 comments
Closed

on_named_slashcommand event for bot #1261

ruslan-ilesik opened this issue Oct 7, 2024 · 8 comments
Assignees

Comments

@ruslan-ilesik
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Every bot has multiple commands and handling all of them in one on_slashcommand handler with a lot of if else statements feels not right

Describe the solution you'd like
We will create on_named_slashcommand event in cluster where u can attach callback to command by name. in my view bot on_slashcommand and it will execute on every matching slashcommand event, this way we will not break back compatibility

Describe alternatives you've considered
Alternatives is using global maps-handlers or creating custom slash command handlers which is annoying to do for every bot dev, also it is not simply extendable

Additional context
There is couple of questions i would like to ask.

  1. Should we have warning on bulk_commands_create if user specified slash command handler for command we do not register?
  2. Can we add some prerequirement for on_named_slashcommand which should return bool if we should proceed or no (it should be executed on every on_slashcommand event and execute named commands only if precondition satisfied or execute only when we have matching on_named_slashcommand (?))
  3. Should we allow multiple handlers for same slashcommand as other dpp events do or should we throw an error if second one is being attached? (in general behavior in such case is questionable)
@ruslan-ilesik
Copy link
Contributor Author

A bit more about precondition details. For example implementing rate limits for users on commands and do it once globally rather than doing it in every command (as we can do it rn in on_slashcommand for example)

@braindigitalis
Copy link
Contributor

For 1, no, no warning, because at scale youd register your commands in your first cluster for example only but all clusters would have a command handler...

@braindigitalis
Copy link
Contributor

braindigitalis commented Oct 7, 2024

For 3, not sure if it makes sense to register twice. the first to catch the event will call event.reply and you cant do event.reply twice.

@ruslan-ilesik
Copy link
Contributor Author

For 3, not sure if it makes sense to register twice. the first to catch the event will call event.reply and you cant do event.reply twice.

so we should throw an exception here. right?

@ruslan-ilesik
Copy link
Contributor Author

Also, i was looking at event router class and i don't see away to implement it without doing custom type/ wrapper around it. my simplest solution would be to have internal on_slashcommand handler and play from it. But if u have better ideas how to addopt event router, please share them

@braindigitalis
Copy link
Contributor

I would just have an internal map of <string,event_router_t*> routed internally where we call on_slashcommand events, either just before or just after.
a simple find on that map and if found, call the .call() method of it. youll need to add a new parameter struct derived from event_dispatch_t to pass to the handler.

@ruslan-ilesik
Copy link
Contributor Author

I would just have an internal map of <string,event_router_t*> routed internally where we call on_slashcommand events, either just before or just after. a simple find on that map and if found, call the .call() method of it. youll need to add a new parameter struct derived from event_dispatch_t to pass to the handler.

why so? can't we just use existing slashcommand_t we have? why should we have different struct for this?

@ruslan-ilesik
Copy link
Contributor Author

Implementation of this type of feature is here
#1262

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

No branches or pull requests

2 participants