[Refactor] Move the remaining zone commands out of game_graphics/zones/ (#7418)

* [Refactor] Move the remaining zone commands out of game_graphics/zones/

* [Game] Move zone dump request onto ZoneViewZoneLogic per review

---------

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

cockatrice/src/game/player/player_actions.cpp

@@ -50,6 +50,7 @@
 #include <libcockatrice/protocol/pb/command_change_zone_properties.pb.h>
 #include <libcockatrice/protocol/pb/command_create_token.pb.h>
 #include <libcockatrice/protocol/pb/command_draw_cards.pb.h>
+#include <libcockatrice/protocol/pb/command_dump_zone.pb.h>
 #include <libcockatrice/protocol/pb/command_flip_card.pb.h>
 #include <libcockatrice/protocol/pb/command_game_say.pb.h>
 #include <libcockatrice/protocol/pb/command_inc_counter.pb.h>
@@ -2035,6 +2036,40 @@ void PlayerActions::moveCards(CardZoneLogic *startZone,
     sendGameCommand(cmd);
 }
 
+PendingCommand *PlayerActions::prepareZoneDump(CardZoneLogic *zone, int numberCards)
+{
+    if (!zone) {
+        return nullptr;
+    }
+
+    Command_DumpZone cmd;
+    cmd.set_player_id(zone->getPlayer()->getPlayerInfo()->getId());
+    cmd.set_zone_name(zone->getName().toStdString());
+    cmd.set_number_cards(numberCards);
+    if (auto *viewLogic = qobject_cast<ZoneViewZoneLogic *>(zone)) {
+        cmd.set_is_reversed(viewLogic->getIsReversed());
+    }
+
+    return prepareGameCommand(cmd);
+}
+
+void PlayerActions::actToggleTapped(const QString &zoneName, const QList<CardItem *> &cards, bool tapped)
+{
+    QList<const ::google::protobuf::Message *> cmdList;
+    for (CardItem *card : cards) {
+        if (card->getTapped() != tapped) {
+            auto *cmd = new Command_SetCardAttr;
+            cmd->set_zone(zoneName.toStdString());
+            cmd->set_card_id(card->getId());
+            cmd->set_attribute(AttrTapped);
+            cmd->set_attr_value(tapped ? "1" : "0");
+            cmdList.append(cmd);
+        }
+    }
+
+    sendGameCommand(prepareGameCommand(cmdList));
+}
+
 void PlayerActions::sendGameCommand(const google::protobuf::Message &command)
 {
     if (player->getPlayerInfo()->getJudge() && !player->getPlayerInfo()->getLocal()) {

cockatrice/src/game/player/player_actions.h

@@ -71,6 +71,21 @@ public:
     void
     moveCards(CardZoneLogic *startZone, CardZoneLogic *targetZone, int x, int y, const QList<CardMoveRequest> &cards);
 
+    /**
+     * @brief Prepares a dump of the given zone, for the caller to connect to and send.
+     * @param zone Zone to dump.
+     * @param numberCards How many cards to dump, -1 for all.
+     */
+    PendingCommand *prepareZoneDump(CardZoneLogic *zone, int numberCards);
+
+    /**
+     * @brief Sets the tapped state of the cards that do not already have it.
+     * @param zoneName Zone the cards belong to.
+     * @param cards Cards the view wants toggled.
+     * @param tapped Desired tapped state.
+     */
+    void actToggleTapped(const QString &zoneName, const QList<CardItem *> &cards, bool tapped);
+
     void moveOneCardUntil(CardItem *card);
     void stopMoveTopCardsUntil();
 

cockatrice/src/game/zones/view_zone_logic.cpp

@@ -3,6 +3,8 @@
 #include "../../client/settings/cache_settings.h"
 #include "../../game_graphics/board/card_item.h"
 #include "../board/card_list.h"
+#include "../player/player_actions.h"
+#include "../player/player_logic.h"
 
 #include <QtGlobal>
 #include <libcockatrice/settings/interface_settings.h>
@@ -177,4 +179,14 @@ void ZoneViewZoneLogic::setWriteableRevealZone(bool _writeableRevealZone)
         emit removeFromViews();
     }
     writeableRevealZone = _writeableRevealZone;
-}
+}
+
+PendingCommand *ZoneViewZoneLogic::requestZoneDump()
+{
+    PlayerActions *actions = getPlayer()->getPlayerActions();
+    PendingCommand *pend = actions->prepareZoneDump(this, numberCards);
+    if (pend != nullptr) {
+        actions->sendGameCommand(pend);
+    }
+    return pend;
+}

cockatrice/src/game/zones/view_zone_logic.h

@@ -12,6 +12,7 @@
 
 class PlayerLogic;
 class QObject;
+class PendingCommand;
 
 class ZoneViewZoneLogic : public CardZoneLogic
 {
@@ -73,6 +74,16 @@ public:
         return origZone;
     }
 
+    /**
+     * @brief Prepares and sends the zone-dump command for this view.
+     * @return The pending command, or nullptr when there is no zone to dump.
+     *
+     * Keeps command construction in game/ so graphics views don't reach
+     * through the player into PlayerActions to set it up. The command is sent
+     * asynchronously, so callers connect to finished() after calling this.
+     */
+    PendingCommand *requestZoneDump();
+
 protected:
     void addCardImpl(CardItem *card, int x, int y) override;
 };

cockatrice/src/game_graphics/zones/table_zone.cpp

@@ -32,7 +32,6 @@
 #include <algorithm>
 #include <iterator>
 #include <libcockatrice/card/card_info.h>
-#include <libcockatrice/protocol/pb/command_set_card_attr.pb.h>
 #include <libcockatrice/settings/interface_settings.h>
 #include <libcockatrice/utility/zone_names.h>
 #include <string>
@@ -267,35 +266,18 @@ void TableZone::reorganizeCards()
 
 void TableZone::toggleTapped()
 {
-    QList<QGraphicsItem *> selectedItemsRaw = scene()->selectedItems();
-    QList<QGraphicsItem *> selectedItems;
-
-    auto isCardOnTable = [](const QGraphicsItem *item) {
-        if (auto card = qgraphicsitem_cast<const CardItem *>(item)) {
-            return card->getZone()->getName() == ZoneNames::TABLE;
-        }
-        return false;
-    };
-
-    std::copy_if(selectedItemsRaw.begin(), selectedItemsRaw.end(), std::back_inserter(selectedItems), isCardOnTable);
-
-    bool tapAll = std::any_of(selectedItems.begin(), selectedItems.end(), [](const QGraphicsItem *item) {
-        return !qgraphicsitem_cast<const CardItem *>(item)->getTapped();
-    });
-    QList<const ::google::protobuf::Message *> cmdList;
-    for (const auto &selectedItem : selectedItems) {
-        CardItem *temp = qgraphicsitem_cast<CardItem *>(selectedItem);
-        if (temp->getTapped() != tapAll) {
-            Command_SetCardAttr *cmd = new Command_SetCardAttr;
-            cmd->set_zone(getLogic()->getName().toStdString());
-            cmd->set_card_id(temp->getId());
-            cmd->set_attribute(AttrTapped);
-            cmd->set_attr_value(tapAll ? "1" : "0");
-            cmdList.append(cmd);
+    QList<CardItem *> selectedCards;
+    for (auto *item : scene()->selectedItems()) {
+        auto *card = qgraphicsitem_cast<CardItem *>(item);
+        if (card && card->getZone()->getName() == ZoneNames::TABLE) {
+            selectedCards.append(card);
         }
     }
-    getLogic()->getPlayer()->getPlayerActions()->sendGameCommand(
-        getLogic()->getPlayer()->getPlayerActions()->prepareGameCommand(cmdList));
+
+    const bool tapAll = std::any_of(selectedCards.begin(), selectedCards.end(),
+                                    [](const CardItem *card) { return !card->getTapped(); });
+
+    getLogic()->getPlayer()->getPlayerActions()->actToggleTapped(getLogic()->getName(), selectedCards, tapAll);
 }
 
 void TableZone::resizeToContents()

cockatrice/src/game_graphics/zones/view_zone.cpp

@@ -26,7 +26,6 @@
 #include <QtMath>
 #include <functional>
 #include <libcockatrice/card/card_localization.h>
-#include <libcockatrice/protocol/pb/command_dump_zone.pb.h>
 #include <libcockatrice/protocol/pb/response_dump_zone.pb.h>
 #include <libcockatrice/protocol/pb/serverinfo_card.pb.h>
 #include <libcockatrice/protocol/pending_command.h>
@@ -98,15 +97,10 @@ void ZoneViewZone::initializeCards(const QList<const ServerInfo_Card *> &cardLis
         }
         reorganizeCards();
     } else if (!qobject_cast<ZoneViewZoneLogic *>(getLogic())->getOriginalZone()->contentsKnown()) {
-        Command_DumpZone cmd;
-        cmd.set_player_id(getLogic()->getPlayer()->getPlayerInfo()->getId());
-        cmd.set_zone_name(getLogic()->getName().toStdString());
-        cmd.set_number_cards(numberCards);
-        cmd.set_is_reversed(qobject_cast<ZoneViewZoneLogic *>(getLogic())->getIsReversed());
-
-        PendingCommand *pend = getLogic()->getPlayer()->getPlayerActions()->prepareGameCommand(cmd);
-        connect(pend, &PendingCommand::finished, this, &ZoneViewZone::zoneDumpReceived);
-        getLogic()->getPlayer()->getPlayerActions()->sendGameCommand(pend);
+        PendingCommand *pend = qobject_cast<ZoneViewZoneLogic *>(getLogic())->requestZoneDump();
+        if (pend != nullptr) {
+            connect(pend, &PendingCommand::finished, this, &ZoneViewZone::zoneDumpReceived);
+        }
     } else {
         const CardList &c = qobject_cast<ZoneViewZoneLogic *>(getLogic())->getOriginalZone()->getCards();
         int number = numberCards == -1 ? c.size() : (numberCards < c.size() ? numberCards : c.size());

cockatrice/src/game_graphics/zones/view_zone_widget.cpp

@@ -45,7 +45,6 @@
 #include <QVariant>
 #include <QWidget>
 #include <QtGlobal>
-#include <libcockatrice/protocol/pb/command_shuffle.pb.h>
 #include <libcockatrice/settings/cards_display_settings.h>
 #include <libcockatrice/settings/interface_settings.h>
 #include <qnamespace.h>
@@ -570,7 +569,7 @@ void ZoneViewWidget::closeEvent(QCloseEvent *event)
     // manually call zone->close in order to remove it from the origZones views
     zone->close();
     if (shuffleCheckBox.isChecked()) {
-        player->getPlayerActions()->sendGameCommand(Command_Shuffle());
+        player->getPlayerActions()->actShuffle();
     }
     zoneDeleted();
     event->accept();