Merge 802edeb8de into 9345050868

CHANGELOG.md

@@ -110,6 +110,7 @@
 - Dev: Emojis now use flags instead of a set of strings for capabilities. (#5616)
 - Dev: Refactored static `MessageBuilder` helpers to standalone functions. (#5652)
 - Dev: Decoupled reply parsing from `MessageBuilder`. (#5660)
+- Dev: Refactored IRC message building. (#5663)
 
 ## 2.5.1
 

benchmarks/CMakeLists.txt

@@ -5,7 +5,6 @@ set(benchmark_SOURCES
     resources/bench.qrc
 
     src/Emojis.cpp
-    src/Highlights.cpp
     src/FormatTime.cpp
     src/Helpers.cpp
     src/LimitedQueue.cpp

benchmarks/src/Highlights.cpp

@@ -1,102 +0,0 @@
-#include "Application.hpp"
-#include "common/Channel.hpp"
-#include "controllers/accounts/AccountController.hpp"
-#include "controllers/highlights/HighlightController.hpp"
-#include "controllers/highlights/HighlightPhrase.hpp"
-#include "messages/Message.hpp"
-#include "messages/MessageBuilder.hpp"
-#include "mocks/BaseApplication.hpp"
-#include "mocks/UserData.hpp"
-#include "util/Helpers.hpp"
-
-#include <benchmark/benchmark.h>
-#include <QDebug>
-#include <QString>
-#include <QTemporaryDir>
-
-using namespace chatterino;
-
-class BenchmarkMessageBuilder : public MessageBuilder
-{
-public:
-    explicit BenchmarkMessageBuilder(
-        Channel *_channel, const Communi::IrcPrivateMessage *_ircMessage,
-        const MessageParseArgs &_args)
-        : MessageBuilder(_channel, _ircMessage, _args)
-    {
-    }
-
-    virtual MessagePtr build()
-    {
-        // PARSE
-        this->parse();
-        this->usernameColor_ = getRandomColor(this->ircMessage->nick());
-
-        // words
-        // this->addWords(this->originalMessage_.split(' '));
-
-        this->message().messageText = this->originalMessage_;
-        this->message().searchText = this->message().localizedName + " " +
-                                     this->userName + ": " +
-                                     this->originalMessage_;
-        return nullptr;
-    }
-
-    void bench()
-    {
-        this->parseHighlights();
-    }
-};
-
-class MockApplication : public mock::BaseApplication
-{
-public:
-    MockApplication()
-        : highlights(this->settings, &this->accounts)
-    {
-    }
-
-    AccountController *getAccounts() override
-    {
-        return &this->accounts;
-    }
-    HighlightController *getHighlights() override
-    {
-        return &this->highlights;
-    }
-
-    IUserDataController *getUserData() override
-    {
-        return &this->userData;
-    }
-
-    AccountController accounts;
-    HighlightController highlights;
-    mock::UserDataController userData;
-};
-
-static void BM_HighlightTest(benchmark::State &state)
-{
-    MockApplication mockApplication;
-
-    std::string message =
-        R"(@badge-info=subscriber/34;badges=moderator/1,subscriber/24;color=#FF0000;display-name=테스트계정420;emotes=41:6-13,15-22;flags=;id=a3196c7e-be4c-4b49-9c5a-8b8302b50c2a;mod=1;room-id=11148817;subscriber=1;tmi-sent-ts=1590922213730;turbo=0;user-id=117166826;user-type=mod :testaccount_420!testaccount_420@testaccount_420.tmi.twitch.tv PRIVMSG #pajlada :-tags Kreygasm,Kreygasm (no space))";
-    auto ircMessage = Communi::IrcMessage::fromData(message.c_str(), nullptr);
-    auto privMsg = dynamic_cast<Communi::IrcPrivateMessage *>(ircMessage);
-    assert(privMsg != nullptr);
-    MessageParseArgs args;
-    auto emptyChannel = Channel::getEmpty();
-
-    for (auto _ : state)
-    {
-        state.PauseTiming();
-        BenchmarkMessageBuilder b(emptyChannel.get(), privMsg, args);
-
-        b.build();
-        state.ResumeTiming();
-
-        b.bench();
-    }
-}
-
-BENCHMARK(BM_HighlightTest);

src/messages/Message.hpp

@@ -22,6 +22,7 @@ class ScrollbarHighlight;
 
 struct Message;
 using MessagePtr = std::shared_ptr<const Message>;
+using MessagePtrMut = std::shared_ptr<Message>;
 struct Message {
     Message();
     ~Message();

src/messages/MessageBuilder.hpp

@@ -14,8 +14,6 @@
 
 #include <ctime>
 #include <memory>
-#include <optional>
-#include <tuple>
 #include <unordered_map>
 #include <utility>
 
@@ -31,6 +29,7 @@ struct AutomodUserAction;
 struct AutomodInfoAction;
 struct Message;
 using MessagePtr = std::shared_ptr<const Message>;
+using MessagePtrMut = std::shared_ptr<Message>;
 
 class MessageElement;
 class TextElement;
@@ -68,6 +67,7 @@ struct LiveUpdatesUpdateEmoteSetMessageTag {
 struct ImageUploaderResultTag {
 };
 
+// NOLINTBEGIN(readability-identifier-naming)
 const SystemMessageTag systemMessage{};
 const RaidEntryMessageTag raidEntryMessage{};
 const TimeoutMessageTag timeoutMessage{};
@@ -79,6 +79,7 @@ const LiveUpdatesUpdateEmoteSetMessageTag liveUpdatesUpdateEmoteSetMessage{};
 // This signifies that you want to construct a message containing the result of
 // a successful image upload.
 const ImageUploaderResultTag imageUploaderResultMessage{};
+// NOLINTEND(readability-identifier-naming)
 
 MessagePtr makeSystemMessage(const QString &text);
 MessagePtr makeSystemMessage(const QString &text, const QTime &time);
@@ -90,26 +91,22 @@ struct MessageParseArgs {
     bool trimSubscriberUsername = false;
     bool isStaffOrBroadcaster = false;
     bool isSubscriptionMessage = false;
+    bool allowIgnore = true;
+    bool isAction = false;
     QString channelPointRewardId = "";
 };
 
+struct HighlightAlert {
+    QUrl customSound;
+    bool playSound = false;
+    bool windowAlert = false;
+};
 class MessageBuilder
 {
 public:
     /// Build a message without a base IRC message.
     MessageBuilder();
 
-    /// Build a message based on an incoming IRC PRIVMSG
-    explicit MessageBuilder(Channel *_channel,
-                            const Communi::IrcPrivateMessage *_ircMessage,
-                            const MessageParseArgs &_args);
-
-    /// Build a message based on an incoming IRC message (e.g. notice)
-    explicit MessageBuilder(Channel *_channel,
-                            const Communi::IrcMessage *_ircMessage,
-                            const MessageParseArgs &_args, QString content,
-                            bool isAction);
-
     MessageBuilder(SystemMessageTag, const QString &text,
                    const QTime &time = QTime::currentTime());
     MessageBuilder(RaidEntryMessageTag, const QString &text,
@@ -157,17 +154,10 @@ public:
 
     ~MessageBuilder() = default;
 
-    QString userName;
-
-    /// The Twitch Channel the message was received in
-    TwitchChannel *twitchChannel = nullptr;
-    /// The Twitch Channel the message was sent in, according to the Shared Chat feature
-    TwitchChannel *sourceChannel = nullptr;
-
     Message *operator->();
     Message &message();
-    MessagePtr release();
+    MessagePtrMut release();
-    std::weak_ptr<Message> weakOf();
+    std::weak_ptr<const Message> weakOf();
 
     void append(std::unique_ptr<MessageElement> element);
     void addLink(const linkparser::Parsed &parsedLink, const QString &source);
@@ -184,14 +174,8 @@ public:
         return pointer;
     }
 
-    [[nodiscard]] bool isIgnored() const;
+    static void triggerHighlights(const Channel *channel,
-    bool isIgnoredReply() const;
+                                  const HighlightAlert &alert);
-    void triggerHighlights();
-    MessagePtr build();
-
-    void setThread(std::shared_ptr<MessageThread> thread);
-    void setParent(MessagePtr parent);
-    void setMessageOffset(int offset);
 
     void appendChannelPointRewardMessage(const ChannelPointReward &reward,
                                          bool isMod, bool isBroadcaster);
@@ -231,96 +215,122 @@ public:
     static MessagePtr makeLowTrustUpdateMessage(
         const PubSubLowTrustUsersMessage &action);
 
-protected:
+    /// @brief Builds a message out of an `ircMessage`.
-    void addTextOrEmoji(EmotePtr emote);
+    ///
-    void addTextOrEmoji(const QString &string_);
+    /// Building a message won't cause highlights to be triggered. They will
+    /// only be parsed. To trigger highlights (play sound etc.), use
+    /// triggerHighlights().
+    ///
+    /// @param channel The channel this message was sent to. Must not be
+    ///                `nullptr`.
+    /// @param ircMessage The original message. This can be any message
+    ///                   (PRIVMSG, USERNOTICE, etc.). Its content is not
+    ///                   accessed through this parameter but through `content`,
+    ///                   as the content might be inside a tag (e.g. gifts in a
+    ///                   USERNOTICE).
+    /// @param args Arguments from parsing a chat message.
+    /// @param content The message text. This isn't always the entire text. In
+    ///                replies, the leading mention can be cut off.
+    ///                See `messageOffset`.
+    /// @param messageOffset Starting offset to be used on index-based
+    ///                      operations on `content` such as parsing emotes.
+    ///                      For example:
+    ///                         ircMessage = "@hi there"
+    ///                         content = "there"
+    ///                         messageOffset_ = 4
+    ///                      The index 6 would resolve to 6 - 4 = 2 => 'e'
+    /// @param thread The reply thread this message is part of. If there's no
+    ///               thread, this is an empty `shared_ptr`.
+    /// @param parent The direct parent this message is replying to. This does
+    ///               not need to be the `thread`s root. If this message isn't
+    ///               replying to anything, this is an empty `shared_ptr`.
+    ///
+    /// @returns The built message and a highlight result.
+    static std::pair<MessagePtrMut, HighlightAlert> makeIrcMessage(
+        Channel *channel, const Communi::IrcMessage *ircMessage,
+        const MessageParseArgs &args, QString content,
+        QString::size_type messageOffset,
+        const std::shared_ptr<MessageThread> &thread = {},
+        const MessagePtr &parent = {});
+
+private:
+    struct TextState {
+        TwitchChannel *twitchChannel = nullptr;
+        bool hasBits = false;
+        bool bitsStacked = false;
+        int bitsLeft = 0;
+    };
+    void addEmoji(const EmotePtr &emote);
+    void addTextOrEmote(TextState &state, QString string);
+
+    Outcome tryAppendCheermote(TextState &state, const QString &string);
+    Outcome tryAppendEmote(TwitchChannel *twitchChannel, const EmoteName &name);
 
     bool isEmpty() const;
     MessageElement &back();
     std::unique_ptr<MessageElement> releaseBack();
 
-    MessageColor textColor_ = MessageColor::Text;
-
     // Helper method that emplaces some text stylized as system text
     // and then appends that text to the QString parameter "toUpdate".
     // Returns the TextElement that was emplaced.
     TextElement *emplaceSystemTextAndUpdate(const QString &text,
                                             QString &toUpdate);
 
-    std::shared_ptr<Message> message_;
-
     void parse();
-    void parseUsernameColor();
+    void parseUsernameColor(const QVariantMap &tags, const QString &userID);
-    void parseUsername();
+    void parseUsername(const Communi::IrcMessage *ircMessage,
-    void parseMessageID();
+                       TwitchChannel *twitchChannel,
-    void parseRoomID();
+                       bool trimSubscriberUsername);
+    void parseMessageID(const QVariantMap &tags);
+
+    /// Parses the room-ID this message was received in
+    ///
+    /// @returns The room-ID
+    static QString parseRoomID(const QVariantMap &tags,
+                               TwitchChannel *twitchChannel);
+
+    /// Parses the shared-chat information from this message.
+    ///
+    /// @param tags The tags of the received message
+    /// @param twitchChannel The channel this message was received in
+    /// @returns The source channel - the channel this message originated from.
+    ///          If there's no channel currently open, @a twitchChannel is
+    ///          returned.
+    TwitchChannel *parseSharedChatInfo(const QVariantMap &tags,
+                                       TwitchChannel *twitchChannel);
+
     // Parse & build thread information into the message
     // Will read information from thread_ or from IRC tags
-    void parseThread();
+    void parseThread(const QString &messageContent, const QVariantMap &tags,
+                     const Channel *channel,
+                     const std::shared_ptr<MessageThread> &thread,
+                     const MessagePtr &parent);
     // parseHighlights only updates the visual state of the message, but leaves the playing of alerts and sounds to the triggerHighlights function
-    void parseHighlights();
+    HighlightAlert parseHighlights(const QVariantMap &tags,
-    void appendChannelName();
+                                   const QString &originalMessage,
-    void appendUsername();
+                                   const MessageParseArgs &args);
 
-    /// Return the Twitch Channel this message originated from
+    void appendChannelName(const Channel *channel);
-    ///
+    void appendUsername(const QVariantMap &tags, const MessageParseArgs &args);
-    /// Useful to handle messages from the "Shared Chat" feature
-    ///
-    /// Can return nullptr
-    const TwitchChannel *getSourceChannel() const;
-
-    std::tuple<std::optional<EmotePtr>, MessageElementFlags, bool> parseEmote(
-        const EmoteName &name) const;
-    Outcome tryAppendEmote(const EmoteName &name);
 
     void addWords(const QStringList &words,
-                  const std::vector<TwitchEmoteOccurrence> &twitchEmotes);
+                  const std::vector<TwitchEmoteOccurrence> &twitchEmotes,
+                  TextState &state);
 
-    void appendTwitchBadges();
+    void appendTwitchBadges(const QVariantMap &tags,
-    void appendChatterinoBadges();
+                            TwitchChannel *twitchChannel);
-    void appendFfzBadges();
+    void appendChatterinoBadges(const QString &userID);
-    void appendSeventvBadges();
+    void appendFfzBadges(TwitchChannel *twitchChannel, const QString &userID);
-    Outcome tryParseCheermote(const QString &string);
+    void appendSeventvBadges(const QString &userID);
 
-    bool shouldAddModerationElements() const;
+    [[nodiscard]] static bool isIgnored(const QString &originalMessage,
+                                        const QString &userID,
+                                        const Channel *channel);
 
-    QString roomID_;
+    std::shared_ptr<Message> message_;
-    bool hasBits_ = false;
+    MessageColor textColor_ = MessageColor::Text;
-    QString bits;
-    int bitsLeft{};
-    bool bitsStacked = false;
-    bool historicalMessage_ = false;
-    std::shared_ptr<MessageThread> thread_;
-    MessagePtr parent_;
-
-    /**
-     * Starting offset to be used on index-based operations on `originalMessage_`.
-     *
-     * For example:
-     * originalMessage_ = "there"
-     * messageOffset_ = 4
-     * (the irc message is "hey there")
-     *
-     * then the index 6 would resolve to 6 - 4 = 2 => 'e'
-     */
-    int messageOffset_ = 0;
-
-    QString userId_;
-    bool senderIsBroadcaster{};
-
-    Channel *channel = nullptr;
-    const Communi::IrcMessage *ircMessage;
-    MessageParseArgs args;
-    const QVariantMap tags;
-    QString originalMessage_;
-
-    const bool action_{};
 
     QColor usernameColor_ = {153, 153, 153};
-
-    bool highlightAlert_ = false;
-    bool highlightSound_ = false;
-    std::optional<QUrl> highlightSoundCustomUrl_{};
 };
 
 }  // namespace chatterino

src/providers/twitch/IrcMessageHandler.cpp

@@ -508,15 +508,20 @@ std::vector<MessagePtr> parseUserNoticeMessage(Channel *channel,
         {
             MessageParseArgs args;
             args.trimSubscriberUsername = true;
+            args.allowIgnore = false;
 
-            MessageBuilder builder(channel, message, args, content, false);
+            auto [built, highlight] = MessageBuilder::makeIrcMessage(
-            builder->flags.set(MessageFlag::Subscription);
+                channel, message, args, content, 0);
-            builder->flags.unset(MessageFlag::Highlighted);
+            if (built)
+            {
+                built->flags.set(MessageFlag::Subscription);
+                built->flags.unset(MessageFlag::Highlighted);
                 if (mirrored)
                 {
-                builder->flags.set(MessageFlag::SharedMessage);
+                    built->flags.set(MessageFlag::SharedMessage);
+                }
+                builtMessages.emplace_back(std::move(built));
             }
-            builtMessages.emplace_back(builder.build());
         }
     }
 
@@ -661,12 +666,13 @@ std::vector<MessagePtr> parsePrivMessage(Channel *channel,
 
     std::vector<MessagePtr> builtMessages;
     MessageParseArgs args;
-    MessageBuilder builder(channel, message, args, message->content(),
+    args.isAction = message->isAction();
-                           message->isAction());
+    auto [built, alert] = MessageBuilder::makeIrcMessage(channel, message, args,
-    if (!builder.isIgnored())
+                                                         message->content(), 0);
+    if (built)
     {
-        builtMessages.emplace_back(builder.build());
+        builtMessages.emplace_back(std::move(built));
-        builder.triggerHighlights();
+        MessageBuilder::triggerHighlights(channel, alert);
     }
 
     return builtMessages;
@@ -709,22 +715,21 @@ std::vector<MessagePtr> IrcMessageHandler::parseMessageWithReply(
         {
             args.channelPointRewardId = it.value().toString();
         }
-        MessageBuilder builder(channel, message, args, content,
+        args.isAction = privMsg->isAction();
-                               privMsg->isAction());
-        builder.setMessageOffset(messageOffset);
 
         auto replyCtx = getReplyContext(tc, message, otherLoaded);
-        builder.setThread(std::move(replyCtx.thread));
+        auto [built, alert] = MessageBuilder::makeIrcMessage(
-        builder.setParent(std::move(replyCtx.parent));
+            channel, message, args, content, messageOffset, replyCtx.thread,
+            replyCtx.parent);
+
+        if (built)
+        {
             if (replyCtx.highlight)
             {
-            builder.message().flags.set(MessageFlag::SubscribedThread);
+                built->flags.set(MessageFlag::SubscribedThread);
             }
-
+            builtMessages.emplace_back(built);
-        if (!builder.isIgnored())
+            MessageBuilder::triggerHighlights(channel, alert);
-        {
-            builtMessages.emplace_back(builder.build());
-            builder.triggerHighlights();
         }
 
         if (message->tags().contains(u"pinned-chat-paid-amount"_s))
@@ -1016,20 +1021,18 @@ void IrcMessageHandler::handleWhisperMessage(Communi::IrcMessage *ircMessage)
 
     auto *c = getApp()->getTwitch()->getWhispersChannel().get();
 
-    MessageBuilder builder(c, ircMessage, args,
+    auto [message, alert] = MessageBuilder::makeIrcMessage(
-                           unescapeZeroWidthJoiner(ircMessage->parameter(1)),
+        c, ircMessage, args, unescapeZeroWidthJoiner(ircMessage->parameter(1)),
-                           false);
+        0);
-
+    if (!message)
-    if (builder.isIgnored())
     {
         return;
     }
 
-    builder->flags.set(MessageFlag::Whisper);
+    message->flags.set(MessageFlag::Whisper);
-    MessagePtr message = builder.build();
+    MessageBuilder::triggerHighlights(c, alert);
-    builder.triggerHighlights();
 
-    getApp()->getTwitch()->setLastUserThatWhisperedMe(builder.userName);
+    getApp()->getTwitch()->setLastUserThatWhisperedMe(message->loginName);
 
     if (message->flags.has(MessageFlag::ShowInMentions))
     {
@@ -1504,6 +1507,7 @@ void IrcMessageHandler::addMessage(Communi::IrcMessage *message,
     {
         args.isStaffOrBroadcaster = true;
     }
+    args.isAction = isAction;
 
     auto *channel = dynamic_cast<TwitchChannel *>(chan.get());
 
@@ -1605,24 +1609,22 @@ void IrcMessageHandler::addMessage(Communi::IrcMessage *message,
         }
     }
 
-    MessageBuilder builder(channel, message, args, content, isAction);
+    args.allowIgnore = !isSub;
-    builder.setMessageOffset(messageOffset);
+    auto [msg, alert] = MessageBuilder::makeIrcMessage(
+        channel, message, args, content, messageOffset, replyCtx.thread,
+        replyCtx.parent);
 
-    builder.setThread(std::move(replyCtx.thread));
+    if (msg)
-    builder.setParent(std::move(replyCtx.parent));
-    if (replyCtx.highlight)
-    {
-        builder.message().flags.set(MessageFlag::SubscribedThread);
-    }
-
-    if (isSub || !builder.isIgnored())
     {
         if (isSub)
         {
-            builder->flags.set(MessageFlag::Subscription);
+            msg->flags.set(MessageFlag::Subscription);
-            builder->flags.unset(MessageFlag::Highlighted);
+            msg->flags.unset(MessageFlag::Highlighted);
+        }
+        if (replyCtx.highlight)
+        {
+            msg->flags.set(MessageFlag::SubscribedThread);
         }
-        auto msg = builder.build();
 
         IrcMessageHandler::setSimilarityFlags(msg, chan);
 
@@ -1630,7 +1632,7 @@ void IrcMessageHandler::addMessage(Communi::IrcMessage *message,
             (!getSettings()->hideSimilar &&
              getSettings()->shownSimilarTriggerHighlights))
         {
-            builder.triggerHighlights();
+            MessageBuilder::triggerHighlights(channel, alert);
         }
 
         const auto highlighted = msg->flags.has(MessageFlag::Highlighted);

tests/snapshots/IrcMessageHandler/bad-emotes.json

@@ -153,7 +153,7 @@
             "searchText": "mm2pl  mm2pl: Kappa ",
             "serverReceivedTime": "2022-09-03T10:31:42Z",
             "timeoutUser": "",
-            "usernameColor": "#ff000000"
+            "usernameColor": "#ffdaa521"
         }
     ]
 }

tests/snapshots/IrcMessageHandler/bad-emotes2.json

@@ -153,7 +153,7 @@
             "searchText": "mm2pl  mm2pl: Kappa ",
             "serverReceivedTime": "2022-09-03T10:31:42Z",
             "timeoutUser": "",
-            "usernameColor": "#ff000000"
+            "usernameColor": "#ffdaa521"
         }
     ]
 }

tests/snapshots/IrcMessageHandler/emote.json

@@ -153,7 +153,7 @@
             "searchText": "mm2pl  mm2pl: Keepo ",
             "serverReceivedTime": "2022-09-03T10:31:35Z",
             "timeoutUser": "",
-            "usernameColor": "#ff000000"
+            "usernameColor": "#ffdaa521"
         }
     ]
 }

tests/snapshots/IrcMessageHandler/emotes.json

@@ -221,7 +221,7 @@
             "searchText": "mm2pl  mm2pl: Kappa Keepo PogChamp ",
             "serverReceivedTime": "2022-09-03T10:31:42Z",
             "timeoutUser": "",
-            "usernameColor": "#ff000000"
+            "usernameColor": "#ffdaa521"
         }
     ]
 }

tests/src/Filters.cpp

@@ -285,9 +285,9 @@ TEST_F(FiltersF, TypingContextChecks)
 
     QString originalMessage = privmsg->content();
 
-    MessageBuilder builder(&channel, privmsg, MessageParseArgs{});
+    auto [msg, alert] = MessageBuilder::makeIrcMessage(
+        &channel, privmsg, MessageParseArgs{}, originalMessage, 0);
 
-    auto msg = builder.build();
     EXPECT_NE(msg.get(), nullptr);
 
     auto contextMap = buildContextMap(msg, &channel);