jchristgit / nosedrum

a command framework for nostrum
https://hexdocs.pm/nosedrum/Nosedrum.html
ISC License
34 stars 12 forks source link

First-class command alias support #1

Closed jchristgit closed 3 years ago

jchristgit commented 5 years ago

Currently, the way to do "aliases" is to just mount the same command module under a different name. Not nice.

In the Nosedrum.Storage behaviour:

In the Nosedrum.Invoker.Split implementation, make adjustments as necessary.

jchristgit commented 4 years ago

this can be achieved with add_command and friends just fine at the moment

jchristgit commented 4 years ago

... but a proper implementation would still be great!

Bentheburrito commented 3 years ago

Hello, if this isn't being worked on I'd like to take this issue up :)

Bentheburrito commented 3 years ago

Question though: do you think it would be better to have an optional callback "aliases" in the command behaviour that a Nosedrum.Storage uses if present? (Opposed to add_aliases and remove_aliases optional callbacks.) I think this would be better because you don't have to worry about removing/adding aliases whenever you add/remove a command

For example, in a command module:

@impl true
def aliases, do: ["alias1", "alias2"]

and Nosedrum.Storage implementations can have something like this in add_command/3:

if function_exported?(command_module, :aliases, 0) do
  aliases = apply(command_module, :aliases, [])
  # register `command_module` as a command under each alias
  # ...
end

Let me know what you think!

jchristgit commented 3 years ago

Hey @Bentheburrito, thanks for your interest in this library and issue!

I think your idea with having an optional aliases function is great! I was thinking that it may be a bit problematic to hard-couple the command module to the way that it's added in the command invoker, but given the current usage function, that would be a bit odd anyways.

The ability to register & unregister all aliases along with the command automatically sounds like a perfectly good idea to me. 🙂

I'll assign you since you mentioned you'd be happy to take this issue up.

jchristgit commented 3 years ago

Also, if you have the time, I'd be happy for any feedback about the library, since this has mostly grown out of my own command framework for my bot, and I'm curious if it fits well into other people's apps!

Bentheburrito commented 3 years ago

Awesome! I'll work on this soon then :)

This has been a great fit for one of my bots! I would normally write my own bare-bones command handler, but this project is big enough that I wanted to focus on actually building out its features instead of the more foundational stuff, so this was a nice surprise once I found it :) I came to Elixir from Node.JS/Discord.JS, and I think the only thing I miss is automatically loading commands on start-up - though I know from trying this myself that this is easier said than done. Slash Command are also something I think would fit nicely in this lib, and is something I was considering opening an issue for and working on myself (if you're interested!)

jchristgit commented 3 years ago

Thanks for the feedback, it's very valuable!

Slash commands would definitely be something that would fit in well! From what I remember, the interactions received via the gateway also contain the original message. Maybe we could expand the exiting Nosedrum.Command behaviour to maybe a callback slash_command/1, which would retrieve the interaction. Then you also have permissions though, and it starts getting a bit odd. Maybe we should have a separate behaviour for it to keep it clean?

As for loading commands at start-up - yeah, I think that would be possible if we started our own application supervisor and had a command storage module & list of commands configured at startup. Something like that was an intentional non-goal though, since I wanted people to be able to choose which parts of the library they use themselves.

Bentheburrito commented 3 years ago

Awesome, it's my pleasure!

I think Slash Commands are different enough to warrant their own behaviour imo. I have a rudimentary slash command handler on another bot that I could clean up and use as a starting point? Also, should I open a separate issue for further discussion?

Regarding command loading on start-up: I see your concern there - one way I can think of to have this without spinning up additional processes would be passing an option to a Storage's start_link...though I'm not sure how you'd represent that in the behaviour.

jchristgit commented 3 years ago

I think Slash Commands are different enough to warrant their own behaviour imo. I have a rudimentary slash command handler on another bot that I could clean up and use as a starting point? Also, should I open a separate issue for further discussion?

Yeah, I agree with making a new issue. I went ahead and made one: https://github.com/jchristgit/nosedrum/issues/14

Regarding command loading on start-up: I see your concern there - one way I can think of to have this without spinning up additional processes would be passing an option to a Storage's start_link...though I'm not sure how you'd represent that in the behaviour.

I very much like the idea of using the start_link options! But I'm also not sure how to represent that properly in the launch options. I'll see if I can find another library using it somewhere, since it's not that uncommon.