-
-
Notifications
You must be signed in to change notification settings - Fork 477
fix: Message.system_content() error with application_command message type #2825
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
Open
BOXERRMD
wants to merge
4
commits into
Pycord-Development:master
Choose a base branch
from
BOXERRMD:Fix_bugs
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
system_content shows the rendered content on the client for certain message types, the client does not render it like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah.. See below:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see what the problem is... Is it the message that the function returns that's the problem?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, system_content shows, as I said, the content rendered in the client side. Application_command messages are just bot messages to respond to interactions, there is no actual system_content apart from the "user used command"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I got that, thanks for the clarification. But there's still a problem: if we use it to get the ForwadedMessage and the bot picks up the message from a bot, the function returns None when it should have returned a string as indicated in the typing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forwarded messages have their own content, and the info provided by discord is pretty limited, could you detail more?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function is supposed to return the content according to the type of message required. For all those that are private, such as calls, they're not, and that's normal, given that you can't call a bot. But for application commands, if we use Message.system_content() it will return None because it doesn't handle the case of a message from a bot responding to an interaction.
As for the snapshots attribute, it does exist, but the version it's based on is not yet available (unless you use the main branch without going through version 2.6.1). So the only way to get snapshots is to use system_content(). That's why I wanted to deal with the application_command case, to avoid any more bugs. But if I need to change the message, I can replace it with self.content, which corresponds to an empty string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since here, from my understanding, the only thing that could fit as system content would be
{username} used {command}
, AND it is not currently possible to obtain the associated command's name, I am unsure how any attempt at this could work. @DA-344 Can you confirm I am not misunderstanding something here ?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently, they want to obtain the contents of a application command response in a message snapshot, but for that, we wouldn't use system_content. And no, you are not misunderstanding anything as far as I understand what this PR is meant to "solve".
No, system_content should not be used to get message snapshots content, just wait until the release that includes Message.snapshots is released (or use the master branch).