search

JDA-Applications / JDA-Utilities

A series of tools and utilities for JDA to assist in bot creation
Apache License 2.0
218 stars 111 forks source link

Add message attribute to onTerminatedCommand #81

Closed itsmefox closed 5 years ago

itsmefox commented 6 years ago

Pull Request

Pull Request Checklist

Please follow the following steps before opening this PR.
PRs that do not complete the checklist will be subject to denial for missing information.

Pull Request Information

Check and fill in the blanks for all that apply:

Description

To handle the onTerminatedCommand event in a custom CommandListener it would be nice to have access to message attribute which was passed to the terminate(CommandEvent event, String message) method.

Artuto commented 6 years ago

Maybe it would be benefitial the adding of an enum with some termination reasons?

That way devs can compare and do things on X rather than comparing strings.

jagrosh commented 6 years ago

I don't think adding this (in String format) is a good idea, as upcoming internationalization changes will be changing the internals here. Do you have an example of a specific use-case you need this for?

itsmefox commented 6 years ago

@Artuto I also thought about this and I would love it to have an enum for this so i can decide which message I want to show to my users. Because in some cases I'd like to show a random message to keep the conversation with my bot fresh and interesting :)

itsmefox commented 6 years ago

@jagrosh When will this internationalization changes come? Because my bot already supports multi-language support except for the command terminations.

jagrosh commented 6 years ago

There is an in-progress branch for it, see https://github.com/JDA-Applications/JDA-Utilities/issues/54 for more info

itsmefox commented 6 years ago

@jagrosh Oh nice. Will there also be a possibility to define if the terminated message is send in DM or not?

jagrosh commented 6 years ago

That might be possible, but the terminate method specifically is only an internal and private method; you shouldn't rely on any specific functionality from it. Additionally, the system in limited in some ways because users of the library can't "terminate" their own commands easily.

In my opinion, the entire 'terminate' system should be replaced by an exception that can be thrown (either by Command internals or within the code for an implementation of a command), with a default handler that can be overwritten. With a system like this, you could take care of the exceptions however you wanted to, and would have resources such as the message, original event, etc.

jagrosh commented 6 years ago

For example, in one of my bots, I have this command exception listener: https://github.com/jagrosh/Vortex/blob/master/src/main/java/com/jagrosh/vortex/commands/CommandExceptionListener.java

Notice how I can short-circuit a command by throwing an exception: https://github.com/jagrosh/Vortex/blob/master/src/main/java/com/jagrosh/vortex/commands/tools/InvitepruneCmd.java#L66

Something like this might be useful for other users too, and this kind of system would get adopted into the internals as well if we added it. However, my examples above use Strings for messages; we'd want to support i18n for this kind of exception system (if we add it at all).