diff --git a/cockatrice/CMakeLists.txt b/cockatrice/CMakeLists.txt index 06255b5f1..ca0be91a2 100644 --- a/cockatrice/CMakeLists.txt +++ b/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 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..4933e2e36 --- /dev/null +++ b/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 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(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 diff --git a/cockatrice/src/game/zones/logic/card_zone_logic.cpp b/cockatrice/src/game/zones/logic/card_zone_logic.cpp index c7149c307..625f2244f 100644 --- a/cockatrice/src/game/zones/logic/card_zone_logic.cpp +++ b/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: diff --git a/cockatrice/src/game/zones/logic/card_zone_logic.h b/cockatrice/src/game/zones/logic/card_zone_logic.h index 1aa46effd..7ce220227 100644 --- a/cockatrice/src/game/zones/logic/card_zone_logic.h +++ b/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 views; bool hasCardAttr; bool isShufflable; - bool alwaysRevealTopCard; + bool alwaysRevealTopCard = false; virtual void addCardImpl(CardItem *card, int x, int y) = 0; }; diff --git a/cockatrice/src/game/zones/logic/command_zone_logic.cpp b/cockatrice/src/game/zones/logic/command_zone_logic.cpp new file mode 100644 index 000000000..3a50de8f9 --- /dev/null +++ b/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); +} diff --git a/cockatrice/src/game/zones/logic/command_zone_logic.h b/cockatrice/src/game/zones/logic/command_zone_logic.h new file mode 100644 index 000000000..ecd9be3bc --- /dev/null +++ b/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 diff --git a/tests/command_zone/CMakeLists.txt b/tests/command_zone/CMakeLists.txt index ad78a0bf5..c8f9c408a 100644 --- a/tests/command_zone/CMakeLists.txt +++ b/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() diff --git a/tests/command_zone/command_zone_logic_test.cpp b/tests/command_zone/command_zone_logic_test.cpp new file mode 100644 index 000000000..e0567b6b8 --- /dev/null +++ b/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 +#include + +/** + * @brief Standalone CardList implementation for testing. + * + * Mimics the behavior of the real CardList class without dependencies. + */ +class TestCardList : public QList +{ + 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(); +} diff --git a/tests/command_zone/mocks/mock_card_item.cpp b/tests/command_zone/mocks/mock_card_item.cpp new file mode 100644 index 000000000..c1dbbdaa6 --- /dev/null +++ b/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. diff --git a/tests/command_zone/mocks/mock_card_item.h b/tests/command_zone/mocks/mock_card_item.h new file mode 100644 index 000000000..894591ebf --- /dev/null +++ b/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 +#include + +/** + * @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