refactor: Remove Emoji's EmojiMap with a vector (#4980)

Chatterino · Nov 26, 2023 · 5b741a8 · 5b741a8
1 parent 9f97398
commit 5b741a8
Show file tree

Hide file tree

Showing 7 changed files with 59 additions and 39 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -61,6 +61,7 @@
 - Dev: Replace `boost::optional` with `std::optional`. (#4877)
 - Dev: Improve performance of selecting text. (#4889, #4911)
 - Dev: Removed direct dependency on Qt 5 compatibility module. (#4906)
+- Dev: Refactor `Emoji`'s EmojiMap into a vector. (#4980)
 - Dev: Refactor `DebugCount` and add copy button to debug popup. (#4921)
 - Dev: Changed lifetime of context menus. (#4924)
 - Dev: Refactor `ChannelView`, removing a bunch of clang-tidy warnings. (#4926)

diff --git a/benchmarks/src/Emojis.cpp b/benchmarks/src/Emojis.cpp
@@ -68,9 +68,20 @@ static void BM_EmojiParsing(benchmark::State &state)
     };
 
     const auto &emojiMap = emojis.getEmojis();
-    std::shared_ptr<EmojiData> penguin;
-    emojiMap.tryGet("1F427", penguin);
-    auto penguinEmoji = penguin->emote;
+    auto getEmoji = [&](auto code) {
+        std::shared_ptr<EmojiData> emoji;
+        for (const auto &e : emojis.getEmojis())
+        {
+            if (e->unifiedCode == code)
+            {
+                emoji = e;
+                break;
+            }
+        }
+        return emoji->emote;
+    };
+    auto penguinEmoji = getEmoji("1F427");
+    assert(penguinEmoji.get() != nullptr);
 
     std::vector<TestCase> tests{
         {

diff --git a/src/controllers/completion/sources/EmoteSource.cpp b/src/controllers/completion/sources/EmoteSource.cpp
@@ -27,9 +27,11 @@ namespace {
         }
     }
 
-    void addEmojis(std::vector<EmoteItem> &out, const EmojiMap &map)
+    void addEmojis(std::vector<EmoteItem> &out,
+                   const std::vector<EmojiPtr> &map)
     {
-        map.each([&](const QString &, const std::shared_ptr<EmojiData> &emoji) {
+        for (const auto &emoji : map)
+        {
             for (auto &&shortCode : emoji->shortCodes)
             {
                 out.push_back(
@@ -40,7 +42,7 @@ namespace {
                      .providerName = "Emoji",
                      .isEmoji = true});
             }
-        });
+        };
     }
 
 }  // namespace

diff --git a/src/providers/emoji/Emojis.cpp b/src/providers/emoji/Emojis.cpp
@@ -196,7 +196,7 @@ void Emojis::loadEmojis()
 
         this->emojiFirstByte_[emojiData->value.at(0)].append(emojiData);
 
-        this->emojis.insert(emojiData->unifiedCode, emojiData);
+        this->emojis.push_back(emojiData);
 
         if (unparsedEmoji.HasMember("skin_variations"))
         {
@@ -218,8 +218,7 @@ void Emojis::loadEmojis()
                 this->emojiFirstByte_[variationEmojiData->value.at(0)].append(
                     variationEmojiData);
 
-                this->emojis.insert(variationEmojiData->unifiedCode,
-                                    variationEmojiData);
+                this->emojis.push_back(variationEmojiData);
             }
         }
     }
@@ -244,10 +243,8 @@ void Emojis::sortEmojis()
 void Emojis::loadEmojiSet()
 {
     getSettings()->emojiSet.connect([this](const auto &emojiSet) {
-        this->emojis.each([=](const auto &name,
-                              std::shared_ptr<EmojiData> &emoji) {
-            (void)name;
-
+        for (const auto &emoji : this->emojis)
+        {
             QString emojiSetToUse = emojiSet;
             // clang-format off
             static std::map<QString, QString> emojiSets = {
@@ -289,7 +286,7 @@ void Emojis::loadEmojiSet()
             emoji->emote = std::make_shared<Emote>(Emote{
                 EmoteName{emoji->value}, ImageSet{Image::fromUrl({url}, 0.35)},
                 Tooltip{":" + emoji->shortCodes[0] + ":<br/>Emoji"}, Url{}});
-        });
+        }
     });
 }
 
@@ -430,7 +427,7 @@ QString Emojis::replaceShortCodes(const QString &text) const
     return ret;
 }
 
-const EmojiMap &Emojis::getEmojis() const
+const std::vector<EmojiPtr> &Emojis::getEmojis() const
 {
     return this->emojis;
 }

diff --git a/src/providers/emoji/Emojis.hpp b/src/providers/emoji/Emojis.hpp
@@ -1,12 +1,11 @@
 #pragma once
 
-#include "util/ConcurrentMap.hpp"
-
 #include <boost/variant.hpp>
 #include <QMap>
 #include <QRegularExpression>
 #include <QVector>
 
+#include <memory>
 #include <set>
 #include <vector>
 
@@ -37,7 +36,7 @@ struct EmojiData {
     EmotePtr emote;
 };
 
-using EmojiMap = ConcurrentMap<QString, std::shared_ptr<EmojiData>>;
+using EmojiPtr = std::shared_ptr<EmojiData>;
 
 class IEmojis
 {
@@ -46,7 +45,7 @@ class IEmojis
 
     virtual std::vector<boost::variant<EmotePtr, QString>> parse(
         const QString &text) const = 0;
-    virtual const EmojiMap &getEmojis() const = 0;
+    virtual const std::vector<EmojiPtr> &getEmojis() const = 0;
     virtual const std::vector<QString> &getShortCodes() const = 0;
     virtual QString replaceShortCodes(const QString &text) const = 0;
 };
@@ -59,18 +58,19 @@ class Emojis : public IEmojis
     std::vector<boost::variant<EmotePtr, QString>> parse(
         const QString &text) const override;
 
-    EmojiMap emojis;
     std::vector<QString> shortCodes;
     QString replaceShortCodes(const QString &text) const override;
 
-    const EmojiMap &getEmojis() const override;
+    const std::vector<EmojiPtr> &getEmojis() const override;
     const std::vector<QString> &getShortCodes() const override;
 
 private:
     void loadEmojis();
     void sortEmojis();
     void loadEmojiSet();
 
+    std::vector<EmojiPtr> emojis;
+
     /// Emojis
     QRegularExpression findShortCodesRegex_{":([-+\\w]+):"};
 

diff --git a/src/widgets/dialogs/EmotePopup.cpp b/src/widgets/dialogs/EmotePopup.cpp
@@ -72,23 +72,22 @@ auto makeEmoteMessage(const EmoteMap &map, const MessageElementFlag &emoteFlag)
     return builder.release();
 }
 
-auto makeEmojiMessage(EmojiMap &emojiMap)
+auto makeEmojiMessage(const std::vector<EmojiPtr> &emojiMap)
 {
     MessageBuilder builder;
     builder->flags.set(MessageFlag::Centered);
     builder->flags.set(MessageFlag::DisableCompactEmotes);
 
-    emojiMap.each([&builder](const auto &key, const auto &value) {
-        (void)key;  // unused
-
+    for (const auto &value : emojiMap)
+    {
         builder
             .emplace<EmoteElement>(
                 value->emote,
                 MessageElementFlags{MessageElementFlag::AlwaysShow,
                                     MessageElementFlag::EmojiAll})
             ->setLink(
                 Link(Link::Type::InsertText, ":" + value->shortCodes[0] + ":"));
-    });
+    }
 
     return builder.release();
 }
@@ -165,15 +164,16 @@ void addEmotes(Channel &channel, const EmoteMap &map, const QString &title,
     channel.addMessage(makeEmoteMessage(map, emoteFlag));
 }
 
-void loadEmojis(ChannelView &view, EmojiMap &emojiMap)
+void loadEmojis(ChannelView &view, const std::vector<EmojiPtr> &emojiMap)
 {
     ChannelPtr emojiChannel(new Channel("", Channel::Type::None));
     emojiChannel->addMessage(makeEmojiMessage(emojiMap));
 
     view.setChannel(emojiChannel);
 }
 
-void loadEmojis(Channel &channel, EmojiMap &emojiMap, const QString &title)
+void loadEmojis(Channel &channel, const std::vector<EmojiPtr> &emojiMap,
+                const QString &title)
 {
     channel.addMessage(makeTitleMessage(title));
     channel.addMessage(makeEmojiMessage(emojiMap));
@@ -269,7 +269,8 @@ EmotePopup::EmotePopup(QWidget *parent)
     this->globalEmotesView_ = makeView("Global");
     this->viewEmojis_ = makeView("Emojis");
 
-    loadEmojis(*this->viewEmojis_, getApp()->emotes->emojis.emojis);
+    loadEmojis(*this->viewEmojis_,
+               getApp()->getEmotes()->getEmojis()->getEmojis());
     this->addShortcuts();
     this->signalHolder_.managedConnect(getApp()->hotkeys->onItemsUpdated,
                                        [this]() {
@@ -546,17 +547,18 @@ void EmotePopup::filterEmotes(const QString &searchText)
         this->filterTwitchEmotes(searchChannel, searchText);
     }
 
-    EmojiMap filteredEmojis{};
+    std::vector<EmojiPtr> filteredEmojis{};
     int emojiCount = 0;
 
-    getApp()->emotes->emojis.emojis.each(
-        [&, searchText](const auto &name, std::shared_ptr<EmojiData> &emoji) {
-            if (emoji->shortCodes[0].contains(searchText, Qt::CaseInsensitive))
-            {
-                filteredEmojis.insert(name, emoji);
-                emojiCount++;
-            }
-        });
+    const auto &emojis = getIApp()->getEmotes()->getEmojis()->getEmojis();
+    for (const auto &emoji : emojis)
+    {
+        if (emoji->shortCodes[0].contains(searchText, Qt::CaseInsensitive))
+        {
+            filteredEmojis.push_back(emoji);
+            emojiCount++;
+        }
+    }
     // emojis
     if (emojiCount > 0)
     {

diff --git a/tests/src/Emojis.cpp b/tests/src/Emojis.cpp
@@ -70,7 +70,14 @@ TEST(Emojis, Parse)
 
     auto getEmoji = [&](auto code) {
         std::shared_ptr<EmojiData> emoji;
-        emojis.getEmojis().tryGet(code, emoji);
+        for (const auto &e : emojis.getEmojis())
+        {
+            if (e->unifiedCode == code)
+            {
+                emoji = e;
+                break;
+            }
+        }
         return emoji->emote;
     };