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

fix(app-cmds): before and after invoke hooks not working for function inside cogs #1004

Merged
merged 6 commits into from
Oct 8, 2024

Conversation

okay1204
Copy link
Contributor

@okay1204 okay1204 commented Mar 4, 2023

Summary

Fixes an issue where the before_invoke and after_invoke decorators were not able to be used on functions inside Cogs because of the self parameter not being addressed. This fix simply checks how many arguments the passed in hook has and decides if the parent cog should be passed in for the self argument or not.

Checklist

  • If code changes were made then they have been tested.
    • I have updated the documentation to reflect the changes.
    • I have run task pyright and fixed the relevant issues.
  • This PR fixes an issue.
  • This PR adds something new (e.g. new method or parameters).
  • This PR is a breaking change (e.g. methods or parameters removed/renamed)
  • This PR is not a code change (e.g. documentation, README, ...)

nextcord/application_command.py Show resolved Hide resolved
Co-authored-by: teaishealthy <76410798+teaishealthy@users.noreply.github.com>
@EmmmaTech EmmmaTech changed the title Fix before and after invoke hooks not working for function inside cogs fix(app-cmds): before and after invoke hooks not working for function inside cogs Mar 10, 2023
@EmmmaTech EmmmaTech added t: bug Type: bug - something isn't working p: medium Priority: medium - should be worked on in the near future s: awaiting review Status: the issue or PR is awaiting reviews labels Mar 10, 2023
@okay1204
Copy link
Contributor Author

@teaishealthy Reverted the previous change because it broke the fix. It needs the else statement or else the hook is called again with just the interaction parameter.

@teaishealthy
Copy link
Collaborator

Reverted the previous change because it broke the fix. It needs the else statement or else the hook is called again with just the interaction parameter.

Yeah I completely forgot the return in the if statement

Copy link
Collaborator

@Skelmis Skelmis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Untested but overall looks fine to me

@alentoghostflame
Copy link
Collaborator

@okay1204 If you fix the merge conflicts, I'll look into merging this in.

@ooliver1 ooliver1 enabled auto-merge (squash) October 8, 2024 22:04
@ooliver1 ooliver1 merged commit 3d0661c into nextcord:master Oct 8, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p: medium Priority: medium - should be worked on in the near future s: awaiting review Status: the issue or PR is awaiting reviews t: bug Type: bug - something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants