Fix color @usernames sometimes not working at all (#3170)

Definitely memory fuckery involved - The comment from @lubieerror https://github.com/Chatterino/chatterino2/issues/2822#issuecomment-897252673 is finally what led me to adding tests and hopefully fixing this.
This commit is contained in:
pajlada 2021-08-21 12:38:38 +02:00 committed by GitHub
parent 07454d0537
commit d7fd08b1d6
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 128 additions and 1 deletions

View file

@ -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)

View file

@ -35,6 +35,7 @@ public:
lru_cache(lru_cache<key_t, value_t> &&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;

View file

@ -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();

View file

@ -25,9 +25,13 @@ public:
void setUserColor(const QString &user, const QColor &color);
void updateOnlineChatters(const std::unordered_set<QString> &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

View file

@ -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

View file

@ -0,0 +1,113 @@
#include "common/ChannelChatters.hpp"
#include <gtest/gtest.h>
#include <QColor>
#include <QStringList>
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"));
}