Refactored emote reloading (#2857)

Co-authored-by: Rasmus Karlsson <rasmus.karlsson@pajlada.com>

CHANGELOG.md

@@ -21,7 +21,7 @@
 - Bugfix: Fixed FFZ emote links for global emotes (#2807, #2808)
 - Bugfix: Fixed pasting text with URLs included (#1688, #2855)
 - Bugfix: Fix reconnecting when IRC write connection is lost (#1831, #2356, #2850)
-- Bugfix: Fixed bit emotes not loading in some rare cases. (#2856)
+- Bugfix: Fixed bit and new subscriber emotes not (re)loading in some rare cases. (#2856, #2857)
 
 ## 2.3.2
 

src/providers/twitch/IrcMessageHandler.cpp

@@ -494,7 +494,16 @@ void IrcMessageHandler::handleClearMessageMessage(Communi::IrcMessage *message)
 
 void IrcMessageHandler::handleUserStateMessage(Communi::IrcMessage *message)
 {
-    auto app = getApp();
+    auto currentUser = getApp()->accounts->twitch.getCurrent();
+
+    // set received emote-sets, used in TwitchAccount::loadUserstateEmotes
+    bool emoteSetsChanged = currentUser->setUserstateEmoteSets(
+        message->tag("emote-sets").toString().split(","));
+
+    if (emoteSetsChanged)
+    {
+        currentUser->loadUserstateEmotes();
+    }
 
     QString channelName;
     if (!trimChannelName(message->parameter(0), channelName))
@@ -502,7 +511,7 @@ void IrcMessageHandler::handleUserStateMessage(Communi::IrcMessage *message)
         return;
     }
 
-    auto c = app->twitch.server->getChannelOrEmpty(channelName);
+    auto c = getApp()->twitch.server->getChannelOrEmpty(channelName);
     if (c->isEmpty())
     {
         return;
@@ -529,10 +538,6 @@ void IrcMessageHandler::handleUserStateMessage(Communi::IrcMessage *message)
             tc->setMod(_mod == "1");
         }
     }
-
-    // handle emotes
-    app->accounts->twitch.getCurrent()->loadUserstateEmotes(
-        message->tag("emote-sets").toString().split(","));
 }
 
 void IrcMessageHandler::handleWhisperMessage(Communi::IrcMessage *message)

src/providers/twitch/TwitchAccount.cpp

@@ -21,10 +21,6 @@
 #include "util/RapidjsonHelpers.hpp"
 
 namespace chatterino {
-namespace {
-    constexpr int USERSTATE_EMOTES_REFRESH_PERIOD = 10 * 60 * 1000;
-}  // namespace
-
 TwitchAccount::TwitchAccount(const QString &username, const QString &oauthToken,
                              const QString &oauthClient, const QString &userID)
     : Account(ProviderId::Twitch)
@@ -199,18 +195,14 @@ void TwitchAccount::loadEmotes()
 
     if (this->getOAuthClient().isEmpty() || this->getOAuthToken().isEmpty())
     {
-        qCDebug(chatterinoTwitch) << "Missing Client ID or OAuth token";
+        qCDebug(chatterinoTwitch) << "Missing Client ID and/or OAuth token";
         return;
     }
 
+    // Getting subscription emotes from kraken
     getKraken()->getUserEmotes(
         this,
         [this](KrakenEmoteSets data) {
-            // clear emote data
-            auto emoteData = this->emotes_.access();
-            emoteData->emoteSets.clear();
-            emoteData->allEmoteNames.clear();
-
             // no emotes available
             if (data.emoteSets.isEmpty())
             {
@@ -220,78 +212,98 @@ void TwitchAccount::loadEmotes()
                 return;
             }
 
-            for (auto emoteSetIt = data.emoteSets.begin();
-                 emoteSetIt != data.emoteSets.end(); ++emoteSetIt)
             {
-                auto emoteSet = std::make_shared<EmoteSet>();
+                // Clearing emote data
+                auto emoteData = this->emotes_.access();
+                emoteData->emoteSets.clear();
+                emoteData->allEmoteNames.clear();
 
-                emoteSet->key = emoteSetIt.key();
-                this->loadEmoteSetData(emoteSet);
-
-                for (const auto emoteArrObj : emoteSetIt.value().toArray())
+                for (auto emoteSetIt = data.emoteSets.begin();
+                     emoteSetIt != data.emoteSets.end(); ++emoteSetIt)
                 {
-                    if (!emoteArrObj.isObject())
+                    auto emoteSet = std::make_shared<EmoteSet>();
+
+                    emoteSet->key = emoteSetIt.key();
+                    this->loadEmoteSetData(emoteSet);
+
+                    for (const auto emoteArrObj : emoteSetIt.value().toArray())
                     {
-                        qCWarning(chatterinoTwitch)
-                            << QString("Emote value from set %1 was invalid")
-                                   .arg(emoteSet->key);
+                        if (!emoteArrObj.isObject())
+                        {
+                            qCWarning(chatterinoTwitch)
+                                << QString(
+                                       "Emote value from set %1 was invalid")
+                                       .arg(emoteSet->key);
+                            continue;
+                        }
+                        KrakenEmote krakenEmote(emoteArrObj.toObject());
+
+                        auto id = EmoteId{krakenEmote.id};
+                        auto code = EmoteName{krakenEmote.code};
+
+                        auto cleanCode =
+                            EmoteName{TwitchEmotes::cleanUpEmoteCode(code)};
+                        emoteSet->emotes.emplace_back(
+                            TwitchEmote{id, cleanCode});
+                        emoteData->allEmoteNames.push_back(cleanCode);
+
+                        auto emote =
+                            getApp()->emotes->twitch.getOrCreateEmote(id, code);
+                        emoteData->emotes.emplace(code, emote);
                     }
-                    KrakenEmote krakenEmote(emoteArrObj.toObject());
 
-                    auto id = EmoteId{krakenEmote.id};
-                    auto code = EmoteName{krakenEmote.code};
-
-                    auto cleanCode =
-                        EmoteName{TwitchEmotes::cleanUpEmoteCode(code)};
-                    emoteSet->emotes.emplace_back(TwitchEmote{id, cleanCode});
-                    emoteData->allEmoteNames.push_back(cleanCode);
-
-                    auto emote =
-                        getApp()->emotes->twitch.getOrCreateEmote(id, code);
-                    emoteData->emotes.emplace(code, emote);
+                    std::sort(emoteSet->emotes.begin(), emoteSet->emotes.end(),
+                              [](const TwitchEmote &l, const TwitchEmote &r) {
+                                  return l.name.string < r.name.string;
+                              });
+                    emoteData->emoteSets.emplace_back(emoteSet);
                 }
-
-                std::sort(emoteSet->emotes.begin(), emoteSet->emotes.end(),
-                          [](const TwitchEmote &l, const TwitchEmote &r) {
-                              return l.name.string < r.name.string;
-                          });
-                emoteData->emoteSets.emplace_back(emoteSet);
             }
+            // Getting userstate emotes from Ivr
+            this->loadUserstateEmotes();
         },
         [] {
-            // request failed
+            // kraken request failed
         });
 }
 
-void TwitchAccount::loadUserstateEmotes(QStringList emoteSetKeys)
+bool TwitchAccount::setUserstateEmoteSets(QStringList newEmoteSets)
 {
-    // do not attempt to load emotes too often
-    if (!this->userstateEmotesTimer_.isValid())
+    newEmoteSets.sort();
+
+    if (this->userstateEmoteSets_ == newEmoteSets)
     {
-        this->userstateEmotesTimer_.start();
+        // Nothing has changed
+        return false;
     }
-    else if (this->userstateEmotesTimer_.elapsed() <
-             USERSTATE_EMOTES_REFRESH_PERIOD)
+
+    this->userstateEmoteSets_ = newEmoteSets;
+
+    return true;
+}
+
+void TwitchAccount::loadUserstateEmotes()
+{
+    if (this->userstateEmoteSets_.isEmpty())
     {
         return;
     }
-    this->userstateEmotesTimer_.restart();
+
+    QStringList newEmoteSetKeys, krakenEmoteSetKeys;
 
     auto emoteData = this->emotes_.access();
     auto userEmoteSets = emoteData->emoteSets;
 
-    QStringList newEmoteSetKeys, currentEmoteSetKeys;
-
     // get list of already fetched emote sets
     for (const auto &userEmoteSet : userEmoteSets)
     {
-        currentEmoteSetKeys.push_back(userEmoteSet->key);
+        krakenEmoteSetKeys.push_back(userEmoteSet->key);
     }
 
     // filter out emote sets from userstate message, which are not in fetched emote set list
-    for (const auto &emoteSetKey : emoteSetKeys)
+    for (const auto &emoteSetKey : this->userstateEmoteSets_)
     {
-        if (!currentEmoteSetKeys.contains(emoteSetKey))
+        if (!krakenEmoteSetKeys.contains(emoteSetKey))
         {
             newEmoteSetKeys.push_back(emoteSetKey);
         }
@@ -302,6 +314,9 @@ void TwitchAccount::loadUserstateEmotes(QStringList emoteSetKeys)
     {
         return;
     }
+    qCDebug(chatterinoTwitch) << QString("Loading %1 emotesets from IVR: %2")
+                                     .arg(newEmoteSetKeys.size())
+                                     .arg(newEmoteSetKeys.join(", "));
 
     // splitting newEmoteSetKeys to batches of 100, because Ivr API endpoint accepts a maximum of 100 emotesets at once
     constexpr int batchSize = 100;
@@ -323,6 +338,7 @@ void TwitchAccount::loadUserstateEmotes(QStringList emoteSetKeys)
         batches.emplace_back(batch);
     }
 
+    // requesting emotes
     for (const auto &batch : batches)
     {
         getIvr()->getBulkEmoteSets(
@@ -374,7 +390,6 @@ void TwitchAccount::loadUserstateEmotes(QStringList emoteSetKeys)
                 // fetching emotes failed, ivr API might be down
             });
     };
-    return;
 }
 
 SharedAccessGuard<const TwitchAccount::TwitchAccountEmoteData>
@@ -496,7 +511,6 @@ void TwitchAccount::loadEmoteSetData(std::shared_ptr<EmoteSet> emoteSet)
     NetworkRequest(Env::get().twitchEmoteSetResolverUrl.arg(emoteSet->key))
         .cache()
         .onSuccess([emoteSet](NetworkResult result) -> Outcome {
-            auto rootOld = result.parseRapidJson();
             auto root = result.parseJson();
             if (root.isEmpty())
             {
@@ -519,10 +533,11 @@ void TwitchAccount::loadEmoteSetData(std::shared_ptr<EmoteSet> emoteSet)
 
             return Success;
         })
-        .onError([](NetworkResult result) {
+        .onError([emoteSet](NetworkResult result) {
             qCWarning(chatterinoTwitch)
-                << QString("Error code %1 while loading emote set data")
-                       .arg(result.status());
+                << QString("Error code %1 while loading emote set data for %2")
+                       .arg(result.status())
+                       .arg(emoteSet->key);
         })
         .execute();
 }

src/providers/twitch/TwitchAccount.hpp

@@ -112,7 +112,12 @@ public:
     SharedAccessGuard<const std::set<TwitchUser>> accessBlocks() const;
 
     void loadEmotes();
-    void loadUserstateEmotes(QStringList emoteSetKeys);
+    // loadUserstateEmotes loads emote sets that are part of the USERSTATE emote-sets key
+    // this function makes sure not to load emote sets that have already been loaded
+    void loadUserstateEmotes();
+    // setUserStateEmoteSets sets the emote sets that were parsed from the USERSTATE emote-sets key
+    // Returns true if the newly inserted emote sets differ from the ones previously saved
+    [[nodiscard]] bool setUserstateEmoteSets(QStringList newEmoteSets);
     SharedAccessGuard<const TwitchAccountEmoteData> accessEmotes() const;
 
     // Automod actions
@@ -130,7 +135,7 @@ private:
     Atomic<QColor> color_;
 
     mutable std::mutex ignoresMutex_;
-    QElapsedTimer userstateEmotesTimer_;
+    QStringList userstateEmoteSets_;
     UniqueAccess<std::set<TwitchUser>> ignores_;
     UniqueAccess<std::set<QString>> ignoresUserIds_;
 

src/providers/twitch/api/Kraken.cpp

@@ -14,7 +14,7 @@ void Kraken::getUserEmotes(TwitchAccount *account,
 {
     this->makeRequest(QString("users/%1/emotes").arg(account->getUserId()), {})
         .authorizeTwitchV5(account->getOAuthClient(), account->getOAuthToken())
-        .onSuccess([successCallback, failureCallback](auto result) -> Outcome {
+        .onSuccess([successCallback](auto result) -> Outcome {
             auto data = result.parseJson();
 
             KrakenEmoteSets emoteSets(data);