Preventing use of complete
avurro opened this issue · 8 comments
I realized that the problem arises when creating the bot in "light" mode:
JDABuilder.createLight
In fact, in this way, when the following method is invoked:
paginator.getEmoteMap ()
you get an empty list.
The list is full if I create the bot in "default" mode:
JDABuilder.createDefault
I also noticed that the same code works when loading the Emotes related to the arrows. I have no idea what the mistake is, but I think it's a good idea to pre-load all the EMOTEs you are going to use.
I hope I have been helpful
ok.. i found the difference. createLight
disables the cache dedicated to EMOTE. Unlike createDefault
, the EMOTEs aren't loaded in advance.
With the following code I resolve the error:
.enableCache(CacheFlag.EMOTE)
However i would like to avoid loading the EMOTEs, of all the servers in which the bot will enter, into RAM.
So I return to the conclusion that maybe, passing all the EMOTE used in the construction phase (and not just those relating to the arrows) is the best solution.
In order to avoid scrolling through all the guilds in which the bot is present to search for emojis, especially considering that, in the case of thousands of servers, it could become a serious problem.
The library has an internal cache for emotes, so it only searches for a given emote once (or if it's no longer valid). This is essential because it's impossible to know emote availability during construction without using JDA's cache, so it's the drawback of going cache-less.
About the complete issue, that's easy to fix, just need to use submit instead, I'll add to the next release.
Wait.. you with this code are looking for EMOTEs in all servers:
for (Guild guild : handler.getGuilds()) {
try {
e = guild.retrieveEmoteById(id).complete();
break;
} catch (ErrorResponseException ignore) {}
}
Here you are searching all the servers where the bot is present, a very expensive operation if the bot in question is on thousands of servers.
The cache you manage with Pagination is a must.
But I am not convinced that this cycle is the best solution.
In the end we talk about a few EMOTEs, why not directly define the IDs in the construction phase of Pagination? (and the guilds where this EMOTEs are loaded if needed)
Because the only emotes that are fixed are those from paginate
and the "X" button, all the other buttons allow any emote to be used during runtime, making it necessary to declare them during construction would severely limit the possible use-cases for them.
That solution might not be the best solution, but since Discord doesn't point which server an emote belongs to there's really no way to retrieve it just from its ID, it's necessary to find out the source server first.
If you look where getOrRetrieveEmote
is called you'll notice that it's only used during method start (for event-handling it's all done through IDs) so it's actually less costly than it appears.
Cache has the benefit of drastically reducing API calls, it's one of the reasons it exist. The emote cache from JDA has very little footprint for the amount of benefit it brings, I'd really suggest using it unless you're running on severely limited specs (like heroku).
Also, do note that this
Guild g = handler.getGuildById(paginator.getEmoteMap().getOrDefault(id, "0"));
retrieves the guild ID from past retrievals, so it further reduces looping over guilds to find the desired Emote.
I've added a way to whitelist server IDs for Emote lookup, this should reduce the amount of places the library will search for the supplied Emote ID.
Both that and the complete()
fix are in release 2.1.7