-
-
Notifications
You must be signed in to change notification settings - Fork 465
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
feat: implement Invokable & BaseContext #1606
Closed
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ik fake merge commit but like I can't merge anything really
* added can_run method * move cooldowns/max conc. * copy methods * compatibility fixes etc (including circular imports) * it works now btw
like bridging all the attributes and subclassed properties etc.
Middledot
added
bug
Something isn't working
help wanted
Extra attention is needed
status: in progress
Work in Progess
status: awaiting review
Awaiting review from a maintainer
feature
Implements a feature
labels
Aug 29, 2022
Signed-off-by: Middledot <[email protected]>
Pushing this to v2.6 |
Signed-off-by: Middledot <[email protected]>
BaseCommand could be more fitting than Invokable. |
this needs a redo, too many conflicts |
This is outdated and needs a rework - Closing |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
bug
Something isn't working
feature
Implements a feature
help wanted
Extra attention is needed
on hold
status: in progress
Work in Progess
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Even though app commands and text commands share many features, some are only implemented on one of the two AND a huge chunk of these features get re-implemented each time they get used. Invokables make it easier to share common features between the 2. BaseContext does the same but for contexts.
ApplicationCommand
andext.commands.Command
are now both subclassed fromInvokable
. Same thing withApplicationContext
andext.commands.Context
withBaseContext
.This tries to be as least breaking as possible btw.
But should probably be tested before merging.
Now that I see it, this is kind of related to #387
Summary
(Possible) breaking/Important changes are highlighted
Additions because of Parity:
_invoke
:args
andkwargs
attributes for argument passing. These arguments are prepared in.prepare
and then passed by.invoke
, just like in ext.commands.._parse_arguments
which is called by.prepare
, just like ext.commandsparents
,root_parent
,cog_name
properties.error
decorator.cooldown_after_parsing
attribute & parameter.ApplicationContext
now has thereinvoke
method.ext.commands.Context
misc attributes related to invocation.App commands do not run their parent's checks anymore for parity.With fix: run only local hooks forSlashCommandGroup
#2139 I realized this could be useful with slash commandsOther Additions:
BaseContext
provides.source
now as a way to get either message or interaction on their respective contexts.Improvements from Parity:
discord/commands
folder (ext.commands.cooldown now acts as an alias).wrap_callback
,unwrap_callback
, andhooked_wrapped_callback
) are now not duplicated.Other Improvements:
qualified_name
andfull_parent_name
now use recursion by attributes instead of with a while loop.ContextMenuCommand
dispatch_error
to_dispatch_error
(and subsequently_dispatch_error
is now__dispatch_error
).Renamedprepare
for command object & help impl. to_prepare
.Renamedcall_before_hooks
&call_after_hooks
in the command objects to_call_before_hooks
&_call_after_hooks
.Information