From 975b39fe10ed5cba4ba3ae5485e8bb524d524d5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82?= Date: Sun, 11 Jul 2021 13:33:35 +0200 Subject: [PATCH] Made username and color in AutoMod mod messages use correct values (#2967) We use values given in pubsub messages and handle their edge-cases properly. --- CHANGELOG.md | 1 + src/Application.cpp | 21 ++-- src/messages/MessageBuilder.cpp | 66 +++++------ src/providers/twitch/PubsubActions.hpp | 6 +- src/providers/twitch/PubsubClient.cpp | 105 +++++++++++++----- src/providers/twitch/PubsubHelpers.cpp | 4 +- src/providers/twitch/TwitchMessageBuilder.cpp | 10 +- 7 files changed, 132 insertions(+), 81 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe730d54c..09d2df90e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Minor: Channel name in ` has gone offline. Exiting host mode.` messages is now clickable. (#2922) - Minor: Added `/openurl` command. Usage: `/openurl `. Opens the provided URL in the browser. (#2461, #2926) - Minor: Updated to Emoji v13.1 (#2958) +- Minor: Sender username in automod messages shown to moderators shows correct color and display name. (#2967) - Bugfix: Now deleting cache files that weren't modified in the past 14 days. (#2947) - Bugfix: Fixed large timeout durations in moderation buttons overlapping with usernames or other buttons. (#2865, #2921) - Bugfix: Middle mouse click no longer scrolls in not fully populated usercards and splits. (#2933) diff --git a/src/Application.cpp b/src/Application.cpp index 76015d91c..f0f79c42f 100644 --- a/src/Application.cpp +++ b/src/Application.cpp @@ -207,7 +207,7 @@ void Application::initPubsub() } QString text = - QString("%1 cleared the chat").arg(action.source.name); + QString("%1 cleared the chat").arg(action.source.login); auto msg = makeSystemMessage(text); postToThread([chan, msg] { @@ -226,15 +226,14 @@ void Application::initPubsub() QString text = QString("%1 turned %2 %3 mode") - .arg(action.source.name) + .arg(action.source.login) .arg(action.state == ModeChangedAction::State::On ? "on" : "off") .arg(action.getModeName()); if (action.duration > 0) { - text.append(" (" + QString::number(action.duration) + - " seconds)"); + text += QString(" (%1 seconds)").arg(action.duration); } auto msg = makeSystemMessage(text); @@ -254,16 +253,10 @@ void Application::initPubsub() QString text; - if (action.modded) - { - text = QString("%1 modded %2") - .arg(action.source.name, action.target.name); - } - else - { - text = QString("%1 unmodded %2") - .arg(action.source.name, action.target.name); - } + text = QString("%1 %2 %3") + .arg(action.source.login, + (action.modded ? "modded" : "unmodded"), + action.target.login); auto msg = makeSystemMessage(text); postToThread([chan, msg] { diff --git a/src/messages/MessageBuilder.cpp b/src/messages/MessageBuilder.cpp index e38ade177..5b44b6aec 100644 --- a/src/messages/MessageBuilder.cpp +++ b/src/messages/MessageBuilder.cpp @@ -140,25 +140,26 @@ std::pair makeAutomodMessage( // Builder for offender's message builder2.emplace(); builder2.emplace(); - builder2.message().loginName = action.target.name; + builder2.message().loginName = action.target.login; builder2.message().flags.set(MessageFlag::PubSub); // sender username builder2 .emplace( - action.target.name + ":", MessageElementFlag::BoldUsername, - MessageColor(QColor("red")), FontStyle::ChatMediumBold) - ->setLink({Link::UserInfo, action.target.name}); + action.target.displayName + ":", MessageElementFlag::BoldUsername, + MessageColor(action.target.color), FontStyle::ChatMediumBold) + ->setLink({Link::UserInfo, action.target.login}); builder2 - .emplace(action.target.name + ":", + .emplace(action.target.displayName + ":", MessageElementFlag::NonBoldUsername, - MessageColor(QColor("red"))) - ->setLink({Link::UserInfo, action.target.name}); + MessageColor(action.target.color)) + ->setLink({Link::UserInfo, action.target.login}); // sender's message caught by AutoMod builder2.emplace(action.message, MessageElementFlag::Text, MessageColor::Text); builder2.message().flags.set(MessageFlag::AutoMod); - auto text2 = QString("%1: %2").arg(action.target.name, action.message); + auto text2 = + QString("%1: %2").arg(action.target.displayName, action.message); builder2.message().messageText = text2; builder2.message().searchText = text2; @@ -277,7 +278,7 @@ MessageBuilder::MessageBuilder(const BanAction &action, uint32_t count) this->emplace(); this->message().flags.set(MessageFlag::System); this->message().flags.set(MessageFlag::Timeout); - this->message().timeoutUser = action.target.name; + this->message().timeoutUser = action.target.login; this->message().count = count; QString text; @@ -296,13 +297,13 @@ MessageBuilder::MessageBuilder(const BanAction &action, uint32_t count) text); } - if (!action.source.name.isEmpty()) + if (!action.source.login.isEmpty()) { this->emplaceSystemTextAndUpdate("by", text); this->emplaceSystemTextAndUpdate( - action.source.name + (action.reason.isEmpty() ? "." : ":"), + action.source.login + (action.reason.isEmpty() ? "." : ":"), text) - ->setLink({Link::UserInfo, action.source.name}); + ->setLink({Link::UserInfo, action.source.login}); } if (!action.reason.isEmpty()) @@ -315,29 +316,30 @@ MessageBuilder::MessageBuilder(const BanAction &action, uint32_t count) { if (action.isBan()) { - this->emplaceSystemTextAndUpdate(action.source.name, text) - ->setLink({Link::UserInfo, action.source.name}); + this->emplaceSystemTextAndUpdate(action.source.login, text) + ->setLink({Link::UserInfo, action.source.login}); this->emplaceSystemTextAndUpdate("banned", text); if (action.reason.isEmpty()) { - this->emplaceSystemTextAndUpdate(action.target.name, text) - ->setLink({Link::UserInfo, action.target.name}); + this->emplaceSystemTextAndUpdate(action.target.login, text) + ->setLink({Link::UserInfo, action.target.login}); } else { - this->emplaceSystemTextAndUpdate(action.target.name + ":", text) - ->setLink({Link::UserInfo, action.target.name}); + this->emplaceSystemTextAndUpdate(action.target.login + ":", + text) + ->setLink({Link::UserInfo, action.target.login}); this->emplaceSystemTextAndUpdate( QString("\"%1\".").arg(action.reason), text); } } else { - this->emplaceSystemTextAndUpdate(action.source.name, text) - ->setLink({Link::UserInfo, action.source.name}); + this->emplaceSystemTextAndUpdate(action.source.login, text) + ->setLink({Link::UserInfo, action.source.login}); this->emplaceSystemTextAndUpdate("timed out", text); - this->emplaceSystemTextAndUpdate(action.target.name, text) - ->setLink({Link::UserInfo, action.target.name}); + this->emplaceSystemTextAndUpdate(action.target.login, text) + ->setLink({Link::UserInfo, action.target.login}); if (action.reason.isEmpty()) { this->emplaceSystemTextAndUpdate( @@ -371,16 +373,16 @@ MessageBuilder::MessageBuilder(const UnbanAction &action) this->message().flags.set(MessageFlag::System); this->message().flags.set(MessageFlag::Untimeout); - this->message().timeoutUser = action.target.name; + this->message().timeoutUser = action.target.login; QString text; - this->emplaceSystemTextAndUpdate(action.source.name, text) - ->setLink({Link::UserInfo, action.source.name}); + this->emplaceSystemTextAndUpdate(action.source.login, text) + ->setLink({Link::UserInfo, action.source.login}); this->emplaceSystemTextAndUpdate( action.wasBan() ? "unbanned" : "untimedout", text); - this->emplaceSystemTextAndUpdate(action.target.name, text) - ->setLink({Link::UserInfo, action.target.name}); + this->emplaceSystemTextAndUpdate(action.target.login, text) + ->setLink({Link::UserInfo, action.target.login}); this->message().messageText = text; this->message().searchText = text; @@ -397,31 +399,31 @@ MessageBuilder::MessageBuilder(const AutomodUserAction &action) { case AutomodUserAction::AddPermitted: { text = QString("%1 added %2 as a permitted term on AutoMod.") - .arg(action.source.name, action.message); + .arg(action.source.login, action.message); } break; case AutomodUserAction::AddBlocked: { text = QString("%1 added %2 as a blocked term on AutoMod.") - .arg(action.source.name, action.message); + .arg(action.source.login, action.message); } break; case AutomodUserAction::RemovePermitted: { text = QString("%1 removed %2 as a permitted term on AutoMod.") - .arg(action.source.name, action.message); + .arg(action.source.login, action.message); } break; case AutomodUserAction::RemoveBlocked: { text = QString("%1 removed %2 as a blocked term on AutoMod.") - .arg(action.source.name, action.message); + .arg(action.source.login, action.message); } break; case AutomodUserAction::Properties: { text = QString("%1 modified the AutoMod properties.") - .arg(action.source.name); + .arg(action.source.login); } break; } diff --git a/src/providers/twitch/PubsubActions.hpp b/src/providers/twitch/PubsubActions.hpp index 8e93e5e47..abe106df1 100644 --- a/src/providers/twitch/PubsubActions.hpp +++ b/src/providers/twitch/PubsubActions.hpp @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include @@ -10,7 +11,10 @@ namespace chatterino { struct ActionUser { QString id; - QString name; + QString login; + // displayName should be in format "login(localizedName)" for non-ascii usernames + QString displayName; + QColor color; }; struct PubSubAction { diff --git a/src/providers/twitch/PubsubClient.cpp b/src/providers/twitch/PubsubClient.cpp index d1100315c..f7440ead7 100644 --- a/src/providers/twitch/PubsubClient.cpp +++ b/src/providers/twitch/PubsubClient.cpp @@ -2,6 +2,7 @@ #include "providers/twitch/PubsubActions.hpp" #include "providers/twitch/PubsubHelpers.hpp" +#include "singletons/Settings.hpp" #include "util/Helpers.hpp" #include "util/RapidjsonHelpers.hpp" @@ -333,7 +334,7 @@ PubSub::PubSub() return; } - if (!rj::getSafe(args[0], action.target.name)) + if (!rj::getSafe(args[0], action.target.login)) { return; } @@ -398,7 +399,7 @@ PubSub::PubSub() return; } - if (!rj::getSafe(args[0], action.target.name)) + if (!rj::getSafe(args[0], action.target.login)) { return; } @@ -444,7 +445,7 @@ PubSub::PubSub() return; } - if (!rj::getSafe(args[0], action.target.name)) + if (!rj::getSafe(args[0], action.target.login)) { return; } @@ -484,7 +485,7 @@ PubSub::PubSub() return; } - if (!rj::getSafe(args[0], action.target.name)) + if (!rj::getSafe(args[0], action.target.login)) { return; } @@ -524,7 +525,7 @@ PubSub::PubSub() return; } - if (!rj::getSafe(args[0], action.target.name)) + if (!rj::getSafe(args[0], action.target.login)) { return; } @@ -556,7 +557,7 @@ PubSub::PubSub() return; } - if (!rj::getSafe(args[0], action.target.name)) + if (!rj::getSafe(args[0], action.target.login)) { return; } @@ -588,7 +589,7 @@ PubSub::PubSub() return; } - if (!rj::getSafe(args[0], action.target.name)) + if (!rj::getSafe(args[0], action.target.login)) { return; } @@ -658,7 +659,7 @@ PubSub::PubSub() return; } - if (!rj::getSafe(data, "requester_login", action.source.name)) + if (!rj::getSafe(data, "requester_login", action.source.login)) { return; } @@ -686,7 +687,7 @@ PubSub::PubSub() return; } - if (!rj::getSafe(data, "requester_login", action.source.name)) + if (!rj::getSafe(data, "requester_login", action.source.login)) { return; } @@ -743,7 +744,7 @@ PubSub::PubSub() return; } - if (!rj::getSafe(data, "requester_login", action.source.name)) + if (!rj::getSafe(data, "requester_login", action.source.login)) { return; } @@ -802,7 +803,7 @@ PubSub::PubSub() return; } - if (!rj::getSafe(data, "requester_login", action.source.name)) + if (!rj::getSafe(data, "requester_login", action.source.login)) { return; } @@ -1367,7 +1368,7 @@ void PubSub::handleMessageResponse(const rapidjson::Value &outerData) QString status; if (!rj::getSafe(data, "status", status)) { - qDebug() << "Failed to get status"; + qCDebug(chatterinoPubsub) << "Failed to get status"; return; } if (status == "PENDING") @@ -1377,20 +1378,22 @@ void PubSub::handleMessageResponse(const rapidjson::Value &outerData) if (!rj::getSafeObject(data, "content_classification", classification)) { - qDebug() << "Failed to get content_classification"; + qCDebug(chatterinoPubsub) + << "Failed to get content_classification"; return; } QString contentCategory; if (!rj::getSafe(classification, "category", contentCategory)) { - qDebug() << "Failed to get content category"; + qCDebug(chatterinoPubsub) + << "Failed to get content category"; return; } int contentLevel; if (!rj::getSafe(classification, "level", contentLevel)) { - qDebug() << "Failed to get content level"; + qCDebug(chatterinoPubsub) << "Failed to get content level"; return; } action.reason = QString("%1 level %2") @@ -1400,25 +1403,26 @@ void PubSub::handleMessageResponse(const rapidjson::Value &outerData) rapidjson::Value messageData; if (!rj::getSafeObject(data, "message", messageData)) { - qDebug() << "Failed to get message data"; + qCDebug(chatterinoPubsub) << "Failed to get message data"; return; } rapidjson::Value messageContent; if (!rj::getSafeObject(messageData, "content", messageContent)) { - qDebug() << "Failed to get message content"; + qCDebug(chatterinoPubsub) + << "Failed to get message content"; return; } if (!rj::getSafe(messageData, "id", action.msgID)) { - qDebug() << "Failed to get message id"; + qCDebug(chatterinoPubsub) << "Failed to get message id"; return; } if (!rj::getSafe(messageContent, "text", action.message)) { - qDebug() << "Failed to get message text"; + qCDebug(chatterinoPubsub) << "Failed to get message text"; return; } @@ -1428,23 +1432,68 @@ void PubSub::handleMessageResponse(const rapidjson::Value &outerData) rapidjson::Value senderData; if (!rj::getSafeObject(messageData, "sender", senderData)) { - qDebug() << "Failed to get sender"; + qCDebug(chatterinoPubsub) << "Failed to get sender"; return; } - QString sender_id; - if (!rj::getSafe(senderData, "user_id", sender_id)) + QString senderId; + if (!rj::getSafe(senderData, "user_id", senderId)) { - qDebug() << "Failed to get sender user id"; + qCDebug(chatterinoPubsub) << "Failed to get sender user id"; return; } - QString sender_login; - if (!rj::getSafe(senderData, "login", sender_login)) + QString senderLogin; + if (!rj::getSafe(senderData, "login", senderLogin)) { - qDebug() << "Failed to get sender login"; + qCDebug(chatterinoPubsub) << "Failed to get sender login"; return; } - action.target = ActionUser{sender_id, sender_login}; - qDebug() << action.msgID; + QString senderDisplayName = senderLogin; + bool hasLocalizedName = false; + if (rj::getSafe(senderData, "display_name", senderDisplayName)) + { + // check for non-ascii display names + if (QString::compare(senderLogin, senderDisplayName, + Qt::CaseInsensitive) != 0) + { + hasLocalizedName = true; + } + } + QColor senderColor; + QString senderColor_; + if (rj::getSafe(senderData, "chat_color", senderColor_)) + { + senderColor = QColor(senderColor_); + } + else if (getSettings()->colorizeNicknames) + { + // color may be not present if user is a grey-name + senderColor = getRandomColor(senderId); + } + // handle username style based on prefered setting + switch (getSettings()->usernameDisplayMode.getValue()) + { + case UsernameDisplayMode::Username: { + if (hasLocalizedName) + { + senderDisplayName = senderLogin; + } + break; + } + case UsernameDisplayMode::LocalizedName: { + break; + } + case UsernameDisplayMode::UsernameAndLocalizedName: { + if (hasLocalizedName) + { + senderDisplayName = QString("%1(%2)").arg( + senderLogin, senderDisplayName); + } + break; + } + } + + action.target = ActionUser{senderId, senderLogin, + senderDisplayName, senderColor}; this->signals_.moderation.automodMessage.invoke(action); } // "ALLOWED" and "DENIED" statuses remain unimplemented diff --git a/src/providers/twitch/PubsubHelpers.cpp b/src/providers/twitch/PubsubHelpers.cpp index f6f9b38e9..e3ea28561 100644 --- a/src/providers/twitch/PubsubHelpers.cpp +++ b/src/providers/twitch/PubsubHelpers.cpp @@ -37,7 +37,7 @@ const rapidjson::Value &getMsgID(const rapidjson::Value &data) bool getCreatedByUser(const rapidjson::Value &data, ActionUser &user) { - return rj::getSafe(data, "created_by", user.name) && + return rj::getSafe(data, "created_by", user.login) && rj::getSafe(data, "created_by_user_id", user.id); } @@ -48,7 +48,7 @@ bool getTargetUser(const rapidjson::Value &data, ActionUser &user) bool getTargetUserName(const rapidjson::Value &data, ActionUser &user) { - return rj::getSafe(data, "target_user_login", user.name); + return rj::getSafe(data, "target_user_login", user.login); } rapidjson::Document createListenMessage(const std::vector &topicsVec, diff --git a/src/providers/twitch/TwitchMessageBuilder.cpp b/src/providers/twitch/TwitchMessageBuilder.cpp index ed3280a3e..ed862ca4c 100644 --- a/src/providers/twitch/TwitchMessageBuilder.cpp +++ b/src/providers/twitch/TwitchMessageBuilder.cpp @@ -1416,17 +1416,19 @@ void TwitchMessageBuilder::deletionMessage(const DeleteAction &action, builder->message().flags.set(MessageFlag::Timeout); builder - ->emplace(action.source.name, MessageElementFlag::Username, + ->emplace(action.source.login, + MessageElementFlag::Username, MessageColor::System, FontStyle::ChatMediumBold) - ->setLink({Link::UserInfo, action.source.name}); + ->setLink({Link::UserInfo, action.source.login}); // TODO(mm2pl): If or when jumping to a single message gets implemented a link, // add a link to the originalMessage builder->emplace( "deleted message from", MessageElementFlag::Text, MessageColor::System); builder - ->emplace(action.target.name, MessageElementFlag::Username, + ->emplace(action.target.login, + MessageElementFlag::Username, MessageColor::System, FontStyle::ChatMediumBold) - ->setLink({Link::UserInfo, action.target.name}); + ->setLink({Link::UserInfo, action.target.login}); builder->emplace("saying:", MessageElementFlag::Text, MessageColor::System); if (action.messageText.length() > 50)