Show historic timeouts and bans in usercard (#4760)

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

CHANGELOG.md

@@ -40,6 +40,7 @@
 - Bugfix: Fixed crash that could occurr when closing the usercard too quickly after blocking or unblocking a user. (#4711)
 - Bugfix: Fixed highlights sometimes not working after changing sound device, or switching users in your operating system. (#4729)
 - Bugfix: Fixed key bindings not showing in context menus on Mac. (#4722)
+- Bugfix: Fixed timeouts from history messages not behaving consistently. (#4760)
 - Bugfix: Fixed tab completion rarely completing the wrong word. (#4735)
 - Bugfix: Fixed an issue where Subscriptions & Announcements that contained ignored phrases would still appear if the Block option was enabled. (#4748)
 - Dev: Added command to set Qt's logging filter/rules at runtime (`/c2-set-logging-rules`). (#4637)

src/CMakeLists.txt

@@ -397,6 +397,7 @@ set(SOURCE_FILES
         util/AttachToConsole.cpp
         util/AttachToConsole.hpp
         util/CancellationToken.hpp
+        util/ChannelHelpers.hpp
         util/Clipboard.cpp
         util/Clipboard.hpp
         util/ConcurrentMap.hpp

src/common/Channel.cpp

@@ -10,6 +10,7 @@
 #include "singletons/Logging.hpp"
 #include "singletons/Settings.hpp"
 #include "singletons/WindowManager.hpp"
+#include "util/ChannelHelpers.hpp"
 
 #include <QJsonArray>
 #include <QJsonDocument>
@@ -113,95 +114,15 @@ void Channel::addMessage(MessagePtr message,
 
 void Channel::addOrReplaceTimeout(MessagePtr message)
 {
-    LimitedQueueSnapshot<MessagePtr> snapshot = this->getMessageSnapshot();
+    addOrReplaceChannelTimeout(
-    int snapshotLength = snapshot.size();
+        this->getMessageSnapshot(), std::move(message), QTime::currentTime(),
-
+        [this](auto /*idx*/, auto msg, auto replacement) {
-    int end = std::max(0, snapshotLength - 20);
+            this->replaceMessage(msg, replacement);
-
+        },
-    bool addMessage = true;
+        [this](auto msg) {
-
+            this->addMessage(msg);
-    QTime minimumTime = QTime::currentTime().addSecs(-5);
+        },
-
+        true);
-    auto timeoutStackStyle = static_cast<TimeoutStackStyle>(
-        getSettings()->timeoutStackStyle.getValue());
-
-    for (int i = snapshotLength - 1; i >= end; --i)
-    {
-        auto &s = snapshot[i];
-
-        if (s->parseTime < minimumTime)
-        {
-            break;
-        }
-
-        if (s->flags.has(MessageFlag::Untimeout) &&
-            s->timeoutUser == message->timeoutUser)
-        {
-            break;
-        }
-
-        if (timeoutStackStyle == TimeoutStackStyle::DontStackBeyondUserMessage)
-        {
-            if (s->loginName == message->timeoutUser &&
-                s->flags.hasNone({MessageFlag::Disabled, MessageFlag::Timeout,
-                                  MessageFlag::Untimeout}))
-            {
-                break;
-            }
-        }
-
-        if (s->flags.has(MessageFlag::Timeout) &&
-            s->timeoutUser == message->timeoutUser)
-        {
-            if (message->flags.has(MessageFlag::PubSub) &&
-                !s->flags.has(MessageFlag::PubSub))
-            {
-                this->replaceMessage(s, message);
-                addMessage = false;
-                break;
-            }
-            if (!message->flags.has(MessageFlag::PubSub) &&
-                s->flags.has(MessageFlag::PubSub))
-            {
-                addMessage = timeoutStackStyle == TimeoutStackStyle::DontStack;
-                break;
-            }
-
-            int count = s->count + 1;
-
-            MessageBuilder replacement(timeoutMessage, message->timeoutUser,
-                                       message->loginName, message->searchText,
-                                       count);
-
-            replacement->timeoutUser = message->timeoutUser;
-            replacement->count = count;
-            replacement->flags = message->flags;
-
-            this->replaceMessage(s, replacement.release());
-
-            addMessage = false;
-            break;
-        }
-    }
-
-    // disable the messages from the user
-    for (int i = 0; i < snapshotLength; i++)
-    {
-        auto &s = snapshot[i];
-        if (s->loginName == message->timeoutUser &&
-            s->flags.hasNone({MessageFlag::Timeout, MessageFlag::Untimeout,
-                              MessageFlag::Whisper}))
-        {
-            // FOURTF: disabled for now
-            // PAJLADA: Shitty solution described in Message.hpp
-            s->flags.set(MessageFlag::Disabled);
-        }
-    }
-
-    if (addMessage)
-    {
-        this->addMessage(message);
-    }
 
     // XXX: Might need the following line
     // WindowManager::instance().repaintVisibleChatWidgets(this);

src/providers/recentmessages/Impl.cpp

@@ -20,52 +20,6 @@ const auto &LOG = chatterinoRecentMessages;
 
 namespace chatterino::recentmessages::detail {
 
-// convertClearchatToNotice takes a Communi::IrcMessage that is a CLEARCHAT
-// command and converts it to a readable NOTICE message. This has
-// historically been done in the Recent Messages API, but this functionality
-// has been moved to Chatterino instead.
-Communi::IrcMessage *convertClearchatToNotice(Communi::IrcMessage *message)
-{
-    auto channelName = message->parameter(0);
-    QString noticeMessage{};
-    if (message->tags().contains("target-user-id"))
-    {
-        auto target = message->parameter(1);
-
-        if (message->tags().contains("ban-duration"))
-        {
-            // User was timed out
-            noticeMessage =
-                QString("%1 has been timed out for %2.")
-                    .arg(target)
-                    .arg(formatTime(message->tag("ban-duration").toString()));
-        }
-        else
-        {
-            // User was permanently banned
-            noticeMessage =
-                QString("%1 has been permanently banned.").arg(target);
-        }
-    }
-    else
-    {
-        // Chat was cleared
-        noticeMessage = "Chat has been cleared by a moderator.";
-    }
-
-    // rebuild the raw IRC message so we can convert it back to an ircmessage again!
-    // this could probably be done in a smarter way
-
-    auto s = QString(":tmi.twitch.tv NOTICE %1 :%2")
-                 .arg(channelName)
-                 .arg(noticeMessage);
-
-    auto *newMessage = Communi::IrcMessage::fromData(s.toUtf8(), nullptr);
-    newMessage->setTags(message->tags());
-
-    return newMessage;
-}
-
 // Parse the IRC messages returned in JSON form into Communi messages
 std::vector<Communi::IrcMessage *> parseRecentMessages(
     const QJsonObject &jsonRoot)
@@ -89,11 +43,6 @@ std::vector<Communi::IrcMessage *> parseRecentMessages(
         auto *message =
             Communi::IrcMessage::fromData(content.toUtf8(), nullptr);
 
-        if (message->command() == "CLEARCHAT")
-        {
-            message = convertClearchatToNotice(message);
-        }
-
         messages.emplace_back(message);
     }
 

src/providers/recentmessages/Impl.hpp

@@ -13,12 +13,6 @@
 
 namespace chatterino::recentmessages::detail {
 
-// convertClearchatToNotice takes a Communi::IrcMessage that is a CLEARCHAT
-// command and converts it to a readable NOTICE message. This has
-// historically been done in the Recent Messages API, but this functionality
-// has been moved to Chatterino instead.
-Communi::IrcMessage *convertClearchatToNotice(Communi::IrcMessage *message);
-
 // Parse the IRC messages returned in JSON form into Communi messages
 std::vector<Communi::IrcMessage *> parseRecentMessages(
     const QJsonObject &jsonRoot);

src/providers/twitch/IrcMessageHandler.cpp

@@ -1,4 +1,4 @@
-#include "IrcMessageHandler.hpp"
+#include "providers/twitch/IrcMessageHandler.hpp"
 
 #include "Application.hpp"
 #include "common/Literals.hpp"
@@ -22,6 +22,7 @@
 #include "singletons/Resources.hpp"
 #include "singletons/Settings.hpp"
 #include "singletons/WindowManager.hpp"
+#include "util/ChannelHelpers.hpp"
 #include "util/FormatTime.hpp"
 #include "util/Helpers.hpp"
 #include "util/IrcHelpers.hpp"
@@ -377,7 +378,7 @@ void IrcMessageHandler::handlePrivMessage(Communi::IrcPrivateMessage *message,
 
 std::vector<MessagePtr> IrcMessageHandler::parseMessageWithReply(
     Channel *channel, Communi::IrcMessage *message,
-    const std::vector<MessagePtr> &otherLoaded)
+    std::vector<MessagePtr> &otherLoaded)
 {
     std::vector<MessagePtr> builtMessages;
 
@@ -416,6 +417,33 @@ std::vector<MessagePtr> IrcMessageHandler::parseMessageWithReply(
         return this->parseNoticeMessage(
             static_cast<Communi::IrcNoticeMessage *>(message));
     }
+    else if (command == u"CLEARCHAT"_s)
+    {
+        auto cc = this->parseClearChatMessage(message);
+        if (!cc)
+        {
+            return builtMessages;
+        }
+        auto &clearChat = *cc;
+        if (clearChat.disableAllMessages)
+        {
+            builtMessages.emplace_back(std::move(clearChat.message));
+        }
+        else
+        {
+            addOrReplaceChannelTimeout(
+                otherLoaded, std::move(clearChat.message),
+                calculateMessageTime(message).time(),
+                [&](auto idx, auto /*msg*/, auto &&replacement) {
+                    replacement->flags.set(MessageFlag::RecentMessage);
+                    otherLoaded[idx] = replacement;
+                },
+                [&](auto &&msg) {
+                    builtMessages.emplace_back(msg);
+                },
+                false);
+        }
+    }
 
     return builtMessages;
 }
@@ -662,40 +690,24 @@ void IrcMessageHandler::handleRoomStateMessage(Communi::IrcMessage *message)
     twitchChannel->roomModesChanged.invoke();
 }
 
-void IrcMessageHandler::handleClearChatMessage(Communi::IrcMessage *message)
+std::optional<ClearChatMessage> IrcMessageHandler::parseClearChatMessage(
+    Communi::IrcMessage *message)
 {
     // check parameter count
     if (message->parameters().length() < 1)
     {
-        return;
+        return std::nullopt;
-    }
-
-    QString chanName;
-    if (!trimChannelName(message->parameter(0), chanName))
-    {
-        return;
-    }
-
-    // get channel
-    auto chan = getApp()->twitch->getChannelOrEmpty(chanName);
-
-    if (chan->isEmpty())
-    {
-        qCDebug(chatterinoTwitch)
-            << "[IrcMessageHandler:handleClearChatMessage] Twitch channel"
-            << chanName << "not found";
-        return;
     }
 
     // check if the chat has been cleared by a moderator
     if (message->parameters().length() == 1)
     {
-        chan->disableAllMessages();
+        return ClearChatMessage{
-        chan->addMessage(
+            .message =
                 makeSystemMessage("Chat has been cleared by a moderator.",
-                              calculateMessageTime(message).time()));
+                                  calculateMessageTime(message).time()),
-
+            .disableAllMessages = true,
-        return;
+        };
     }
 
     // get username, duration and message of the timed out user
@@ -711,7 +723,46 @@ void IrcMessageHandler::handleClearChatMessage(Communi::IrcMessage *message)
         MessageBuilder(timeoutMessage, username, durationInSeconds, false,
                        calculateMessageTime(message).time())
             .release();
-    chan->addOrReplaceTimeout(timeoutMsg);
+
+    return ClearChatMessage{.message = timeoutMsg, .disableAllMessages = false};
+}
+
+void IrcMessageHandler::handleClearChatMessage(Communi::IrcMessage *message)
+{
+    auto cc = this->parseClearChatMessage(message);
+    if (!cc)
+    {
+        return;
+    }
+    auto &clearChat = *cc;
+
+    QString chanName;
+    if (!trimChannelName(message->parameter(0), chanName))
+    {
+        return;
+    }
+
+    // get channel
+    auto chan = getApp()->twitch->getChannelOrEmpty(chanName);
+
+    if (chan->isEmpty())
+    {
+        qCDebug(chatterinoTwitch)
+            << "[IrcMessageHandler::handleClearChatMessage] Twitch channel"
+            << chanName << "not found";
+        return;
+    }
+
+    // chat has been cleared by a moderator
+    if (clearChat.disableAllMessages)
+    {
+        chan->disableAllMessages();
+        chan->addMessage(std::move(clearChat.message));
+
+        return;
+    }
+
+    chan->addOrReplaceTimeout(std::move(clearChat.message));
 
     // refresh all
     getApp()->windows->repaintVisibleChatWidgets(chan.get());

src/providers/twitch/IrcMessageHandler.hpp

@@ -4,6 +4,7 @@
 
 #include <IrcMessage>
 
+#include <optional>
 #include <vector>
 
 namespace chatterino {
@@ -16,6 +17,11 @@ using MessagePtr = std::shared_ptr<const Message>;
 class TwitchChannel;
 class TwitchMessageBuilder;
 
+struct ClearChatMessage {
+    MessagePtr message;
+    bool disableAllMessages;
+};
+
 class IrcMessageHandler
 {
     IrcMessageHandler() = default;
@@ -29,7 +35,7 @@ public:
 
     std::vector<MessagePtr> parseMessageWithReply(
         Channel *channel, Communi::IrcMessage *message,
-        const std::vector<MessagePtr> &otherLoaded);
+        std::vector<MessagePtr> &otherLoaded);
 
     // parsePrivMessage arses a single IRC PRIVMSG into 0-1 Chatterino messages
     std::vector<MessagePtr> parsePrivMessage(
@@ -38,6 +44,8 @@ public:
                            TwitchIrcServer &server);
 
     void handleRoomStateMessage(Communi::IrcMessage *message);
+    std::optional<ClearChatMessage> parseClearChatMessage(
+        Communi::IrcMessage *message);
     void handleClearChatMessage(Communi::IrcMessage *message);
     void handleClearMessageMessage(Communi::IrcMessage *message);
     void handleUserStateMessage(Communi::IrcMessage *message);

src/util/ChannelHelpers.hpp

@@ -0,0 +1,121 @@
+#pragma once
+
+#include "common/Channel.hpp"
+#include "messages/Message.hpp"
+#include "messages/MessageBuilder.hpp"
+#include "singletons/Settings.hpp"
+
+namespace chatterino {
+
+/// Adds a timeout or replaces a previous one sent in the last 20 messages and in the last 5s.
+/// This function accepts any buffer to store the messsages in.
+/// @param replaceMessage A function of type `void (int index, MessagePtr toReplace, MessagePtr replacement)`
+///                       - replace `buffer[i]` (=toReplace) with `replacement`
+/// @param addMessage A function of type `void (MessagePtr message)`
+///                   - adds the `message`.
+/// @param disableUserMessages If set, disables all message by the timed out user.
+template <typename Buf, typename Replace, typename Add>
+void addOrReplaceChannelTimeout(const Buf &buffer, MessagePtr message,
+                                QTime now, Replace replaceMessage,
+                                Add addMessage, bool disableUserMessages)
+{
+    // NOTE: This function uses the messages PARSE time to figure out whether they should be replaced
+    // This works as expected for incoming messages, but not for historic messages.
+    // This has never worked before, but would be nice in the future.
+    // For this to work, we need to make sure *all* messages have a "server received time".
+
+    auto snapshotLength = static_cast<qsizetype>(buffer.size());
+
+    auto end = std::max<qsizetype>(0, snapshotLength - 20);
+
+    bool shouldAddMessage = true;
+
+    QTime minimumTime = now.addSecs(-5);
+
+    auto timeoutStackStyle = static_cast<TimeoutStackStyle>(
+        getSettings()->timeoutStackStyle.getValue());
+
+    for (auto i = snapshotLength - 1; i >= end; --i)
+    {
+        const MessagePtr &s = buffer[i];
+
+        if (s->parseTime < minimumTime)
+        {
+            break;
+        }
+
+        if (s->flags.has(MessageFlag::Untimeout) &&
+            s->timeoutUser == message->timeoutUser)
+        {
+            break;
+        }
+
+        if (timeoutStackStyle == TimeoutStackStyle::DontStackBeyondUserMessage)
+        {
+            if (s->loginName == message->timeoutUser &&
+                s->flags.hasNone({MessageFlag::Disabled, MessageFlag::Timeout,
+                                  MessageFlag::Untimeout}))
+            {
+                break;
+            }
+        }
+
+        if (s->flags.has(MessageFlag::Timeout) &&
+            s->timeoutUser == message->timeoutUser)
+        {
+            if (message->flags.has(MessageFlag::PubSub) &&
+                !s->flags.has(MessageFlag::PubSub))
+            {
+                replaceMessage(i, s, message);
+                shouldAddMessage = false;
+                break;
+            }
+            if (!message->flags.has(MessageFlag::PubSub) &&
+                s->flags.has(MessageFlag::PubSub))
+            {
+                shouldAddMessage =
+                    timeoutStackStyle == TimeoutStackStyle::DontStack;
+                break;
+            }
+
+            uint32_t count = s->count + 1;
+
+            MessageBuilder replacement(timeoutMessage, message->timeoutUser,
+                                       message->loginName, message->searchText,
+                                       count);
+
+            replacement->timeoutUser = message->timeoutUser;
+            replacement->count = count;
+            replacement->flags = message->flags;
+
+            replaceMessage(i, s, replacement.release());
+
+            shouldAddMessage = false;
+            break;
+        }
+    }
+
+    // disable the messages from the user
+    if (disableUserMessages)
+    {
+        for (qsizetype i = 0; i < snapshotLength; i++)
+        {
+            auto &s = buffer[i];
+            if (s->loginName == message->timeoutUser &&
+                s->flags.hasNone({MessageFlag::Timeout, MessageFlag::Untimeout,
+                                  MessageFlag::Whisper}))
+            {
+                // FOURTF: disabled for now
+                // PAJLADA: Shitty solution described in Message.hpp
+                s->flags.set(MessageFlag::Disabled);
+            }
+        }
+    }
+
+    if (shouldAddMessage)
+    {
+        addMessage(message);
+    }
+}
+
+}  // namespace chatterino