refactor: extract shared card insertion algorithm from hand/stack zones (#6701)

Hand and stack zones had near-identical addCardImpl() implementations, differing only in whether resetState() preserves annotations.
Extract the shared pattern into a template function (CardZoneAlgorithms::addCardToList) to eliminate duplication and enable isolated testing without Qt dependencies.
Pile, table, and zone-view logic are intentionally excluded — their post-add behavior (signals, coordinate placement, hidden cards) is materially different.

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

@@ -0,0 +1,45 @@
+#ifndef COCKATRICE_CARD_ZONE_ALGORITHMS_H
+#define COCKATRICE_CARD_ZONE_ALGORITHMS_H
+
+namespace CardZoneAlgorithms
+{
+
+/**
+ * Shared insertion logic for zones where cards become visible on add and follow
+ * the standard pattern: clamp index, insert, clear identity if contents unknown,
+ * reset state, show card.
+ *
+ * Zones with different post-add behavior (signal connections, positional resets,
+ * hidden cards, or coordinate-based placement) should NOT use this — implement
+ * addCardImpl directly instead.
+ *
+ * Template parameters allow testing with lightweight mocks that avoid Qt graphics
+ * dependencies.
+ *
+ * @tparam CardList Must provide: size() -> int, insert(int, CardType*),
+ *                  getContentsKnown() -> bool
+ * @tparam CardType Must provide: setId(int), setCardRef(CardRefType),
+ *                  resetState(bool), setVisible(bool)
+ * @param keepAnnotations Forwarded to card->resetState(). Stack-like zones preserve
+ *                        annotations across zone transitions; hand-like zones clear them.
+ */
+template <typename CardList, typename CardType>
+void addCardToList(CardList &cards, CardType *card, int x, bool keepAnnotations)
+{
+    if (x < 0 || x >= cards.size()) {
+        x = static_cast<int>(cards.size());
+    }
+    cards.insert(x, card);
+
+    if (!cards.getContentsKnown()) {
+        card->setId(-1);
+        card->setCardRef({});
+    }
+
+    card->resetState(keepAnnotations);
+    card->setVisible(true);
+}
+
+} // namespace CardZoneAlgorithms
+
+#endif // COCKATRICE_CARD_ZONE_ALGORITHMS_H

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

@@ -1,6 +1,7 @@
 #include "hand_zone_logic.h"
 
 #include "../../board/card_item.h"
+#include "card_zone_algorithms.h"
 
 HandZoneLogic::HandZoneLogic(Player *_player,
                              const QString &_name,
@@ -14,16 +15,5 @@ HandZoneLogic::HandZoneLogic(Player *_player,
 
 void HandZoneLogic::addCardImpl(CardItem *card, int x, int /*y*/)
 {
-    // if x is negative set it to add at end
-    if (x < 0 || x >= cards.size()) {
-        x = cards.size();
-    }
-    cards.insert(x, card);
-
-    if (!cards.getContentsKnown()) {
-        card->setId(-1);
-        card->setCardRef({});
-    }
-    card->resetState();
-    card->setVisible(true);
-}
+    CardZoneAlgorithms::addCardToList(cards, card, x, false);
+}

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

@@ -1,6 +1,7 @@
 #include "stack_zone_logic.h"
 
 #include "../../board/card_item.h"
+#include "card_zone_algorithms.h"
 
 StackZoneLogic::StackZoneLogic(Player *_player,
                                const QString &_name,
@@ -14,16 +15,5 @@ StackZoneLogic::StackZoneLogic(Player *_player,
 
 void StackZoneLogic::addCardImpl(CardItem *card, int x, int /*y*/)
 {
-    // if x is negative set it to add at end
-    if (x < 0 || x >= cards.size()) {
-        x = static_cast<int>(cards.size());
-    }
-    cards.insert(x, card);
-
-    if (!cards.getContentsKnown()) {
-        card->setId(-1);
-        card->setCardRef({});
-    }
-    card->resetState(true);
-    card->setVisible(true);
-}
+    CardZoneAlgorithms::addCardToList(cards, card, x, true);
+}

tests/CMakeLists.txt

@@ -60,6 +60,7 @@ target_link_libraries(
   ${TEST_QT_MODULES}
 )
 
+add_subdirectory(card_zone_algorithms)
 add_subdirectory(carddatabase)
 add_subdirectory(loading_from_clipboard)
 add_subdirectory(oracle)

tests/card_zone_algorithms/CMakeLists.txt

@@ -0,0 +1,15 @@
+add_executable(card_zone_algorithms_test card_zone_algorithms_test.cpp)
+
+target_include_directories(card_zone_algorithms_test PRIVATE ${CMAKE_SOURCE_DIR}/cockatrice/src/game/zones/logic)
+
+target_link_libraries(
+  card_zone_algorithms_test
+  PRIVATE Threads::Threads
+  PRIVATE ${GTEST_BOTH_LIBRARIES}
+)
+
+add_test(NAME card_zone_algorithms_test COMMAND card_zone_algorithms_test)
+
+if(NOT GTEST_FOUND)
+  add_dependencies(card_zone_algorithms_test gtest)
+endif()

tests/card_zone_algorithms/card_zone_algorithms_test.cpp

@@ -0,0 +1,159 @@
+#include "card_zone_algorithms.h"
+
+#include <gtest/gtest.h>
+#include <vector>
+
+struct MockCardRef
+{
+};
+
+struct MockCard
+{
+    int idSet = 0;
+    bool idWasCalled = false;
+    MockCardRef cardRefSet{};
+    bool cardRefWasCalled = false;
+    bool resetStateCalled = false;
+    bool resetStateKeepAnnotations = false;
+    bool visibleSet = false;
+
+    void setId(int id)
+    {
+        idSet = id;
+        idWasCalled = true;
+    }
+
+    void setCardRef(MockCardRef ref)
+    {
+        cardRefSet = ref;
+        cardRefWasCalled = true;
+    }
+
+    void resetState(bool keepAnnotations)
+    {
+        resetStateCalled = true;
+        resetStateKeepAnnotations = keepAnnotations;
+    }
+
+    void setVisible(bool visible)
+    {
+        visibleSet = visible;
+    }
+};
+
+class MockCardList
+{
+    std::vector<MockCard *> cards;
+    bool contentsKnown;
+
+public:
+    explicit MockCardList(bool _contentsKnown) : contentsKnown(_contentsKnown)
+    {
+    }
+
+    int size() const
+    {
+        return static_cast<int>(cards.size());
+    }
+
+    void insert(int index, MockCard *card)
+    {
+        cards.insert(cards.begin() + index, card);
+    }
+
+    bool getContentsKnown() const
+    {
+        return contentsKnown;
+    }
+
+    MockCard *at(int index) const
+    {
+        return cards.at(index);
+    }
+};
+
+class AddCardAlgorithmTest : public ::testing::Test
+{
+protected:
+    MockCardList knownList{true};
+    MockCardList unknownList{false};
+};
+
+TEST_F(AddCardAlgorithmTest, NegativeIndexClampsToEnd)
+{
+    MockCard a, b;
+    CardZoneAlgorithms::addCardToList(knownList, &a, 0, false);
+    CardZoneAlgorithms::addCardToList(knownList, &b, -1, false);
+
+    EXPECT_EQ(knownList.at(0), &a);
+    EXPECT_EQ(knownList.at(1), &b);
+    EXPECT_EQ(knownList.size(), 2);
+}
+
+TEST_F(AddCardAlgorithmTest, IndexBeyondSizeClampsToEnd)
+{
+    MockCard a, b;
+    CardZoneAlgorithms::addCardToList(knownList, &a, 0, false);
+    CardZoneAlgorithms::addCardToList(knownList, &b, 999, false);
+
+    EXPECT_EQ(knownList.at(0), &a);
+    EXPECT_EQ(knownList.at(1), &b);
+    EXPECT_EQ(knownList.size(), 2);
+}
+
+TEST_F(AddCardAlgorithmTest, ContentsKnownPreservesIdentity)
+{
+    MockCard card;
+    CardZoneAlgorithms::addCardToList(knownList, &card, 0, false);
+
+    EXPECT_FALSE(card.idWasCalled);
+    EXPECT_FALSE(card.cardRefWasCalled);
+    EXPECT_TRUE(card.visibleSet);
+}
+
+TEST_F(AddCardAlgorithmTest, ContentsUnknownClearsIdentity)
+{
+    MockCard card;
+    CardZoneAlgorithms::addCardToList(unknownList, &card, 0, false);
+
+    EXPECT_TRUE(card.idWasCalled);
+    EXPECT_EQ(card.idSet, -1);
+    EXPECT_TRUE(card.cardRefWasCalled);
+}
+
+TEST_F(AddCardAlgorithmTest, MidListInsertionPreservesOrder)
+{
+    MockCard a, b, c;
+    CardZoneAlgorithms::addCardToList(knownList, &a, 0, 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), &c);
+    EXPECT_EQ(knownList.at(2), &b);
+}
+
+TEST_F(AddCardAlgorithmTest, KeepAnnotationsFalsePassedThrough)
+{
+    MockCard card;
+    CardZoneAlgorithms::addCardToList(knownList, &card, 0, false);
+
+    EXPECT_TRUE(card.resetStateCalled);
+    EXPECT_FALSE(card.resetStateKeepAnnotations);
+}
+
+TEST_F(AddCardAlgorithmTest, KeepAnnotationsTruePassedThrough)
+{
+    MockCard card;
+    CardZoneAlgorithms::addCardToList(knownList, &card, 0, true);
+
+    EXPECT_TRUE(card.resetStateCalled);
+    EXPECT_TRUE(card.resetStateKeepAnnotations);
+}
+
+int main(int argc, char **argv)
+{
+    ::testing::InitGoogleTest(&argc, argv);
+    return RUN_ALL_TESTS();
+}