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

Hybrid commands only accept Discord flag decorators in certain orders #10058

Open
3 tasks done
Flame442 opened this issue Dec 28, 2024 · 2 comments
Open
3 tasks done

Hybrid commands only accept Discord flag decorators in certain orders #10058

Flame442 opened this issue Dec 28, 2024 · 2 comments
Labels
unconfirmed bug A bug report that needs triaging

Comments

@Flame442
Copy link
Contributor

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

  • I have searched the open issues for duplicates.
  • I have shown the entire traceback, if possible.
  • 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.


if isinstance(f, (Command, Group, ContextMenu)):
allowed_installs = f.allowed_installs or AppInstallationType()
f.allowed_installs = allowed_installs
else:
allowed_installs = getattr(f, '__discord_app_commands_installation_types__', None) or AppInstallationType()
f.__discord_app_commands_installation_types__ = allowed_installs # type: ignore # Runtime attribute assignment

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 Flame442 added the unconfirmed bug A bug report that needs triaging label Dec 28, 2024
@DA-344
Copy link
Contributor

DA-344 commented Dec 28, 2024

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

@Flame442
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
unconfirmed bug A bug report that needs triaging
Projects
None yet
Development

No branches or pull requests

2 participants