[Refactor] Move card selection actions to GameScene and p/t parsing to CardState (#7426)

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

cockatrice/src/game/board/card_state.cpp

@@ -66,6 +66,37 @@ void CardState::setFaceDown(bool _facedown)
     emit stateChanged();
 }
 
+QVariantList CardState::parsePT(const QString &pt)
+{
+    QVariantList ptList = QVariantList();
+    if (!pt.isEmpty()) {
+        int sep = pt.indexOf('/');
+        if (sep == 0) {
+            ptList.append(QVariant(pt.mid(1))); // cut off starting '/' and take full string
+        } else {
+            int start = 0;
+            for (;;) {
+                QString item = pt.mid(start, sep - start);
+                if (item.isEmpty()) {
+                    ptList.append(QVariant(QString()));
+                } else if (item[0] == '+') {
+                    ptList.append(QVariant(item.mid(1).toInt())); // add as int
+                } else if (item[0] == '-') {
+                    ptList.append(QVariant(item.toInt())); // add as int
+                } else {
+                    ptList.append(QVariant(item)); // add as qstring
+                }
+                if (sep == -1) {
+                    break;
+                }
+                start = sep + 1;
+                sep = pt.indexOf('/', start);
+            }
+        }
+    }
+    return ptList;
+}
+
 void CardState::setZone(CardZoneLogic *_zone)
 {
     if (zone == _zone) {

cockatrice/src/game/board/card_state.h

@@ -5,6 +5,7 @@
 #include <QObject>
 #include <QPoint>
 #include <QString>
+#include <QVariant>
 #include <libcockatrice/card/printing/exact_card.h>
 #include <libcockatrice/utility/card_ref.h>
 #include <qtmetamacros.h>
@@ -151,6 +152,26 @@ public:
 
     void setPT(const QString &_pt);
 
+    /**
+     * @brief Parses a string representing a p/t in order to extract the values from it.
+     *
+     * If the string contains '/', the string will be split at the '/' and each side will be parsed separately,
+     * which means the result list will have two elements.
+     *
+     * If '/' is not found, then the entire string is parsed together, which means the result list will
+     * have a single element.
+     *
+     * If either side of the split is empty, there will also only be a single element in the result list.
+     *
+     * This function will attempt to parse each substring as an int first, handling plus and minus prefixes.
+     * If successful, it will put the parsed value into the QVariant as an int.
+     * If failed, it will just put the substring into the QVariant as a QString.
+     *
+     * @param pt The p/t string
+     * @return A QVariantList that can contain one or two elements, where each QVariant can be either int or QString
+     */
+    static QVariantList parsePT(const QString &pt);
+
     bool getDoesntUntap() const
     {
         return doesntUntap;

cockatrice/src/game/player/player_actions.cpp

@@ -795,58 +795,6 @@ void PlayerActions::actMoveBottomCardToTop()
     sendGameCommand(cmd);
 }
 
-/**
- * Selects all cards in the given zone.
- *
- * @param zone The zone to select from
- * @param filter A predicate to filter which cards are selected. Defaults to always returning true.
- */
-static void selectCardsInZone(
-    const CardZoneLogic *zone,
-    std::function<bool(const CardItem *)> filter = [](const CardItem *) { return true; })
-{
-    if (!zone) {
-        return;
-    }
-
-    for (auto &cardItem : zone->getCards()) {
-        if (cardItem && filter(cardItem)) {
-            cardItem->setSelected(true);
-        }
-    }
-}
-
-void PlayerActions::actSelectAll(const CardItem *card)
-{
-    if (!card) {
-        return;
-    }
-
-    selectCardsInZone(card->getZone());
-}
-
-void PlayerActions::actSelectRow(const CardItem *card)
-{
-    if (!card) {
-        return;
-    }
-
-    auto isSameRow = [card](const CardItem *cardItem) {
-        return qAbs(card->scenePos().y() - cardItem->scenePos().y()) < 50;
-    };
-    selectCardsInZone(card->getZone(), isSameRow);
-}
-
-void PlayerActions::actSelectColumn(const CardItem *card)
-{
-    if (!card) {
-        return;
-    }
-
-    auto isSameColumn = [card](const CardItem *cardItem) { return cardItem->x() == card->x(); };
-    selectCardsInZone(card->getZone(), isSameColumn);
-}
-
 void PlayerActions::actDrawBottomCard()
 {
     if (player->getDeckZone()->getCards().empty()) {
@@ -1356,7 +1304,7 @@ void PlayerActions::actIncPT(QList<CardItem *> selectedCards, int deltaP, int de
     QList<const ::google::protobuf::Message *> commandList;
     for (auto card : selectedCards) {
         QString pt = card->getPT();
-        const auto ptList = CardItem::parsePT(pt);
+        const auto ptList = CardState::parsePT(pt);
         QString newpt;
         if (ptList.isEmpty()) {
             newpt = QString::number(deltaP) + (deltaT ? "/" + QString::number(deltaT) : "");
@@ -1432,7 +1380,7 @@ void PlayerActions::actSetPT(QList<CardItem *> selectedCards, const QString &pt)
 {
     int playerid = player->getPlayerInfo()->getId();
 
-    const auto ptList = CardItem::parsePT(pt);
+    const auto ptList = CardState::parsePT(pt);
     bool empty = ptList.isEmpty();
 
     QList<const ::google::protobuf::Message *> commandList;
@@ -1440,7 +1388,7 @@ void PlayerActions::actSetPT(QList<CardItem *> selectedCards, const QString &pt)
         auto *cmd = new Command_SetCardAttr;
         QString newpt = QString();
         if (!empty) {
-            const auto oldpt = CardItem::parsePT(card->getPT());
+            const auto oldpt = CardState::parsePT(card->getPT());
             int ptIter = 0;
             for (const auto &_item : ptList) {
                 if (_item.typeId() == QMetaType::Type::Int) {
@@ -1537,7 +1485,7 @@ void PlayerActions::actReduceLifeByPower(QList<CardItem *> selectedCards)
     // calculate total power;
     int total = 0;
     for (auto card : selectedCards) {
-        QVariantList parsed = CardItem::parsePT(card->getPT());
+        QVariantList parsed = CardState::parsePT(card->getPT());
         if (!parsed.isEmpty()) {
             int power = parsed.first().toInt(); // toInt will default to 0 if it's not an int
             total += qMax(power, 0);

cockatrice/src/game/player/player_actions.h

@@ -211,10 +211,6 @@ public slots:
     void actRequestMoveBottomCardsToDialog(const QString &targetZone, const QString &zoneDisplayName, bool faceDown);
     void moveBottomCardsTo(int number, const QString &targetZone, bool faceDown);
 
-    void actSelectAll(const CardItem *card);
-    void actSelectRow(const CardItem *card);
-    void actSelectColumn(const CardItem *card);
-
     void actViewLibrary();
     void actViewHand();
     void actRequestViewTopCardsDialog();

cockatrice/src/game_graphics/board/card_item.cpp

@@ -440,37 +440,6 @@ void CardItem::playCard(bool faceDown)
     }
 }
 
-QVariantList CardItem::parsePT(const QString &pt)
-{
-    QVariantList ptList = QVariantList();
-    if (!pt.isEmpty()) {
-        int sep = pt.indexOf('/');
-        if (sep == 0) {
-            ptList.append(QVariant(pt.mid(1))); // cut off starting '/' and take full string
-        } else {
-            int start = 0;
-            for (;;) {
-                QString item = pt.mid(start, sep - start);
-                if (item.isEmpty()) {
-                    ptList.append(QVariant(QString()));
-                } else if (item[0] == '+') {
-                    ptList.append(QVariant(item.mid(1).toInt())); // add as int
-                } else if (item[0] == '-') {
-                    ptList.append(QVariant(item.toInt())); // add as int
-                } else {
-                    ptList.append(QVariant(item)); // add as qstring
-                }
-                if (sep == -1) {
-                    break;
-                }
-                start = sep + 1;
-                sep = pt.indexOf('/', start);
-            }
-        }
-    }
-    return ptList;
-}
-
 /**
  * @brief returns true if the zone is a unwritable reveal zone view (eg a card reveal window). Will return false if zone
  * is nullptr.

cockatrice/src/game_graphics/board/card_item.h

@@ -143,26 +143,6 @@ public:
     void drawAttachArrow();
     void playCard(bool faceDown);
 
-    /**
-     * @brief Parses a string representing a p/t in order to extract the values from it.
-     *
-     * If the string contains '/', the string will be split at the '/' and each side will be parsed separately,
-     * which means the result list will have two elements.
-     *
-     * If '/' is not found, then the entire string is parsed together, which means the result list will
-     * have a single element.
-     *
-     * If either side of the split is empty, there will also only be a single element in the result list.
-     *
-     * This function will attempt to parse each substring as an int first, handling plus and minus prefixes.
-     * If successful, it will put the parsed value into the QVariant as an int.
-     * If failed, it will just put the substring into the QVariant as a QString.
-     *
-     * @param pt The p/t string
-     * @return A QVariantList that can contain one or two elements, where each QVariant can be either int or QString
-     */
-    static QVariantList parsePT(const QString &pt);
-
 protected:
     void mouseMoveEvent(QGraphicsSceneMouseEvent *event) override;
     void mouseReleaseEvent(QGraphicsSceneMouseEvent *event) override;

cockatrice/src/game_graphics/game_scene.cpp

@@ -40,6 +40,7 @@
 #include <QSharedPointer>
 #include <QtMath>
 #include <algorithm>
+#include <functional>
 #include <libcockatrice/settings/interface_settings.h>
 #include <libcockatrice/utility/zone_names.h>
 #include <numeric>
@@ -124,6 +125,56 @@ QList<CardItem *> GameScene::selectedCards() const
     return selectedCards;
 }
 
+/**
+ * @brief Selects every card of @p zone for which @p filter holds.
+ * @param filter A predicate to filter which cards are selected.
+ */
+static void selectCardsInZone(
+    const CardZoneLogic *zone,
+    std::function<bool(const CardItem *)> filter = [](const CardItem *) { return true; })
+{
+    if (!zone) {
+        return;
+    }
+
+    for (auto &cardItem : zone->getCards()) {
+        if (cardItem && filter(cardItem)) {
+            cardItem->setSelected(true);
+        }
+    }
+}
+
+void GameScene::selectAllInZone(const CardItem *card)
+{
+    if (!card) {
+        return;
+    }
+
+    selectCardsInZone(card->getZone());
+}
+
+void GameScene::selectSameRow(const CardItem *card)
+{
+    if (!card) {
+        return;
+    }
+
+    auto isSameRow = [card](const CardItem *cardItem) {
+        return qAbs(card->scenePos().y() - cardItem->scenePos().y()) < 50;
+    };
+    selectCardsInZone(card->getZone(), isSameRow);
+}
+
+void GameScene::selectSameColumn(const CardItem *card)
+{
+    if (!card) {
+        return;
+    }
+
+    auto isSameColumn = [card](const CardItem *cardItem) { return cardItem->x() == card->x(); };
+    selectCardsInZone(card->getZone(), isSameColumn);
+}
+
 CardItem *GameScene::getActiveCard() const
 {
     return activeCard;

cockatrice/src/game_graphics/game_scene.h

@@ -113,6 +113,15 @@ public:
     /** @brief Gets all selected CardItems. */
     QList<CardItem *> selectedCards() const;
 
+    /** @brief Selects every card sharing the zone of @p card. */
+    void selectAllInZone(const CardItem *card);
+
+    /** @brief Selects the cards laid out in the same visual row as @p card. */
+    void selectSameRow(const CardItem *card);
+
+    /** @brief Selects the cards laid out in the same visual column as @p card. */
+    void selectSameColumn(const CardItem *card);
+
     /** @brief Card the currently open context menu targets, or null. */
     CardItem *getActiveCard() const;
 

cockatrice/src/game_graphics/player/menu/card_menu.cpp

@@ -117,9 +117,9 @@ CardMenu::CardMenu(PlayerGraphicsItem *_player, CardItem *_card, bool _shortcuts
 
     connect(aAttach, &QAction::triggered, this, &CardMenu::attachCard);
     connect(aDrawArrow, &QAction::triggered, this, &CardMenu::drawArrow);
-    connect(aSelectAll, &QAction::triggered, this, [this, actions]() { actions->actSelectAll(card); });
+    connect(aSelectAll, &QAction::triggered, this, [this, gameScene]() { gameScene->selectAllInZone(card); });
-    connect(aSelectRow, &QAction::triggered, this, [this, actions]() { actions->actSelectRow(card); });
+    connect(aSelectRow, &QAction::triggered, this, [this, gameScene]() { gameScene->selectSameRow(card); });
-    connect(aSelectColumn, &QAction::triggered, this, [this, actions]() { actions->actSelectColumn(card); });
+    connect(aSelectColumn, &QAction::triggered, this, [this, gameScene]() { gameScene->selectSameColumn(card); });
 
     aRevealToAll = new QAction(this);
 

cockatrice/src/game_graphics/tally/stats_tally.cpp

@@ -16,7 +16,7 @@ static int sumPowers(const QList<CardItem *> &cards)
     // calculate total power;
     int total = 0;
     for (auto card : cards) {
-        QVariantList parsed = CardItem::parsePT(card->getPT());
+        QVariantList parsed = CardState::parsePT(card->getPT());
         if (!parsed.isEmpty()) {
             int power = parsed.first().toInt(); // toInt will default to 0 if it's not an int
             total += qMax(power, 0);
@@ -44,7 +44,7 @@ static int sumToughness(const QList<CardItem *> &cards)
 {
     int total = 0;
     for (auto card : cards) {
-        QVariantList parsed = CardItem::parsePT(card->getPT());
+        QVariantList parsed = CardState::parsePT(card->getPT());
         if (parsed.size() == 2) {
             int toughness = parsed.at(1).toInt(); // toInt will default to 0 if it's not an int
             total += qMax(toughness, 0);