Implement a least breaking approach to slash commands #39
Loading…
x
Reference in New Issue
Block a user
No description provided.
Delete Branch "2.0"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
This PR implements a system on top of ext.commands for slash commands allowing for minimal breakage by constructing fake message objects
on_interaction
.Checklist
I need to test it further. I've noted that the PR contains a few style fixes for unrelated files, would you be able to remove them (or send them to another PR) so as not to clutter the commit view ?
Great work !
Small nitpic, shouldn't this say client ?
@ -118,6 +160,35 @@ def _is_submodule(parent: str, child: str) -> bool:
return parent == child or child.startswith(parent + ".")
def _unwrap_slash_groups(
Maybe this could be made a list of guild ids, instead of just a single guild.
Given this might be overridden by subclasses, I'd suggest having a process_slash_commands coro called from there, as in on_message.
Combine the two calls in the init
Or, even better,
_FakeSlashMessage.from_interaction(interaction)
Why isn't this in BotBase ?
You should add docs for
message_commands
andslash_commands
@ -281,2 +326,4 @@
If this is set, only upload this slash command to these guild IDs.
This overwrites the global ``slash_command_guilds`` parameter of :class:`.Bot`.
message_command
would be more appropriate here for consistency.Should this really be done by the library ?
if this isn't done by the lib, the interaction fails (no response in the channel), as this is the default help command I thought it appropriate.
@ -118,6 +160,35 @@ def _is_submodule(parent: str, child: str) -> bool:
return parent == child or child.startswith(parent + ".")
def _unwrap_slash_groups(
resolved in #bikeshedding
Bot is used in other docstrings throughout Client, such as
clear
andcreate_guild
.@ -118,6 +160,35 @@ def _is_submodule(parent: str, child: str) -> bool:
return parent == child or child.startswith(parent + ".")
def _unwrap_slash_groups(
I think the idea was to discourage use of guild slash commands where global slash commands could be used, then have a kwarg in command deco for specific guilds:

@ -118,6 +160,35 @@ def _is_submodule(parent: str, child: str) -> bool:
return parent == child or child.startswith(parent + ".")
def _unwrap_slash_groups(
yes, maybe I marked resolved prematurely
needs access to
self.application_id or (await self.application_info())
which are from discord.Client, if you can think of a better way to do it let me knowAdded, alongside docs for
message_command
andslash_command
in the command decorators.@ -118,6 +160,35 @@ def _is_submodule(parent: str, child: str) -> bool:
return parent == child or child.startswith(parent + ".")
def _unwrap_slash_groups(
decided to do both
I'll help with tthe typing later, but tons of
type:ignore
Needs a versionadd
Nitpick, but I think avoiding
# type :ignore
is a good precedentbetter to
if
and raise an error, I think that comes to api design. But I think a better error for the user is necessary instead of an simple assertionerrorDocstring for function?
@ -405,0 +465,4 @@
kwargs.pop("nonce", None)
kwargs.pop("stickers", None)
kwargs.pop("reference", None)
kwargs.pop("delete_after", None)
Content not in func docstring
ValueError? use an ext.command error right like the registration error
Reads better as "Parameter 'intents' must be of :class:
discord.Intent
"where would I put it then, I have to somewhere to make the fake message into a discord.Message for typing purposes
this is a situation that should never happen, and the assert is only for typing purposes, None was popped from the dict above.
@ -405,0 +465,4 @@
kwargs.pop("nonce", None)
kwargs.pop("stickers", None)
kwargs.pop("reference", None)
kwargs.pop("delete_after", None)
Content is from
abc.Messageable.send
andNot a change from this PR, weird git stuff is going on.
Resolved in
8a779ef595
Resolved in
8a779ef595
Resolved in
8a779ef595
TypeError isnt the right type of error to raise here.
Consider ValueError
try pulling from cache first here maybe?
This also should be a ValueError
interaction.channel is a property that does a cache lookup using the ID, if it is None or PartialMessageable it couldn't get from cache