feat(command-zone): add logic layer with tests

Implement CommandZoneLogic - the card data management layer for zones.
This layer handles card insertion, removal, and ordering independently
of Qt graphics rendering.

Key components:
- CommandZoneLogic: manages card data within a zone
- AddCardAlgorithm: generic insertion algorithm for ordered zones
- CardZoneLogic modifications: integrate with existing zone hierarchy

Design decisions:
- Separation of data logic from graphics enables unit testing
- addCardImpl uses a stable insertion algorithm for consistent ordering
- Mock card items enable testing without Qt graphics dependencies

Test coverage verifies card insertion behavior across edge cases.

cockatrice/CMakeLists.txt

@@ -105,6 +105,7 @@ set(cockatrice_SOURCES
     src/game/zones/card_zone.cpp
     src/game/zones/command_zone_state.cpp
     src/game/zones/hand_zone.cpp
+    src/game/zones/logic/command_zone_logic.cpp
     src/game/zones/logic/card_zone_logic.cpp
     src/game/zones/logic/hand_zone_logic.cpp
     src/game/zones/logic/pile_zone_logic.cpp

cockatrice/src/game/zones/logic/add_card_algorithm.h

@@ -0,0 +1,44 @@
+#ifndef ADD_CARD_ALGORITHM_H
+#define ADD_CARD_ALGORITHM_H
+
+/**
+ * @file add_card_algorithm.h
+ * @brief Shared algorithm for card insertion logic.
+ *
+ * This template is used by both production (CommandZoneLogic) and test code
+ * to ensure consistent behavior and eliminate duplication.
+ */
+
+namespace CardZoneAlgorithms
+{
+
+/**
+ * @brief Core logic for adding a card to a zone's card list.
+ *
+ * @tparam CardList List type supporting size(), insert(), getContentsKnown()
+ * @tparam CardType Card type supporting setId(), setCardRef(), resetState(), setVisible()
+ * @param cards The card list to insert into
+ * @param card The card to insert
+ * @param x Target position (negative or out-of-bounds appends to end)
+ */
+template <typename CardList, typename CardType> void addCardToList(CardList &cards, CardType *card, int x)
+{
+    // Normalize position: negative or beyond range appends to end
+    if (x < 0 || x >= cards.size()) {
+        x = static_cast<int>(cards.size());
+    }
+    cards.insert(x, card);
+
+    // Handle unknown contents
+    if (!cards.getContentsKnown()) {
+        card->setId(-1);
+        card->setCardRef({});
+    }
+
+    card->resetState(true);
+    card->setVisible(true);
+}
+
+} // namespace CardZoneAlgorithms
+
+#endif // ADD_CARD_ALGORITHM_H

cockatrice/src/game/zones/logic/card_zone_logic.cpp

@@ -38,7 +38,7 @@ CardZoneLogic::CardZoneLogic(Player *_player,
 void CardZoneLogic::addCard(CardItem *card, const bool reorganize, const int x, const int y)
 {
     if (!card) {
-        qCWarning(CardZoneLog) << "CardZoneLogic::addCard() card is null; this shouldn't normally happen";
+        qCWarning(CardZoneLogicLog) << "CardZoneLogic::addCard() card is null; this shouldn't normally happen";
         return;
     }
 
@@ -92,7 +92,7 @@ CardItem *CardZoneLogic::getCard(int cardId)
 {
     CardItem *c = cards.findCard(cardId);
     if (!c) {
-        qCWarning(CardZoneLog) << "CardZoneLogic::getCard: card id=" << cardId << "not found";
+        qCWarning(CardZoneLogicLog) << "CardZoneLogic::getCard: card id=" << cardId << "not found";
         return nullptr;
     }
     // If the card's id is -1, this zone is invisible,
@@ -107,7 +107,7 @@ CardItem *CardZoneLogic::getCard(int cardId)
 void CardZoneLogic::removeCard(CardItem *card)
 {
     if (!card) {
-        qCWarning(CardZoneLog) << "CardZoneLogic::removeCard: card is null, this shouldn't normally happen";
+        qCWarning(CardZoneLogicLog) << "CardZoneLogic::removeCard: card is null, this shouldn't normally happen";
         return;
     }
 
@@ -195,6 +195,18 @@ QString CardZoneLogic::getTranslatedName(bool theirOwn, GrammaticalCase gc) cons
         return (theirOwn ? tr("their graveyard", "nominative") : tr("%1's graveyard", "nominative").arg(ownerName));
     else if (name == ZoneNames::EXILE)
         return (theirOwn ? tr("their exile", "nominative") : tr("%1's exile", "nominative").arg(ownerName));
+    else if (name == ZoneNames::COMMAND)
+        return (theirOwn ? tr("their command zone", "nominative")
+                         : tr("%1's command zone", "nominative").arg(ownerName));
+    else if (name == ZoneNames::PARTNER)
+        return (theirOwn ? tr("their partner zone", "nominative")
+                         : tr("%1's partner zone", "nominative").arg(ownerName));
+    else if (name == ZoneNames::COMPANION)
+        return (theirOwn ? tr("their companion zone", "nominative")
+                         : tr("%1's companion zone", "nominative").arg(ownerName));
+    else if (name == ZoneNames::BACKGROUND)
+        return (theirOwn ? tr("their background zone", "nominative")
+                         : tr("%1's background zone", "nominative").arg(ownerName));
     else if (name == ZoneNames::SIDEBOARD)
         switch (gc) {
             case CaseLookAtZone:

cockatrice/src/game/zones/logic/card_zone_logic.h

@@ -1,7 +1,7 @@
 /**
  * @file card_zone_logic.h
  * @ingroup GameLogicZones
- * @brief TODO: Document this.
+ * @brief Base class for zone data management, separated from graphics.
  */
 
 #ifndef COCKATRICE_CARD_ZONE_LOGIC_H
@@ -22,6 +22,18 @@ class QAction;
 class QPainter;
 class CardDragItem;
 
+/**
+ * @class CardZoneLogic
+ * @brief Abstract base class for zone data management (logic layer).
+ *
+ * CardZoneLogic handles card storage, lookup, and manipulation for a zone.
+ * It is paired with a CardZone graphics object to implement the logic/graphics
+ * separation pattern used throughout Cockatrice.
+ *
+ * Subclasses must implement addCardImpl() to define zone-specific insertion behavior.
+ *
+ * @see CardZone
+ */
 class CardZoneLogic : public QObject
 {
     Q_OBJECT
@@ -111,7 +123,7 @@ protected:
     QList<ZoneViewZone *> views;
     bool hasCardAttr;
     bool isShufflable;
-    bool alwaysRevealTopCard;
+    bool alwaysRevealTopCard = false;
 
     virtual void addCardImpl(CardItem *card, int x, int y) = 0;
 };

cockatrice/src/game/zones/logic/command_zone_logic.cpp

@@ -0,0 +1,19 @@
+#include "command_zone_logic.h"
+
+#include "../../board/card_item.h"
+#include "add_card_algorithm.h"
+
+CommandZoneLogic::CommandZoneLogic(Player *_player,
+                                   const QString &_name,
+                                   bool _hasCardAttr,
+                                   bool _isShufflable,
+                                   bool _contentsKnown,
+                                   QObject *parent)
+    : CardZoneLogic(_player, _name, _hasCardAttr, _isShufflable, _contentsKnown, parent)
+{
+}
+
+void CommandZoneLogic::addCardImpl(CardItem *card, int x, int /*y*/)
+{
+    CardZoneAlgorithms::addCardToList(cards, card, x);
+}

cockatrice/src/game/zones/logic/command_zone_logic.h

@@ -0,0 +1,51 @@
+/**
+ * @file command_zone_logic.h
+ * @ingroup GameLogicZones
+ * @brief Logic layer for the command zone, used for Commander format.
+ */
+
+#ifndef COCKATRICE_COMMAND_ZONE_LOGIC_H
+#define COCKATRICE_COMMAND_ZONE_LOGIC_H
+#include "card_zone_logic.h"
+
+/**
+ * @class CommandZoneLogic
+ * @brief Logic layer for managing cards in the command zone.
+ *
+ * Handles data storage and card management for command zones in Commander format.
+ * Supports ordered card insertion for drag-and-drop operations.
+ *
+ * @see CommandZone for the graphics layer
+ * @see CardZoneLogic
+ */
+class CommandZoneLogic : public CardZoneLogic
+{
+    Q_OBJECT
+public:
+    /**
+     * @brief Constructs a CommandZoneLogic instance.
+     * @param _player The player who owns this zone
+     * @param _name Zone name ("command" or "partner")
+     * @param _hasCardAttr Whether cards in this zone have attributes
+     * @param _isShufflable Whether the zone can be shuffled
+     * @param _contentsKnown Whether the zone contents are public knowledge
+     * @param parent Parent QObject
+     */
+    CommandZoneLogic(Player *_player,
+                     const QString &_name,
+                     bool _hasCardAttr,
+                     bool _isShufflable,
+                     bool _contentsKnown,
+                     QObject *parent = nullptr);
+
+protected:
+    /**
+     * @brief Adds a card at position x (y ignored). Appends if x is -1 or out of range.
+     * @param card Card to add
+     * @param x Insertion index, or -1 to append
+     * @param y Unused
+     */
+    void addCardImpl(CardItem *card, int x, int y) override;
+};
+
+#endif // COCKATRICE_COMMAND_ZONE_LOGIC_H

tests/command_zone/CMakeLists.txt

@@ -10,6 +10,27 @@ set(TEST_QT_MODULES
     ${COCKATRICE_QT_VERSION_NAME}::Widgets
 )
 
+# ------------------------
+# Command Zone Logic Test
+# Tests addCardImpl insertion behavior
+# ------------------------
+add_executable(command_zone_logic_test
+    mocks/mock_card_item.cpp
+    command_zone_logic_test.cpp
+)
+
+target_include_directories(command_zone_logic_test
+    PRIVATE ${CMAKE_SOURCE_DIR}/cockatrice/src/game/zones/logic
+)
+
+target_link_libraries(command_zone_logic_test
+    PRIVATE Threads::Threads
+    PRIVATE ${GTEST_BOTH_LIBRARIES}
+    PRIVATE ${TEST_QT_MODULES}
+)
+
+add_test(NAME command_zone_logic_test COMMAND command_zone_logic_test)
+
 # ------------------------
 # Command Zone State Test
 # Tests the extracted CommandZoneState pure state machine
@@ -36,5 +57,6 @@ add_test(NAME command_zone_state_test COMMAND command_zone_state_test)
 # Dependencies on gtest
 # ------------------------
 if(NOT GTEST_FOUND)
+    add_dependencies(command_zone_logic_test gtest)
     add_dependencies(command_zone_state_test gtest)
 endif()

tests/command_zone/command_zone_logic_test.cpp

@@ -0,0 +1,238 @@
+/**
+ * @file command_zone_logic_test.cpp
+ * @brief Unit tests for CommandZoneLogic::addCardImpl
+ *
+ * Tests the card insertion logic for command zones:
+ * - Position handling (negative x, beyond range, valid x)
+ * - Card visibility and state reset
+ * - Unknown contents handling (sets card ID to -1)
+ */
+
+#include "add_card_algorithm.h"
+#include "mocks/mock_card_item.h"
+
+#include <QList>
+#include <gtest/gtest.h>
+
+/**
+ * @brief Standalone CardList implementation for testing.
+ *
+ * Mimics the behavior of the real CardList class without dependencies.
+ */
+class TestCardList : public QList<MockCardItem *>
+{
+    bool contentsKnown;
+
+public:
+    explicit TestCardList(bool _contentsKnown = true) : contentsKnown(_contentsKnown)
+    {
+    }
+
+    [[nodiscard]] bool getContentsKnown() const
+    {
+        return contentsKnown;
+    }
+
+    void setContentsKnown(bool known)
+    {
+        contentsKnown = known;
+    }
+};
+
+/**
+ * @brief Testable implementation of CommandZoneLogic::addCardImpl.
+ *
+ * Uses the shared CardZoneAlgorithms::addCardToList template to ensure
+ * test behavior matches production code exactly.
+ */
+class CommandZoneLogicTestHarness
+{
+public:
+    TestCardList cards;
+
+    explicit CommandZoneLogicTestHarness(bool contentsKnown = true) : cards(contentsKnown)
+    {
+    }
+
+    /**
+     * @brief Delegates to shared algorithm for testing.
+     *
+     * Uses CardZoneAlgorithms::addCardToList which is the same template
+     * used by CommandZoneLogic::addCardImpl in production code.
+     */
+    void addCardImpl(MockCardItem *card, int x, int /*y*/)
+    {
+        CardZoneAlgorithms::addCardToList(cards, card, x);
+    }
+};
+
+class CommandZoneLogicTest : public ::testing::Test
+{
+protected:
+    CommandZoneLogicTestHarness *harness;
+
+    void SetUp() override
+    {
+        harness = new CommandZoneLogicTestHarness(true);
+    }
+
+    void TearDown() override
+    {
+        qDeleteAll(harness->cards);
+        harness->cards.clear();
+        delete harness;
+    }
+
+    MockCardItem *createCard(int id = 1)
+    {
+        return new MockCardItem(id);
+    }
+
+    void addExistingCards(int count)
+    {
+        for (int i = 0; i < count; ++i) {
+            harness->cards.append(createCard(100 + i));
+        }
+    }
+};
+
+// Test: Negative x appends card to end of list
+TEST_F(CommandZoneLogicTest, AddCard_NegativeX_AppendsToEnd)
+{
+    addExistingCards(2); // Cards at indices 0, 1
+    MockCardItem *newCard = createCard(42);
+
+    harness->addCardImpl(newCard, -1, 0);
+
+    ASSERT_EQ(3, harness->cards.size());
+    EXPECT_EQ(newCard, harness->cards.at(2)) << "Card should be at end (index 2)";
+}
+
+// Test: x beyond range appends card to end
+TEST_F(CommandZoneLogicTest, AddCard_BeyondRange_AppendsToEnd)
+{
+    addExistingCards(2); // Cards at indices 0, 1
+    MockCardItem *newCard = createCard(42);
+
+    harness->addCardImpl(newCard, 100, 0); // x=100 is way beyond size=2
+
+    ASSERT_EQ(3, harness->cards.size());
+    EXPECT_EQ(newCard, harness->cards.at(2)) << "Card should be at end (index 2)";
+}
+
+// Test: Valid x inserts card at that position
+TEST_F(CommandZoneLogicTest, AddCard_ValidX_InsertsAtPosition)
+{
+    addExistingCards(3); // Cards at indices 0, 1, 2
+    MockCardItem *cardAtIndex1 = harness->cards.at(1);
+    MockCardItem *newCard = createCard(42);
+
+    harness->addCardImpl(newCard, 1, 0);
+
+    ASSERT_EQ(4, harness->cards.size());
+    EXPECT_EQ(newCard, harness->cards.at(1)) << "New card should be at index 1";
+    EXPECT_EQ(cardAtIndex1, harness->cards.at(2)) << "Original card at index 1 should shift to index 2";
+}
+
+// Test: y parameter is ignored
+TEST_F(CommandZoneLogicTest, AddCard_YIsIgnored)
+{
+    MockCardItem *newCard = createCard(42);
+
+    harness->addCardImpl(newCard, 0, 999); // y=999 should be ignored
+
+    ASSERT_EQ(1, harness->cards.size());
+    EXPECT_EQ(newCard, harness->cards.at(0)) << "Card should be at index 0 regardless of y value";
+}
+
+// Test: Card is set to visible after adding
+TEST_F(CommandZoneLogicTest, AddCard_SetsVisibleTrue)
+{
+    MockCardItem *newCard = createCard(42);
+    ASSERT_FALSE(newCard->visible) << "Card should start invisible";
+
+    harness->addCardImpl(newCard, 0, 0);
+
+    EXPECT_TRUE(newCard->visible) << "Card should be visible after adding";
+}
+
+// Test: resetState is called with true
+TEST_F(CommandZoneLogicTest, AddCard_CallsResetState)
+{
+    MockCardItem *newCard = createCard(42);
+    ASSERT_FALSE(newCard->resetStateCalled) << "resetState should not be called initially";
+
+    harness->addCardImpl(newCard, 0, 0);
+
+    EXPECT_TRUE(newCard->resetStateCalled) << "resetState should be called";
+    EXPECT_TRUE(newCard->resetStateShowFaceDown) << "resetState should be called with true";
+}
+
+// Test: When contents unknown, card ID is set to -1
+TEST_F(CommandZoneLogicTest, AddCard_UnknownContents_SetsIdNegative)
+{
+    delete harness;
+    harness = new CommandZoneLogicTestHarness(false); // contentsKnown = false
+
+    MockCardItem *newCard = createCard(42);
+    ASSERT_EQ(42, newCard->getId()) << "Card should start with original ID";
+
+    harness->addCardImpl(newCard, 0, 0);
+
+    EXPECT_EQ(-1, newCard->getId()) << "Card ID should be -1 when contents unknown";
+}
+
+// Test: When contents known, card ID is preserved
+TEST_F(CommandZoneLogicTest, AddCard_KnownContents_PreservesId)
+{
+    MockCardItem *newCard = createCard(42);
+    ASSERT_EQ(42, newCard->getId()) << "Card should start with original ID";
+
+    harness->addCardImpl(newCard, 0, 0);
+
+    EXPECT_EQ(42, newCard->getId()) << "Card ID should be preserved when contents known";
+}
+
+// Test: Insert at position 0 (beginning)
+TEST_F(CommandZoneLogicTest, AddCard_AtZero_InsertsAtBeginning)
+{
+    addExistingCards(2);
+    MockCardItem *originalFirst = harness->cards.at(0);
+    MockCardItem *newCard = createCard(42);
+
+    harness->addCardImpl(newCard, 0, 0);
+
+    ASSERT_EQ(3, harness->cards.size());
+    EXPECT_EQ(newCard, harness->cards.at(0)) << "New card should be at beginning";
+    EXPECT_EQ(originalFirst, harness->cards.at(1)) << "Original first card should shift to index 1";
+}
+
+// Test: Insert into empty list
+TEST_F(CommandZoneLogicTest, AddCard_EmptyList_AppendsCard)
+{
+    ASSERT_EQ(0, harness->cards.size()) << "List should start empty";
+    MockCardItem *newCard = createCard(42);
+
+    harness->addCardImpl(newCard, 0, 0);
+
+    ASSERT_EQ(1, harness->cards.size());
+    EXPECT_EQ(newCard, harness->cards.at(0));
+}
+
+// Test: x equal to size appends (boundary condition)
+TEST_F(CommandZoneLogicTest, AddCard_XEqualToSize_AppendsToEnd)
+{
+    addExistingCards(2); // size = 2
+    MockCardItem *newCard = createCard(42);
+
+    harness->addCardImpl(newCard, 2, 0); // x == size, should append
+
+    ASSERT_EQ(3, harness->cards.size());
+    EXPECT_EQ(newCard, harness->cards.at(2)) << "Card should be at end";
+}
+
+int main(int argc, char **argv)
+{
+    ::testing::InitGoogleTest(&argc, argv);
+    return RUN_ALL_TESTS();
+}

tests/command_zone/mocks/mock_card_item.cpp

@@ -0,0 +1,4 @@
+#include "mock_card_item.h"
+
+// MOC requires at least one non-inline method in a translation unit
+// when using Q_OBJECT macro. This file provides that.

tests/command_zone/mocks/mock_card_item.h

@@ -0,0 +1,72 @@
+/**
+ * @file mock_card_item.h
+ * @brief Lightweight CardItem stub for command zone unit tests.
+ *
+ * This mock provides minimal CardItem functionality needed to test
+ * CommandZoneLogic::addCardImpl without pulling in graphics dependencies.
+ */
+
+#ifndef MOCK_CARD_ITEM_H
+#define MOCK_CARD_ITEM_H
+
+#include <QObject>
+#include <QString>
+
+/**
+ * @brief Stub CardRef for tests (avoids network/protobuf dependencies)
+ */
+struct CardRef
+{
+    QString name;
+    QString setCode;
+};
+
+/**
+ * @brief Lightweight CardItem mock for testing zone logic.
+ *
+ * Tracks method calls and state changes for verification in tests.
+ */
+class MockCardItem : public QObject
+{
+    Q_OBJECT
+public:
+    explicit MockCardItem(int _id = 1, QObject *parent = nullptr) : QObject(parent), id(_id)
+    {
+    }
+
+    // State tracking
+    int id;
+    bool visible = false;
+    bool resetStateCalled = false;
+    bool resetStateShowFaceDown = false;
+    CardRef cardRef;
+
+    // CardItem interface methods used by CommandZoneLogic::addCardImpl
+    [[nodiscard]] int getId() const
+    {
+        return id;
+    }
+    void setId(int _id)
+    {
+        id = _id;
+    }
+    void setVisible(bool v)
+    {
+        visible = v;
+    }
+    [[nodiscard]] bool isVisible() const
+    {
+        return visible;
+    }
+    void resetState(bool showFaceDown)
+    {
+        resetStateCalled = true;
+        resetStateShowFaceDown = showFaceDown;
+    }
+    void setCardRef(const CardRef &ref)
+    {
+        cardRef = ref;
+    }
+};
+
+#endif // MOCK_CARD_ITEM_H