fix: take indices to messages as a hint (#5683)

Chatterino · Nov 2, 2024 · 5f76f5b · 5f76f5b
1 parent ecfb35c
commit 5f76f5b
Show file tree

Hide file tree

Showing 7 changed files with 246 additions and 27 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -63,6 +63,7 @@
 - Bugfix: Fixed double-click selection not working when clicking outside a message. (#5617)
 - Bugfix: Fixed emotes starting with ":" not tab-completing. (#5603)
 - Bugfix: Fixed 7TV emotes messing with Qt's HTML. (#5677)
+- Bugfix: Fixed incorrect messages getting replaced visually. (#5683)
 - Dev: Update Windows build from Qt 6.5.0 to Qt 6.7.1. (#5420)
 - Dev: Update vcpkg build Qt from 6.5.0 to 6.7.0, boost from 1.83.0 to 1.85.0, openssl from 3.1.3 to 3.3.0. (#5422)
 - Dev: Unsingletonize `ISoundController`. (#5462)

diff --git a/src/common/Channel.cpp b/src/common/Channel.cpp
@@ -253,21 +253,33 @@ void Channel::fillInMissingMessages(const std::vector<MessagePtr> &messages)
     }
 }
 
-void Channel::replaceMessage(MessagePtr message, MessagePtr replacement)
+void Channel::replaceMessage(const MessagePtr &message,
+                             const MessagePtr &replacement)
 {
     int index = this->messages_.replaceItem(message, replacement);
 
     if (index >= 0)
     {
-        this->messageReplaced.invoke((size_t)index, replacement);
+        this->messageReplaced.invoke((size_t)index, message, replacement);
     }
 }
 
-void Channel::replaceMessage(size_t index, MessagePtr replacement)
+void Channel::replaceMessage(size_t index, const MessagePtr &replacement)
 {
-    if (this->messages_.replaceItem(index, replacement))
+    MessagePtr prev;
+    if (this->messages_.replaceItem(index, replacement, &prev))
     {
-        this->messageReplaced.invoke(index, replacement);
+        this->messageReplaced.invoke(index, prev, replacement);
+    }
+}
+
+void Channel::replaceMessage(size_t hint, const MessagePtr &message,
+                             const MessagePtr &replacement)
+{
+    auto index = this->messages_.replaceItem(hint, message, replacement);
+    if (index >= 0)
+    {
+        this->messageReplaced.invoke(hint, message, replacement);
     }
 }
 

diff --git a/src/common/Channel.hpp b/src/common/Channel.hpp
@@ -66,7 +66,9 @@ class Channel : public std::enable_shared_from_this<Channel>
     pajlada::Signals::Signal<MessagePtr &, std::optional<MessageFlags>>
         messageAppended;
     pajlada::Signals::Signal<std::vector<MessagePtr> &> messagesAddedAtStart;
-    pajlada::Signals::Signal<size_t, MessagePtr &> messageReplaced;
+    /// (index, prev-message, replacement)
+    pajlada::Signals::Signal<size_t, const MessagePtr &, const MessagePtr &>
+        messageReplaced;
     /// Invoked when some number of messages were filled in using time received
     pajlada::Signals::Signal<const std::vector<MessagePtr> &> filledInMessages;
     pajlada::Signals::NoArgSignal destroyed;
@@ -96,8 +98,11 @@ class Channel : public std::enable_shared_from_this<Channel>
 
     void addOrReplaceTimeout(MessagePtr message);
     void disableAllMessages();
-    void replaceMessage(MessagePtr message, MessagePtr replacement);
-    void replaceMessage(size_t index, MessagePtr replacement);
+    void replaceMessage(const MessagePtr &message,
+                        const MessagePtr &replacement);
+    void replaceMessage(size_t index, const MessagePtr &replacement);
+    void replaceMessage(size_t hint, const MessagePtr &message,
+                        const MessagePtr &replacement);
     void deleteMessage(QString messageID);
 
     /// Removes all messages from this channel and invokes #messagesCleared

diff --git a/src/messages/LimitedQueue.hpp b/src/messages/LimitedQueue.hpp
@@ -8,6 +8,7 @@
 #include <mutex>
 #include <optional>
 #include <shared_mutex>
+#include <utility>
 #include <vector>
 
 namespace chatterino {
@@ -212,9 +213,10 @@ class LimitedQueue
      *
      * @param[in] index the index of the item to replace
      * @param[in] replacement the item to put in place of the item at index
+     * @param[out] prev (optional) the item located at @a index before replacing
      * @return true if a replacement took place
      */
-    bool replaceItem(size_t index, const T &replacement)
+    bool replaceItem(size_t index, const T &replacement, T *prev = nullptr)
     {
         std::unique_lock lock(this->mutex_);
 
@@ -223,10 +225,46 @@ class LimitedQueue
             return false;
         }
 
-        this->buffer_[index] = replacement;
+        if (prev)
+        {
+            *prev = std::exchange(this->buffer_[index], replacement);
+        }
+        else
+        {
+            this->buffer_[index] = replacement;
+        }
         return true;
     }
 
+    /**
+     * @brief Replace the needle with the given item
+     *
+     * @param hint A hint on where the needle _might_ be
+     * @param[in] needle the item to search for
+     * @param[in] replacement the item to replace needle with
+     * @return the index of the replaced item, or -1 if no replacement took place
+     */
+    int replaceItem(size_t hint, const T &needle, const T &replacement)
+    {
+        std::unique_lock lock(this->mutex_);
+
+        if (hint < this->buffer_.size() && this->buffer_[hint] == needle)
+        {
+            this->buffer_[hint] = replacement;
+            return static_cast<int>(hint);
+        }
+
+        for (size_t i = 0; i < this->buffer_.size(); ++i)
+        {
+            if (this->buffer_[i] == needle)
+            {
+                this->buffer_[i] = replacement;
+                return static_cast<int>(i);
+            }
+        }
+        return -1;
+    }
+
     /**
      * @brief Inserts the given item before another item
      * 
@@ -315,6 +353,32 @@ class LimitedQueue
         return std::nullopt;
     }
 
+    /**
+     * @brief Find an item with a hint
+     *
+     * @param hint A hint on where the needle _might_ be
+     * @param predicate that will used to find the item
+     * @return the item and its index or none if it's not found
+     */
+    std::optional<std::pair<size_t, T>> find(size_t hint, auto &&predicate)
+    {
+        std::unique_lock lock(this->mutex_);
+
+        if (hint < this->buffer_.size() && predicate(this->buffer_[hint]))
+        {
+            return std::pair{hint, this->buffer_[hint]};
+        };
+
+        for (size_t i = 0; i < this->buffer_.size(); i++)
+        {
+            if (predicate(this->buffer_[i]))
+            {
+                return std::pair{i, this->buffer_[i]};
+            }
+        }
+        return std::nullopt;
+    }
+
     /**
      * @brief Returns the first item matching a predicate, checking in reverse
      * 

diff --git a/src/widgets/helper/ChannelView.cpp b/src/widgets/helper/ChannelView.cpp
@@ -967,10 +967,10 @@ void ChannelView::setChannel(const ChannelPtr &underlyingChannel)
 
     this->channelConnections_.managedConnect(
         underlyingChannel->messageReplaced,
-        [this](auto index, const auto &replacement) {
+        [this](auto index, const auto &prev, const auto &replacement) {
             if (this->shouldIncludeMessage(replacement))
             {
-                this->channel_->replaceMessage(index, replacement);
+                this->channel_->replaceMessage(index, prev, replacement);
             }
         });
 
@@ -1051,8 +1051,9 @@ void ChannelView::setChannel(const ChannelPtr &underlyingChannel)
     // on message replaced
     this->channelConnections_.managedConnect(
         this->channel_->messageReplaced,
-        [this](size_t index, MessagePtr replacement) {
-            this->messageReplaced(index, replacement);
+        [this](size_t index, const MessagePtr &prev,
+               const MessagePtr &replacement) {
+            this->messageReplaced(index, prev, replacement);
         });
 
     // on messages filled in
@@ -1258,27 +1259,29 @@ void ChannelView::messageAddedAtStart(std::vector<MessagePtr> &messages)
     this->queueLayout();
 }
 
-void ChannelView::messageReplaced(size_t index, MessagePtr &replacement)
+void ChannelView::messageReplaced(size_t hint, const MessagePtr &prev,
+                                  const MessagePtr &replacement)
 {
-    auto oMessage = this->messages_.get(index);
-    if (!oMessage)
+    auto optItem = this->messages_.find(hint, [&](const auto &it) {
+        return it->getMessagePtr() == prev;
+    });
+    if (!optItem)
     {
         return;
     }
-
-    auto message = *oMessage;
+    const auto &[index, oldItem] = *optItem;
 
     auto newItem = std::make_shared<MessageLayout>(replacement);
 
-    if (message->flags.has(MessageLayoutFlag::AlternateBackground))
+    if (oldItem->flags.has(MessageLayoutFlag::AlternateBackground))
     {
         newItem->flags.set(MessageLayoutFlag::AlternateBackground);
     }
 
     this->scrollBar_->replaceHighlight(index,
                                        replacement->getScrollBarHighlight());
 
-    this->messages_.replaceItem(message, newItem);
+    this->messages_.replaceItem(index, newItem);
     this->queueLayout();
 }
 

diff --git a/src/widgets/helper/ChannelView.hpp b/src/widgets/helper/ChannelView.hpp
@@ -268,7 +268,8 @@ class ChannelView final : public BaseWidget
                          std::optional<MessageFlags> overridingFlags);
     void messageAddedAtStart(std::vector<MessagePtr> &messages);
     void messageRemoveFromStart(MessagePtr &message);
-    void messageReplaced(size_t index, MessagePtr &replacement);
+    void messageReplaced(size_t hint, const MessagePtr &prev,
+                         const MessagePtr &replacement);
     void messagesUpdated();
 
     void performLayout(bool causedByScrollbar = false,