Discord.Net icon indicating copy to clipboard operation
Discord.Net copied to clipboard

Preconditions Default to Base Command Upon Failing

Open FireController1847 opened this issue 4 years ago • 5 comments

The best way I can describe this is with a code example:

namespace Divibot.Modules.Moderation {

    [Name("Roles")]
    [Group("roles")]
    public class RolesModule : ModuleBase<SocketCommandContext> {

        [Command]
        [Name("roles")]
        [Summary("Provides a list of sub-commands that are able to be used in the roles group.")]
        [Priority(-1)]
        public async Task RolesAsync() {
            // do stuff
        }

        [Command("give")]
        [Summary("Gives the specified user the specified role.\n<user|userid|username> <role|roleid|rolename>)]")]
        [RequireBotPermission(GuildPermission.ManageRoles, ErrorMessage = Messages.NoBotPermission + " I am missing the Manage Roles permission.")]
        public async Task GiveAsync(SocketGuildUser user, SocketRole role) {
            // do stuff
        }

    }

}

When a precondition for a sub-command fails (so, in this example c#roles give), the precondition for the base command will be the one reported. In this situation, it's the "too many arguments" error because the GiveAsync method has two parameters, whereas the RolesAsync method has none. So even if I give the correct parameters, if ANY of the preconditions fail OR parameters fail, then it will default to the base command.

This behavior is hard to describe and is easiest to understand in an example bot.

FireController1847 avatar Sep 25 '20 06:09 FireController1847

I encountered this issue as well, I believe the issue is due to the CommandService#L504 first filtering out commands which fail preconditions.

In our case all the SubCommand preconditions fail but the BaseCommand succeeds but as the BaseCommand doesn't support the arguments being passed it results in a failed ParseResult being returned.

I've workaround this issue in my bot by implementing a more basic version of what the CommandService does but backwards, I find the first command that has a successful ParseResult then I do the precondition checks and return that if it fails, for my specific use case this works fine but I'm not entirely sure if this would cause issues elsewhere.

LXGaming avatar Jan 30 '21 13:01 LXGaming

This is my current workaround for this. It isn't pretty, but it gets the job done:

/**
* If a precondition in a sub-command fails, it will (for some reason) default to the base command's precondition.
* To get around this, I manually call each precondition's "checkpermissionsasync" method.
* https://github.com/discord-net/Discord.Net/issues/1635
*/
public async static Task<bool> PreconditionWorkaround(SocketCommandContext context, PreconditionAttribute attribute, string errorMessage) {
    if (!(await attribute.CheckPermissionsAsync(context, null, null)).IsSuccess) {
        await context.Channel.SendMessageAsync(errorMessage);
        return false;
    } else {
        return true;
    }
}

Its usage is as so:

public async Task GiveAsync(SocketGuildUser user, SocketRole role) {
    if (
        !await Divibot.PreconditionWorkaround(Context, new RequireContextAttribute(ContextType.Guild), Messages.GuildOnly) ||
        !await Divibot.PreconditionWorkaround(Context, new RequireBotPermissionAttribute(GuildPermission.ManageRoles), Messages.NoBotPermission + " I am missing the Manage Roles permission.") ||
        !await Divibot.PreconditionWorkaround(Context, new RequireUserPermissionAttribute(GuildPermission.ManageRoles), Messages.NoPermission + " You are missing the Manage Roles permission.")
    ) {
        return;
    }
}

FireController1847 avatar May 27 '21 05:05 FireController1847

@Cenngo Can you take a look here?

quinchs avatar Nov 26 '21 15:11 quinchs

@Cenngo Can you take a look here?

How do you want this handled? If i remember corretly, It is correct that the precondition check is prioritized over the method signature check. I guess we can change the piplelines order of execution.

Cenngo avatar Nov 26 '21 19:11 Cenngo

How do you want this handled?

It's up to you how you want to come to a solution

quinchs avatar Nov 26 '21 20:11 quinchs