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.

Sign up for GitHub

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

Jump to bottom

feat(emoji): implement app emojis #1224

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

feat(emoji): implement app emojis #1224

wants to merge 17 commits into from

Conversation

Snipy7374
Copy link
Collaborator

@Snipy7374 Snipy7374 commented Aug 10, 2024 •
edited
Loading

Summary

Checklist

  • If code changes were made, then they have been tested
    • I have updated the documentation to reflect the changes
    • I have formatted the code properly by running pdm lint
    • I have type-checked the code by running pdm pyright
  • 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, ...)

@shiftinv shiftinv added t: enhancement New feature t: api support Support of Discord API features s: in progress Issue/PR is being worked on labels Aug 11, 2024
@Snipy7374 Snipy7374 marked this pull request as ready for review August 21, 2024 09:56
@Snipy7374
Copy link
Collaborator Author

I marked this ready for review even though i still don't know if we should provide the ability to cache app emojis at startup, as you can see i did put a boolean argument but didn't implement any logic for it so far.

Victorsitou
Victorsitou reviewed Oct 22, 2024
View reviewed changes
Copy link
Member

@Victorsitou Victorsitou left a comment

Choose a reason for hiding this comment

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

In the documentation, referring the emojis as "application emojis" instead of "app emojis" would be more consistent

disnake/client.py Outdated Show resolved Hide resolved
disnake/client.py Show resolved Hide resolved
disnake/emoji.py
@@ -51,6 +51,10 @@ class Emoji(_EmojiTag, AssetMixin):

Returns the emoji rendered for Discord.

.. versionchanched:: 2.10

This class can now represents app emojis too. Denoted by having :attr:`.Emoji.guild_id` as ``None``.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe a is_app_emoji property would be nice here

disnake/emoji.py Outdated Show resolved Hide resolved
disnake/emoji.py Outdated Show resolved Hide resolved
disnake/emoji.py Outdated Show resolved Hide resolved
disnake/client.py Outdated Show resolved Hide resolved
disnake/emoji.py Outdated Show resolved Hide resolved
@shiftinv shiftinv self-requested a review November 14, 2024 14:33
Snipy7374 and others added 8 commits November 26, 2024 17:47
@Snipy7374 @Victorsitou
Update disnake/client.py
51dae3b 
Co-authored-by: Victor <[email protected]>
Signed-off-by: Snipy7374 <[email protected]>
@Snipy7374 @Victorsitou
Update disnake/client.py
b2d343a 
Co-authored-by: Victor <[email protected]>
Signed-off-by: Snipy7374 <[email protected]>
@Snipy7374 @Victorsitou
Update disnake/emoji.py
9cba32c 
Co-authored-by: Victor <[email protected]>
Signed-off-by: Snipy7374 <[email protected]>
@Snipy7374 @Victorsitou
Update disnake/emoji.py
3b3cbb9 
Co-authored-by: Victor <[email protected]>
Signed-off-by: Snipy7374 <[email protected]>
@Snipy7374 @Victorsitou
Update disnake/emoji.py
8247fa3 
Co-authored-by: Victor <[email protected]>
Signed-off-by: Snipy7374 <[email protected]>
@Snipy7374 @Victorsitou
Update disnake/client.py
e38bda8 
Co-authored-by: Victor <[email protected]>
Signed-off-by: Snipy7374 <[email protected]>
@Snipy7374 @Victorsitou
Update disnake/emoji.py
74bdc75 
Co-authored-by: Victor <[email protected]>
Signed-off-by: Snipy7374 <[email protected]>
@Snipy7374
Merge branch 'master' into feat/app_emojis
0e7ad20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Victorsitou Victorsitou Victorsitou left review comments

@shiftinv shiftinv Awaiting requested review from shiftinv

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
s: in progress Issue/PR is being worked on t: api support Support of Discord API features t: enhancement New feature
Projects
disnake
Status: Todo
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

implement app emojis
3 participants
@Snipy7374 @Victorsitou @shiftinv