From 9bb399606ced58ed15cbb85dfb9737c130854fcb Mon Sep 17 00:00:00 2001 From: DawnFire42 Date: Sun, 15 Mar 2026 03:39:44 -0400 Subject: [PATCH] refactor: extract shared card insertion algorithm from hand/stack zones (#6701) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../game/zones/logic/card_zone_algorithms.h | 45 +++++ .../src/game/zones/logic/hand_zone_logic.cpp | 16 +- .../src/game/zones/logic/stack_zone_logic.cpp | 16 +- tests/CMakeLists.txt | 1 + tests/card_zone_algorithms/CMakeLists.txt | 15 ++ .../card_zone_algorithms_test.cpp | 159 ++++++++++++++++++ 6 files changed, 226 insertions(+), 26 deletions(-) create mode 100644 cockatrice/src/game/zones/logic/card_zone_algorithms.h create mode 100644 tests/card_zone_algorithms/CMakeLists.txt create mode 100644 tests/card_zone_algorithms/card_zone_algorithms_test.cpp diff --git a/cockatrice/src/game/zones/logic/card_zone_algorithms.h b/cockatrice/src/game/zones/logic/card_zone_algorithms.h new file mode 100644 index 000000000..118a7d29d --- /dev/null +++ b/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 +void addCardToList(CardList &cards, CardType *card, int x, bool keepAnnotations) +{ + if (x < 0 || x >= cards.size()) { + x = static_cast(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 diff --git a/cockatrice/src/game/zones/logic/hand_zone_logic.cpp b/cockatrice/src/game/zones/logic/hand_zone_logic.cpp index 292b9f7e3..efa85d649 100644 --- a/cockatrice/src/game/zones/logic/hand_zone_logic.cpp +++ b/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); -} \ No newline at end of file + CardZoneAlgorithms::addCardToList(cards, card, x, false); +} diff --git a/cockatrice/src/game/zones/logic/stack_zone_logic.cpp b/cockatrice/src/game/zones/logic/stack_zone_logic.cpp index a477a8025..6f3cb6e99 100644 --- a/cockatrice/src/game/zones/logic/stack_zone_logic.cpp +++ b/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(cards.size()); - } - cards.insert(x, card); - - if (!cards.getContentsKnown()) { - card->setId(-1); - card->setCardRef({}); - } - card->resetState(true); - card->setVisible(true); -} \ No newline at end of file + CardZoneAlgorithms::addCardToList(cards, card, x, true); +} diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index d80ccce4f..3a9eb55cf 100644 --- a/tests/CMakeLists.txt +++ b/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) diff --git a/tests/card_zone_algorithms/CMakeLists.txt b/tests/card_zone_algorithms/CMakeLists.txt new file mode 100644 index 000000000..889e92eaa --- /dev/null +++ b/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() diff --git a/tests/card_zone_algorithms/card_zone_algorithms_test.cpp b/tests/card_zone_algorithms/card_zone_algorithms_test.cpp new file mode 100644 index 000000000..cc098cae9 --- /dev/null +++ b/tests/card_zone_algorithms/card_zone_algorithms_test.cpp @@ -0,0 +1,159 @@ +#include "card_zone_algorithms.h" + +#include +#include + +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 cards; + bool contentsKnown; + +public: + explicit MockCardList(bool _contentsKnown) : contentsKnown(_contentsKnown) + { + } + + int size() const + { + return static_cast(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(); +}