Closed bivashy closed 1 year ago
Excellent work! I have a few questions though, as I haven't got a chance to fully explore the code
@Default
and @Optional
?@OptionData
with a Java enum? I think this would be more idiomatic and align better with the designI'll merge the PR in a separate branch, in case it needs any more modifications or cleanups before moving it upstream. Once again, appreciate the effort!
Thank you, I appreciate your compliments
Answers:
Yes definitely. If you are interested how it converts built-in annotations to OptionData
, it was handled in /core/BasicSlashCommandMapper
, i've used CommandParameter#isOptional
for defining when it is option required or not.
Also Thank you for reminding me. I completely forgot to organize the optional parameters, so they should be placed at the end. This is necessary because
https://discord.com/developers/docs/interactions/application-commands#application-command-object-application-command-option-structure
I think i will create another Pull Request for "fixing" that case
Yes, this is possible, it is handled in /core/BasicSlashCommandMapper
too. I've used CommandParameter#getType
for obtaining Class<?>
instance. And then I used Type#getEnumConstants
for getting all enum values
Here is code permalinks:
Optional related: https://github.com/bivashy/Lamp/blob/master/jda/src/main/java/revxrsal/commands/jda/core/BasicSlashCommandMapper.java#L73
Enum related: https://github.com/bivashy/Lamp/blob/master/jda/src/main/java/revxrsal/commands/jda/core/BasicSlashCommandMapper.java#L78-L81
What is this change supposed to do?
This change adds support of JDA slash commands. And solved several problems with Lamp and Discord slash commands compatibility. Also this change should be backward compatible. This pull request adds:
SuggestionProvider
will also handle JDA eventCommandAutoCompleteInteractionEvent
@OptionData
annotation, otherwise Lamp will handle automatically.Backward compatibility
getMessage()
,getEvent()
.getGenericEvent()
method. Also i've createdactor
package in root directory (/src/main/java/revxrsal/commands/jda/actor
) where we defineMessageJDAActor
,SlashCommandJDAActor
that represents actors of specific events. But i didn't moveJDAActor
So I think there shouldn't be any problem for migrating to new slash command support. Also i've mentioned alternative methods for Deprecated methods
How to use?
Register slash commands:
Manually describe parameter
Screenshots:
![animal_manual_execution_example](https://github.com/Revxrsal/Lamp/assets/85439143/66f5ee4c-5ce8-44f7-b832-9a37989e2ee1)
If we don't want to manually write parameter, we can do like this: Automatic parameter description
Screenshots:
![animal_manual_execution_example](https://github.com/Revxrsal/Lamp/assets/85439143/c2d3ddf2-634b-491d-8f97-2f8e53583f15)
Also we can use other parameters too, we can define complex commands like this:
Screenshots: Autocomplete:![autocomplete_complex_example](https://github.com/Revxrsal/Lamp/assets/85439143/ecc49261-8e70-46fe-b804-b80b69f2654c)
Proper typing:![integer_type_example](https://github.com/Revxrsal/Lamp/assets/85439143/7116d39b-a165-4dea-907a-c70bf90af7a8)
When we execute command:![full_input_example](https://github.com/Revxrsal/Lamp/assets/85439143/c4ebe891-c113-4684-b1a8-68c372777255)
Switch parameter:
![switch_parameter_execution](https://github.com/Revxrsal/Lamp/assets/85439143/80505d4a-fe63-42f5-b79c-8fa01b4d527c)
Note: We can remove
@Named
annotation, but it will result naming our parameters: arg0, arg1, arg2...Limitations
If we read documentation of discord slash commands, we can see that we cannot define long chain of subcommands, cannot have base command and subcommand at the same time according to:
Note: Every edge case was handled in
/core/SlashCommandConverter
.And we can see how this pull request handles such edge cases:
Expected exception
This is considered invalid because
permission
have at the same time subcommand (add
andremove
), and command implementation (@DefaultFor("permission")
)Another similar limitation
Expected exception:
Where we have two paths
permission add
andpermission add hide
. Wherepermission add
is base path, andpermission add hide
is subcommand of base path. And base path have 'implementation' and 'subcommand' at the same time, which doesn't allowed.Another problem is, we cannot have too deep subcommands. For example it is impossible create such command path in discord:
/permission firstsubcommand secondsubcommand thirdsubcommand
Because we can only create subcommand groups, and subcommands.Related issue: https://github.com/Revxrsal/Lamp/issues/50
If you have any questions, feel free to ask :)