refactor: load Twitch emotes from Helix #5239
Conversation
|
With this approach we're going to need to use image
|
|
notes: a cavalcade of funni errors
f5-ing a single channel will label the `Twitch` section of global emotes incorrectly:loaded naturally emote menu can populate gif emotes twice in some channels (maybe specifically an issue with perma subs)
this does not effect the : menu |
I'm hoping for Twitch to utilize the pages better. But we'll still need some rate-limiting.
Fixed by grouping everything to
Seems to be twitchdev/issues#922 - I'm hoping this gets fixed - having yet another map to detect duplicates seems pointless. Btw, it's only in the popup. |
Nerixyz
force-pushed
the
fix/twitch-emotes
branch
2 times, most recently
from
71c1eea
to
4b3f78a
Compare
|
#5289 - should make sure the emotes listed as well ordered |
Nerixyz
force-pushed
the
fix/twitch-emotes
branch
from
4b3f78a
to
84fc815
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
So this is functionally "complete" but still really slow. It tries to cache emote sets for channels that haven't resolved yet. But from my testing, all channels resolved equally slow, so there wouldn't be any benefit. I'm waiting for Twitch to reply to the issues mentioned in the description and for them to potentially use better page sizes (some are tiny, costing a few RTTs - this adds up). |
Are you able to retest this? It doesn't seem right; I have 4 active subscriptions right now and it fetched everything in 6 calls (no broadcaster_id specified) |
It seems weird that no broadcaster_id was specified... I've added some additional logging for each request and for the total (I'll probably remove this if this gets merged). For my user, it's actually 28 requests. They're probably from event emotes that have accumulated throughout the years (as you can see, the size of each page is small). Logschatterino.twitch: [TwitchChannel "nerixyz"] Loading Twitch emotes - userID: "129546453", broadcasterID: "129546453", manualRefresh: false
chatterino.twitch: [TwitchChannel "nymn"] Loading Twitch emotes - userID: "129546453", broadcasterID: "62300805", manualRefresh: false
chatterino.twitch: [TwitchChannel "pajlada"] Loading Twitch emotes - userID: "129546453", broadcasterID: "11148817", manualRefresh: false
chatterino.twitch: [TwitchChannel "uint128"] Loading Twitch emotes - userID: "129546453", broadcasterID: "489584266", manualRefresh: false
chatterino.twitch: [TwitchChannel "nerixyz"] Got 16 more emote(s)
chatterino.twitch: [TwitchChannel "nymn"] Got 21 more emote(s)
chatterino.twitch: [TwitchChannel "pajlada"] Got 21 more emote(s)
chatterino.twitch: [TwitchChannel "uint128"] Got 16 more emote(s)
chatterino.twitch: [TwitchChannel "nerixyz"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nymn"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "pajlada"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "uint128"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nerixyz"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nymn"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "uint128"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "pajlada"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nymn"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nerixyz"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "pajlada"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "uint128"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nymn"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nerixyz"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "pajlada"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nymn"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nerixyz"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "uint128"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nerixyz"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nymn"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "pajlada"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "uint128"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nerixyz"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "pajlada"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "uint128"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nymn"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nerixyz"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "pajlada"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nymn"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "uint128"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "pajlada"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nerixyz"] Got 10 more emote(s)
chatterino.twitch: [TwitchChannel "nymn"] Got 10 more emote(s)
chatterino.twitch: [TwitchChannel "uint128"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "pajlada"] Got 10 more emote(s)
chatterino.twitch: [TwitchChannel "nerixyz"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nymn"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "uint128"] Got 10 more emote(s)
chatterino.twitch: [TwitchChannel "pajlada"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nerixyz"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nymn"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "uint128"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "pajlada"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nerixyz"] Got 47 more emote(s)
chatterino.twitch: [TwitchChannel "nymn"] Got 47 more emote(s)
chatterino.twitch: [TwitchChannel "uint128"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "pajlada"] Got 47 more emote(s)
chatterino.twitch: [TwitchChannel "nerixyz"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nymn"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "uint128"] Got 47 more emote(s)
chatterino.twitch: [TwitchChannel "pajlada"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nerixyz"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nymn"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "uint128"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "pajlada"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nerixyz"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nymn"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "uint128"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "pajlada"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nerixyz"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nymn"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "uint128"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "pajlada"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nerixyz"] Got 4 more emote(s)
chatterino.twitch: [TwitchChannel "nymn"] Got 4 more emote(s)
chatterino.twitch: [TwitchChannel "uint128"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "pajlada"] Got 4 more emote(s)
chatterino.twitch: [TwitchChannel "nymn"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nerixyz"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "uint128"] Got 4 more emote(s)
chatterino.twitch: [TwitchChannel "pajlada"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nymn"] Got 46 more emote(s)
chatterino.twitch: [TwitchChannel "nerixyz"] Got 46 more emote(s)
chatterino.twitch: [TwitchChannel "uint128"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "pajlada"] Got 46 more emote(s)
chatterino.twitch: [TwitchChannel "nymn"] Got 21 more emote(s)
chatterino.twitch: [TwitchChannel "nerixyz"] Got 21 more emote(s)
chatterino.twitch: [TwitchChannel "uint128"] Got 46 more emote(s)
chatterino.twitch: [TwitchChannel "pajlada"] Got 21 more emote(s)
chatterino.twitch: [TwitchChannel "nymn"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nerixyz"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "uint128"] Got 21 more emote(s)
chatterino.twitch: [TwitchChannel "pajlada"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nymn"] Got 9 more emote(s)
chatterino.twitch: [TwitchChannel "nerixyz"] Got 9 more emote(s)
chatterino.twitch: [TwitchChannel "uint128"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "pajlada"] Got 9 more emote(s)
chatterino.twitch: [TwitchChannel "nymn"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nerixyz"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "uint128"] Got 9 more emote(s)
chatterino.twitch: [TwitchChannel "pajlada"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nymn"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nerixyz"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "uint128"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "pajlada"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nymn"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nerixyz"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "uint128"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "pajlada"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nymn"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nerixyz"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "uint128"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "pajlada"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "nymn"] Got 275 more emote(s)
chatterino.twitch: [TwitchChannel "nymn"] Loaded 533 Twitch emotes (28 requests)
chatterino.twitch: [TwitchChannel "nerixyz"] Got 275 more emote(s)
chatterino.twitch: [TwitchChannel "nerixyz"] Loaded 528 Twitch emotes (28 requests)
chatterino.twitch: [TwitchChannel "uint128"] Got 5 more emote(s)
chatterino.twitch: [TwitchChannel "pajlada"] Got 275 more emote(s)
chatterino.twitch: [TwitchChannel "pajlada"] Loaded 528 Twitch emotes (28 requests)
chatterino.twitch: [TwitchChannel "uint128"] Got 275 more emote(s)
chatterino.twitch: [TwitchChannel "uint128"] Loaded 528 Twitch emotes (28 requests) |
Yeah we can paginate once without broadcaster_id, but we need to specify broadcaster_id for the channels we have joined but are not subscribed (can leverage userstate for this logic) |
I believe 'Get Channel Emotes' would work better for this, as it requires no pagination to get all emotes |
good point, can use |
I don't like this, but it's probably the best idea looking at the request count. My main concern is that Twitch could add more emotes that are local, and |
This changed a bunch of stuff but I'm too tired to look at it now maybe tomorrow. Also in total there are a few unrelated changes. I would love something like Rust's async-await here but I don't have it so the code looks really ugly (blame shifting, I'll refactor it later hopefully).
There was a problem hiding this comment.
Not a full review, overall looks good. Less changes required for this than I would have imagined.
I've pushed some documentation changes as way for me to say: "I think this is how it works"
Some emotes seem to be duplicated for me, might have to do with t3 sub
They only show up once in the emote completion popup & the emote tab complketino
| CancellationToken token(false); | ||
| ASSERT_FALSE(token.isCancelled()); | ||
| { | ||
| ScopedCancellationToken scoped(std::move(token)); |
There was a problem hiding this comment.
Correct me if I'm wrong:
Since ScopedCancellationToken has no accessors on its own, moving a CancellationToken into a ScopedCancellationToken means no one can check its state, I feel like unless ScopedCancellationToken has its own accessors for its backingToken_, the move ctor & move assignment operator should be deleted
There was a problem hiding this comment.
It takes the CancellationToken by value now. But generally, the idea is that using a move, you can avoid one copy of the shared_ptr. It's not really visible in the constructor as much but more in the assignment operator:
CancellationToken token(false);
sendRequest("/foo", token /* copied */);
this->myToken = std::move(token); // avoid any copy of the shared_ptrno way this doesn't break anything
There was a problem hiding this comment.
On testaccount_420, where I only have one t3 sub to pajlada:
Sub emotes correctly show up as sub emotes.
Global Twitch emotes correctly show up as Global Twitch emotes.
pajlada's follower emotes wrongly show up as Global Twitch emotes
Loading 342 emotes took 3 requests
On pajlada, where I have a bunch of various subs:
Bit emotes show up correctly
Sub emotes show up correctly
Follower emotes show up as global emotes if you're subbed
When opening the emote popup in a channel that hasn't been fully joined yet (i.e. the room ID hasn't loaded), global emotes show up as sub emotes before things have
Loading 1068 emotes took 46 requests
It's probably not a fix, but for a user it's a fix.
This PR refactors how Twitch emotes are loaded - using the new Helix API.
Although the API returns decent data, it's slow. For my user (two subscriptions), I need to make 28 requests to get all emotes for one channel. One request takes about 100-250ms.
To help reduce latency, the emotes are fetched once for the user (by leaving
broadcaster_idempty) and for each channel, the follow-status is checked and follower emotes are loaded. This requires (user:read:follows, Chatterino/website#527).Related twitchdev issues: