discord.py icon indicating copy to clipboard operation
discord.py copied to clipboard

Hybrid commands only accept Discord flag decorators in certain orders

Open Flame442 opened this issue 1 year ago • 2 comments

Summary

The ordering of @app_commands.allowed_installs and @commands.hybrid_command affects whether the allows_installs decorator will apply.

Reproduction Steps

  1. Load the minimal reproduction cog below
  2. bot.tree.get_command("beforecom").allowed_installs is the expected AppInstallationType object with the flags applied
  3. bot.tree.get_command("aftercom").allowed_installs is None

Minimal Reproducible Code

import discord
from discord.ext import commands
from discord import app_commands

class Test(commands.Cog):
    def __init__(self, bot):
        self.bot = bot
    
    @commands.hybrid_command()
    @app_commands.allowed_installs(guilds=True, users=True)
    async def beforecom(self, ctx: commands.Context) -> None:
        await ctx.send("Example")
    
    @app_commands.allowed_installs(guilds=True, users=True)
    @commands.hybrid_command()
    async def aftercom(self, ctx: commands.Context) -> None:
        await ctx.send("Example")

async def setup(bot):
    await bot.add_cog(Test(bot))

Expected Results

Either orientation of the decorator should apply the allowed_installs setting to the hybrid command.

Actual Results

Only cases where the allowed_installs decorator runs first (ie, is lower in the decorator list) actually cause the decorator to work.

Intents

discord.Intents.all()

System Information

  • Python v3.8.5-final
  • discord.py v2.4.0-final
  • aiohttp v3.9.5
  • system info: Windows 10 10.0.19041

Checklist

  • [X] I have searched the open issues for duplicates.
  • [X] I have shown the entire traceback, if possible.
  • [X] I have removed my token from display, if visible.

Additional Context

Also tested with app_commands.guild_only, which has the same issue. I do not know the full scope of decorators affected by this issue, but I would imagine it is all non-check app command decorators.


https://github.com/Rapptz/discord.py/blob/9806aeb83179d0d1e90d903e30db7e69e0d492e5/discord/app_commands/commands.py#L2806-L2811

In the decorator logic, the else case handles the currently working case, which is when the decorator is used while the object is still a function. Later, when the app command is initialized, it properly includes the allowed_installs setting.

For hybrid commands specifically, the object that is returned from @commands.hybrid_command is a commands.HybridCommand, which does not inherit from app_commands.Command. Only commands.HybridAppCommand inherits from app_commands.Command, which would require accessing the .app_command attribute of the HybridCommand.

A potential fix is to add a 3rd case to these decorators, which checks for HybridCommands and pulls out the .app_command. I am not familiar enough with the flow of information for app command internals to know if that would be a satisfactory solution.

Flame442 avatar Dec 28 '24 19:12 Flame442

This is a Python design and noted in the documentation of the allowed_installs decorator

DA-344 avatar Dec 28 '24 21:12 DA-344

Where in the documentation for the decorator is it noted? The example has it in the "working" order, but only for an @app_commands.command() case, which is not order dependent. There is no note stating a required order, and no note about differences for hybrid_commands. The only note is regarding subcommands, which does not apply in this case.

Flame442 avatar Dec 28 '24 21:12 Flame442