Fix emoji unified/non-qualified version for sending & parsing (#4840)

Co-authored-by: nerix <nerixdev@outlook.de>

CHANGELOG.md

@@ -6,6 +6,7 @@
 - Minor: The account switcher is now styled to match your theme. (#4817)
 - Minor: Add an invisible resize handle to the bottom of frameless user info popups and reply thread popups. (#4795)
 - Minor: The installer now checks for the VC Runtime version and shows more info when it's outdated. (#4847)
+- Bugfix: Fixed an issue where certain emojis did not send to Twitch chat correctly. (#4840)
 - Bugfix: Fixed capitalized channel names in log inclusion list not being logged. (#4848)
 - Bugfix: Trimmed custom streamlink paths on all platforms making sure you don't accidentally add spaces at the beginning or end of its path. (#4834)
 - Bugfix: Fixed a performance issue when displaying replies to certain messages. (#4807)

benchmarks/src/Emojis.cpp

@@ -55,3 +55,120 @@ static void BM_ShortcodeParsing(benchmark::State &state)
 }
 
 BENCHMARK(BM_ShortcodeParsing);
+
+static void BM_EmojiParsing(benchmark::State &state)
+{
+    Emojis emojis;
+
+    emojis.load();
+
+    struct TestCase {
+        QString input;
+        std::vector<boost::variant<EmotePtr, QString>> expectedOutput;
+    };
+
+    const auto &emojiMap = emojis.getEmojis();
+    std::shared_ptr<EmojiData> penguin;
+    emojiMap.tryGet("1F427", penguin);
+    auto penguinEmoji = penguin->emote;
+
+    std::vector<TestCase> tests{
+        {
+            // 1 emoji
+            "foo 🐧 bar",
+            // expected output
+            {
+                "foo ",
+                penguinEmoji,
+                " bar",
+            },
+        },
+        {
+            // no emoji
+            "foo bar",
+            // expected output
+            {
+                "foo bar",
+            },
+        },
+        {
+            // many emoji
+            "foo 🐧 bar 🐧🐧🐧🐧🐧",
+            // expected output
+            {
+                "foo ",
+                penguinEmoji,
+                " bar ",
+                penguinEmoji,
+                penguinEmoji,
+                penguinEmoji,
+                penguinEmoji,
+                penguinEmoji,
+            },
+        },
+    };
+
+    for (auto _ : state)
+    {
+        for (const auto &test : tests)
+        {
+            auto output = emojis.parse(test.input);
+
+            bool areEqual = std::equal(output.begin(), output.end(),
+                                       test.expectedOutput.begin());
+
+            if (!areEqual)
+            {
+                qDebug() << "BAD BENCH";
+                for (const auto &v : output)
+                {
+                    if (v.type() == typeid(QString))
+                    {
+                        qDebug() << "output:" << boost::get<QString>(v);
+                    }
+                }
+            }
+        }
+    }
+}
+
+BENCHMARK(BM_EmojiParsing);
+
+template <class... Args>
+static void BM_EmojiParsing2(benchmark::State &state, Args &&...args)
+{
+    Emojis emojis;
+
+    emojis.load();
+
+    auto argsTuple = std::make_tuple(std::move(args)...);
+    auto input = std::get<0>(argsTuple);
+    auto expectedNumEmojis = std::get<1>(argsTuple);
+    for (auto _ : state)
+    {
+        auto output = emojis.parse(input);
+        int actualNumEmojis = 0;
+        for (const auto &part : output)
+        {
+            if (part.type() == typeid(EmotePtr))
+            {
+                ++actualNumEmojis;
+            }
+        }
+
+        if (actualNumEmojis != expectedNumEmojis)
+        {
+            qDebug() << "BAD BENCH, EXPECTED NUM EMOJIS IS WRONG"
+                     << actualNumEmojis;
+        }
+    }
+}
+
+BENCHMARK_CAPTURE(BM_EmojiParsing2, one_emoji, "foo 🐧 bar", 1);
+BENCHMARK_CAPTURE(BM_EmojiParsing2, two_emoji, "foo 🐧 bar 🐧", 2);
+BENCHMARK_CAPTURE(
+    BM_EmojiParsing2, many_emoji,
+    "😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 "
+    "😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 "
+    "😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 ",
+    61);

src/providers/emoji/Emojis.cpp

@@ -12,22 +12,23 @@
 #include <rapidjson/error/error.h>
 #include <rapidjson/rapidjson.h>
 
-#include <array>
+#include <map>
 #include <memory>
 
-namespace chatterino {
 namespace {
 
-    auto toneNames = std::map<QString, QString>{
+using namespace chatterino;
+
+const std::map<QString, QString> TONE_NAMES{
     {"1F3FB", "tone1"}, {"1F3FC", "tone2"}, {"1F3FD", "tone3"},
     {"1F3FE", "tone4"}, {"1F3FF", "tone5"},
 };
 
 void parseEmoji(const std::shared_ptr<EmojiData> &emojiData,
                 const rapidjson::Value &unparsedEmoji,
-                    QString shortCode = QString())
+                const QString &shortCode = {})
 {
-        std::array<uint32_t, 9> unicodeBytes{};
+    std::vector<uint32_t> unicodeBytes{};
 
     struct {
         bool apple;
@@ -42,16 +43,17 @@ namespace {
     }
     else
     {
-            const auto &shortCodes = unparsedEmoji["short_names"];
-            for (const auto &_shortCode : shortCodes.GetArray())
+        // Load short codes from the suggested short_names
+        const auto &shortNames = unparsedEmoji["short_names"];
+        for (const auto &shortName : shortNames.GetArray())
         {
-                emojiData->shortCodes.emplace_back(_shortCode.GetString());
+            emojiData->shortCodes.emplace_back(shortName.GetString());
         }
     }
 
-        rj::getSafe(unparsedEmoji, "non_qualified",
-                    emojiData->nonQualifiedCode);
+    rj::getSafe(unparsedEmoji, "non_qualified", emojiData->nonQualifiedCode);
     rj::getSafe(unparsedEmoji, "unified", emojiData->unifiedCode);
+    assert(!emojiData->unifiedCode.isEmpty());
 
     rj::getSafe(unparsedEmoji, "has_img_apple", capabilities.apple);
     rj::getSafe(unparsedEmoji, "has_img_google", capabilities.google);
@@ -75,31 +77,48 @@ namespace {
         emojiData->capabilities.insert("Facebook");
     }
 
-        QStringList unicodeCharacters;
-        if (!emojiData->nonQualifiedCode.isEmpty())
-        {
-            unicodeCharacters =
-                emojiData->nonQualifiedCode.toLower().split('-');
-        }
-        else
-        {
-            unicodeCharacters = emojiData->unifiedCode.toLower().split('-');
-        }
-        if (unicodeCharacters.length() < 1)
-        {
-            return;
-        }
-
-        int numUnicodeBytes = 0;
+    QStringList unicodeCharacters = emojiData->unifiedCode.toLower().split('-');
 
     for (const QString &unicodeCharacter : unicodeCharacters)
     {
-            unicodeBytes.at(numUnicodeBytes++) =
-                QString(unicodeCharacter).toUInt(nullptr, 16);
+        bool ok{false};
+        unicodeBytes.push_back(QString(unicodeCharacter).toUInt(&ok, 16));
+        if (!ok)
+        {
+            qCWarning(chatterinoEmoji)
+                << "Failed to parse emoji" << emojiData->shortCodes;
+            return;
+        }
     }
 
-        emojiData->value =
-            QString::fromUcs4(unicodeBytes.data(), numUnicodeBytes);
+    // We can safely do a narrowing static cast since unicodeBytes will never be a large number
+    emojiData->value = QString::fromUcs4(unicodeBytes.data(),
+                                         static_cast<int>(unicodeBytes.size()));
+
+    if (!emojiData->nonQualifiedCode.isEmpty())
+    {
+        QStringList nonQualifiedCharacters =
+            emojiData->nonQualifiedCode.toLower().split('-');
+        std::vector<uint32_t> nonQualifiedBytes{};
+        for (const QString &unicodeCharacter : nonQualifiedCharacters)
+        {
+            bool ok{false};
+            nonQualifiedBytes.push_back(
+                QString(unicodeCharacter).toUInt(&ok, 16));
+            if (!ok)
+            {
+                qCWarning(chatterinoEmoji)
+                    << "Failed to parse emoji nonQualified"
+                    << emojiData->shortCodes;
+                return;
+            }
+        }
+
+        // We can safely do a narrowing static cast since unicodeBytes will never be a large number
+        emojiData->nonQualified =
+            QString::fromUcs4(nonQualifiedBytes.data(),
+                              static_cast<int>(nonQualifiedBytes.size()));
+    }
 }
 
 // getToneNames takes a tones and returns their names in the same order
@@ -111,8 +130,8 @@ namespace {
     QStringList toneNameResults;
     for (const auto &tonePart : toneParts)
     {
-            auto toneNameIt = toneNames.find(tonePart);
-            if (toneNameIt == toneNames.end())
+        auto toneNameIt = TONE_NAMES.find(tonePart);
+        if (toneNameIt == TONE_NAMES.end())
         {
             qDebug() << "Tone with key" << tonePart
                      << "does not exist in tone names map";
@@ -129,6 +148,8 @@ namespace {
 
 }  // namespace
 
+namespace chatterino {
+
 void Emojis::load()
 {
     this->loadEmojis();
@@ -219,6 +240,8 @@ void Emojis::loadEmojiSet()
     getSettings()->emojiSet.connect([this](const auto &emojiSet) {
         this->emojis.each([=](const auto &name,
                               std::shared_ptr<EmojiData> &emoji) {
+            (void)name;
+
             QString emojiSetToUse = emojiSet;
             // clang-format off
             static std::map<QString, QString> emojiSets = {
@@ -243,7 +266,7 @@ void Emojis::loadEmojiSet()
             };
             // clang-format on
 
-            if (emoji->capabilities.count(emojiSetToUse) == 0)
+            if (!emoji->capabilities.contains(emojiSetToUse))
             {
                 emojiSetToUse = "Twitter";
             }
@@ -268,7 +291,7 @@ std::vector<boost::variant<EmotePtr, QString>> Emojis::parse(
     const QString &text) const
 {
     auto result = std::vector<boost::variant<EmotePtr, QString>>();
-    int lastParsedEmojiEndIndex = 0;
+    QString::size_type lastParsedEmojiEndIndex = 0;
 
     for (auto i = 0; i < text.length(); ++i)
     {
@@ -288,32 +311,22 @@ std::vector<boost::variant<EmotePtr, QString>> Emojis::parse(
 
         const auto &possibleEmojis = it.value();
 
-        int remainingCharacters = text.length() - i - 1;
+        auto remainingCharacters = text.length() - i - 1;
 
         std::shared_ptr<EmojiData> matchedEmoji;
 
-        int matchedEmojiLength = 0;
+        QString::size_type matchedEmojiLength = 0;
 
         for (const std::shared_ptr<EmojiData> &emoji : possibleEmojis)
         {
-            int emojiExtraCharacters = emoji->value.length() - 1;
-            if (emojiExtraCharacters > remainingCharacters)
+            auto emojiNonQualifiedExtraCharacters =
+                emoji->nonQualified.length() - 1;
+            auto emojiExtraCharacters = emoji->value.length() - 1;
+            if (remainingCharacters >= emojiExtraCharacters)
             {
-                // It cannot be this emoji, there's not enough space for it
-                continue;
-            }
-
-            bool match = true;
-
-            for (int j = 1; j < emoji->value.length(); ++j)
-            {
-                if (text.at(i + j) != emoji->value.at(j))
-                {
-                    match = false;
-
-                    break;
-                }
-            }
+                // look in emoji->value
+                bool match = QStringView{emoji->value}.mid(1) ==
+                             QStringView{text}.mid(i + 1, emojiExtraCharacters);
 
                 if (match)
                 {
@@ -323,16 +336,34 @@ std::vector<boost::variant<EmotePtr, QString>> Emojis::parse(
                     break;
                 }
             }
+            if (!emoji->nonQualified.isNull() &&
+                remainingCharacters >= emojiNonQualifiedExtraCharacters)
+            {
+                // This checking here relies on the fact that the nonQualified string
+                // always starts with the same byte as value (the unified string)
+                bool match = QStringView{emoji->nonQualified}.mid(1) ==
+                             QStringView{text}.mid(
+                                 i + 1, emojiNonQualifiedExtraCharacters);
+
+                if (match)
+                {
+                    matchedEmoji = emoji;
+                    matchedEmojiLength = emoji->nonQualified.length();
+
+                    break;
+                }
+            }
+        }
 
         if (matchedEmojiLength == 0)
         {
             continue;
         }
 
-        int currentParsedEmojiFirstIndex = i;
-        int currentParsedEmojiEndIndex = i + (matchedEmojiLength);
+        auto currentParsedEmojiFirstIndex = i;
+        auto currentParsedEmojiEndIndex = i + (matchedEmojiLength);
 
-        int charactersFromLastParsedEmoji =
+        auto charactersFromLastParsedEmoji =
             currentParsedEmojiFirstIndex - lastParsedEmojiEndIndex;
 
         if (charactersFromLastParsedEmoji > 0)
@@ -382,7 +413,7 @@ QString Emojis::replaceShortCodes(const QString &text) const
             continue;
         }
 
-        auto emojiData = emojiIt.value();
+        const auto &emojiData = emojiIt.value();
 
         ret.replace(offset + match.capturedStart(), match.capturedLength(),
                     emojiData->value);

src/providers/emoji/Emojis.hpp

@@ -7,7 +7,6 @@
 #include <QRegularExpression>
 #include <QVector>
 
-#include <map>
 #include <set>
 #include <vector>
 
@@ -21,6 +20,9 @@ struct EmojiData {
     // :male:)
     QString value;
 
+    // actual byte-representation of the non qualified emoji
+    QString nonQualified;
+
     // i.e. 204e-50a2
     QString unifiedCode;
     QString nonQualifiedCode;

src/util/XDGDirectory.cpp

@@ -3,6 +3,8 @@
 #include "util/CombinePath.hpp"
 #include "util/Qt.hpp"
 
+#include <unordered_map>
+
 namespace chatterino {
 
 #if defined(Q_OS_UNIX) and !defined(Q_OS_DARWIN)