From 7f2168a9d4748952df64bbfce1f202d6319352c7 Mon Sep 17 00:00:00 2001 From: pajlada Date: Sun, 6 Jul 2025 15:05:28 +0200 Subject: [PATCH] fix: correctly override user color in sub gifts (#6322) --- CHANGELOG.md | 1 + src/CMakeLists.txt | 2 + src/common/ChannelChatters.cpp | 2 +- src/common/ChannelChatters.hpp | 2 +- src/messages/MessageBuilder.cpp | 36 ++++- src/messages/MessageBuilder.hpp | 3 +- src/providers/twitch/IrcMessageHandler.cpp | 4 +- src/providers/twitch/UserColor.cpp | 48 ++++++ src/providers/twitch/UserColor.hpp | 37 +++++ src/util/SampleData.cpp | 3 + tests/CMakeLists.txt | 1 + .../IrcMessageHandler/sub-gift-02.json | 131 ++++++++++++++++ tests/src/IrcMessageHandler.cpp | 11 ++ tests/src/TwitchUserColor.cpp | 141 ++++++++++++++++++ 14 files changed, 410 insertions(+), 12 deletions(-) create mode 100644 src/providers/twitch/UserColor.cpp create mode 100644 src/providers/twitch/UserColor.hpp create mode 100644 tests/snapshots/IrcMessageHandler/sub-gift-02.json create mode 100644 tests/src/TwitchUserColor.cpp diff --git a/CHANGELOG.md b/CHANGELOG.md index f5c947c9..7c94bece 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ - Bugfix: Fixed blocked users showing up in "Users joined:" and "Users parted:" messages. (#6181) - Bugfix: Fixed an issue where text boxes in the settings dialog could be stuck with an old value. (#6286) - Bugfix: Fixed an issue where Splits could get lost by dragging it onto your Recycle Bin. (#6147) +- Bugfix: Correctly color gifter & recipient usernames in subscription gift messages, taking all color sources into consideration. (#6322) - Bugfix: Fixed some Twitch commands not getting tab-completed correctly. (#6143) - Bugfix: Fixed shared chat badges displaying pixelated when Chatterino is scaled too much. (#6146) - Bugfix: Fixed a few crashes that could occur when Chatterino was shutting down, some related to network tasks still firing despite us shutting down. (#6187) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 8c43c4f7..0c1ee130 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -440,6 +440,8 @@ set(SOURCE_FILES providers/twitch/TwitchUser.hpp providers/twitch/TwitchUsers.cpp providers/twitch/TwitchUsers.hpp + providers/twitch/UserColor.cpp + providers/twitch/UserColor.hpp providers/twitch/eventsub/Connection.cpp providers/twitch/eventsub/Connection.hpp diff --git a/src/common/ChannelChatters.cpp b/src/common/ChannelChatters.cpp index 52f52262..5cce20a8 100644 --- a/src/common/ChannelChatters.cpp +++ b/src/common/ChannelChatters.cpp @@ -112,7 +112,7 @@ size_t ChannelChatters::colorsSize() const return size; } -const QColor ChannelChatters::getUserColor(const QString &user) +QColor ChannelChatters::getUserColor(const QString &user) const { const auto chatterColors = this->chatterColors_.access(); diff --git a/src/common/ChannelChatters.hpp b/src/common/ChannelChatters.hpp index 8ba4b130..1718a02f 100644 --- a/src/common/ChannelChatters.hpp +++ b/src/common/ChannelChatters.hpp @@ -24,7 +24,7 @@ public: void addRecentChatter(const QString &user); void addJoinedUser(const QString &user, bool isMod, bool isBroadcaster); void addPartedUser(const QString &user, bool isMod, bool isBroadcaster); - const QColor getUserColor(const QString &user); + QColor getUserColor(const QString &user) const; void setUserColor(const QString &user, const QColor &color); void updateOnlineChatters(const std::unordered_set &usernames); diff --git a/src/messages/MessageBuilder.cpp b/src/messages/MessageBuilder.cpp index 2a3aefbf..94e6e3fa 100644 --- a/src/messages/MessageBuilder.cpp +++ b/src/messages/MessageBuilder.cpp @@ -32,6 +32,7 @@ #include "providers/twitch/TwitchIrc.hpp" #include "providers/twitch/TwitchIrcServer.hpp" #include "providers/twitch/TwitchUsers.hpp" +#include "providers/twitch/UserColor.hpp" #include "singletons/Emotes.hpp" #include "singletons/Resources.hpp" #include "singletons/Settings.hpp" @@ -587,8 +588,12 @@ MessagePtrMut MessageBuilder::makeSystemMessageWithUser( MessagePtrMut MessageBuilder::makeSubgiftMessage(const QString &text, const QVariantMap &tags, - const QTime &time) + const QTime &time, + TwitchChannel *channel) { + const auto *userDataController = getApp()->getUserData(); + assert(userDataController != nullptr); + MessageBuilder builder; builder.emplace(time); @@ -598,11 +603,16 @@ MessagePtrMut MessageBuilder::makeSubgiftMessage(const QString &text, { gifterDisplayName = gifterLogin; } - MessageColor gifterColor = MessageColor::System; - if (auto colorTag = tags.value("color").value(); colorTag.isValid()) - { - gifterColor = MessageColor(colorTag); - } + + auto gifterColor = + twitch::getUserColor({ + .userLogin = gifterLogin, + .userID = tags.value("user-id").toString(), + .userDataController = userDataController, + .channelChatters = channel, + .color = tags.value("color").value(), + }) + .value_or(MessageColor::System); auto recipientLogin = tags.value("msg-param-recipient-user-name").toString(); @@ -617,6 +627,17 @@ MessagePtrMut MessageBuilder::makeSubgiftMessage(const QString &text, recipientDisplayName = recipientLogin; } + auto recipientColor = + twitch::getUserColor( + { + .userLogin = recipientLogin, + .userID = tags.value("msg-param-recipient-id").toString(), + + .userDataController = userDataController, + .channelChatters = channel, + }) + .value_or(MessageColor::System); + const auto textFragments = text.split(SPACE_REGEX, Qt::SkipEmptyParts); for (const auto &word : textFragments) { @@ -632,8 +653,7 @@ MessagePtrMut MessageBuilder::makeSubgiftMessage(const QString &text, { builder .emplace(recipientDisplayName, recipientLogin, - MessageColor::System, - MessageColor::System) + MessageColor::System, recipientColor) ->setTrailingSpace(false); builder.emplace(u"!"_s, MessageElementFlag::Text, MessageColor::System); diff --git a/src/messages/MessageBuilder.hpp b/src/messages/MessageBuilder.hpp index e9f50580..cfd6bffc 100644 --- a/src/messages/MessageBuilder.hpp +++ b/src/messages/MessageBuilder.hpp @@ -240,7 +240,8 @@ public: static MessagePtrMut makeSubgiftMessage(const QString &text, const QVariantMap &tags, - const QTime &time); + const QTime &time, + TwitchChannel *channel); static MessagePtrMut makeMissingScopesMessage(const QString &missingScopes); diff --git a/src/providers/twitch/IrcMessageHandler.cpp b/src/providers/twitch/IrcMessageHandler.cpp index 6cc6b3d0..7a2a52b8 100644 --- a/src/providers/twitch/IrcMessageHandler.cpp +++ b/src/providers/twitch/IrcMessageHandler.cpp @@ -667,6 +667,8 @@ void IrcMessageHandler::parseUserNoticeMessageInto(Communi::IrcMessage *message, MessageSink &sink, TwitchChannel *channel) { + assert(channel != nullptr); + auto tags = message->tags(); auto parameters = message->parameters(); @@ -775,7 +777,7 @@ void IrcMessageHandler::parseUserNoticeMessageInto(Communi::IrcMessage *message, // subgifts are special because they include two users auto msg = MessageBuilder::makeSubgiftMessage( parseTagString(messageText), tags, - calculateMessageTime(message).time()); + calculateMessageTime(message).time(), channel); msg->flags.set(MessageFlag::Subscription); if (mirrored) diff --git a/src/providers/twitch/UserColor.cpp b/src/providers/twitch/UserColor.cpp new file mode 100644 index 00000000..4e23d717 --- /dev/null +++ b/src/providers/twitch/UserColor.cpp @@ -0,0 +1,48 @@ +#include "providers/twitch/UserColor.hpp" + +#include "common/ChannelChatters.hpp" +#include "controllers/userdata/UserDataController.hpp" +#include "messages/MessageColor.hpp" + +namespace chatterino::twitch { + +std::optional getUserColor(const GetUserColorParams ¶ms) +{ + if (params.userDataController != nullptr) + { + if (!params.userID.isEmpty()) + { + if (const auto &oUser = + params.userDataController->getUser(params.userID)) + { + const auto &user = *oUser; + if (user.color && user.color->isValid()) + { + return {*user.color}; + } + } + } + } + + if (params.color.isValid()) + { + return {params.color}; + } + + if (params.channelChatters != nullptr) + { + if (!params.userLogin.isEmpty()) + { + if (auto color = + params.channelChatters->getUserColor(params.userLogin); + color.isValid()) + { + return {color}; + } + } + } + + return std::nullopt; +} + +} // namespace chatterino::twitch diff --git a/src/providers/twitch/UserColor.hpp b/src/providers/twitch/UserColor.hpp new file mode 100644 index 00000000..bcbe30a8 --- /dev/null +++ b/src/providers/twitch/UserColor.hpp @@ -0,0 +1,37 @@ +#pragma once + +#include "messages/MessageColor.hpp" + +#include + +#include + +class QColor; + +namespace chatterino { + +class IUserDataController; +class ChannelChatters; + +namespace twitch { + +struct GetUserColorParams { + QString userLogin; + QString userID; + + const IUserDataController *userDataController{}; + const ChannelChatters *channelChatters{}; + + QColor color; +}; + +/// Attempt to get the color of the user based on the provided parameters. +/// +/// If there's a valid color for the given user ID in the user data controller, return it. +/// If there's a valid `color` parameter, return it. +/// If there's a valid color for the given user login in the channel chatters set, return it. +/// Otherwise, return nullopt. +std::optional getUserColor(const GetUserColorParams ¶ms); + +} // namespace twitch +} // namespace chatterino diff --git a/src/util/SampleData.cpp b/src/util/SampleData.cpp index a31850d1..48429cbf 100644 --- a/src/util/SampleData.cpp +++ b/src/util/SampleData.cpp @@ -104,6 +104,9 @@ const QStringList &getSampleSubMessages() // resub without message R"(@badges=subscriber/12;color=#CC00C2;display-name=cspice;emotes=;id=6fc4c3e0-ca61-454a-84b8-5669dee69fc9;login=cspice;mod=0;msg-id=resub;msg-param-months=12;msg-param-sub-plan-name=Channel\sSubscription\s(forsenlol):\s$9.99\sSub;msg-param-sub-plan=2000;room-id=22484632;subscriber=1;system-msg=cspice\sjust\ssubscribed\swith\sa\sTier\s2\ssub.\scspice\ssubscribed\sfor\s12\smonths\sin\sa\srow!;tmi-sent-ts=1528192510808;turbo=0;user-id=47894662;user-type= :tmi.twitch.tv USERNOTICE #pajlada)", + + // pajlada gifted a sub to rustafur + R"(@tmi-sent-ts=1751793597378;subscriber=1;id=ac51c358-0525-468f-9e0f-e9f2b7953c29;room-id=11148817;user-id=11148817;login=pajlada;display-name=pajlada;badges=broadcaster/1,subscriber/3072,partner/1;badge-info=subscriber/114;color=#CC44FF;flags=;user-type=;emotes=;msg-param-sub-plan-name=look\sat\sthose\sshitty\semotes,\srip\s$5\sLUL;msg-param-gift-months=1;system-msg=pajlada\sgifted\sa\sTier\s1\ssub\sto\srustafur!;msg-param-months=6;msg-param-origin-id=4645652708379472175;msg-param-recipient-id=27787997;msg-param-sub-plan=1000;msg-id=subgift;msg-param-recipient-display-name=rustafur;msg-param-recipient-user-name=rustafur;msg-param-community-gift-id=4645652708379472175;msg-param-sender-count=0 :tmi.twitch.tv USERNOTICE #pajlada)", }; return list; } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 2aff24c8..e2e2168f 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -57,6 +57,7 @@ set(test_SOURCES ${CMAKE_CURRENT_LIST_DIR}/src/NativeMessaging.cpp ${CMAKE_CURRENT_LIST_DIR}/src/ImageUploader.cpp ${CMAKE_CURRENT_LIST_DIR}/src/TwitchChannel.cpp + ${CMAKE_CURRENT_LIST_DIR}/src/TwitchUserColor.cpp ${CMAKE_CURRENT_LIST_DIR}/src/lib/Snapshot.cpp ${CMAKE_CURRENT_LIST_DIR}/src/lib/Snapshot.hpp diff --git a/tests/snapshots/IrcMessageHandler/sub-gift-02.json b/tests/snapshots/IrcMessageHandler/sub-gift-02.json new file mode 100644 index 00000000..4a3dc451 --- /dev/null +++ b/tests/snapshots/IrcMessageHandler/sub-gift-02.json @@ -0,0 +1,131 @@ +{ + "input": "@tmi-sent-ts=1751793597378;subscriber=1;id=ac51c358-0525-468f-9e0f-e9f2b7953c29;room-id=11148817;user-id=11148817;login=pajlada;display-name=pajlada;badges=broadcaster/1,subscriber/3072,partner/1;badge-info=subscriber/114;color=#CC44FF;flags=;user-type=;emotes=;msg-param-sub-plan-name=look\\sat\\sthose\\sshitty\\semotes,\\srip\\s$5\\sLUL;msg-param-gift-months=1;system-msg=pajlada\\sgifted\\sa\\sTier\\s1\\ssub\\sto\\srustafur!;msg-param-months=6;msg-param-origin-id=4645652708379472175;msg-param-recipient-id=27787997;msg-param-sub-plan=1000;msg-id=subgift;msg-param-recipient-display-name=rustafur;msg-param-recipient-user-name=rustafur;msg-param-community-gift-id=4645652708379472175;msg-param-sender-count=0 :tmi.twitch.tv USERNOTICE #pajlada", + "output": [ + { + "badgeInfos": { + }, + "badges": [ + ], + "channelName": "", + "count": 1, + "displayName": "", + "elements": [ + { + "element": { + "color": "System", + "flags": "Timestamp", + "link": { + "type": "None", + "value": "" + }, + "style": "TimestampMedium", + "tooltip": "", + "trailingSpace": true, + "type": "TextElement", + "words": [ + "9:19" + ] + }, + "flags": "Timestamp", + "format": "", + "link": { + "type": "None", + "value": "" + }, + "time": "09:19:57", + "tooltip": "", + "trailingSpace": true, + "type": "TimestampElement" + }, + { + "color": "Text", + "fallbackColor": "System", + "flags": "Text|Mention", + "link": { + "type": "None", + "value": "" + }, + "style": "ChatMedium", + "tooltip": "", + "trailingSpace": true, + "type": "MentionElement", + "userColor": "#ffcc44ff", + "userLoginName": "pajlada", + "words": [ + "pajlada" + ] + }, + { + "color": "System", + "flags": "Text", + "link": { + "type": "None", + "value": "" + }, + "style": "ChatMedium", + "tooltip": "", + "trailingSpace": true, + "type": "TextElement", + "words": [ + "gifted", + "a", + "Tier", + "1", + "sub", + "to" + ] + }, + { + "color": "Text", + "fallbackColor": "System", + "flags": "Text|Mention", + "link": { + "type": "None", + "value": "" + }, + "style": "ChatMedium", + "tooltip": "", + "trailingSpace": false, + "type": "MentionElement", + "userColor": "#ffff00ff", + "userLoginName": "rustafur", + "words": [ + "rustafur" + ] + }, + { + "color": "System", + "flags": "Text", + "link": { + "type": "None", + "value": "" + }, + "style": "ChatMedium", + "tooltip": "", + "trailingSpace": true, + "type": "TextElement", + "words": [ + "!" + ] + } + ], + "flags": "System|DoNotTriggerNotification|Subscription", + "id": "", + "localizedName": "", + "loginName": "", + "messageText": "pajlada gifted a Tier 1 sub to rustafur!", + "searchText": "pajlada gifted a Tier 1 sub to rustafur!", + "serverReceivedTime": "", + "timeoutUser": "", + "userID": "", + "usernameColor": "#ff000000" + } + ], + "params": { + "userData": { + "27787997": { + "color": "#FF00FF" + } + } + } +} diff --git a/tests/src/IrcMessageHandler.cpp b/tests/src/IrcMessageHandler.cpp index a0fc31de..abfadbad 100644 --- a/tests/src/IrcMessageHandler.cpp +++ b/tests/src/IrcMessageHandler.cpp @@ -576,6 +576,17 @@ TEST_P(TestIrcMessageHandlerP, Run) VectorMessageSink sink; + const auto &userData = snapshot->param("userData").toObject(); + for (auto it = userData.begin(); it != userData.end(); ++it) + { + const auto &userID = it.key(); + const auto &data = it.value().toObject(); + if (auto color = data.value("color").toString(); !color.isEmpty()) + { + this->mockApplication->getUserData()->setUserColor(userID, color); + } + } + for (auto prevInput : snapshot->param("prevMessages").toArray()) { auto *ircMessage = Communi::IrcMessage::fromData( diff --git a/tests/src/TwitchUserColor.cpp b/tests/src/TwitchUserColor.cpp new file mode 100644 index 00000000..ad8a316b --- /dev/null +++ b/tests/src/TwitchUserColor.cpp @@ -0,0 +1,141 @@ +#include "common/ChannelChatters.hpp" +#include "mocks/BaseApplication.hpp" +#include "mocks/Channel.hpp" +#include "mocks/Logging.hpp" +#include "mocks/UserData.hpp" +#include "providers/twitch/UserColor.hpp" +#include "Test.hpp" + +#include + +namespace chatterino::twitch { + +namespace { + +class MockApplication : public mock::BaseApplication +{ +public: + MockApplication() = default; + + ILogging *getChatLogger() override + { + return &this->logging; + } + + mock::EmptyLogging logging; +}; + +} // namespace + +class TwitchUserColor : public ::testing::Test +{ +protected: + void SetUp() override + { + this->app = std::make_unique(); + this->userDataController = std::make_unique(); + this->channel = std::make_unique("test"); + this->chatters = std::make_unique(*this->channel); + } + + void TearDown() override + { + this->chatters.reset(); + this->channel.reset(); + this->userDataController.reset(); + this->app.reset(); + } + + std::unique_ptr app; + std::unique_ptr userDataController; + std::unique_ptr channel; + std::unique_ptr chatters; +}; + +TEST_F(TwitchUserColor, NoDataForUser) +{ + ASSERT_EQ(getUserColor({ + .userLogin = "pajlada", + .userID = "11148817", + .userDataController = userDataController.get(), + .channelChatters = chatters.get(), + .color = {}, + }), + std::nullopt); +} + +TEST_F(TwitchUserColor, ChannelChattersData) +{ + // User exists in channel chatters. + // Their color should be fetched from there. + + chatters->setUserColor("pajlada", QColor("#FF0000")); + + ASSERT_EQ(getUserColor({ + .userLogin = "pajlada", + .userID = "11148817", + .userDataController = userDataController.get(), + .channelChatters = chatters.get(), + .color = {}, + }), + QColor("#FF0000")); +} + +TEST_F(TwitchUserColor, ChannelChattersDataButBaseColor) +{ + // User exists in channel chatters + // The base color is valid and should be used. + + chatters->setUserColor("pajlada", QColor("#FF0000")); + + ASSERT_EQ(getUserColor({ + .userLogin = "pajlada", + .userID = "11148817", + .userDataController = userDataController.get(), + .channelChatters = chatters.get(), + .color = QColor("#00FF00"), + }), + QColor("#00FF00")); +} + +TEST_F(TwitchUserColor, ChannelChattersDataButBaseColorInvalid) +{ + // User exists in channel chatters + // A base color has been defined but it's invalid + // The channel chatter color should be used + + ASSERT_EQ(0, chatters->colorsSize()); + chatters->setUserColor("pajlada", QColor("#FF0000")); + + ASSERT_EQ(getUserColor({ + .userLogin = "pajlada", + .userID = "11148817", + .userDataController = userDataController.get(), + .channelChatters = chatters.get(), + .color = QColor("this is an invalid color"), + }), + QColor("#FF0000")); +} + +TEST_F(TwitchUserColor, UCD) +{ + // User exists in channel chatters + // A base color has been defined + // User exists in user data controller + // The user data controller color should be used + + chatters->setUserColor("pajlada", QColor("#FF0000")); + + userDataController->setUserColor("11148817", "#0000FF"); + + ASSERT_EQ(getUserColor({ + .userLogin = "pajlada", + .userID = "11148817", + .userDataController = userDataController.get(), + .channelChatters = chatters.get(), + .color = QColor("#00FF00"), + }), + QColor("#0000FF")); +} + +} // namespace chatterino::twitch