diff --git a/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model.cpp b/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model.cpp index 5e9fb6b4a..76afca0c4 100644 --- a/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model.cpp +++ b/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model.cpp @@ -479,6 +479,13 @@ QModelIndex DeckListModel::addCard(const ExactCard &card, const QString &zoneNam auto *boardNode = dynamic_cast(root->findChild(zoneName)); auto *customZoneNode = boardNode ? nullptr : DeckListModelCustomZones::findSubZoneByName(root, zoneName); + // Mirroring flattens nested deck sub-zones into shadow rows, so a shadow row + // index is only usable as a deck-tree position while both sides have the same + // direct-children shape. When they diverge, the card is appended to the deck + // zone instead of being written out of range. + InnerDecklistNode *deckCardParent = nullptr; + bool customZoneNeedsAppend = false; + if (boardNode) { // Board zone: cards are grouped by the active criteria. QString groupCriteria = extractGroupCriteriaValue(cardInfo, activeGroupCriteria); @@ -486,6 +493,27 @@ QModelIndex DeckListModel::addCard(const ExactCard &card, const QString &zoneNam } else if (customZoneNode) { // Custom zone: cards live flat inside the zone. cardParent = customZoneNode; + auto *listRoot = deckList->getTree()->getRoot(); + for (int i = 0; i < listRoot->size(); ++i) { + auto *boardZone = dynamic_cast(listRoot->at(i)); + if (!boardZone) { + continue; + } + deckCardParent = dynamic_cast(boardZone->findChild(zoneName)); + if (deckCardParent) { + break; + } + } + // A deck custom zone holding nested sub-zones mirrors with flattened rows, + // so a shadow row index does not map onto its direct children. + if (deckCardParent) { + for (int i = 0; i < deckCardParent->size(); ++i) { + if (dynamic_cast(deckCardParent->at(i))) { + customZoneNeedsAppend = true; + break; + } + } + } } else { // Not present in the shadow tree. The deck tree may still hold a custom // zone that has not been mirrored (callers can add a zone and then a @@ -495,7 +523,10 @@ QModelIndex DeckListModel::addCard(const ExactCard &card, const QString &zoneNam bool hasDeckZone = false; for (int i = 0; i < listRoot->size(); ++i) { if (auto *boardZone = dynamic_cast(listRoot->at(i))) { - if (boardZone->findChild(zoneName)) { + // Only real zones count: a card sitting directly under the board + // shares the name comparison but is not a zone, and treating it as + // one would recurse forever without mirroring anything. + if (dynamic_cast(boardZone->findChild(zoneName))) { hasDeckZone = true; break; } @@ -522,8 +553,9 @@ QModelIndex DeckListModel::addCard(const ExactCard &card, const QString &zoneNam if (!cardNode) { // Determine the correct index int insertRow = findSortedInsertRow(cardParent, cardInfo); + int deckInsertRow = customZoneNeedsAppend ? -1 : insertRow; - auto *decklistCard = deckList->addCard(cardInfo->getName(), zoneName, insertRow, cardSetName, + auto *decklistCard = deckList->addCard(cardInfo->getName(), zoneName, deckInsertRow, cardSetName, printingInfo.getProperty("num"), printingInfo.getProperty("uuid")); beginInsertRows(parentIndex, insertRow, insertRow); diff --git a/tests/deck_list_model/deck_list_model_zone_integration_test.cpp b/tests/deck_list_model/deck_list_model_zone_integration_test.cpp index d49d1b443..a4562c95d 100644 --- a/tests/deck_list_model/deck_list_model_zone_integration_test.cpp +++ b/tests/deck_list_model/deck_list_model_zone_integration_test.cpp @@ -239,6 +239,43 @@ TEST(DeckListModelZoneIntegration, EmptyCustomZoneSurvivesMirrorAndPruning) EXPECT_TRUE(findZoneRow(model, mainIndex).isValid()); } +// Regression: a board card named like the requested zone must not be mistaken for +// a zone. Previously `findChild` matched any child by name, so a mainboard card +// called "Lightning Bolt" made addCard believe a "Lightning Bolt" zone existed and +// recurse through rebuildTree forever. +TEST(DeckListModelZoneIntegration, AddCardToCardNamedZoneDoesNotRecurse) +{ + QSharedPointer deck(new DeckList()); + DeckListModel model(nullptr, deck); + auto *tree = deck->getTree(); + + tree->addCard("Lightning Bolt", 2, DECK_ZONE_MAIN, -1); + + QModelIndex added = model.addCard(ExactCard(CardInfo::newInstance("Swords to Plowshares")), "Lightning Bolt"); + ASSERT_TRUE(added.isValid()); +} + +// Regression: adding to a custom zone that holds a nested sub-zone mirrored the +// nested cards as flattened shadow rows, so the sorted shadow row index pointed +// past the deck zone's direct children. The card must be appended to the deck +// zone instead of being written out of range. +TEST(DeckListModelZoneIntegration, AddCardToCustomZoneWithNestedSubZoneAppends) +{ + QSharedPointer deck(new DeckList()); + DeckListModel model(nullptr, deck); + auto *tree = deck->getTree(); + + auto *removal = tree->addCustomZone(DECK_ZONE_MAIN, "Removal"); + ASSERT_NE(removal, nullptr); + auto *deeper = new InnerDecklistNode("Deeper", removal); + new DecklistCardNode("Lightning Bolt", 2, deeper, -1); + model.rebuildTree(); + + QModelIndex added = model.addCard(ExactCard(CardInfo::newInstance("Swords to Plowshares")), "Removal"); + ASSERT_TRUE(added.isValid()); + ASSERT_TRUE(added.parent().data(DeckRoles::IsCustomZoneRole).toBool()); +} + int main(int argc, char **argv) { ::testing::InitGoogleTest(&argc, argv);