serenity icon indicating copy to clipboard operation
serenity copied to clipboard

The slash commands wrapper needs to become more declarative

Open the-unbot opened this issue 3 years ago • 17 comments

The current experience of processing and registering slash commands is just a madness. Especially the amount of .excepts (or .unwraps) needed to do to deserialize parameters.

I would love it if you added a procedural macro that would automatically implement the registration and deserialization of commands.

the-unbot avatar Aug 07 '21 00:08 the-unbot

At first glance, that's what I thought the command framework was for when initially looking at the library.

I wonder if it would be possible to integrate the existing command framework into Discord's slash commands. I'm not saying they're 1-to-1, but using the same (or similar) system for both would make it easier to learn and easier to migrate existing code over.

zedseven avatar Aug 31 '21 14:08 zedseven

I'm just use poise (a command framework for serenity) to handle slash command processing for me currently.

MrDogeBro avatar Aug 31 '21 14:08 MrDogeBro

@zedseven Denis was working on a new framework for serenity https://github.com/serenity-rs/framework/ because the current framework is limiting, due to is heavy use of macros, and a ton of static values; but development stopped due to discord announcing the message content intent, and now the plan is to either: 1: rework the new framework to be slash commands only 2: rework the new framework to support both slash and prefix commands, just like how poise does.

Option 2 would be best, but option 1 is what will be easiest, and it's what every other library has chosen to do.

vicky5124 avatar Aug 31 '21 15:08 vicky5124

Interesting - that makes sense. I think for the time being, I'll use poise and keep an eye on this.

zedseven avatar Aug 31 '21 15:08 zedseven

How about a macro implemented directly into serenity? Believe this is asking for something in the framework. For example:

let (name: &str, type: &str) = slash_command_option! {
   option name, type
}

That's just something I thought of on the spot.

Milo123459 avatar Sep 28 '21 17:09 Milo123459

Yes, that is exactly what I had in mind. This also doubles down as a way of using dropdown menus

// sub command group
enum Info {
	#[some_cool_macro(rename = "ping")] // this is the name of the slash command displayed to users
    Ping,
    #[some_cool_macro(rename = "stats")]
    Stats
}

// drop down menu
enum DropDown {
   #[some_cool_macro(rename = "guilds")]
   Guilds
}

let (command: &str, group: &str) = slash_command_sub_group! {
   Info
   // other sub command groups can go here, it'll deserailize it and tell you what command was run, optionally
   // telling you from which sub command group
}
// Now, if you do /ping you would get variables like this:
// command: "ping"
// group: "info" // group is what is returned for groups from the discord API.

// get options from the command (options are: global, fast)

let (global: bool, fast: Option<bool>) = slash_command_option! {
   option global, fast? // optional options use a ? at the end
}

// dropdown:
let (what: DropDown) = slash_command_option! {
   dropdown DropDown: what, // options....
}

Rough mock up. Would be very, very cool

Milo123459 avatar Sep 29 '21 19:09 Milo123459

Yes, that is exactly what I had in mind. This also doubles down as a way of using dropdown menus

// sub command group
enum Info {
	#[some_cool_macro(rename = "ping")] // this is the name of the slash command displayed to users
    Ping,
    #[some_cool_macro(rename = "stats")]
    Stats
}

// drop down menu
enum DropDown {
   #[some_cool_macro(rename = "guilds")]
   Guilds
}

let (command: &str, group: &str) = slash_command_sub_group! {
   Info
   // other sub command groups can go here, it'll deserailize it and tell you what command was run, optionally
   // telling you from which sub command group
}
// Now, if you do /ping you would get variables like this:
// command: "ping"
// group: "info" // group is what is returned for groups from the discord API.

// get options from the command (options are: global, fast)

let (global: bool, fast: Option<bool>) = slash_command_option! {
   option global, fast? // optional options use a ? at the end
}

// dropdown:
let (what: DropDown) = slash_command_option! {
   dropdown DropDown: what, // options....
}

Rough mock up. Would be very, very cool

Milo123459 avatar Sep 29 '21 19:09 Milo123459

Indeed this would be amazing.

To point out, the (now "dead", here's hoping for the forks) Discord.py library has a similar concept with Python's decorators and cogs.

It made making a simple, modular bot trivial. You just wrote a class with some methods, marked them with a few decorators and you were done, full command and group support.

I'm unfortunately new to the Rust space, so I can't contribute with the syntax discussion, but the end result with D.py was quite elegant, being something along the lines of:

import discord
from discord.ext import commands

class MyGroup(command.Cog):
    @commands.group()
    def my_group(self, ctx: discord.Context):
        pass
    
    @my_group.command()
    def my_command(self, ctx: discord.Context, my_arg: int):
        # some stuff
    
    @my_group.command()
    def my_other_command(self, ctx: discord.Context, foo: str):
        # some stuff

It made for very little boilerplate and repetition of information: parameters were gotten from the method's arguments, the name from the method name, etc. You, of course, can override any of these.

I think trying to port some of this "style" to Serenity would do wonders for the library.

Angelin01 avatar Nov 28 '21 02:11 Angelin01

I've been trying to implement some slash commands to my bot today and I must admit, it's very intuitive at best and just a pure madness at worst when you try to combine it with slash command permissions.

You declare your commands in one place, then set permissions in another place, and then run those commands in yet another place. And to make it all worse, a lot of struct and function names look basically the same way with just tiny differences, making you mistake one thing with another, and within just a moment you no longer have any idea what were you thinking about. It's confusing, both in code and in documentation.

Personally, I've just ended up giving up on implementing permission system to my slash commands, simply because just doing a quick and easy role/user check during command execution was much easier and cleaner.

While I am also just a newbie in Rust, so I can't help much with making things better just yet, I feel like it might be a good idea to elevate the slash command system "out of EventHandler space" and provide an option to map out your slash commands inside ClientBuilder.

#[tokio::main]
async fn main() {
    let mut client =
        Client::builder(TOKEN)
            .intents(GatewayIntents::all())
            .slash_commands(|slash_builder|

                slash_builder.create_command(|command| command
                    // Works pretty much the same way as CreateApplicationCommand
                    .name("do_things")
                    .description("does things")
                    .create_option(/* ... */)
                    .create_option(/* ... */)

                    // Allows us to plan out permissions right away
                    .permissions(|perms|
                        perms.create_permissions(|details| // ApplicationCommandPermissionData
                           details.kind(ApplicationCommandPermissionType::Role)
                               .id(RoleId(123456789012345678))
                               .permission(true))
                    )

                    // Allows us to implement an action that will be executed upon triggering
                    .action(|ctx, aci| {
                        /*
                        Here you define what action your command will execute when called.
                        aci stands for ApplicationCommandInteraction that would be passed over alongside ctx (Context).
                        Is expected to return CreateInteractionResponse
                        */
                    })
                )
            )
            .event_handler(Handler)
            .await
            .expect("Err creating client");
    if let Err(why) = client.start().await {
        println!("Client error: {:?}", why);
    }
}

With something like this, it would be much easier for newbies like me to implement new commands, everything would be much easier to handle, as everything is in the same place, and we could safely assume that the framework already knows what's the most performant and safe way to implement our slash commands.

tiritto avatar Nov 28 '21 19:11 tiritto

Personally, I disagree with having a builder. It looks long and un-needed. I still think a macro is the way to go.

Milo123459 avatar Nov 28 '21 19:11 Milo123459

In case anyone want some inspiration or just to add a possible example of such a macro, this is the macros I currently use to declare slash commands.

The interaction_setup macro generates a static struct with a reference to the command function, rate limits, etc, as well as a JSON value containing the slash command config as accepted by Discord. The command struct gets added to a static distributed slice matching the group name, which is then easily accessible to the guild initialization code.

unknown-48 unknown-56 unknown-139

anden3 avatar Nov 28 '21 19:11 anden3

maybe it can be integrated with clap 3 (structopt), they have derive based construction of cli command. Then the generated structure can be used to create/parse application commands.

PS: I might be able to do a POC for this.

Gentoli avatar Dec 29 '21 16:12 Gentoli

they have derive based construction of cli command

I am already developing a library of that nature. It is similar to structopt in that you attach #[derive(Command)] or #[derive(Group)] on structs to generate code for registering/parsing commands and subcommand groups, respectively. There is no documentation yet, but the surface API is very simple.

Minimal usage of the library involves just defining a single command, an enum with a #[derive(Commands)] (for consolidating registration and parsing of all commands at once), registering either with register_commands_globally or register_commands_in_guild, and a parse call. If the parse call is successful, you will be able to match on the #[derive(Commands)] enum to dispatch a command. You may find an example of this as a ping bot at:

https://github.com/acdenisSK/serenity_commands/blob/master/examples/e01_basic_ping_bot/src/main.rs

arqunis avatar Dec 29 '21 21:12 arqunis

My framework poise has become relatively popular for declarative slash commands (and text commands, and context menu commands)

kangalio avatar Sep 12 '22 08:09 kangalio

I've written my own derive macro for this in the style of clap.

https://github.com/vidhanio/serenity-commands

Extremely intuitive to use:

use serenity::all::{
    async_trait, Client, Context, CreateInteractionResponse, CreateInteractionResponseMessage,
    EventHandler, GatewayIntents, GuildId, Interaction,
};
use serenity_commands::{Command, Commands, SubCommand};

#[derive(Debug, Commands)]
enum AllCommands {
    /// Ping the bot.
    Ping,

    /// Echo a message.
    Echo {
        /// The message to echo.
        message: String,
    },

    /// Perform math operations.
    Math(MathCommand),
}

impl AllCommands {
    fn run(self) -> String {
        match self {
            Self::Ping => "Pong!".to_string(),
            Self::Echo { message } => message,
            Self::Math(math) => math.run().to_string(),
        }
    }
}

#[derive(Debug, Command)]
enum MathCommand {
    /// Add two numbers.
    Add(BinaryOperation),

    /// Subtract two numbers.
    Subtract(BinaryOperation),

    /// Multiply two numbers.
    Multiply(BinaryOperation),

    /// Divide two numbers.
    Divide(BinaryOperation),

    /// Negate a number.
    Negate {
        /// The number to negate.
        a: f64,
    },
}

impl MathCommand {
    fn run(self) -> f64 {
        match self {
            Self::Add(BinaryOperation { a, b }) => a + b,
            Self::Subtract(BinaryOperation { a, b }) => a - b,
            Self::Multiply(BinaryOperation { a, b }) => a * b,
            Self::Divide(BinaryOperation { a, b }) => a / b,
            Self::Negate { a } => -a,
        }
    }
}

#[derive(Debug, SubCommand)]
struct BinaryOperation {
    /// The first number.
    a: f64,

    /// The second number.
    b: f64,
}

struct Handler {
    guild_id: GuildId,
}

#[async_trait]
impl EventHandler for Handler {
    async fn ready(&self, ctx: Context, _: serenity::model::gateway::Ready) {
        self.guild_id
            .set_commands(&ctx, AllCommands::create_commands())
            .await
            .unwrap();
    }

    async fn interaction_create(&self, ctx: Context, interaction: Interaction) {
        if let Interaction::Command(command) = interaction {
            let command_data = AllCommands::from_command_data(&command.data).unwrap();
            command
                .create_response(
                    ctx,
                    CreateInteractionResponse::Message(
                        CreateInteractionResponseMessage::new().content(command_data.run()),
                    ),
                )
                .await
                .unwrap();
        }
    }
}

Then you can call AllCommands::to_command_data() in your ready handler to get a Vec<CreateCommand> to register, and call AllCommands::from_command_data(&command.data) to parse the command into the struct. From there you can work with regular old structs/enums as you usually would in Rust!

vidhanio avatar Dec 04 '23 18:12 vidhanio

You should see poise, it is under the serenity organisation now and is widely used.

GnomedDev avatar Dec 04 '23 18:12 GnomedDev

You should see poise, it is under the serenity organisation now and is widely used.

I have tried out poise and found it pretty good, but I ended up needing to drop down to serenity primitives so much that it just made more sense to write the entire bot in the "lower-level" serenity to get the fine tuning I wanted. I wrote this library because I found myself repeating a lot of the same parsing code with janky declarative macros, so I just wanted to write a nice proc-macro library to handle it easier.

Also, I wanted parameters to be in a struct rather than just normal function parameters, as I find being able to add methods to these structs (say, to convert them into an embed) very useful. It's a lot harder if you have, say, 6 options in the parameter list you have to pass around everywhere.

vidhanio avatar Dec 04 '23 18:12 vidhanio