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

Slash command service #1733

Closed
wants to merge 23 commits into from
Closed

Conversation

quinchs
Copy link
Member

@quinchs quinchs commented Dec 27, 2020

Summary

This PR is for discussion and implementation of a command service for #1717.

Discussion, questions, and feedback is welcome as this PR is based off of how you want slash commands to be written.

This PR also includes changes made in #1717

Issues to overcome

You should not need to rewrite all your current commands to implement slash commands. Basically, the slash command service should be able to use the attributes from the base command service, like Command, Summary, Name, etc.

A proposed idea from @Mijago is as follows:

Couldn't we parse the currently existing parameter attributes and use them for the SlashCommand?

[Command("item")]
[Summary("Search for an item in the manifest.")]
public async Task ShowItems(
    [Remainder]
    [Name("Name")]
    [Summary("Specify the name of the item here. You can use RegExp to give a more specific search term.")]
    string name)
{
    await ShowItemsLang(name, "en");
}

This way, we would have all the information your example above would also have. The types are already defined, so I doubt there is a need to re-define them.

  • Name and description of the command
  • Name and description of the argument
  • Type of the argument
  • Whether the argument is required or not

Another form of attribute based commands was suggested by @Cenngo:

I think creating choices can also be accomplished by attributes since there are only 2 fields that need to be filled. I don't think there will be a lot of people who will add more than a handful of choices per option so i think this should'nt be that messy, relatively speaking.

[SlashCommand("rainbow6", "description")]
    public class Rainbow6
    {
        [SubCommandGroup("player", "description")]
        [SubCommand("casual", "description")]
        public async Task GetCasual ( [Description("")] [Required]string username, 
            [Description("")] [Choices("UPlay", "uplay")][Choices("XBox Live", "xbl")][Choices("PlayStation Network", "psn")]string platform)
        {

        }

        [SubCommandGroup("player", "description")]
        [SubCommand("ranked", "description")]
        public void GetRanked ( [Description("")] [Required]string username, 
            [Choices("UPlay", "uplay")][Choices("XBox Live", "xbl")][Choices("PlayStation Network", "psn")] string platform )
        {

        }

        [SubCommandGroup("operator", "description")]
        [SubCommand("stats", "description")]
        public void GetStats ( [Description("")] [Required]string username,
            [Choices("Seasonal", "seasonal")][Choices("Overall", "overall")]string scope, 
            [Choices("Close", 5)][Choices("Long", 10)]int saesons)
        {

        }
    }

Changes

The first comit of this PR adds the SlashCommandBuilder to the command service, as suggested by @ZargorNET.
The SlashCommandBuilder is used to build SlashCommandCreationProperties in a much nicer way.

Questions

  • Should a slash command service/handler be a new project? For example: Discord.Net.SlashCommands
  • Should a slash command service implemented into CommandService or should it be its own service?

quinchs and others added 11 commits December 16, 2020 18:49
…correct documentation. Added AlwaysAcknowledgeInteractions to the socket client config
New Rest entities: RestApplicationCommand,RestGlobalCommand, RestGuildCommand, RestApplicationCommandOption, RestApplicationCommandChoice, RestApplicationCommandType.

Added public methods to the RestClient to fetch/create/edit interactions.
…LICATION_COMMAND_CREATE gateway events.

Added SocketApplicationCommands, Added method in SocketGuild to fetch that guilds ApplicationCommands.

Tested all rest routes and fixed them accordingly.

Did more testing and I think its ready to go
Copy link

@ZargorNET ZargorNET left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should a slash command service/handler be a new project? For example: Discord.Net.SlashCommands

I think yes. It'd go well with the current design pattern and with this way you could omit unused files to speed up compilation time and reduce binary file size. And on top of that, you can easily replace for example the current CommandService to your own (because to be honest, it's not that good in the current state)

docs/guides/commands/application-commands.md Show resolved Hide resolved
docs/guides/commands/application-commands.md Show resolved Hide resolved
docs/guides/commands/application-commands.md Show resolved Hide resolved
@TheHeartOfFire
Copy link

Really looking forward to this feature. How can i help?

@quinchs
Copy link
Member Author

quinchs commented Jan 3, 2021

If you want to help write the nitty gritty reflection with me, just dm me on discord: quin#3017

@Mijago
Copy link

Mijago commented Jan 28, 2021

Hi, anything i could do to help you with this PR?
I dont have much spare time but as I would love to see this feature, I'd like to help, even if it would only be concept work.

@quinchs
Copy link
Member Author

quinchs commented Jan 28, 2021

Providing ideas on how you would want to write slash command and how a slash command service should work would be ideal if you don't have a lot of time. I have also run out of free time because of work and school so I can only collect ideas as of now.

If you want to just write code you can make a PR on my fork and I'll check it out but be warned writing a command service requires the bravest of nerds due to the godsent amount of reflection.

@ginomessmer
Copy link

Hi @quinchs. Thanks for your work so far.

If you want to just write code you can make a PR on my fork and I'll check it out but be warned writing a command service requires the bravest of nerds due to the godsent amount of reflection.

I'm new to this repository but I would love to take a look. Could you please briefly describe the current state of your work and elaborate on the pending work? That would help others to take over your progress if necessary.

…poses and to be a starting-off point for newcomers to the slash command ecosystem,
@quinchs
Copy link
Member Author

quinchs commented Feb 10, 2021

Could you please briefly describe the current state of your work and elaborate on the pending work?

The support for slash commands (like creating/receiving them) is done in #1717 while a SlashCommandService still needs to be worked on.

Me and @TheHeartOfFire have worked a tad on the service and we decided we want to support class based slash commands and attribute based, what I mean by this is that the service will allow command as seen below

Attribute based

[Command("item")]
[Summary("Search for an item in the manifest.")]
public async Task ShowItems(
    [Remainder]
    [Name("Name")]
    [Summary("Specify the name of the item here. You can use RegExp to give a more specific search term.")]
    string name)
{
    await ShowItemsLang(name, "en");
}

Class based

//                                                 allow DiscordSocketClient or DiscordShardedClient
public class MySlashCommand : SlashCommandBase<DiscordSocketClient>
{
    private DiscordSocketClient client;
    public MySlashCommand(DiscordSocketClient client)
        : base()
    {
        // Init
        this.client = client;

        // Maybe listen for this command with an event?
        this.Executed += HandleExecute;

        // Register the command here with all the command args?
        // Would use the SlashCommandBuilder here? https://github.com/quinchs/Discord.Net/blob/SlashCommandService/Discord.Net.SlashCommands/Builders/SlashCommandBuilder.cs
        base.Create("some arguments");
    }

    // Instead of passing the raw interaction, maybe use a context like https://github.com/quinchs/Discord.Net/blob/SlashCommandService/src/Discord.Net.Commands/SlashCommands/Types/SlashCommandContext.cs
    public async Task HandleExecute(SocketInteraction arg)
    {
        await arg.RespondAsync("Success!");
    }
}

Although is could all be changed I feel like most people would use one of these methods, and reflection could boil methods down to a single class like this https://github.com/quinchs/Discord.Net/blob/SlashCommandService/src/Discord.Net.Commands/SlashCommands/Types/SlashCommandModule.cs then just execute based on that. None of the reflection is done yet as I didn't get time to work on it.

TL;DR: reflection needs to be written, basically everything needs to be written.

…parse arguments, nor does it register commands. Also added to the aforementioned dev-test project.
Details:
Subcommands and Subcommand groups not yet implemented, they will require for some parts of the code to be re-done. More attributes can and should be implemented, such as [Required] and [Choice(... , ...)].
Breakdown:
* Rectified line endings to LF, as per the settings of the project.
* Added a new command to SlashCommandService and SlashCommandServiceHelper to register the found commands to discord.
* Implemented CommandRegistrationOptions that can be used to configure the behaviour on registration - what to do with old commands, and with commands that already exist with the same name. A default version exists and can be accessed with CommandRegistrationOptions.Default
* Modified the sample program to reflect the changes made to the SlashCommandService and to also register a new command that tests all 6 types of CommandOptions (except subcommand and subcommand group)
* At the moment all commands are registered in my test guild, because the update for global commands is not instant. See SlashCommandServiceHelper.RegisterCommands(...) or line 221.
* Modified SlashCommandInfo to parse arguments given from Interaction, unde in ExecuteAsync, and added method BuilDcommand that returns SlashCommandCreationProperties - which can be registered to Discord.
* Renamed in the sample project PingCommand.cs to DevModule.cs
* Added custom attribute Description for the command method's parameters.
* Implemented SlashParameterInfo - and extension of the OptionBuilder that implements a method name Parse - takes DataOptions and gives out a cast object to be passed to the command Delegate. Planning on doing more with it.
* Moved SlashCommandBuilder.cs to the same directory structure
* Moved SlashCommandModule.cs and ISlashCommandModule.cs to its own folder.
Details:
To implement them I had to get creative. First thing i did was manually register a command that uses sub commands and sub command groups. Two things I noticed immediately:
1) I can create a subcommand on a "root" command - where no SubCommandGroup is used
2) The current implementation of the Interactions doesn't know what type of value an option is. Good thing is that there is only 1 option when querying subcommands and subcommand groups, so I can find out what the "path" of the subcommand is.
TOP/root/rng
TOP/root/usr/zero
TOP/root/usr/johnny (i misspelled it in the source files, woops)
[See SlashCommandsExample/DiscordClient.cs]

Next I wanted to make command groups (I'll use this term as to mean a slash command with subcommands and regular slash command groups) to be implemented in code in a sort of hierarchical manner - so I made them classes with attributes. Unfortunately to make this work I had to make them re-inherit the same things as the base module - UGLY but I see no other option to do this other than making them inherit from another class that remembers the instance of the upper class and implements the same methods aka a whole mess that I decided I won't want to partake in.
[See SlashCommandsExample/Modules/DevModule.cs]

Next-up is to search for these sub-groups. I decided that the most intuitive way of implementing these was to make SlashModuleInfo have children and parent of the same type -- from which arose different problems, but we'll get to that.
So I gave them some children and a parent and a reference to the CommandGroup attribute they have on themselves. The boolean isCommandGroup is unused, but could be useful in the future... maybe. Also I've added a path variable to internally store structure.
I wanted (after the whole reflections business) for commands to be easly accessed and deal WITH NO REFLECTION because those are slow, so I changed the final string - SlashCommandInfo dictionary to containt paths instead of command infos, something like what I exemplefied above.

In any case, I edited the service helper (the search for modules method) to ignore command groups and only store top level commands. After that I made a command to instantiate command groups, and the command creation and registration were changed as to be recursive - because recurion is the simpest way to do this and it's efficient enough for what we want - we only run this once anyway.

The biggest change was with command building - commands no longer build themselves, but now we command each module to build itself. There are 3 cases:
Top-Level commands
Top-Level subcommands (or level 1 command group)
subcommands within slash command groups

The code is uncommented, untidy and I'll fix that in a future commit. One last thing to note is that SlashCommands can have 0 options! - fixed that bug. Also SlashCommandBuilder.WithName() for some reason was implemented wrongly - I pressume a copy-paste error,
Also I implemented 0 types of enforcing rules - I'm going to leave this to other people to do.
… in particualr guilds.

Details:
There's nothing much to say, just added a variable to track with modules/commands/command groups have the global attribute and depending on that I register them globally or locally. There is currently no way to implement two commands with the same name, but one on the guild level and one on the global level. There is also currently no implemented way to register only some commands to only some guilds - this could be done through attributes, but another solution for users who want to do complex stuff would be to just give them the built commands and let them maually register them as they see fit.
Details:
There is nothing really to be said, it's simple and self-explanatory. The only thing I would want to implement is to add support for bool? and int? param types because, as of now, you don't know if the user passed False or just didn't pass anything. In any case, I personally don't suspect it's going to be hard to do. After I implement this I will create a PR on the original Discord.Net Repo.
…dCommands for those who want to register commands themselves and added support for bool? and int?. There will be a PR soon.
@quinchs
Copy link
Member Author

quinchs commented Feb 18, 2021

Merged @SlenderPlays PR on my repo because it looks like a good start to work off of.

@MoonieGZ
Copy link
Contributor

Heads up, been trying this around a little bit and it hits an error in
Discord.Net.WebSocket\Entities\Interaction\SocketApplicationCommand.cs line 59

model.Options can be null and adding a null check seems to fix it.

@quinchs
Copy link
Member Author

quinchs commented Feb 25, 2021

Il take a look, keep in mind this pr is not done and may contain lots of bugs

@MoonieGZ
Copy link
Contributor

I am aware, I'm very happy with the experiments I'm able to run with it so far, thank you so much for your work!

@quinchs
Copy link
Member Author

quinchs commented Feb 25, 2021

Thank you for using it!

@erxkk
Copy link

erxkk commented Feb 25, 2021

Quick heads up: after somebody had problems with the pr throwing a null ref internally:
SlashCommands/SlashCommandServiceHelper.cs#L42 does not account for Interfaces, which have no BaseType and thus throws a null ref if an interface is declared inside the assembly.
I did not try out the pr myself, but this is my conclusion after they sent some debugger info.

@webmilio
Copy link

webmilio commented Feb 25, 2021

Quick heads up: after somebody had problems with the pr throwing a null ref internally:
SlashCommands/SlashCommandServiceHelper.cs#L42 does not account for Interfaces, which have no BaseType and thus throws a null ref if an interface is declared inside the assembly.
I did not try out the pr myself, but this is my conclusion after they sent some debugger info.

Should be fixed (on another branch). Instead of checking for null we check for !typeInfo.IsAbstract && !typeInfo.IsInterface .

@george-cosma
Copy link

Just as an FYI, I've written my implementation of the slash command service with the goal to work, with little regard to programming norms. My biggest "sin" in all of this has to be The Command Service Helper. It's just an excuse to put all of the ugly code in. Personally, if I were to do it again I would split it in two - one for transforming the code that we get from the assembly into INFO's and another to handle them properly - in this section, I would include just the registration bit, but I can totally see preconditions and other stuff like that be introduced here as well.

Also, it really needs to be cleaned up. This whole block could be totally put into a function that takes as a parameter the type of attribute we want to test for. Also, a lot of stuff is public when in reality we don't really want it to be.

To finish off, there are a lot of bugs, and I'm not super proud of this code, but I will let you folk with way more experience in building an API handle all of my war crimes against programming itself 😃 .

@MoonieGZ
Copy link
Contributor

MoonieGZ commented Mar 1, 2021

Potentially dumb question, I notice in the sample there is a comment section about registering commands manually for example only certain commands in certain guilds...

Is this possible with this PR? If so, any pointers on implementing it? I'm prepping my servers for when this becomes "official".

Ideally (just throwing out ideas right now, don't burn me please), it'd be really nice to have some tags/attributes to restrict it to certain guild IDs

@quinchs
Copy link
Member Author

quinchs commented Mar 1, 2021

Currently idk. Will it be added? Yes.

@george-cosma
Copy link

Potentially dumb question, I notice in the sample there is a comment section about registering commands manually for example only certain commands in certain guilds...

Is this possible with this PR? If so, any pointers on implementing it? I'm prepping my servers for when this becomes "official".

Technically you can do that right now but I didn't bother to test it. The purpose of the BuildCommands method is to build the commands and give you the option to manually register them.
Here's a bit of pseudo-C#.

// Build the commands
List<SlashCommandCreationProperties> commands = await _commands.BuildCommands();

ulong guild1 = ... ;
ulong guild2 = ... ;

// Register half of the commands to guild1
for(int i = 0; i < commands.Count/2; i++)
{
    await socketClient.Rest.CreateGuildCommand(commands[i], guild1);
}

// and the other half to guild2
for(int i = commands.Count/2; i < commands.Count; i++)
{
    await socketClient.Rest.CreateGuildCommand(commands[i], guild2);
}

As I've said before, I didn't test this and there is a non-zero chance that there is a bug out there somewhere. I'm a bit swamped right now so I can't test it myself.

I also recommend taking a look at this class.

Ideally (just throwing out ideas right now, don't burn me please), it'd be really nice to have some tags/attributes to restrict it to certain guild IDs

Don't worry about being burned. I did want to implement a tag such as

[Guild(0301...)]
[Guild(0204...)]
public async Task PingAsync()
{
...
}

But the issue is that all of the uids have to be constant, so you can't set the guild ids from a config file or from anywhere for that matter. There might be a way to do this programmatically, but I'm not sure. I guess for people that make their bot for a particular discord server this might work, but otherwise I don't think this could scale.

I guess if there is a demand for it then it might be useful to implement.

@MoonieGZ
Copy link
Contributor

MoonieGZ commented Mar 3, 2021

Appreciate the answer @SlenderPlays my current thought process is to have a "normal" bot command to register either individual commands or all of them, that way you're getting the guild ID from the context in which the command is run (therefore not in code)... However, I can see this causing problems when the bot is brought offline for example.

Then again this is all theoretical because the person whose server I'm trying to implement these on has never gotten back to me to give me the permissions to even create slash commands so doesn't really matter if I don't find a solution right away...

{
class Program
{
static void Main(string[] args)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: You can declare Main as Task Main(string[] args) and simply await your RunAsync() call to avoid mixing synchronous and asynchronous code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't really matter here, as they both ultimately compile down to the same IL internally; at least, as far as this example is concerned.

@Cenngo
Copy link
Collaborator

Cenngo commented Jul 6, 2021

After 3 days worth of speed programming i finally got a working prototype that incorporates the REST, Web Socket, Message Component and Command Service functionalities. I'm currently in the process of creating the documentation but the code needs a reasonable amount of polishing. Since I don't have much free time and i had a different vision in mind i created my fork from ground up. Command registration is almost a carbon copy of the original Commands extension. Message component interactions are handled with an [Interaction] attribute.

[SlashGroup("utility", "utility commands")]
    public class Module : CommandBase<SocketSlashCommandContext>
    {
        [SlashCommand("echo", "repeat after me")]
        public async Task Echo (string text = "empty")
        {
            await Context.Interaction.PopulateAcknowledgement($"You just said: {text}");
        }

        [SlashCommand("animals", "animal commands")]
        public async Task FavouriteAnimal([Choice("Dog", "dog"), Choice("Cat", "cat")]string text)
        {
            await Context.Interaction.PopulateAcknowledgement($"your favourite animal is {text}");
        }

        [SlashGroup("user", "user management tools")]
        [SlashCommand("tag", "tag a user")]
        public async Task TagUser(IChannel channel, IUser user)
        {
            var socketUser = user as SocketUser;
            await (channel as SocketTextChannel)?.SendMessageAsync($"{socketUser?.Mention}: anan");
        }
    }

    public class SecondModule : CommandBase<SocketSlashCommandContext>
    {
        [SlashCommand("ping", "recieve a pong")]
        public async Task Ping ( )
        {
            var builder = new MessageUIBuilder();
            var select = new SelectMenuBuilder("select_song")
                .AddOption(new SelectOption("Never gonna give you up", "rickroll"))
                .AddOption(new SelectOption("I want it that way", "b99"))
                .WithPlaceholder("Select a song")
                .WithMaxValue(2)
                .Build();

            builder.AddRow(select);
            await Context.Interaction.PopulateAcknowledgement("pong", messageComponents: builder.Build());
        }

        [Interaction("select_song")]
        public async Task Song ( params string[] values )
        {
            await Context.Interaction.SendFollowupAsync($"you selected {string.Join(',', values)}");
        }

I'll keep working on this but if anyone wants to try it out you check out the fork here

@quinchs
Copy link
Member Author

quinchs commented Jul 8, 2021

@Cenngo If you could make a PR over on DNet Labs that would be great!

@Cenngo
Copy link
Collaborator

Cenngo commented Jul 8, 2021

@Cenngo If you could make a PR over on DNet Labs that would be great!

Sure, i will. Though since the last commit on this PR is from 6 months ago, i may have misjudged the state of development. I didn't know that repo existed.

@quinchs quinchs mentioned this pull request Sep 20, 2021
34 tasks
@quinchs
Copy link
Member Author

quinchs commented Sep 20, 2021

Closing as this is implemented in #1923

@quinchs quinchs closed this Sep 20, 2021
@JKamsker
Copy link

JKamsker commented Dec 2, 2021

Is there a replacement for this? @quinchs
Manually switching and seperately building commands isn't nice at all :(

@Cenngo
Copy link
Collaborator

Cenngo commented Dec 2, 2021

Is there a replacement for this? @quinchs Manually switching and seperately building commands isn't nice at all :(

Head over to D.Net-Labs for that.

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

Successfully merging this pull request may close these issues.