-
-
Notifications
You must be signed in to change notification settings - Fork 737
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
Implemented emoji endpoints #835
Conversation
/// <summary> Creates a new emote in this guild. </summary> | ||
Task<GuildEmote> CreateEmoteAsync(string name, Image image, Optional<IEnumerable<IRole>> roles = default(Optional<IEnumerable<IRole>>), RequestOptions options = null); | ||
/// <summary> Modifies an existing emote in this guild. </summary> | ||
Task<GuildEmote> ModifyEmoteAsync(GuildEmote emote, Action<EmoteProperties> func, RequestOptions options = null); |
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.
Any reason these methods were left on IGuild instead of GuildEmote?
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.
GuildEmote does not hold the Guild object required for this to work. This would require some deeper changes, and I didn't feel confident enough to make them without some guidance.
@@ -117,5 +117,16 @@ public interface IGuild : IDeletable, ISnowflakeEntity | |||
Task DownloadUsersAsync(); | |||
/// <summary> Removes all users from this guild if they have not logged on in a provided number of days or, if simulate is true, returns the number of users that would be removed. </summary> | |||
Task<int> PruneUsersAsync(int days = 30, bool simulate = false, RequestOptions options = null); | |||
|
|||
/// <summary> Gets a collection of all emotes in this guild. </summary> | |||
Task<IReadOnlyCollection<GuildEmote>> ListEmotesAsync(RequestOptions options = null); |
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.
Doesn't this already exist in the form of IGuild.Emotes
?
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.
It also exists in the API. See: linked Discord API documentation.
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.
Is there an advantage or reason to run ListEmotesAsync
(which should probably be GetEmotesAsync
) vs reading the cached collection?
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.
AFAIK, not really. But it exists, so I implemented it.
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.
Better to just leave this off - we can always readd it if there's any API changes that make it necessary
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.
A'ight, is commenting out acceptable? Will make it easier to re-add 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.
it's only 10 lines, just remove it
* Implemented emoji endpoints. * Fixed: now using API entities for REST client. * Removed emoji listing endpoint, as per @foxbot's request.
This PR implements the emoji endpoints, as documented here. This enables bots to manipulate guild emoji.
There is one caveat, however. Bots are only able to get, modify, and delete emoji they've created. They can, however, list all emoji, but it requires manage emoji permission.
Everything is tested, and it works.