Implementing new Discord features: Interaction & Slash commands

[quinchs]

Hello, I've started writing support for the new discord interaction features.

I think we should talk about Slash commands here, There are some questions I think we all have:

• How will Slash commands be written?
• Will they take on the same style as writing normal commands?
• Will Discord.Net.Commands handle invoking and managing Slash Commands?

I have wrote in some methods for the RestClient to get, create, edit, and delete global and guild slash commands, I've also created some interfaces to use for Interaction with discord.

[quinchs]

#1716

[Still34]

Thanks for the contribution! However, before the changes can be moved forward to any future code review, please discard any non-essential changes in the csprojs and add proper documentation for all public members. Thanks!

[SubZero0]

I'm aware the latest pushes added an analyzer that complains a lot, but please ignore it and make your classes consistent with the others (like removing the spaces between properties etc). I'll review when this PR gets closer to completion.

[Still34]

Temporarily disabled the style enforcement via 36de7b2

[quinchs]

fae3fbe fixes the above mentioned issues and adds comments to everything except SocketInteraction

[quinchs]

If some people want to test the new event in the socket client, and let me know if there's any issues. I've tested it a bit and have found no issues.

There are still some stuff todo though:

• Implement some way to create Application commands (Read here)
• Implement file uploads for Interaction responses

I think the best way we can implement writing slash command is with the default Discord.Net.Commands command service, I'm not too familiar with that project so if someone else could take a look at it that would be great

[quinchs]

I'm thinking more about command service stuff, should Slash Commands be written with attributes?

[SlashCommand("CommandName", "Description", new Option()
{
    "user",                  // option name
    "a user",               // option description
    OptionType.User, // option type
    true,                     // required?
},
new Option() 
{
    "reason",                // option name
    "the reason",          // option description
    OptionType.String, // option type
    true,                       // required?
})]
public async Task TestCommand(SocketGuildUser user, string reason)
{

}

There are a lot of parameters for slash commands so its kind of hard to decide how to implement them. The downside to the attribute method is that it gets messy really fast

[FiniteReality]

[SlashCommand("CommandName", "Description", new Option()
{
    "user",
    "a user",
    OptionType.User,
    true,
},
new Option() 
{
    "reason",
    "the reason",
    OptionType.String,
    true,
})]
public async Task TestCommand(SocketGuildUser user, string reason)
{

}

Won't work. Attribute parameters must be constants, which means no new.

[quinchs]

Won't work. Attribute parameters must be constants, which means no new.

Just realized that. This is going to be complicated...

Another possibility is to make a class for your slash command, something like this:

public class MySlashCommand : SlashCommandBase
{
    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?
        base.Create("some arguments");
    }

    public async Task HandleExecute(SocketInteraction arg)
    {
        await arg.RespondAsync("Success!");
    }
}

[asmejkal]

Nice job! Maybe the command service changes could be a different PR? Since that's a bit separate from the implementation of the API on the client, and probably going to be complicated. Not everyone needs the Discord.Net command service to use this.

[quinchs]

Nice job! Maybe the command service changes could be a different PR? Since that's a bit separate from the implementation of the API on the client, and probably going to be complicated. Not everyone needs the Discord.Net command service to use this.

I agree, Im going to make a mockup command service for SocketInteractions as a reference. We do still need a public way of fetching, creating, modifying, and deleting Global/guild application commands. I'm going to work on that now, and then I think we're ready for a review!

[quinchs]

I'm unable to test the functionality of the new rest methods until tomorrow so you are all free to go let me know what to fix lol

[quinchs]

All the rest functionality works now, and the socket stuff also works.

[Mijago]

I'm thinking more about command service stuff, should Slash Commands be written with attributes?

[SlashCommand("CommandName", "Description", new Option()
{
    "user",                  // option name
    "a user",               // option description
    OptionType.User, // option type
    true,                     // required?
},
new Option() 
{
    "reason",                // option name
    "the reason",          // option description
    OptionType.String, // option type
    true,                       // required?
})]
public async Task TestCommand(SocketGuildUser user, string reason)
{

}

There are a lot of parameters for slash commands so its kind of hard to decide how to implement them. The downside to the attribute method is that it gets messy really fast

I may be a bit late to the party, but couldn't we parse the currently existing parameter attributes and use them for the SlashCommand?

For example, my current Bot looks like this:

[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");
}

And we could simply modify it to something like this:

[Command("item")]
[SlashCommand("item")] // NEW
[Summary("Search for an item in the manifest.")]
public async Task ShowItems(
    [Remainder]
    [Required] // NEW (if required)
    [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

[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)
        {

        }
    }

[Mijago]

@Cenngo
I don't think choices should be (only) hard-coded, as sometimes it would be nice to add/remove choices depending on the permissions or other situational things. Or must they be set at the registration of the command? Either way, setting them at launch would also be good.

[ZargorNET]

As mentioned before, using Attributes will get really messy. I mean just look at your own code @Cenngo and now imagine more parameters.
I personally think we should just register commands at startup in one's main method via a builder.
Something like:

    client.RegisterSlashCommand(
                new SlashCommandBuilder("testCommand")
                .AddParameter("name", "type", "description", ["choices"])
                .AddParameter(...)
                .MakeGlobal()
                .AddSubCommand(new SlashCommandBuilder(...)...)
                .Handler(typeof(TestCommandHandler))
                .Build()
            )

Inside the handler, which is a class that inherits from - let's say - SlashModuleBase<CommandContext> where you define the methods that get called. In order to map them correctly, we could use Attributes above each method to indicate which (sub)command belongs to which method.
I think that this will result in the most readable code.

[quinchs]

@ZargorNET @Mijago @Cenngo I'm going to open a new PR for implementing slash commands to the command service, this pull is just implementing them into the client.

I like the idea of a SlashCommandBuilder and having attribute based commands, I feel like we should support both attribute and class based. We could have the command service boil those into a base object that represents a slash command.

The reason why I think we should support both is both are easy to write, understandable, and can be interpreted easily with reflection. With my bots and custom command service I have each command in its own class and its much more manageable to write commands that way. Another + for class based slash commands is the class is the context, meaning your context and useful methods lives in this.

Attributes are good because it would be very easy to migrate/add slash commands to your existing commands, meaning you don't have to re-write all your commands. The only down side I see with attributes is there's gonna be a lot of them.

Tomorrow I'l open a PR for discussion and testing the command service with slash commands.

[quinchs]

Updates and TODO

After looking at the changes with slash commands on discord site, this PR is not merge ready anymore. There are some issues to address:

Invalid follow up message types

When sending follow up messages, the API route is based off of websocket and how the current build parses it is to a RestUserMessage.

The issue with this is that you should be able to Modify and Delete your follow up messages based off of the docs. Currently this is unsupported. TODO

Batch edit application command permissions

Currently this is not supported at all, this is a TODO.

Get Guild Application and Application Command permissions

This is currently not supported either, this is a TODO.

Edit and delete original response.

This is also currently unsupported with the current build... Add it to the TODO

Bulk Overwrite Guild Application Command

This is a very useful route and not supported in the build. You guessed it! TODO

Bulk Overwrite Global Application Command

Same as above but for global commands. TODO

default_permission for creating/modifying/getting Guild/Global Application commands.

They are in the rest entities, there's just no way to current set/get them. TODO

Better error messages for 400's on creating application commands.

Discord was really kind to include an error json structure telling you exactly what you did wrong when creating application commands. It would be very helpful if this was included in the exception when creating application commands. TODO

Conclusion

I'm sure I missed a lot of other small changes that need to be implemented but this gets the point across. This PR is still missing a lot of features and I think we should fix them before setting this pull to ready for review.

TL;DR: PR not done but it works.

If you want to help write any code for this PR you are more than welcome to work on any of the TODO items and create a PR over on my fork and I will merge them here. I can't promise I will work on this pr often as I have school and a software job and unfortunately the Discord.net team doesn't pay me enough 🤣. I will pop in every now an then to do some patches and fix any bugs I find while using it, I will also respond to comments.

[quinchs]

The issue i see in this PR is that the same instance of slashmodules is used forever and dependency injection isnt implemented.
Ive made a fork of this which fixes some exceptions and tweaks which "fix" what ive mentioned above.
Since its mainly for personal use, there is a breaking change for registering commands (78ada14) which prevents getting rate limited when you have over 5 commands but removes the ability to keep existing commands.

@quinchs i think you can take some of the stuff i changed over to this PR.

@NotOfficer After going in depth with your branch, it looks like you solved some of the issues I was experiencing. At some point soon I will start merging the two as some stuff has changed with the api

[Slamerz]

The issue i see in this PR is that the same instance of slashmodules is used forever and dependency injection isnt implemented.
Ive made a fork of this which fixes some exceptions and tweaks which "fix" what ive mentioned above.
Since its mainly for personal use, there is a breaking change for registering commands (78ada14) which prevents getting rate limited when you have over 5 commands but removes the ability to keep existing commands.
@quinchs i think you can take some of the stuff i changed over to this PR.

@NotOfficer After going in depth with your branch, it looks like you solved some of the issues I was experiencing. At some point soon I will start merging the two as some stuff has changed with the api

Is the dependency injection issue taken care of in this branch?

[AraHaan]

[SlashCommand("CommandName", "Description", new Option()
{
    "user",
    "a user",
    OptionType.User,
    true,
},
new Option() 
{
    "reason",
    "the reason",
    OptionType.String,
    true,
})]
public async Task TestCommand(SocketGuildUser user, string reason)
{

}

Won't work. Attribute parameters must be constants, which means no new.

Not entirely true, you can use new string[] {} in parameters in attributes (even if it's properties inside the attribute as well it will work).

[GuyInGrey]

What about this: Instead of using attributes, have the class inherit an interface. That interface has a method you need to implement that will return the slash command, using some SlashCommandBuilder class.

[AraHaan]

What about this: Instead of using attributes, have the class inherit an interface. That interface has a method you need to implement that will return the slash command, using some SlashCommandBuilder class.

Why not have it like how we make commands currently, but have an optional bool value that if set to true makes it a slash command.

And with it optional it would be by default false so it does not break anyone at all either.

[GuyInGrey]

What about this: Instead of using attributes, have the class inherit an interface. That interface has a method you need to implement that will return the slash command, using some SlashCommandBuilder class.

Why not have it like how we make commands currently, but have an optional bool value that if set to true makes it a slash command.

And with it optional it would be by default false so it does not break anyone at all either.

I mean, from what I'm seeing, there's a lot more to slash commands (Sub command groups and such) than can be effectively created within an attribute

[nitz]

What about this: Instead of using attributes, have the class inherit an interface. That interface has a method you need to implement that will return the slash command, using some SlashCommandBuilder class.

Why not have it like how we make commands currently, but have an optional bool value that if set to true makes it a slash command.

And with it optional it would be by default false so it does not break anyone at all either.

I mean, from what I'm seeing, there's a lot more to slash commands (Sub command groups and such) than can be effectively created within an attribute

Could that not be handled by the way sub commands are handled with the [Group] attribute now (Command "Submodules")?

[GuyInGrey]

What about this: Instead of using attributes, have the class inherit an interface. That interface has a method you need to implement that will return the slash command, using some SlashCommandBuilder class.

Why not have it like how we make commands currently, but have an optional bool value that if set to true makes it a slash command.

And with it optional it would be by default false so it does not break anyone at all either.

I mean, from what I'm seeing, there's a lot more to slash commands (Sub command groups and such) than can be effectively created within an attribute

Could that not be handled by the way sub commands are handled with the [Group] attribute now (Command "Submodules")?

That could work, yeah. One big class represents the whole slash command. Sub commands are methods, and subcommandgroups are sub-classes, with their own methods as commands.

SlashCommand1
  SubCommand1
  SubCommandGroup1
    SubCommand3

[SlashCommand("Name", "Description")]
public class SlashCommand1 : SlashCommand
{
  // Option types are taken from the parameter types. No sense in defining them twice.
  [SlashCommand("Name1", "Description")]
  public async Task SubCommand1([SlashOption("Name", "Description")]SocketGuildUser user) { }
  
  public class SubCommandGroup1 {
    [SlashCommand("Name2", "Description")]
    public async Task SubCommand2() { }
  }
}

[AraHaan]

I think the best option is to use the pre-existing attributes, and use the Groups attributes like we have now, but every command in a group must be set to be a slash command in the same current attribute that makes them a normal command (just would have to set the optional value to make them a slash command to true).

[christopherfowers]

With the recent chatter and the PR owners statements (@quinchs) I’m not sure where this thing actually is in terms of getting pushed along to being usable. Where is slash command support currently? Where is this wonderful group of people trying to take it? And how can I help?

[quinchs]

With the recent chatter and the PR owners statements (@quinchs) I’m not sure where this thing actually is in terms of getting pushed along to being usable. Where is slash command support currently? Where is this wonderful group of people trying to take it? And how can I help?

The lib maintainers are reworking dnet, once thats done il update the PR's as they are currently out of date.

Slash commands and message components are being developed in Discord.NET Labs. They are stable enough to use there and thats where the PRs to dnet will originate. If you want to help out you can check the labs repo and join our Discord server

[quinchs]

Closing as this is implemented in #1923

[leocb]

Awesome work! Is there a documentation or example project we can use to reference these new features?

[quinchs]

Awesome work! Is there a documentation or example project we can use to reference these new features?

The docs are sort of limited but yes, The readme of Discord.Net Labs has some documentation of the new features

[ghost]

Hi,

When will this be implemented in Discord.NET instead of labs?

[leocb]

AFAIK it's already merged but there's no oficial nuget yet, you can download the source and compile it yourself if you want to start using it

[Francoimora]

And when will it be available in the official nuget streamline ? We use CI/CD so it's easier for us to setup if we just have to update the nuget.

[AraHaan]

Remora.Discord supports slash commands as well, so if you want you could switch to that.

I have also migrated from Discord.NET and overall it simplified my bot to the point 75% of it's code was due to problems with Discord.NET and hacks to try to mitigate them. After the migrations I was able to nuke all of it and it would work the same.

[Francoimora]

That's a shame but I think I'll give it a try soon, thanks.

[quinchs]

AFAIK it's already merged but there's no oficial nuget yet, you can download the source and compile it yourself if you want to start using it

Its currently still in a PR. its a big pr so it will take a bit to get reviewed and refined

[ghost]

Alright, lets hope it will be merged soon and by available.

[djinnet]

I am really curious for what is the currently status with the PR?
It looks really close to be done and all the checks has passed green, so what precisely is holding the big PR back?