[Refactor] Build arrow and attach commands in game/ instead of ArrowItem (#7422)

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

cockatrice/src/game/player/player_actions.cpp

@@ -5,6 +5,7 @@
 #include "../../game_graphics/dialogs/dlg_create_token.h"
 #include "../../interface/deck_loader/loaded_deck.h"
 #include "../abstract_game.h"
+#include "../board/arrow_data.h"
 #include "../board/counter_state.h"
 #include "../game_event_handler.h"
 #include "../zones/card_zone_logic.h"
@@ -48,6 +49,7 @@
 #include <libcockatrice/card/relation/card_relation.h>
 #include <libcockatrice/protocol/pb/command_attach_card.pb.h>
 #include <libcockatrice/protocol/pb/command_change_zone_properties.pb.h>
+#include <libcockatrice/protocol/pb/command_create_arrow.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>
@@ -66,6 +68,7 @@
 #include <libcockatrice/settings/card_override_settings.h>
 #include <libcockatrice/settings/interface_settings.h>
 #include <libcockatrice/utility/clamped_arithmetic.h>
+#include <libcockatrice/utility/color.h>
 #include <libcockatrice/utility/counter_limits.h>
 #include <libcockatrice/utility/expression.h>
 #include <libcockatrice/utility/zone_names.h>
@@ -174,6 +177,60 @@ void PlayerActions::playCardToTable(const CardItem *card, bool faceDown)
     sendGameCommand(cmd);
 }
 
+void PlayerActions::attachCards(CardItem *startCard, const CardItem *targetCard)
+{
+    if (targetCard->getAttachedTo() || targetCard->getZone()->getName() != ZoneNames::TABLE) {
+        return;
+    }
+
+    // move card onto table first if attaching from some other zone
+    if (startCard->getZone()->getName() != ZoneNames::TABLE) {
+        playCardToTable(startCard, false);
+    }
+
+    Command_AttachCard cmd;
+    cmd.set_start_zone(ZoneNames::TABLE);
+    cmd.set_card_id(startCard->getId());
+    cmd.set_target_player_id(targetCard->getZone()->getPlayer()->getPlayerInfo()->getId());
+    cmd.set_target_zone(targetCard->getZone()->getName().toStdString());
+    cmd.set_target_card_id(targetCard->getId());
+    sendGameCommand(cmd);
+}
+
+void PlayerActions::createArrow(CardItem *startCard, const ArrowData &arrow, int deleteInPhase)
+{
+    Command_CreateArrow cmd;
+    cmd.mutable_arrow_color()->CopyFrom(convertQColorToColor(arrow.color));
+    cmd.set_start_player_id(arrow.startPlayerId);
+    cmd.set_start_zone(arrow.startZone.toStdString());
+    cmd.set_start_card_id(arrow.startCardId);
+    cmd.set_target_player_id(arrow.targetPlayerId);
+
+    if (!arrow.isPlayerTargeted()) {
+        cmd.set_target_zone(arrow.targetZone.toStdString());
+        cmd.set_target_card_id(arrow.targetCardId);
+    }
+
+    // if the card is in hand then it has just been moved to the stack or table as part of drawing the arrow
+    if (arrow.startZone == ZoneNames::HAND) {
+        CardInfoPtr ci = startCard->getCard().getCardPtr();
+        bool playToStack = SettingsCache::instance().userInterface().getPlayToStack();
+        if (ci && ((!playToStack && ci->getUiAttributes().tableRow == 3) ||
+                   (playToStack && ci->getUiAttributes().tableRow != 0 &&
+                    startCard->getZone()->getName() != ZoneNames::STACK))) {
+            cmd.set_start_zone(ZoneNames::STACK);
+        } else {
+            cmd.set_start_zone(playToStack ? ZoneNames::STACK : ZoneNames::TABLE);
+        }
+    }
+
+    if (deleteInPhase != 0) {
+        cmd.set_delete_in_phase(deleteInPhase);
+    }
+
+    sendGameCommand(cmd);
+}
+
 void PlayerActions::actViewLibrary()
 {
     emit requestZoneViewToggle(ZoneNames::DECK, -1);

cockatrice/src/game/player/player_actions.h

@@ -41,7 +41,7 @@ class CardZoneLogic;
 class Command_MoveCard;
 class PendingCommand;
 class PlayerLogic;
-
+struct ArrowData;
 class PlayerActions : public QObject
 {
     Q_OBJECT
@@ -86,6 +86,25 @@ public:
      */
     void actToggleTapped(const QString &zoneName, const QList<CardItem *> &cards, bool tapped);
 
+    /**
+     * @brief Attaches one card to another on the table on behalf of a view.
+     * @param startCard Card being attached.
+     * @param targetCard Card being attached to.
+     */
+    void attachCards(CardItem *startCard, const CardItem *targetCard);
+
+    /**
+     * @brief Draws an arrow from a card to another card or a player on behalf of a view.
+     *
+     * The view resolves the endpoints before playing the card out of hand, so the
+     * arrow still records the zone the card started in.
+     *
+     * @param startCard Card the arrow starts from.
+     * @param arrow Arrow endpoints and color, as resolved by the view.
+     * @param deleteInPhase Phase the arrow expires in, 0 to keep it until removed.
+     */
+    void createArrow(CardItem *startCard, const ArrowData &arrow, int deleteInPhase);
+
     void moveOneCardUntil(CardItem *card);
     void stopMoveTopCardsUntil();
 

cockatrice/src/game_graphics/board/arrow_item.cpp

@@ -27,12 +27,7 @@
 #include <QTransform>
 #include <QtGlobal>
 #include <QtMath>
-#include <libcockatrice/card/card_info.h>
-#include <libcockatrice/protocol/pb/command_attach_card.pb.h>
-#include <libcockatrice/protocol/pb/command_create_arrow.pb.h>
 #include <libcockatrice/settings/cards_display_settings.h>
-#include <libcockatrice/settings/interface_settings.h>
-#include <libcockatrice/utility/color.h>
 #include <libcockatrice/utility/zone_names.h>
 #include <math.h>
 #include <qnamespace.h>
@@ -426,19 +421,19 @@ void ArrowDragItem::mouseReleaseEvent(QGraphicsSceneMouseEvent *event)
 
         CardZoneLogic *startZone = startCard->getZone();
 
-        Command_CreateArrow cmd;
-        cmd.mutable_arrow_color()->CopyFrom(convertQColorToColor(data->color));
-        cmd.set_start_player_id(startZone->getPlayer()->getPlayerInfo()->getId());
-        cmd.set_start_zone(startZone->getName().toStdString());
-        cmd.set_start_card_id(startCard->getId());
+        ArrowData arrow;
+        arrow.color = data->color;
+        arrow.startPlayerId = startZone->getPlayer()->getPlayerInfo()->getId();
+        arrow.startZone = startZone->getName();
+        arrow.startCardId = startCard->getId();
 
         if (auto *targetCard = qgraphicsitem_cast<CardItem *>(targetItem)) {
             CardZoneLogic *targetZone = targetCard->getZone();
-            cmd.set_target_player_id(targetZone->getPlayer()->getPlayerInfo()->getId());
-            cmd.set_target_zone(targetZone->getName().toStdString());
-            cmd.set_target_card_id(targetCard->getId());
+            arrow.targetPlayerId = targetZone->getPlayer()->getPlayerInfo()->getId();
+            arrow.targetZone = targetZone->getName();
+            arrow.targetCardId = targetCard->getId();
         } else if (auto *targetPlayer = qgraphicsitem_cast<PlayerTarget *>(targetItem)) {
-            cmd.set_target_player_id(targetPlayer->getOwner()->getPlayerInfo()->getId());
+            arrow.targetPlayerId = targetPlayer->getOwner()->getPlayerInfo()->getId();
         } else {
             delArrow();
             return;
@@ -447,22 +442,9 @@ void ArrowDragItem::mouseReleaseEvent(QGraphicsSceneMouseEvent *event)
         // if the card is in hand then we will move the card to stack or table as part of drawing the arrow
         if (startZone->getName() == ZoneNames::HAND) {
             startCard->playCard(false);
-            CardInfoPtr ci = startCard->getCard().getCardPtr();
-            bool playToStack = SettingsCache::instance().userInterface().getPlayToStack();
-            if (ci && ((!playToStack && ci->getUiAttributes().tableRow == 3) ||
-                       (playToStack && ci->getUiAttributes().tableRow != 0 &&
-                        startCard->getZone()->getName() != ZoneNames::STACK))) {
-                cmd.set_start_zone(ZoneNames::STACK);
-            } else {
-                cmd.set_start_zone(playToStack ? ZoneNames::STACK : ZoneNames::TABLE);
-            }
         }
 
-        if (deleteInPhase != 0) {
-            cmd.set_delete_in_phase(deleteInPhase);
-        }
-
-        player->getPlayerActions()->sendGameCommand(cmd);
+        player->getPlayerActions()->createArrow(startCard, arrow, deleteInPhase);
     }
 
     delArrow();
@@ -554,7 +536,7 @@ void ArrowAttachItem::mouseReleaseEvent(QGraphicsSceneMouseEvent *event)
         auto *startCard = qgraphicsitem_cast<CardItem *>(startItem);
         auto *targetCard = qgraphicsitem_cast<CardItem *>(targetItem);
         if (startCard && targetCard) {
-            attachCards(startCard, targetCard);
+            player->getPlayerActions()->attachCards(startCard, targetCard);
         }
     }
 
@@ -563,23 +545,3 @@ void ArrowAttachItem::mouseReleaseEvent(QGraphicsSceneMouseEvent *event)
         child->mouseReleaseEvent(event);
     }
 }
-
-void ArrowAttachItem::attachCards(CardItem *startCard, const CardItem *targetCard)
-{
-    if (targetCard->getAttachedTo() || targetCard->getZone()->getName() != ZoneNames::TABLE) {
-        return;
-    }
-
-    // move card onto table first if attaching from some other zone
-    if (startCard->getZone()->getName() != ZoneNames::TABLE) {
-        player->getPlayerActions()->playCardToTable(startCard, false);
-    }
-
-    Command_AttachCard cmd;
-    cmd.set_start_zone(ZoneNames::TABLE);
-    cmd.set_card_id(startCard->getId());
-    cmd.set_target_player_id(targetCard->getZone()->getPlayer()->getPlayerInfo()->getId());
-    cmd.set_target_zone(targetCard->getZone()->getName().toStdString());
-    cmd.set_target_card_id(targetCard->getId());
-    player->getPlayerActions()->sendGameCommand(cmd);
-}

cockatrice/src/game_graphics/board/arrow_item.h

@@ -131,7 +131,6 @@ private:
     PlayerLogic *player;
     QList<ArrowAttachItem *> childArrows;
     QMetaObject::Connection positionConnection;
-    void attachCards(CardItem *startCard, const CardItem *targetCard);
 
 public:
     explicit ArrowAttachItem(ArrowTarget *_startItem);