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

feat: avatar decorations #68

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

Conversation

Snazzah
Copy link
Contributor

@Snazzah Snazzah commented Apr 18, 2023

Ref: discord/discord-api-docs#5723 discord/discord-api-docs#6464

This also is based on new seasonal avatar decorations being prefixed with v2_ to denote using an avatar decoration preset.

TTtie
TTtie previously requested changes Apr 18, 2023
Copy link
Contributor

@TTtie TTtie left a comment

Choose a reason for hiding this comment

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

Two small nits + please update the TS definitions (if you aren't able to, let me know and I'll update them for you), otherwise it's good to go once the appropriate docs PR is merged

lib/gateway/Shard.js Outdated Show resolved Hide resolved
lib/gateway/Shard.js Outdated Show resolved Hide resolved
@Snazzah Snazzah requested a review from TTtie April 18, 2023 16:56
@TTtie TTtie added type: enhancement New feature or request reason: Discord Issues or pull requests related to Discord's API changes scope: structures Issues or pull requests affecting the structure definitions labels Apr 19, 2023
@TTtie
Copy link
Contributor

TTtie commented Sep 29, 2023

Hey there! 👋

If this PR is still alive, then it might be worth refactoring it to the latest iteration: discord/discord-api-docs#6464

That one is going to stay for a while, I think. Whereas the ones seen before seemed to be seasonal testing rounds…

@Snazzah
Copy link
Contributor Author

Snazzah commented Sep 29, 2023

Will refactor with the latest iteration later this weekend.

@Snazzah
Copy link
Contributor Author

Snazzah commented Oct 1, 2023

Sidenote: I removed dynamicAvatarDecorationURL from the User class since it seems that avatar decoration presets don't use size query strings

@Snazzah Snazzah marked this pull request as ready for review October 1, 2023 03:08
TTtie
TTtie previously requested changes Oct 1, 2023
Copy link
Contributor

@TTtie TTtie left a comment

Choose a reason for hiding this comment

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

A few nitpicks, otherwise, LGTM

lib/gateway/Shard.js Outdated Show resolved Hide resolved
lib/gateway/Shard.js Outdated Show resolved Hide resolved
lib/gateway/Shard.js Outdated Show resolved Hide resolved
lib/gateway/Shard.js Outdated Show resolved Hide resolved
lib/structures/User.js Outdated Show resolved Hide resolved
lib/structures/User.js Outdated Show resolved Hide resolved
lib/structures/User.js Outdated Show resolved Hide resolved
lib/structures/Member.js Outdated Show resolved Hide resolved
Copy link
Contributor

@TTtie TTtie left a comment

Choose a reason for hiding this comment

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

LGTM. BTW, I rebased the branch on top of latest dev, please keep that in mind if you'd like to change something more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
reason: Discord Issues or pull requests related to Discord's API changes scope: structures Issues or pull requests affecting the structure definitions type: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants