From 2253e4ff5eac17c622fe06ab91befeea7acf416f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Wed, 2 Sep 2026 08:27:53 +0200 Subject: [PATCH] [DeckEditor] Notify card set changes after zone edits and drop the board scan - modifyTree emits cardNodesChanged alongside deckHashChanged so the banner-card combo and printing in-deck counts refresh after removing a zone that still holds cards - DecklistNodeTree::findCustomZoneByName is public and moveCustomZone uses it, locating zones under non-standard boards (e.g. tokens) instead of scanning only main/side/maybeboard --- .../deck_editor/deck_state_manager.cpp | 27 ++++++-------- .../deck_list/deck_list_node_tree.h | 11 +++++- .../deck_list_zones/deck_list_zones_test.cpp | 36 +++++++++++++++++++ 3 files changed, 56 insertions(+), 18 deletions(-) diff --git a/cockatrice/src/interface/widgets/deck_editor/deck_state_manager.cpp b/cockatrice/src/interface/widgets/deck_editor/deck_state_manager.cpp index e007e6dd0..e563729a4 100644 --- a/cockatrice/src/interface/widgets/deck_editor/deck_state_manager.cpp +++ b/cockatrice/src/interface/widgets/deck_editor/deck_state_manager.cpp @@ -407,25 +407,15 @@ bool DeckStateManager::moveCustomZone(const QString &zoneName, const QString &ne { const auto *tree = deckList->getTree(); - // Locate the board currently holding the zone. - QString currentBoardName; - for (const QString &boardName : InnerDecklistNode::boardZoneNames()) { - for (const auto *zone : tree->getCustomZones(boardName)) { - if (zone->getName() == zoneName) { - currentBoardName = boardName; - break; - } - } - if (!currentBoardName.isEmpty()) { - break; - } - } - - if (currentBoardName.isEmpty()) { + // Locate the zone through the tree's own lookup, which walks every top-level + // zone (not just the standard boards) and covers the same-board no-op below. + const auto *zone = tree->findCustomZoneByName(zoneName); + if (!zone) { return false; } // Same-board moves are no-ops and must not pollute the history. + const QString currentBoardName = zone->getParent() ? zone->getParent()->getName() : QString(); if (currentBoardName == newBoardZoneName) { return true; } @@ -433,8 +423,8 @@ bool DeckStateManager::moveCustomZone(const QString &zoneName, const QString &ne // Zone names are deck-unique among zones created through this manager, so a // same-named zone on the target board can only come from an imported deck. // Refuse the move instead of silently stacking same-named zones. - for (const auto *zone : tree->getCustomZones(newBoardZoneName)) { - if (zone->getName() == zoneName) { + for (const auto *targetZone : tree->getCustomZones(newBoardZoneName)) { + if (targetZone->getName() == zoneName) { return false; } } @@ -552,6 +542,9 @@ bool DeckStateManager::modifyTree(const QString &reason, const std::functionrebuildTree(); deckList->refreshDeckHash(); emit deckListModel->deckHashChanged(); + // removeCustomZone can drop whole card sets the model never notified + // about (rebuildTree emits no cardNodesChanged), so tell the consumers. + emit deckListModel->cardNodesChanged(); doCardModified(); } diff --git a/libcockatrice_deck_list/libcockatrice/deck_list/deck_list_node_tree.h b/libcockatrice_deck_list/libcockatrice/deck_list/deck_list_node_tree.h index 3c88d6030..5d91cd233 100644 --- a/libcockatrice_deck_list/libcockatrice/deck_list/deck_list_node_tree.h +++ b/libcockatrice_deck_list/libcockatrice/deck_list/deck_list_node_tree.h @@ -124,6 +124,16 @@ public: */ bool hasZoneName(const QString &zoneName) const; + /** + * @brief Finds a custom zone anywhere in the deck by name. + * + * Walks the children of every top-level zone, so a zone nested under any + * board (and not just the standard ones) is found. + * @param zoneName The zone name to find. + * @return The matching zone node, or nullptr if none exists. + */ + InnerDecklistNode *findCustomZoneByName(const QString &zoneName) const; + /** * @brief Applies a function to every card in the deck tree. This can modify the cards. * @@ -137,7 +147,6 @@ private: InnerDecklistNode *getZoneObjFromName(const QString &zoneName) const; InnerDecklistNode *findBoardZone(const QString &boardZoneName) const; InnerDecklistNode *findOrCreateBoardZone(const QString &boardZoneName); - InnerDecklistNode *findCustomZoneByName(const QString &zoneName) const; }; #endif // COCKATRICE_DECKLIST_NODE_TREE_H diff --git a/tests/deck_list_zones/deck_list_zones_test.cpp b/tests/deck_list_zones/deck_list_zones_test.cpp index a5148621d..801f226a9 100644 --- a/tests/deck_list_zones/deck_list_zones_test.cpp +++ b/tests/deck_list_zones/deck_list_zones_test.cpp @@ -213,6 +213,42 @@ TEST(DeckListZones, MoveCustomZoneMovesCards) EXPECT_FALSE(tree->moveCustomZone("Removal", "not_a_board")); } +TEST(DeckListZones, MoveCustomZoneFailsForUnknownBoard) +{ + DeckList deck; + auto *tree = deck.getTree(); + + ASSERT_NE(tree->addCustomZone(DECK_ZONE_MAIN, "Removal"), nullptr); + tree->addCard("Lightning Bolt", 2, "Removal", -1); + + EXPECT_FALSE(tree->moveCustomZone("Removal", "not_a_board")); + + // The zone is still under main. + EXPECT_EQ(tree->getCustomZones(DECK_ZONE_MAIN).size(), 1); +} + +// Regression: findCustomZoneByName walks every top-level zone, so a custom zone +// an imported deck carries under a non-standard board (tokens) is still found and +// movable. The pre-fix manager-level moveCustomZone only scanned the standard +// boards and returned false for these with no feedback. +TEST(DeckListZones, MoveCustomZoneNestedUnderTokensBoard) +{ + DeckList deck; + auto *tree = deck.getTree(); + auto *root = tree->getRoot(); + + auto *tokens = new InnerDecklistNode(DECK_ZONE_TOKENS, root); + auto *removal = new InnerDecklistNode("Removal", tokens); + new DecklistCardNode("Lightning Bolt", 2, removal, -1); + + EXPECT_TRUE(tree->findCustomZoneByName("Removal")); + EXPECT_TRUE(tree->moveCustomZone("Removal", DECK_ZONE_SIDE)); + + auto pairs = collectBoardCardPairs(deck); + EXPECT_FALSE(hasPair(pairs, DECK_ZONE_TOKENS, "Lightning Bolt")); + EXPECT_TRUE(hasPair(pairs, DECK_ZONE_SIDE, "Lightning Bolt")); +} + TEST(DeckListZones, RemoveCustomZoneRemovesCards) { DeckList deck;