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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(Reactions): Support super reactions #1470

Open
wants to merge 15 commits into
base: dev
Choose a base branch
from

Conversation

iiFDCT
Copy link
Contributor

@iiFDCT iiFDCT commented Jan 14, 2024

Super Reactions!

Adds support for super reactions! Pretty sure Discord is still in the process of adding some of it from the docs, so some of the properties Ive used are from open PRs on the Discord API repo

Also adds support for specifying a reaction type when using Message#getReaction() and all getMessageReaction() functions

Changes/Additions

  • burst_colors, count_details & me_burst added to Message#reactions reaction objects
  • messageReactionAdd & messageReactionRemove events send additional burst (boolean) parameter to bound function
  • type option added to the getReaction() function within Message, as well as the getMessageReaction() function within Client, DMChannel & GuildTextableChannel,

Testing:

MESSAGE_REACTION_ADD & MESSAGE_REACTION_REMOVE rawWS screenshots

Message Reaction Add (Normal Emoji)

image

Message Reaction Remove (Normal Emoji)

image

Message Reaction Add (Burst Emoji)

image

Message Reaction Remove (Burst Emoji)

image

Note: burst_colors is never sent with MESSAGE_REACTION_REMOVE, even when it's a burst reaction, so I'll leave that property out completely rather than just sending an empty array to the event.

Message object with burst reaction

image

This screenshot shows a message with 1 normal reaction, and 1 burst reaction. I'll add more screenshots here when I've finished the todo list and can continue testing 馃憤馃徎

@bsian03
Copy link
Collaborator

bsian03 commented Jan 21, 2024

Has someone tested this? I can't reliably test this myself

Copy link
Collaborator

@bsian03 bsian03 left a comment

Choose a reason for hiding this comment

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

Please lint, merge conflict resolution is invalid

@iiFDCT
Copy link
Contributor Author

iiFDCT commented Jan 21, 2024

Has someone tested this? I can't reliably test this myself

This PR is still a draft cuz Discord haven't updated their docs yet to match all of the props I've used here. Probably best to test it when both of those PRs are merged just in case they change anything else 馃憤馃徎

@bsian03
Copy link
Collaborator

bsian03 commented Jan 21, 2024

I personally don't think it's worth waiting until Discord confirms docs. Also we previously had stuff which was undocumented (can't remember which ones exactly), and if there are any changes, we can just update accordingly

@iiFDCT iiFDCT marked this pull request as ready for review January 21, 2024 22:57
Copy link
Collaborator

@bsian03 bsian03 left a comment

Choose a reason for hiding this comment

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

There are some props which aren't documented in the PRs, I've tried to ask the api team what's going on with them but haven't actually received a reply (yes i know that contradicts what i said in my earlier comment)

lib/gateway/Shard.js Outdated Show resolved Hide resolved
if(packet.d.burst) {
message.reactions[reaction].me_burst = true;
} else {
message.reactions[reaction].me = true;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bsian03 Should I always make .me true, even when it's not a super reaction? And then obviously me_burst would also be true when it's a super reaction. Just for backwards compatability reasons really. What d'ya think?

@bsian03
Copy link
Collaborator

bsian03 commented Feb 23, 2024

I'll take a further look at this once I get my PR done

@iiFDCT iiFDCT mentioned this pull request Apr 20, 2024
21 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants