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

The slash commands wrapper needs to become more declarative #1462

Open
the-unbot opened this issue Aug 7, 2021 · 17 comments
Open

The slash commands wrapper needs to become more declarative #1462

the-unbot opened this issue Aug 7, 2021 · 17 comments
Labels
feature-request A new requested functionality. framework Related to the `framework` and `framework::standard` modules and/or the `command_attr` crate

Comments

@the-unbot
Copy link

the-unbot commented Aug 7, 2021

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.

@arqunis arqunis added feature-request A new requested functionality. framework Related to the `framework` and `framework::standard` modules and/or the `command_attr` crate labels Aug 7, 2021
@zedseven
Copy link

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.

@MrDogeBro
Copy link

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

@vicky5124
Copy link
Contributor

@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.

@zedseven
Copy link

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

@Milo123459
Copy link
Contributor

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
Copy link
Contributor

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

1 similar comment
@Milo123459
Copy link
Contributor

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

@Angelin01
Copy link

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.

@tiritto
Copy link

tiritto commented Nov 28, 2021

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.

@Milo123459
Copy link
Contributor

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

@anden3
Copy link
Contributor

anden3 commented Nov 28, 2021

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

@Gentoli
Copy link
Contributor

Gentoli commented Dec 29, 2021

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.

@arqunis
Copy link
Member

arqunis commented Dec 29, 2021

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

@kangalio
Copy link
Collaborator

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

@vidhanio
Copy link
Contributor

vidhanio commented Dec 4, 2023

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!

@GnomedDev
Copy link
Member

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

@vidhanio
Copy link
Contributor

vidhanio commented Dec 4, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A new requested functionality. framework Related to the `framework` and `framework::standard` modules and/or the `command_attr` crate
Projects
None yet
Development

No branches or pull requests

14 participants