search

Cog-Creators / Red-DiscordBot

A multi-function Discord bot
https://docs.discord.red
GNU General Public License v3.0
4.81k stars 2.31k forks source link

Ensure global slash commands don't break in DMs #5995

Closed synrg closed 1 year ago

synrg commented 1 year ago

What Red version are you using?

3.5.0.dev

What were you trying to do?

I was trying to confirm readiness of my bot instance to upgrade to Red 3.5 once it is released by using any of the global slash commands in DM to my V3/develop test bot instance, whether provided by my own cog inatcog from https://github.com/dronefly-garden/dronefly/tree/dpy2, currently in development to support Red 3.5, or any other third-party cog.

What did you expect to happen?

The Discord docs say that this should work, that global slash commands can be used in DM, so long as both you and the bot are both members of at least one server together, which is the case here.

What actually happened?

Instead, I received an error, The application did not respond.

How can we reproduce this error?

In fact, using Trusty's weather cog is not my goal, but provides a convenient example that I know fails. My own cog produces the same result, but development of that is in flux, so I'd rather not use that as an example. These instructions could be adapted to use any other cog, so long as it provides global slash commands:

  1. Install weather from https://github.com/TrustyJAID/Trusty-cogs/tree/dpy-2.0
  2. Use both message commands (e.g. [p]weather halifax) and slash commands (e.g. /weather location halifax) from the cog in a server channel to confirm proper operation. If the API key is not set yet, each command should respond with a message saying the api key needs to be set.
  3. Try the same thing in a DM to the bot. Observe that the message command produces a response from the bot, but the global slash command does not.

Anything else?

In discussion with Zephyrkul about the issue, we determined that this is what prevents the command from working: https://github.com/Cog-Creators/Red-DiscordBot/blob/2ab204438e2d3d45c9e29f6a78a32aa579a042bf/redbot/core/bot.py#L821-L824

While that might be expedient to not consider these messages as eligible, if that's the case, at the very least, I think they should not be published as available commands. That is, when a user presses / in a DM to the bot, they should not see them if they are known to not work. Otherwise, the bot owner and/or cog developer will be bothered by support requests when this happens. While cog developers can opt to publish their commands as guild only instead to avoid the issue, there's little recourse left to the bot owner unless they can fork the cog themselves to change them to guild-only.

I'm not attached to a particular outcome: I'd be happy with anything that doesn't unduly delay the release of Red 3.5. Here are four possibilities:

  1. If global slash commands can be supported in DMs, that's outcome I'd prefer.
    • Many users of my cog have found it difficult to remember the whole of my cog's expansive command set. Slash commands will help with discovery of the right command through command autocompletion.
  2. If global slash commands can simply be dropped from the command tree because they're not easily supportable in time for Red 3.5, that would be an acceptable outcome.
  3. Red could make commands guild-only always, and simply not support global slash commands.
  4. Advise the cog developers preparing their cogs for Red 3.5 that global slash commands won't work yet, and leave it to them to make their cogs provide guild-only commands to avoid the issue.
Zephyrkul commented 1 year ago

As a clarifying note, global slash commands DO work just fine with Red as-is. It is only global HYBRID commands which Red refuses to run, specifically when they are run via the app command route in DMs. "Pure" app commands are unaffected by this issue.

Rapptz commented 1 year ago

During a hybrid invoke, the library creates a "synthetic" message object filled with Interaction.channel but if it isn't available it falls back to PartialMessageable as seen here. I think it made sense to forbid PartialMessageable back then, but now as its scope has expanded a little bit it's a bit more fluid with the other channels so it shares more or less all the attributes that the old DMChannel had so if your code can manage with DMChannel it should be possible to also work with PartialMessageable... with the caveat of me and recipient both being missing.