diff --git a/CHANGELOG.md b/CHANGELOG.md index b48ade145..d79f8c9c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - Minor: Added a setting to hide similar messages by any user. (#2716) - Minor: Duplicate spaces now count towards the display message length. (#3002) - Minor: Commands are now backed up. (#3168) +- Bugfix: Fixed colored usernames sometimes not working. (#3170) - Bugfix: Restored ability to send duplicate `/me` messages. (#3166) - Bugfix: Notifications for moderators about other moderators deleting messages can now be disabled. (#3121) - Bugfix: Moderation mode and active filters are now preserved when opening a split as a popup. (#3113, #3130) diff --git a/lib/lrucache/lrucache/lrucache.hpp b/lib/lrucache/lrucache/lrucache.hpp index aa2c6c511..ef7d031db 100644 --- a/lib/lrucache/lrucache/lrucache.hpp +++ b/lib/lrucache/lrucache/lrucache.hpp @@ -35,6 +35,7 @@ public: lru_cache(lru_cache &&other) : _cache_items_list(std::move(other._cache_items_list)) , _cache_items_map(std::move(other._cache_items_map)) + , _max_size(other._max_size) { other._cache_items_list.clear(); other._cache_items_map.clear(); @@ -44,6 +45,7 @@ public: { _cache_items_list = std::move(other._cache_items_list); _cache_items_map = std::move(other._cache_items_map); + _max_size = other._max_size; other._cache_items_list.clear(); other._cache_items_map.clear(); return *this; diff --git a/src/common/ChannelChatters.cpp b/src/common/ChannelChatters.cpp index 8b72f7b17..f9eb1ea65 100644 --- a/src/common/ChannelChatters.cpp +++ b/src/common/ChannelChatters.cpp @@ -74,6 +74,12 @@ void ChannelChatters::updateOnlineChatters( chatters_->updateOnlineChatters(chatters); } +size_t ChannelChatters::colorsSize() const +{ + auto size = this->chatterColors_.access()->size(); + return size; +} + const QColor ChannelChatters::getUserColor(const QString &user) { const auto chatterColors = this->chatterColors_.access(); diff --git a/src/common/ChannelChatters.hpp b/src/common/ChannelChatters.hpp index 3a2c4e49a..70ca53924 100644 --- a/src/common/ChannelChatters.hpp +++ b/src/common/ChannelChatters.hpp @@ -25,9 +25,13 @@ public: void setUserColor(const QString &user, const QColor &color); void updateOnlineChatters(const std::unordered_set &chatters); -private: + // colorsSize returns the amount of colors stored in `chatterColors_` + // NOTE: This function is only meant to be used in tests and benchmarks + size_t colorsSize() const; + static constexpr int maxChatterColorCount = 5000; +private: Channel &channel_; // maps 2 char prefix to set of names diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 8b1fafe99..4c15a4902 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -2,6 +2,7 @@ project(chatterino-test) set(test_SOURCES ${CMAKE_CURRENT_LIST_DIR}/src/main.cpp + ${CMAKE_CURRENT_LIST_DIR}/src/ChannelChatters.cpp ${CMAKE_CURRENT_LIST_DIR}/src/AccessGuard.cpp ${CMAKE_CURRENT_LIST_DIR}/src/NetworkCommon.cpp ${CMAKE_CURRENT_LIST_DIR}/src/NetworkRequest.cpp diff --git a/tests/src/ChannelChatters.cpp b/tests/src/ChannelChatters.cpp new file mode 100644 index 000000000..e0006a205 --- /dev/null +++ b/tests/src/ChannelChatters.cpp @@ -0,0 +1,113 @@ +#include "common/ChannelChatters.hpp" + +#include +#include +#include + +namespace chatterino { + +class MockChannel : public Channel +{ +public: + MockChannel(const QString &name) + : Channel(name, Channel::Type::Twitch) + { + } +}; + +} // namespace chatterino + +using namespace chatterino; + +// Ensure inserting the same user does not increase the size of the stored colors +TEST(ChatterChatters, insertSameUser) +{ + MockChannel channel("test"); + + ChannelChatters chatters(channel); + + EXPECT_EQ(chatters.colorsSize(), 0); + chatters.setUserColor("pajlada", QColor("#fff")); + EXPECT_EQ(chatters.colorsSize(), 1); + chatters.setUserColor("pajlada", QColor("#fff")); + EXPECT_EQ(chatters.colorsSize(), 1); +} + +// Ensure we can update a chatters color +TEST(ChatterChatters, insertSameUserUpdatesColor) +{ + MockChannel channel("test"); + + ChannelChatters chatters(channel); + + chatters.setUserColor("pajlada", QColor("#fff")); + EXPECT_EQ(chatters.getUserColor("pajlada"), QColor("#fff")); + chatters.setUserColor("pajlada", QColor("#f0f")); + EXPECT_EQ(chatters.getUserColor("pajlada"), QColor("#f0f")); +} + +// Ensure getting a non-existant users color returns an invalid QColor +TEST(ChatterChatters, getNonExistantUser) +{ + MockChannel channel("test"); + + ChannelChatters chatters(channel); + + EXPECT_EQ(chatters.getUserColor("nonexistantuser"), QColor()); +} + +// Ensure getting a user doesn't create an entry +TEST(ChatterChatters, getDoesNotCreate) +{ + MockChannel channel("test"); + + ChannelChatters chatters(channel); + + EXPECT_EQ(chatters.colorsSize(), 0); + chatters.getUserColor("nonexistantuser"); + EXPECT_EQ(chatters.colorsSize(), 0); +} + +// Ensure the least recently used entry is purged when we reach MAX_SIZE +TEST(ChatterChatters, insertMaxSize) +{ + MockChannel channel("test"); + + ChannelChatters chatters(channel); + + // Prime chatters with 2 control entries + chatters.setUserColor("pajlada", QColor("#f00")); + chatters.setUserColor("zneix", QColor("#f0f")); + + EXPECT_EQ(chatters.getUserColor("pajlada"), QColor("#f00")); + EXPECT_EQ(chatters.getUserColor("zneix"), QColor("#f0f")); + EXPECT_EQ(chatters.getUserColor("nonexistantuser"), QColor()); + + EXPECT_EQ(chatters.colorsSize(), 2); + + for (int i = 0; i < ChannelChatters::maxChatterColorCount - 1; ++i) + { + auto username = QString("user%1").arg(i); + chatters.setUserColor(username, QColor("#00f")); + } + + // Should have bumped ONE entry out (pajlada) + + EXPECT_EQ(chatters.getUserColor("pajlada"), QColor()); + EXPECT_EQ(chatters.getUserColor("zneix"), QColor("#f0f")); + EXPECT_EQ(chatters.getUserColor("user1"), QColor("#00f")); + + chatters.setUserColor("newuser", QColor("#00e")); + + for (int i = 0; i < ChannelChatters::maxChatterColorCount; ++i) + { + auto username = QString("user%1").arg(i); + chatters.setUserColor(username, QColor("#00f")); + } + + // One more entry should be bumped out (zneix) + + EXPECT_EQ(chatters.getUserColor("pajlada"), QColor()); + EXPECT_EQ(chatters.getUserColor("zneix"), QColor()); + EXPECT_EQ(chatters.getUserColor("user1"), QColor("#00f")); +}