[Settings] Make chat settings page scrollable (#7271)

When a player restores connection to a game, the client rebuilds each
zone from the cards the server sends in the game state. Non-coordinate
zones (hand, piles, stack) report x == 0 on every card, so inserting
each rebuilt card at that coordinate reversed the received order one
card at a time.

Append rebuilt cards in the order they arrive for zones without
coordinates; coordinate-based zones (table) keep using x/y.

Fixes #2759

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>

cockatrice/src/game/board/counter_state.cpp

@@ -13,12 +13,12 @@ CounterState *CounterState::fromProto(const ServerInfo_Counter &counter, QObject
                             convertColorToQColor(counter.counter_color()), counter.radius(), counter.count(), parent);
 }
 
-void CounterState::setValue(int newValue)
+void CounterState::setValue(int newValue, bool skipDamageAnimation)
 {
     if (newValue == value) {
         return;
     }
     int old = value;
     value = newValue;
-    emit valueChanged(old, newValue);
+    emit valueChanged(old, newValue, skipDamageAnimation);
 }

cockatrice/src/game/board/counter_state.h

@@ -35,10 +35,23 @@ public:
         return value;
     }
 
-    void setValue(int newValue);
+    /**
+     * @brief Set the counter value.
+     * @param newValue The new value.
+     * @param skipDamageAnimation When true, valueChanged is emitted with skipDamageAnimation=true, letting views
+     * suppress damage-related feedback (e.g. battlefield shimmer, life counter flash) for values set during replay
+     * rewinds.
+     */
+    void setValue(int newValue, bool skipDamageAnimation = false);
 
 signals:
-    void valueChanged(int oldValue, int newValue);
+    /**
+     * @brief Emitted whenever the value changes.
+     * @param oldValue The previous value.
+     * @param newValue The new value.
+     * @param skipDamageAnimation True when the change should not trigger damage/life-change feedback in views.
+     */
+    void valueChanged(int oldValue, int newValue, bool skipDamageAnimation);
 
 private:
     int id;

cockatrice/src/game/player/event_processing_options.h

@@ -13,7 +13,8 @@
 enum EventProcessingOption
 {
     SKIP_REVEAL_WINDOW = 0x0001,
-    SKIP_TAP_ANIMATION = 0x0002
+    SKIP_TAP_ANIMATION = 0x0002,
+    SKIP_DAMAGE_ANIMATION = 0x0004
 };
 
 // Wrap it in a QFlags typedef

cockatrice/src/game/player/player_event_handler.cpp

@@ -262,14 +262,15 @@ void PlayerEventHandler::eventCreateCounter(const Event_CreateCounter &event)
     player->addCounter(event.counter_info());
 }
 
-void PlayerEventHandler::eventSetCounter(const Event_SetCounter &event)
+void PlayerEventHandler::eventSetCounter(const Event_SetCounter &event, EventProcessingOptions options)
 {
     CounterState *ctr = player->getCounters().value(event.counter_id(), nullptr);
     if (!ctr) {
         return;
     }
     int oldValue = ctr->getValue();
-    ctr->setValue(event.value());
+    const bool skipDamageAnimation = options.testFlag(SKIP_DAMAGE_ANIMATION);
+    ctr->setValue(event.value(), skipDamageAnimation);
     emit logSetCounter(player, ctr->getName(), event.value(), oldValue);
 }
 
@@ -625,7 +626,7 @@ void PlayerEventHandler::processGameEvent(GameEvent::GameEventType type,
             eventCreateCounter(event.GetExtension(Event_CreateCounter::ext));
             break;
         case GameEvent::SET_COUNTER:
-            eventSetCounter(event.GetExtension(Event_SetCounter::ext));
+            eventSetCounter(event.GetExtension(Event_SetCounter::ext), options);
             break;
         case GameEvent::DEL_COUNTER:
             eventDelCounter(event.GetExtension(Event_DelCounter::ext));

cockatrice/src/game/player/player_event_handler.h

@@ -153,7 +153,7 @@ public:
     void eventCreateCounter(const Event_CreateCounter &event);
 
     /// Set a player-level counter value.
-    void eventSetCounter(const Event_SetCounter &event);
+    void eventSetCounter(const Event_SetCounter &event, EventProcessingOptions options);
 
     /// Delete a player-level counter.
     void eventDelCounter(const Event_DelCounter &event);

cockatrice/src/game/player/player_logic.cpp

@@ -175,7 +175,15 @@ void PlayerLogic::processPlayerInfo(const ServerInfo_Player &info)
                 const ServerInfo_Card &cardInfo = zoneInfo.card_list(j);
                 auto *card = new CardItem(this);
                 card->processCardInfo(cardInfo);
-                zone->addCard(card, false, cardInfo.x(), cardInfo.y());
+                // Zones without coordinates (hand, piles, stack) preserve the order
+                // they arrive in on the server in the positions of their cards list.
+                // The x coordinate of such cards is always 0, so inserting at it
+                // would reverse the list on reconnect. Append instead.
+                if (zoneInfo.with_coords()) {
+                    zone->addCard(card, false, cardInfo.x(), cardInfo.y());
+                } else {
+                    zone->addCard(card, false, -1);
+                }
             }
         }
         if (zoneInfo.has_always_reveal_top_card()) {

cockatrice/src/game_graphics/board/abstract_counter.cpp

@@ -29,9 +29,9 @@ AbstractCounter::AbstractCounter(CounterState *state,
 {
     setAcceptHoverEvents(true);
 
-    connect(state, &CounterState::valueChanged, this, [this](int oldValue, int newValue) {
+    connect(state, &CounterState::valueChanged, this, [this](int oldValue, int newValue, bool skipDamageAnimation) {
         value = newValue;
-        onValueChanged(oldValue, newValue);
+        onValueChanged(oldValue, newValue, skipDamageAnimation);
         update();
     });
 
@@ -230,7 +230,7 @@ void AbstractCounterDialog::changeValue(int diff)
     setTextValue(QString::number(curValue));
 }
 
-void AbstractCounter::onValueChanged(int /*oldValue*/, int /*newValue*/)
+void AbstractCounter::onValueChanged(int /*oldValue*/, int /*newValue*/, bool /*skipDamageAnimation*/)
 {
     // Default: no feedback. Subclasses such as PlayerCounter override this to
     // flash the counter on meaningful changes (life gain/loss).

cockatrice/src/game_graphics/board/abstract_counter.h

@@ -39,8 +39,9 @@ protected:
      * @brief Hook for subclasses that need per-value-change feedback (e.g. life-total flash).
      *
      * Called whenever the counter's value changes, before the item repaints.
+     * @param skipDamageAnimation True when damage-related feedback should be suppressed (replay rewinds).
      */
-    virtual void onValueChanged(int oldValue, int newValue);
+    virtual void onValueChanged(int oldValue, int newValue, bool skipDamageAnimation);
 
     void mousePressEvent(QGraphicsSceneMouseEvent *event) override;
     void hoverEnterEvent(QGraphicsSceneHoverEvent *event) override;

cockatrice/src/game_graphics/game_scene.cpp

@@ -221,7 +221,12 @@ void GameScene::removePlayer(PlayerLogic *player)
 
     clearArrowsForPlayer(player->getPlayerInfo()->getId());
 
-    for (ZoneViewWidget *zone : zoneViews) {
+    // Closing a view removes it from zoneViews synchronously, so iterate over a
+    // copy: otherwise a player with several open views (e.g. library and hand)
+    // only has the first one closed here and the remaining views are left
+    // pointing at a player that is about to be deleted.
+    const QList<ZoneViewWidget *> zoneViewCopy = zoneViews;
+    for (ZoneViewWidget *zone : zoneViewCopy) {
         if (zone->getPlayer() == player) {
             zone->close();
         }
@@ -664,7 +669,10 @@ CardItem *GameScene::findTopmostCardInZone(const QList<QGraphicsItem *> &items,
  */
 void GameScene::toggleZoneView(PlayerLogic *player, const QString &zoneName, int numberCards, bool isReversed)
 {
-    for (auto &view : zoneViews) {
+    // Closing a view removes it from zoneViews synchronously, so iterate over a
+    // copy to make sure every already-open matching view is closed.
+    const QList<ZoneViewWidget *> zoneViewCopy = zoneViews;
+    for (auto *view : zoneViewCopy) {
         ZoneViewZone *temp = view->getZone();
         if (temp->getLogic()->getName() == zoneName && temp->getLogic()->getPlayer() == player &&
             qobject_cast<ZoneViewZoneLogic *>(temp->getLogic())->getNumberCards() == numberCards) {

cockatrice/src/game_graphics/player/player_graphics_item.cpp

@@ -252,8 +252,8 @@ void PlayerGraphicsItem::onCounterAdded(CounterState *state)
     AbstractCounter *widget;
     if (state->getName() == "life") {
         widget = playerTarget->addCounter(state);
-        connect(state, &CounterState::valueChanged, this, [this](int oldValue, int newValue) {
-            if (newValue < oldValue) {
+        connect(state, &CounterState::valueChanged, this, [this](int oldValue, int newValue, bool skipDamageAnimation) {
+            if (newValue < oldValue && !skipDamageAnimation) {
                 tableZoneGraphicsItem->triggerDamageShimmer();
             }
         });

cockatrice/src/game_graphics/player/player_target.cpp

@@ -69,7 +69,7 @@ void PlayerCounter::paint(QPainter *painter, const QStyleOptionGraphicsItem * /*
     }
 }
 
-void PlayerCounter::onValueChanged(int oldValue, int newValue)
+void PlayerCounter::onValueChanged(int oldValue, int newValue, bool skipDamageAnimation)
 {
     flashDelta = newValue - oldValue;
     if (flashDelta == 0) {
@@ -81,6 +81,11 @@ void PlayerCounter::onValueChanged(int oldValue, int newValue)
         return;
     }
 
+    if (skipDamageAnimation) {
+        flashAlpha = 0.0;
+        return;
+    }
+
     flashAlpha = 1.0;
     flashClock.start();
     if (scene()) {
@@ -132,8 +137,18 @@ void PlayerTarget::paint(QPainter *painter, const QStyleOptionGraphicsItem * /*o
     QRectF translatedRect = painter->combinedTransform().mapRect(avatarBoundingRect);
     QSize translatedSize = translatedRect.size().toSize();
     QPixmap cachedPixmap;
+    // The key must cover everything the generated pawn depends on: the rendered
+    // size, the user level, and the pixmap being drawn. fullPixmap.cacheKey() is
+    // 0 for every null pixmap, so the default-pawn branch additionally needs the
+    // pawn's privlevel (lowercased, matching UserLevelPixmapGenerator) and colors
+    // in the key — otherwise two players without a custom avatar (and the same
+    // user level) would share one cached pawn.
     const QString cacheKey = "avatar" + QString::number(translatedSize.width()) + "_" +
-                             QString::number(info->user_level()) + "_" + QString::number(fullPixmap.cacheKey());
+                             QString::number(translatedSize.height()) + "_" + QString::number(info->user_level()) +
+                             "_" + QString::number(fullPixmap.cacheKey()) + "_" +
+                             QString::fromStdString(info->privlevel()).toLower() + "_" +
+                             QString::fromStdString(info->pawn_colors().left_side()) + "_" +
+                             QString::fromStdString(info->pawn_colors().right_side());
     if (!QPixmapCache::find(cacheKey, &cachedPixmap)) {
         cachedPixmap = QPixmap(translatedSize.width(), translatedSize.height());
 

cockatrice/src/game_graphics/player/player_target.h

@@ -21,7 +21,7 @@ class PlayerCounter : public AbstractCounter, public IAnimatedItem
 {
     Q_OBJECT
 protected:
-    void onValueChanged(int oldValue, int newValue) override;
+    void onValueChanged(int oldValue, int newValue, bool skipDamageAnimation) override;
 
 private:
     static constexpr qreal flashDurationMs = 450.0;

cockatrice/src/interface/widgets/dialogs/dlg_settings.cpp

@@ -132,7 +132,7 @@ void DlgSettings::setupUi()
     pagesWidget->addWidget(makeScrollable(userInterfacePage));
     pagesWidget->addWidget(makeScrollable(deckEditorPage));
     pagesWidget->addWidget(makeScrollable(storagePage));
-    pagesWidget->addWidget(messagesPage);
+    pagesWidget->addWidget(makeScrollable(messagesPage));
     pagesWidget->addWidget(soundPage);
     pagesWidget->addWidget(shortcutsPage);
 

cockatrice/src/interface/widgets/replay/replay_manager.cpp

@@ -142,8 +142,10 @@ void ReplayManager::processNewEvents(PlaybackMode playbackMode)
         }
 
         // backwards skip => always skip tap animation
+        // backwards skip => always skip damage animation (battlefield shimmer / life counter flash)
         if (playbackMode == BACKWARD_SKIP) {
             options |= SKIP_TAP_ANIMATION;
+            options |= SKIP_DAMAGE_ANIMATION;
         }
 
         emit eventReplayed(replay->event_list(currentEvent), options);

cockatrice/src/interface/widgets/server/user/user_context_menu.cpp

@@ -439,7 +439,7 @@ void UserContextMenu::showContextMenu(const QPoint &pos,
         }
     }
     aDetails->setEnabled(true);
-    aChat->setEnabled(anotherUser && online);
+    aChat->setEnabled(anotherUser && online && !userListProxy->isUserIgnored(userName));
     aShowGames->setEnabled(online);
     aReport->setEnabled(anotherUser);
     aAddToBuddyList->setEnabled(anotherUser);
@@ -606,7 +606,15 @@ void UserContextMenu::execAddToIgnore(const QString &userName)
     Command_AddToList cmd;
     cmd.set_list("ignore");
     cmd.set_user_name(userName.toStdString());
-    client->sendCommand(client->prepareSessionCommand(cmd));
+    PendingCommand *pend = client->prepareSessionCommand(cmd);
+    connect(pend, &PendingCommand::finished, this,
+            [this, userName](const Response &response, const CommandContainer &, const QVariant &) {
+                if (response.response_code() == Response::RespOk) {
+                    QMessageBox::information(static_cast<QWidget *>(parent()), tr("Ignore list"),
+                                             tr("%1 has been added to your ignore list.").arg(userName));
+                }
+            });
+    client->sendCommand(pend);
 }
 
 void UserContextMenu::execRemoveFromIgnore(const QString &userName)

cockatrice/src/interface/widgets/settings_page/messages_settings_page.cpp

@@ -59,6 +59,10 @@ MessagesSettingsPage::MessagesSettingsPage()
     connect(&roomHistory, &QCheckBox::QT_STATE_CHANGED, &SettingsCache::instance().chat(),
             &ChatSettings::setRoomHistory);
 
+    ignoreAllPrivateMessagesCheckBox.setChecked(SettingsCache::instance().chat().getIgnoreAllPrivateMessages());
+    connect(&ignoreAllPrivateMessagesCheckBox, &QCheckBox::QT_STATE_CHANGED, &SettingsCache::instance().chat(),
+            &ChatSettings::setIgnoreAllPrivateMessages);
+
     customAlertString = new QLineEdit();
     customAlertString->setText(SettingsCache::instance().chat().getHighlightWords());
     connect(customAlertString, &QLineEdit::textChanged, &SettingsCache::instance().chat(),
@@ -76,6 +80,7 @@ MessagesSettingsPage::MessagesSettingsPage()
     chatGrid->addWidget(&messagePopups, 5, 0);
     chatGrid->addWidget(&mentionPopups, 6, 0);
     chatGrid->addWidget(&roomHistory, 7, 0);
+    chatGrid->addWidget(&ignoreAllPrivateMessagesCheckBox, 8, 0);
     chatGroupBox = new QGroupBox;
     chatGroupBox->setLayout(chatGrid);
 
@@ -256,6 +261,7 @@ void MessagesSettingsPage::retranslateUi()
     messagePopups.setText(tr("Enable desktop notifications for private messages"));
     mentionPopups.setText(tr("Enable desktop notification for mentions"));
     roomHistory.setText(tr("Enable room message history on join"));
+    ignoreAllPrivateMessagesCheckBox.setText(tr("Ignore all private messages"));
     hexLabel.setText(tr("(Color is hexadecimal)"));
     hexHighlightLabel.setText(tr("(Color is hexadecimal)"));
     customAlertStringLabel.setText(tr("Separate words with a space, alphanumeric characters only"));

cockatrice/src/interface/widgets/settings_page/messages_settings_page.h

@@ -40,6 +40,7 @@ private:
     QCheckBox messagePopups;
     QCheckBox mentionPopups;
     QCheckBox roomHistory;
+    QCheckBox ignoreAllPrivateMessagesCheckBox;
     QGroupBox *chatGroupBox;
     QGroupBox *highlightGroupBox;
     QGroupBox *messageGroupBox;

cockatrice/src/interface/widgets/tabs/tab_account.cpp

@@ -137,6 +137,11 @@ void TabAccount::retranslateUi()
     buddyList->retranslateUi();
     ignoreList->retranslateUi();
     userInfoBox->retranslateUi();
+
+    buddyList->setToolTip(tr("Buddies are marked with a star in chat, a sound plays when they join or leave the "
+                             "server, and they can be invited to buddy-only games."));
+    ignoreList->setToolTip(tr("Ignored users' chat messages are hidden from you, and they cannot send you private "
+                              "messages or join your games."));
 }
 
 void TabAccount::processListUsersResponse(const Response &response)

cockatrice/src/interface/widgets/tabs/tab_message.cpp

@@ -98,6 +98,12 @@ void TabMessage::closeEvent(QCloseEvent *event)
 
 void TabMessage::sendPrivateMessage(const QString &text)
 {
+    if (tabSupervisor->getUserListManager()->isUserIgnored(getUserName())) {
+        chatView->appendMessage(tr("You have ignored %1; your messages are not delivered.")
+                                    .arg(QString::fromStdString(otherUserInfo->name())));
+        return;
+    }
+
     Command_Message cmd;
     cmd.set_user_name(otherUserInfo->name());
     cmd.set_message(text.toStdString());

cockatrice/src/interface/widgets/tabs/tab_supervisor.cpp

@@ -1063,6 +1063,13 @@ TabMessage *TabSupervisor::addMessageTab(const QString &receiverName, bool focus
         return tab;
     }
 
+    if (focus && userListManager->isUserIgnored(receiverName)) {
+        QMessageBox::information(
+            this, tr("Ignored user"),
+            tr("You have ignored %1. Remove them from your ignore list to open a private chat.").arg(receiverName));
+        return nullptr;
+    }
+
     tab = new TabMessage(this, client, *userInfo, otherUser, userOnline);
     connect(tab, &TabMessage::talkClosing, this, &TabSupervisor::talkLeft);
     connect(tab, &TabMessage::maximizeClient, this, &TabSupervisor::maximizeMainWindow);
@@ -1277,7 +1284,21 @@ void TabSupervisor::processGameEventContainer(const GameEventContainer &cont)
 
 void TabSupervisor::processUserMessageEvent(const Event_UserMessage &event)
 {
+    // "Ignore all private messages" silences every PM, including messages to
+    // already-open tabs — unlike the unregistered/non-buddy filters below,
+    // which only apply when creating a new tab. Messages from moderators/admins
+    // are exempt to ensure warnings still reach users.
     QString senderName = QString::fromStdString(event.sender_name());
+    if (SettingsCache::instance().chat().getIgnoreAllPrivateMessages()) {
+        const ServerInfo_User *onlineUserInfo = userListManager->getOnlineUser(senderName);
+        if (!onlineUserInfo) {
+            return;
+        }
+        const UserLevelFlags userLevel(onlineUserInfo->user_level());
+        if (!userLevel.testFlag(ServerInfo_User::IsModerator) && !userLevel.testFlag(ServerInfo_User::IsAdmin)) {
+            return;
+        }
+    }
     TabMessage *tab = messageTabs.value(senderName);
     if (!tab) {
         tab = messageTabs.value(QString::fromStdString(event.receiver_name()));

libcockatrice_interfaces/libcockatrice/interfaces/interface_chat_settings_provider.h

@@ -20,6 +20,7 @@ public:
     [[nodiscard]] virtual bool getShowMessagePopup() const = 0;
     [[nodiscard]] virtual bool getShowMentionPopup() const = 0;
     [[nodiscard]] virtual bool getRoomHistory() const = 0;
+    [[nodiscard]] virtual bool getIgnoreAllPrivateMessages() const = 0;
     [[nodiscard]] virtual QString getHighlightWords() const = 0;
 };
 

libcockatrice_settings/libcockatrice/settings/chat_settings.cpp

@@ -65,6 +65,11 @@ bool ChatSettings::getRoomHistory() const
     return getValue("roomHistory", QString(), QString(), true).toBool();
 }
 
+bool ChatSettings::getIgnoreAllPrivateMessages() const
+{
+    return getValue("ignoreAllPrivateMessages", QString(), QString(), false).toBool();
+}
+
 QString ChatSettings::getHighlightWords() const
 {
     return getValue("highlightWords").toString();
@@ -131,6 +136,11 @@ void ChatSettings::setRoomHistory(bool _roomHistory)
     setValue(_roomHistory, "roomHistory");
 }
 
+void ChatSettings::setIgnoreAllPrivateMessages(bool _ignoreAllPrivateMessages)
+{
+    setValue(_ignoreAllPrivateMessages, "ignoreAllPrivateMessages");
+}
+
 void ChatSettings::setHighlightWords(const QString &_highlightWords)
 {
     setValue(_highlightWords, "highlightWords");

libcockatrice_settings/libcockatrice/settings/chat_settings.h

@@ -23,6 +23,7 @@ public:
     [[nodiscard]] bool getShowMessagePopup() const override;
     [[nodiscard]] bool getShowMentionPopup() const override;
     [[nodiscard]] bool getRoomHistory() const override;
+    [[nodiscard]] bool getIgnoreAllPrivateMessages() const override;
     [[nodiscard]] QString getHighlightWords() const override;
 
     void setChatMention(bool _chatMention);
@@ -37,6 +38,7 @@ public:
     void setShowMessagePopups(bool _showMessagePopups);
     void setShowMentionPopups(bool _showMentionPopups);
     void setRoomHistory(bool _roomHistory);
+    void setIgnoreAllPrivateMessages(bool _ignoreAllPrivateMessages);
     void setHighlightWords(const QString &_highlightWords);
 
 signals:

tests/card_zone_algorithms/card_zone_algorithms_test.cpp

@@ -134,6 +134,35 @@ TEST_F(AddCardAlgorithmTest, MidListInsertionPreservesOrder)
     EXPECT_EQ(knownList.at(2), &b);
 }
 
+// Reconnecting to a game rebuilds zones from a ServerInfo_Zone. Non-coordinate zones
+// (hand, piles, stack) report x == 0 on every card, so inserting each rebuilt card at
+// that index would reverse the received server order. Appending (-1) keeps it.
+TEST_F(AddCardAlgorithmTest, RebuildInsertAtZeroReversesServerOrder)
+{
+    MockCard a, b, c;
+    CardZoneAlgorithms::addCardToList(knownList, &a, 0, false);
+    CardZoneAlgorithms::addCardToList(knownList, &b, 0, false);
+    CardZoneAlgorithms::addCardToList(knownList, &c, 0, false);
+
+    EXPECT_EQ(knownList.size(), 3);
+    EXPECT_EQ(knownList.at(0), &c);
+    EXPECT_EQ(knownList.at(1), &b);
+    EXPECT_EQ(knownList.at(2), &a);
+}
+
+TEST_F(AddCardAlgorithmTest, RebuildAppendPreservesServerOrder)
+{
+    MockCard a, b, c;
+    CardZoneAlgorithms::addCardToList(knownList, &a, -1, false);
+    CardZoneAlgorithms::addCardToList(knownList, &b, -1, false);
+    CardZoneAlgorithms::addCardToList(knownList, &c, -1, false);
+
+    EXPECT_EQ(knownList.size(), 3);
+    EXPECT_EQ(knownList.at(0), &a);
+    EXPECT_EQ(knownList.at(1), &b);
+    EXPECT_EQ(knownList.at(2), &c);
+}
+
 TEST_F(AddCardAlgorithmTest, KeepAnnotationsFalsePassedThrough)
 {
     MockCard card;

tests/settings/settings_defaults_test.cpp

@@ -300,6 +300,12 @@ TEST_F(SettingsDefaultsTest, Chat_RoomHistory_Default)
     ASSERT_EQ(s.getRoomHistory(), true);
 }
 
+TEST_F(SettingsDefaultsTest, Chat_IgnoreAllPrivateMessages_Default)
+{
+    ChatSettings s(settingsPath, nullptr);
+    ASSERT_EQ(s.getIgnoreAllPrivateMessages(), false);
+}
+
 // --- PersonalSettings ---
 
 TEST_F(SettingsDefaultsTest, Personal_Lang_Default)