ygimenez / Pagination-Utils

A collection of methods to make message pagination with JDA easier.
GNU Lesser General Public License v2.1
27 stars 7 forks source link

setEmote doesn't work #19

Closed avurro closed 3 years ago

avurro commented 3 years ago

Describe the bug I don't understand how to use setEmote for custom emojis, I get error whatever combination I try. Could you give me an example?

ygimenez commented 3 years ago

You need to pass the ID of the Emote, it can be found by putting \ behind it, it'll be in this format: <:name:id>

All you need to do after getting the mention is copy only the ID part of it.

avurro commented 3 years ago

have you done any tests about it? Because it does not work.

ygimenez commented 3 years ago

Yes I did test, what error are you getting? InvalidEmoteException?

avurro commented 3 years ago

Yes, with only ID doesn't work. It doesn't work with whatever combination of custom emoji identifiers

ygimenez commented 3 years ago

Could you show me how you're doing it?

In any case, I'll review it tomorrow to see if I find something, but that's really weird since I've tried it myself before releasing the version.

avurro commented 3 years ago
Paginator paginator = PaginatorBuilder.createPaginator().setHandler(jda)
      .shouldRemoveOnReact(false)
      .setEmote(Emote.SKIP_BACKWARD,"807633784434327563")
      .setEmote(Emote.PREVIOUS,"809239881242247188")
      .setEmote(Emote.SKIP_FORWARD,"807633784580997130") 
      .setEmote(Emote.NEXT,"809239881141977098")
      .setEmote(Emote.CANCEL, "809239881002909737")

this is next button: image

avurro commented 3 years ago

However, even if it's not about this bug, the quick navigation arrows don't take custom emojis. As you can see from the test above. (with version 2.1.1.)

ygimenez commented 3 years ago

Those aren't SKIP buttons, those are GOTO buttons (GOTO_LAST and GOTO_FIRST).

About the ID issue, I'll double check it once I'm at my PC, I'll let you know about updates.

ygimenez commented 3 years ago

Could you be more specific on what error are you experiencing? I'm trying to reproduce it but everything seems normal (except for a Message retrieval error but this isn't related to what you reported).

avurro commented 3 years ago

image

ygimenez commented 3 years ago

Are you sure your bot is in the server those emotes are in? If you loop through those IDs won't any of those return null?

avurro commented 3 years ago

i'm sure because with version 2.1.1. it works. As you can see in screenshot in my previous comment https://github.com/ygimenez/Pagination-Utils/issues/19#issuecomment-777161032

(2.1.1. version code) image

avurro commented 3 years ago

I don't understand why you want keep only id if JDA method take custom emoji with format "name:id". In 2.1.1. version, custom emoji are correctly loaded.

ygimenez commented 3 years ago

As I pointed before, by using name + id means that a single change in the Emote's name will invalidate your buttons, which doesn't happen when using only IDs

avurro commented 3 years ago

I understand this, but if the setReaction method asks for the custom emoji in the format "name: id" why not give it as it asked?

ygimenez commented 3 years ago

It doesn't ask in that format, it gives the option to use that format, addReaction has many signatures actually.

avurro commented 3 years ago

image For custom emojis it's the only one.

ygimenez commented 3 years ago

You're looking at addReaction(String), there's also addReaction(Emote)

avurro commented 3 years ago

image For resource optimization reasons, I avoid using the cache as much as possible. CacheFlag.EMOTE is disabled in my case.

avurro commented 3 years ago

in case of a non-existent EMOTE, try using addReaction(String). It is a fair compromise.

ygimenez commented 3 years ago

That's the reason then, I'll try to see if there's any way to retrieve an Emote without using the cache.

About your suggestion, the issue isn't with addReaction but on how I retrieve the event from the handler, I can only use either the name or the ID of the Emote, not both.

avurro commented 3 years ago

image Same problem with getName. The best way is to switch the map key from EMOTE to String.

ygimenez commented 3 years ago

The map doesn't store Emote objects, it stores their IDs for comparison when pressing buttons. All I need to know is how can I retrieve an Emote without using the cache.

ygimenez commented 3 years ago

Found a way to retrieve Emotes without using the cache, the fix was added in version 2.1.4. I'd still recommend using it tho, since it's generally faster while its footprint is really small even with thousands of Emotes.

avurro commented 3 years ago

Version 2.1.4:

[JDA RateLimit-Worker 2] WARN  net.dv8tion.jda.internal.requests.RateLimiter - Encountered 429 on route PUT/channels/{channel_id}/messages/{message_id}/reactions/{reaction_code}/{user_id} with bucket d6b7697c78814ecd72bb20df05517c78:guild_id:791430572387467286:webhook_id Retry-After: 235 ms

image

avurro commented 3 years ago

image After some tests I noticed 2 things:

ygimenez commented 3 years ago

About first note, you've set shouldRemoveOnReact to false, so it'll not reset the reaction on click. About the second note, I've not understood what you meant, could you reword it please?

Also, there's an issue that I've noticed when using cache-less clients (constantly hitting ratelimits), I'll look into it as soon as I can.

ygimenez commented 3 years ago

Please try the latest release, it should be working without hitting the ratelimits as it was before.

avurro commented 3 years ago

The library is much more responsive now, wow! Good Work! 👍

ygimenez commented 3 years ago

I'm happy your issue is solved, good coding!