From b49083f8d343e1832ee540b09f4c2912093d7164 Mon Sep 17 00:00:00 2001 From: DawnFire42 Date: Sat, 14 Mar 2026 13:14:27 -0400 Subject: [PATCH] refactor: extract shared card insertion algorithm from hand/stack zones 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 — thier post-add behavior (signals, coordinate placement, hidden cards) is materially different. --- .../src/game/zones/logic/add_card_algorithm.h | 41 ++++ .../src/game/zones/logic/hand_zone_logic.cpp | 16 +- .../src/game/zones/logic/stack_zone_logic.cpp | 16 +- tests/CMakeLists.txt | 1 + tests/add_card_algorithm/CMakeLists.txt | 16 ++ .../add_card_algorithm_test.cpp | 208 ++++++++++++++++++ 6 files changed, 272 insertions(+), 26 deletions(-) create mode 100644 cockatrice/src/game/zones/logic/add_card_algorithm.h create mode 100644 tests/add_card_algorithm/CMakeLists.txt create mode 100644 tests/add_card_algorithm/add_card_algorithm_test.cpp diff --git a/cockatrice/src/game/zones/logic/add_card_algorithm.h b/cockatrice/src/game/zones/logic/add_card_algorithm.h new file mode 100644 index 000000000..353037bb8 --- /dev/null +++ b/cockatrice/src/game/zones/logic/add_card_algorithm.h @@ -0,0 +1,41 @@ +#ifndef COCKATRICE_ADD_CARD_ALGORITHM_H +#define COCKATRICE_ADD_CARD_ALGORITHM_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. + * + * @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_ADD_CARD_ALGORITHM_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..e9616994b 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 "add_card_algorithm.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..12a6f88a6 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 "add_card_algorithm.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..5ccca8c50 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -60,6 +60,7 @@ target_link_libraries( ${TEST_QT_MODULES} ) +add_subdirectory(add_card_algorithm) add_subdirectory(carddatabase) add_subdirectory(loading_from_clipboard) add_subdirectory(oracle) diff --git a/tests/add_card_algorithm/CMakeLists.txt b/tests/add_card_algorithm/CMakeLists.txt new file mode 100644 index 000000000..4f27bd179 --- /dev/null +++ b/tests/add_card_algorithm/CMakeLists.txt @@ -0,0 +1,16 @@ +add_executable(add_card_algorithm_test add_card_algorithm_test.cpp) + +target_include_directories(add_card_algorithm_test + PRIVATE ${CMAKE_SOURCE_DIR}/cockatrice/src/game/zones/logic +) + +target_link_libraries(add_card_algorithm_test + PRIVATE Threads::Threads + PRIVATE ${GTEST_BOTH_LIBRARIES} +) + +add_test(NAME add_card_algorithm_test COMMAND add_card_algorithm_test) + +if(NOT GTEST_FOUND) + add_dependencies(add_card_algorithm_test gtest) +endif() diff --git a/tests/add_card_algorithm/add_card_algorithm_test.cpp b/tests/add_card_algorithm/add_card_algorithm_test.cpp new file mode 100644 index 000000000..faf9c3f73 --- /dev/null +++ b/tests/add_card_algorithm/add_card_algorithm_test.cpp @@ -0,0 +1,208 @@ +#include "add_card_algorithm.h" + +#include +#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; + bool setVisibleCalled = 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) + { + setVisibleCalled = true; + 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); + } + + void setContentsKnown(bool known) + { + contentsKnown = known; + } +}; + +class AddCardAlgorithmTest : public ::testing::Test +{ +protected: + MockCardList knownList{true}; + MockCardList unknownList{false}; +}; + +TEST_F(AddCardAlgorithmTest, InsertAtValidPosition) +{ + 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.at(0), &a); + EXPECT_EQ(knownList.at(1), &c); + EXPECT_EQ(knownList.at(2), &b); +} + +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, IndexEqualToSizeAppends) +{ + 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); +} + +TEST_F(AddCardAlgorithmTest, InsertIntoEmptyList) +{ + MockCard a; + CardZoneAlgorithms::addCardToList(knownList, &a, 0, false); + + EXPECT_EQ(knownList.size(), 1); + EXPECT_EQ(knownList.at(0), &a); +} + +TEST_F(AddCardAlgorithmTest, ContentsKnownPreservesIdentity) +{ + MockCard card; + CardZoneAlgorithms::addCardToList(knownList, &card, 0, false); + + EXPECT_FALSE(card.idWasCalled); + EXPECT_FALSE(card.cardRefWasCalled); +} + +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, 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); +} + +TEST_F(AddCardAlgorithmTest, CardSetVisibleAfterAdd) +{ + MockCard card; + CardZoneAlgorithms::addCardToList(knownList, &card, 0, false); + + EXPECT_TRUE(card.setVisibleCalled); + EXPECT_TRUE(card.visibleSet); +} + +TEST_F(AddCardAlgorithmTest, MultipleInsertionsMaintainCorrectOrder) +{ + MockCard a, b, c, d; + CardZoneAlgorithms::addCardToList(knownList, &a, 0, false); + CardZoneAlgorithms::addCardToList(knownList, &b, 0, false); + CardZoneAlgorithms::addCardToList(knownList, &c, 1, false); + CardZoneAlgorithms::addCardToList(knownList, &d, 3, false); + + EXPECT_EQ(knownList.size(), 4); + EXPECT_EQ(knownList.at(0), &b); + EXPECT_EQ(knownList.at(1), &c); + EXPECT_EQ(knownList.at(2), &a); + EXPECT_EQ(knownList.at(3), &d); +} + +int main(int argc, char **argv) +{ + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +}