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

Application hosted Emoji support #2712

Closed

Conversation

Andre601
Copy link
Contributor

@Andre601 Andre601 commented Aug 5, 2024

Pull Request Etiquette

Changes

  • Internal code
  • Library interface (affecting end-user code)
  • Documentation
  • Other: _____

Closes Issue: NaN

Description

Supersedes #2706 with a cleaned up commit list and implementation of freya022's addition of createApplicationEmoji(String, Icon)

@Andre601 Andre601 mentioned this pull request Aug 5, 2024
6 tasks
@Andre601 Andre601 changed the title Feature/application emojis Application hosted Emoji support Aug 5, 2024
RestAction<Void> deleteApplicationEmojiById(long emojiId);

@CheckReturnValue
default RestAction<Void> deleteApplicationEmojiById(@Nonnull String emojiId)
Copy link
Contributor

Choose a reason for hiding this comment

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

The String overload should be implemented, and the long overload should be the default method, to avoid converting twice


@Nullable
@CheckReturnValue
default RestAction<ApplicationEmoji> updateApplicationEmojiName(@Nonnull String emojiId, @Nonnull String name)
Copy link
Contributor

Choose a reason for hiding this comment

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

The String overload should be implemented, and the long overload should be the default method, to avoid converting twice


@Nullable
@CheckReturnValue
default RestAction<ApplicationEmoji> retrieveApplicationEmojiById(@Nonnull String emojiId)
Copy link
Contributor

Choose a reason for hiding this comment

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

The String overload should be implemented, and the long overload should be the default method, to avoid converting twice

* The format {@code :smiley:} is a client-side alias which is replaced by the unicode emoji, not a custom emoji.</b>
*
* @see JDA#retrieveApplicationEmojiById(long)
* @see JDA#retrieveApplicationEmojis()
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps also add JDA#createApplicationEmoji(...)

Comment on lines +50 to +51
*/
RestAction<Void> delete();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
*/
RestAction<Void> delete();
*/
@Nonnull
@CheckReturnValue
RestAction<Void> delete();

@CheckReturnValue
RestAction<List<ApplicationEmoji>> retrieveApplicationEmojis();

@Nullable
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
@Nullable
@Nonnull

@CheckReturnValue
RestAction<ApplicationEmoji> createApplicationEmoji(@Nonnull String name, @Nonnull Icon icon);

@CheckReturnValue
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
@CheckReturnValue
@Nonnull
@CheckReturnValue

JDA getJDA();

/**
* The user who created this emoji
Copy link
Contributor

Choose a reason for hiding this comment

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

Should probably tell when this is null, which to my knowledge is only after creating it

@Override
public UnicodeEmoji asUnicode()
{
throw new IllegalStateException("Cannot convert CustomEmoji to UnicodeEmoji!");
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
throw new IllegalStateException("Cannot convert CustomEmoji to UnicodeEmoji!");
throw new IllegalStateException("Cannot convert ApplicationEmoji to UnicodeEmoji!");

Comment on lines +57 to +59
Checks.notBlank(name, "Name");
name = name.trim();
Checks.inRange(name, 2, 32, "Name");
Copy link
Contributor

Choose a reason for hiding this comment

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

Those aren't the same checks as JDA#updateApplicationEmojiName

I also don't think the input should be changed

Check that this matches Checks#ALPHANUMERIC_WITH_DASH

@yoyosource
Copy link

@Andre601 are you still working on this? I would really like this being in the master.

@Andre601
Copy link
Contributor Author

Andre601 commented Sep 4, 2024

Yeah, I can't find motivation to continue working on this, so I would be happy if someone could take over the changes, so that I can close this PR...

@yoyosource
Copy link

I will take a look. I don't know how fast I can take over the changes. I will message you (@Andre601) when I did it ok?

@yoyosource
Copy link

@Andre601 I created a PR that took over your changes: #2726

@Andre601 Andre601 closed this Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants