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

[Help] Send menus to dm if maxpages is 0 #5375

Merged
merged 7 commits into from
Jan 19, 2024

Conversation

Dav-Git
Copy link
Contributor

@Dav-Git Dav-Git commented Oct 7, 2021

Description of the changes

Blocked by #6282
Closes #5093

This PR will change help to send help menus to DMs if maxpages is 0.
Inconsistent with the "non-menu" mode the menus would always send in the invocation context even if maxpages was 0.

Currently i am a little stuck on the menus in the DM. For a reason i have yet to figure out the menu_ctx context does not play nicely with the menu.
While all messages and reactions can be sent/added, I can not flip the pages in the menu.

@github-actions github-actions bot added Category: Bot Core Category: Core - Help This is related to our help command and/or HelpFormatter API. labels Oct 7, 2021
@Dav-Git
Copy link
Contributor Author

Dav-Git commented Oct 11, 2021

I can‘t seem to find the issue here. The menu works fine with the old implementation when invoked in dms but for some reason my context for m doesn‘t want to work with it.

@Dav-Git Dav-Git marked this pull request as ready for review October 24, 2021 21:17
@Jackenmen Jackenmen added this to the 3.4.x milestone Dec 23, 2021
Copy link
Member

@Flame442 Flame442 left a comment

Choose a reason for hiding this comment

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

[p]helpset maxpages's help currently states:

Note: This setting does not apply to menu help.

This needs to be changed to explain the new behavior specific to menus as a part of this PR.

@Jackenmen Jackenmen added the QA: Changes Requested Used by few QA members. Awaiting changes requested by maintainers or QA. label Dec 27, 2022
@Flame442 Flame442 self-assigned this Jan 2, 2024
@Flame442
Copy link
Member

Flame442 commented Jan 2, 2024

This PR has conflicts due to the addition of button support to help's menu options. Looking at the SimpleMenu class, it requires a ctx object to send the menu in, which won't exist for DMs. It does not support the message= kwarg that the older menu supported, to put a menu onto an existing message. Continuing to include this change would require a different approach to support button menus.

@TrustyJAID, you wrote the PR to add buttons to help, do you have any insight as to the best way to approach this?

@TrustyJAID
Copy link
Member

I think the best path forward would be adding a new option on SimpleMenu called start_dm which accepts a user parameter instead. This way it's a new feature for the menu and would allow this to happen. Looking at the SimpleMenu ctx is only used to extract the author and provide a messageable which can be simply replaced with the author in the new method.

@Flame442 Flame442 added the Blocked By: Other PR Blocked by another PR. label Jan 2, 2024
@Flame442
Copy link
Member

Flame442 commented Jan 2, 2024

Blocked until #6282 is resolved and this PR can use the new method.

@Flame442 Flame442 removed the Blocked By: Other PR Blocked by another PR. label Jan 15, 2024
@github-actions github-actions bot added the Category: Core - Bot Commands This is related to core commands (Core and CogManagerUI cog classes). label Jan 15, 2024
@Flame442 Flame442 removed the QA: Changes Requested Used by few QA members. Awaiting changes requested by maintainers or QA. label Jan 15, 2024
@Flame442 Flame442 removed their assignment Jan 15, 2024
@Flame442 Flame442 removed this from the 3.5.x milestone Jan 15, 2024
@Flame442 Flame442 added this to the 3.5.6 milestone Jan 15, 2024
@Flame442
Copy link
Member

I can‘t seem to find the issue here. The menu works fine with the old implementation when invoked in dms but for some reason my context for m doesn‘t want to work with it.

The issue appears to be that generating ctx from a message sent by the bot sets ctx.author equal to the bot, rather than the invoker of help. This means the menu is listening for reactions from the bot, rather than from the user.

predicates = ReactionPredicate.with_emojis(tuple(controls.keys()), message, ctx.author)

It should be possible to monkeypatch the author attribute to fix this issue.

@Flame442 Flame442 self-assigned this Jan 19, 2024
@Flame442 Flame442 merged commit dbd71db into Cog-Creators:V3/develop Jan 19, 2024
17 checks passed
@red-githubbot red-githubbot bot added the Changelog Entry: Pending Changelog entry for this PR hasn't been added by repo maintainers yet. label Jan 19, 2024
@Jackenmen Jackenmen added Changelog Entry: Added Changelog entry for this PR has already been added to changelog PR. and removed Changelog Entry: Pending Changelog entry for this PR hasn't been added by repo maintainers yet. labels Mar 22, 2024
Dav-Git added a commit to Dav-Git/Red-DiscordBot that referenced this pull request Sep 8, 2024
Co-authored-by: Dav <dav@mail.stopdavabuse.de>
Co-authored-by: Michael Oliveira <34169552+Flame442@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Core - Bot Commands This is related to core commands (Core and CogManagerUI cog classes). Category: Core - Help This is related to our help command and/or HelpFormatter API. Changelog Entry: Added Changelog entry for this PR has already been added to changelog PR. Type: Feature New feature or request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allowing Help to be DMable when Menu is toggled.
4 participants