diff --git a/cockatrice/src/interface/deck_loader/deck_loader.cpp b/cockatrice/src/interface/deck_loader/deck_loader.cpp index 39a0c1071..a406b6976 100644 --- a/cockatrice/src/interface/deck_loader/deck_loader.cpp +++ b/cockatrice/src/interface/deck_loader/deck_loader.cpp @@ -384,6 +384,14 @@ void DeckLoader::saveToStream_DeckZone(QTextStream &out, for (int j = 0; j < zoneNode->size(); j++) { auto *card = dynamic_cast(zoneNode->at(j)); + if (!card) { + // Cards collected in nested sub-zones are exported by recursion so + // they don't end up invisible in the plain text output. + if (auto *subZone = dynamic_cast(zoneNode->at(j))) { + saveToStream_DeckZone(out, subZone, addComments, addSetNameAndNumber); + } + continue; + } CardInfoPtr info = CardDatabaseManager::query()->getCardInfo(card->getName()); QString cardType = info ? info->getMainCardType() : "unknown"; @@ -510,9 +518,26 @@ bool DeckLoader::convertToCockatriceFormat(LoadedDeck &deck) void DeckLoader::printDeckListNode(QTextCursor *cursor, const InnerDecklistNode *node) { + if (!node || node->isEmpty()) { + return; + } + const int totalColumns = 2; - if (node->height() == 1) { + // Dispatch children by type instead of trusting a whole-node height: a deck + // node may hold direct cards and nested zones side by side (custom zones), + // and an empty node would previously crash on at(0). + QVector cards; + QVector subZones; + for (int i = 0; i < node->size(); i++) { + if (auto *card = dynamic_cast(node->at(i))) { + cards.append(card); + } else if (auto *zone = dynamic_cast(node->at(i))) { + subZones.append(zone); + } + } + + if (!cards.isEmpty()) { QTextBlockFormat blockFormat; QTextCharFormat charFormat; charFormat.setFontPointSize(11); @@ -523,9 +548,9 @@ void DeckLoader::printDeckListNode(QTextCursor *cursor, const InnerDecklistNode tableFormat.setCellPadding(0); tableFormat.setCellSpacing(0); tableFormat.setBorder(0); - QTextTable *table = cursor->insertTable(node->size() + 1, totalColumns, tableFormat); - for (int i = 0; i < node->size(); i++) { - auto *card = dynamic_cast(node->at(i)); + QTextTable *table = cursor->insertTable(cards.size() + 1, totalColumns, tableFormat); + for (int i = 0; i < cards.size(); i++) { + const AbstractDecklistCardNode *card = cards[i]; QTextCharFormat cellCharFormat; cellCharFormat.setFontPointSize(9); @@ -540,7 +565,13 @@ void DeckLoader::printDeckListNode(QTextCursor *cursor, const InnerDecklistNode cellCursor = cell.firstCursorPosition(); cellCursor.insertText(card->getName()); } - } else if (node->height() == 2) { + } + + for (const InnerDecklistNode *subZone : subZones) { + if (subZone->isEmpty()) { + continue; + } + QTextBlockFormat blockFormat; QTextCharFormat charFormat; charFormat.setFontPointSize(14); @@ -559,10 +590,8 @@ void DeckLoader::printDeckListNode(QTextCursor *cursor, const InnerDecklistNode tableFormat.setColumnWidthConstraints(constraints); QTextTable *table = cursor->insertTable(1, totalColumns, tableFormat); - for (int i = 0; i < node->size(); i++) { - QTextCursor cellCursor = table->cellAt(0, (i * totalColumns) / node->size()).lastCursorPosition(); - printDeckListNode(&cellCursor, dynamic_cast(node->at(i))); - } + QTextCursor cellCursor = table->cellAt(0, 0).firstCursorPosition(); + printDeckListNode(&cellCursor, subZone); } cursor->movePosition(QTextCursor::End); diff --git a/cockatrice/src/interface/widgets/deck_editor/card_database_view.cpp b/cockatrice/src/interface/widgets/deck_editor/card_database_view.cpp index c51830cf0..00388a3cd 100644 --- a/cockatrice/src/interface/widgets/deck_editor/card_database_view.cpp +++ b/cockatrice/src/interface/widgets/deck_editor/card_database_view.cpp @@ -91,7 +91,7 @@ void CardDatabaseView::decrementCard(const QString &zoneName) } void CardDatabaseView::setZoneMenuProvider(const std::function>()> &provider, - const std::function &newZoneHandler) + const std::function &newZoneHandler) { zoneMenuProvider = provider; this->newZoneHandler = newZoneHandler; @@ -151,35 +151,46 @@ void CardDatabaseView::openCustomMenu(QPoint point) if (zoneMenuProvider) { QMenu *addToZoneMenu = menu.addMenu(tr("Add to Zone")); - for (const QString &boardName : InnerDecklistNode::boardZoneNames()) { - QAction *action = addToZoneMenu->addAction(InnerDecklistNode::visibleNameFromName(boardName)); - connect(action, &QAction::triggered, this, - [this, card, boardName] { emit cardAdded(card->getName(), boardName); }); - } - - bool anyCustomZone = false; const auto zoneBoards = zoneMenuProvider(); - for (const auto &zoneBoard : zoneBoards) { - const QString &boardName = zoneBoard.first; - const QStringList &customZones = zoneBoard.second; + for (const QString &boardName : InnerDecklistNode::boardZoneNames()) { + // Boards with zones nest their children so no two menu entries + // share a visible name: "Maindeck ▸ { Maindeck (whole board), … }". + const QStringList customZones = [&zoneBoards, boardName] { + for (const auto &zoneBoard : zoneBoards) { + if (zoneBoard.first == boardName) { + return zoneBoard.second; + } + } + return QStringList(); + }(); if (customZones.isEmpty()) { - continue; - } - anyCustomZone = true; - QMenu *boardSubmenu = addToZoneMenu->addMenu(InnerDecklistNode::visibleNameFromName(boardName)); - for (const QString &zoneName : customZones) { - QAction *action = boardSubmenu->addAction(zoneName); + QAction *action = addToZoneMenu->addAction(InnerDecklistNode::visibleNameFromName(boardName)); connect(action, &QAction::triggered, this, - [this, card, zoneName] { emit cardAdded(card->getName(), zoneName); }); + [this, card, boardName] { emit cardAdded(card->getName(), boardName); }); + } else { + QMenu *boardSubmenu = addToZoneMenu->addMenu(InnerDecklistNode::visibleNameFromName(boardName)); + QAction *wholeBoardAction = boardSubmenu->addAction(InnerDecklistNode::visibleNameFromName(boardName)); + connect(wholeBoardAction, &QAction::triggered, this, + [this, card, boardName] { emit cardAdded(card->getName(), boardName); }); + for (const QString &zoneName : customZones) { + QAction *action = boardSubmenu->addAction(zoneName); + connect(action, &QAction::triggered, this, + [this, card, zoneName] { emit cardAdded(card->getName(), zoneName); }); + } } } - if (anyCustomZone) { + if (newZoneHandler) { addToZoneMenu->addSeparator(); - } - QAction *newZoneAction = addToZoneMenu->addAction(tr("Create &new zone...")); - connect(newZoneAction, &QAction::triggered, this, [this] { newZoneHandler(); }); + QAction *newZoneAction = addToZoneMenu->addAction(tr("Create &new zone...")); + connect(newZoneAction, &QAction::triggered, this, [this, card] { + const QString zoneName = newZoneHandler(); + if (!zoneName.isEmpty()) { + emit cardAdded(card->getName(), zoneName); + } + }); + } } if (canBeCommander(*card)) { diff --git a/cockatrice/src/interface/widgets/deck_editor/card_database_view.h b/cockatrice/src/interface/widgets/deck_editor/card_database_view.h index 72040b97e..668444199 100644 --- a/cockatrice/src/interface/widgets/deck_editor/card_database_view.h +++ b/cockatrice/src/interface/widgets/deck_editor/card_database_view.h @@ -24,7 +24,8 @@ class CardDatabaseView : public QTreeView /// The list contains (board zone name, custom zone names) pairs for every board. std::function>()> zoneMenuProvider; /// Handler invoked when the user picks "New zone..." from the add-to-zone menu. - std::function newZoneHandler; + /// Returns the name of the created zone, or an empty string if creation was cancelled. + std::function newZoneHandler; public: explicit CardDatabaseView(QWidget *parent, CardDatabaseDisplayModel *model); @@ -45,10 +46,11 @@ public: * If no provider is set, the submenu is not shown. * * @param provider Returns the custom zones of the current deck, grouped by board zone - * @param newZoneHandler Invoked when the user chooses "New zone..." in the submenu + * @param newZoneHandler Creates a new custom zone and returns its name, or an empty string + * if creation was cancelled. The menu entry is hidden when not provided. */ void setZoneMenuProvider(const std::function>()> &provider, - const std::function &newZoneHandler); + const std::function &newZoneHandler); signals: void cardChanged(const QString &cardName); diff --git a/cockatrice/src/interface/widgets/deck_editor/deck_editor_card_database_dock_widget.cpp b/cockatrice/src/interface/widgets/deck_editor/deck_editor_card_database_dock_widget.cpp index d78758375..6269f0323 100644 --- a/cockatrice/src/interface/widgets/deck_editor/deck_editor_card_database_dock_widget.cpp +++ b/cockatrice/src/interface/widgets/deck_editor/deck_editor_card_database_dock_widget.cpp @@ -31,7 +31,7 @@ void DeckEditorCardDatabaseDockWidget::createDatabaseDisplayDock(AbstractTabDeck } return result; }, - [this, deckEditor] { + [this, deckEditor]() -> QString { QString boardName; const QString zoneName = DeckZoneDialog::promptForNewZone(this, {}, &boardName, [deckEditor](const QString &candidate) { @@ -40,6 +40,7 @@ void DeckEditorCardDatabaseDockWidget::createDatabaseDisplayDock(AbstractTabDeck if (!zoneName.isEmpty()) { deckEditor->deckStateManager->createCustomZone(boardName, zoneName); } + return zoneName; }); auto *frame = new QVBoxLayout; diff --git a/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.cpp b/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.cpp index 9296f8697..6f3fff5ae 100644 --- a/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.cpp +++ b/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.cpp @@ -784,18 +784,34 @@ void DeckEditorDeckDockWidget::decklistCustomMenu(QPoint point) const bool isCardRow = sourceIndex.isValid() && !isCustomZoneRow && !isBoardZoneRow && !getModel()->hasChildren(sourceIndex); + // Walk the row up to its top-level node to find the hosting board. Cards in + // the tokens board cannot be moved (moveCardToZone bails for it), so the + // move menu is skipped for them. + QString currentBoardName; + QModelIndex board = sourceIndex.parent(); + while (board.isValid() && board.parent().isValid()) { + board = board.parent(); + } + if (board.isValid()) { + currentBoardName = board.siblingAtColumn(DeckListModelColumns::CARD_NAME).data(Qt::EditRole).toString(); + } + if (isCardRow) { - addMoveToZoneMenu(&menu, sourceIndex); - menu.addSeparator(); + if (currentBoardName != DECK_ZONE_TOKENS) { + addMoveToZoneMenu(&menu, sourceIndex, currentBoardName); + menu.addSeparator(); + } } else if (isCustomZoneRow) { const QString zoneName = sourceIndex.siblingAtColumn(DeckListModelColumns::CARD_NAME).data(Qt::EditRole).toString(); QAction *renameAction = menu.addAction(tr("&Rename zone...")); connect(renameAction, &QAction::triggered, this, [this, zoneName] { - const QString newName = DeckZoneDialog::promptForRename(this, zoneName, [this](const QString &candidate) { - return deckStateManager->validateNewZoneName(candidate); - }); + // The unchanged name must not validate as a duplicate. + const QString newName = + DeckZoneDialog::promptForRename(this, zoneName, [this, zoneName](const QString &candidate) { + return candidate == zoneName ? QString() : deckStateManager->validateNewZoneName(candidate); + }); if (!newName.isEmpty() && newName != zoneName) { deckStateManager->renameCustomZone(zoneName, newName); } @@ -805,8 +821,12 @@ void DeckEditorDeckDockWidget::decklistCustomMenu(QPoint point) addChangeBoardMenu(boardMenu, zoneName); QAction *deleteAction = menu.addAction(tr("&Delete zone")); - deleteAction->setEnabled(!getModel()->hasChildren(sourceIndex)); - deleteAction->setStatusTip(tr("Move or remove all cards first.")); + const bool zoneHasCards = getModel()->hasChildren(sourceIndex); + deleteAction->setEnabled(!zoneHasCards); + if (zoneHasCards) { + deleteAction->setToolTip(tr("Move or remove all cards first.")); + menu.setToolTipsVisible(true); + } connect(deleteAction, &QAction::triggered, this, [this, zoneName] { const auto result = QMessageBox::warning(this, tr("Delete zone"), tr("Delete the zone \"%1\"?").arg(zoneName), @@ -837,37 +857,52 @@ void DeckEditorDeckDockWidget::decklistCustomMenu(QPoint point) menu.exec(deckView->mapToGlobal(point)); } -void DeckEditorDeckDockWidget::addMoveToZoneMenu(QMenu *menu, const QModelIndex &sourceCardIndex) +void DeckEditorDeckDockWidget::addMoveToZoneMenu(QMenu *menu, + const QModelIndex &sourceCardIndex, + const QString ¤tBoardName) { - const auto moveToZone = [this, sourceCardIndex](const QString &targetZoneName) { - deckStateManager->moveCardToZone(sourceCardIndex, targetZoneName); + const auto addMoveAction = [this, sourceCardIndex](QMenu *targetMenu, const QString &targetZoneName, + const QString &label, bool enabled) { + QAction *action = targetMenu->addAction(label); + action->setEnabled(enabled); + if (enabled) { + connect(action, &QAction::triggered, this, [this, sourceCardIndex, targetZoneName] { + deckStateManager->moveCardToZone(sourceCardIndex, targetZoneName); + }); + } }; - for (const QString &boardName : InnerDecklistNode::boardZoneNames()) { - QAction *action = menu->addAction(InnerDecklistNode::visibleNameFromName(boardName)); - connect(action, &QAction::triggered, this, [moveToZone, boardName] { moveToZone(boardName); }); - } - const auto tree = deckStateManager->getDeckListShared()->getTree(); - bool anyCustomZone = false; + + QMenu *moveMenu = menu->addMenu(tr("Move to &zone")); + for (const QString &boardName : InnerDecklistNode::boardZoneNames()) { - QList customZones = tree->getCustomZones(boardName); - if (customZones.isEmpty()) { - continue; - } - anyCustomZone = true; - QMenu *boardSubmenu = menu->addMenu(InnerDecklistNode::visibleNameFromName(boardName)); - for (const auto *customZone : customZones) { - QAction *action = boardSubmenu->addAction(customZone->getName()); - connect(action, &QAction::triggered, this, [moveToZone, customZone] { moveToZone(customZone->getName()); }); + const QString boardLabel = InnerDecklistNode::visibleNameFromName(boardName); + const auto customZones = tree->getCustomZones(boardName); + + // Boards with zones nest their children so no two menu entries share a + // visible name: "Maindeck ▸ { Maindeck (whole board), Removal, … }". + // The board the card already lives on is marked instead of offered. + if (!customZones.isEmpty()) { + QMenu *boardSubmenu = moveMenu->addMenu(boardLabel); + addMoveAction(boardSubmenu, boardName, boardLabel, boardName != currentBoardName); + for (const auto *customZone : customZones) { + addMoveAction(boardSubmenu, customZone->getName(), customZone->getName(), true); + } + } else { + addMoveAction(moveMenu, boardName, boardLabel, boardName != currentBoardName); } } - if (anyCustomZone) { - menu->addSeparator(); - } + moveMenu->addSeparator(); - addNewZoneAction(menu); + QAction *newZoneAction = moveMenu->addAction(tr("Create new zone and move &here...")); + connect(newZoneAction, &QAction::triggered, this, [this, sourceCardIndex, currentBoardName] { + const QString zoneName = createNewCustomZone(currentBoardName); + if (!zoneName.isEmpty()) { + deckStateManager->moveCardToZone(sourceCardIndex, zoneName); + } + }); } void DeckEditorDeckDockWidget::addChangeBoardMenu(QMenu *menu, const QString &zoneName) @@ -900,16 +935,21 @@ void DeckEditorDeckDockWidget::addChangeBoardMenu(QMenu *menu, const QString &zo void DeckEditorDeckDockWidget::addNewZoneAction(QMenu *menu, const QString &initialBoardName) { QAction *newZoneAction = menu->addAction(tr("Create &new zone...")); - connect(newZoneAction, &QAction::triggered, this, [this, initialBoardName] { - QString boardName; - const QString zoneName = - DeckZoneDialog::promptForNewZone(this, initialBoardName, &boardName, [this](const QString &candidate) { - return deckStateManager->validateNewZoneName(candidate); - }); - if (!zoneName.isEmpty()) { - deckStateManager->createCustomZone(boardName, zoneName); - } - }); + connect(newZoneAction, &QAction::triggered, this, + [this, initialBoardName] { createNewCustomZone(initialBoardName); }); +} + +QString DeckEditorDeckDockWidget::createNewCustomZone(const QString &initialBoardName) +{ + QString boardName; + const QString zoneName = + DeckZoneDialog::promptForNewZone(this, initialBoardName, &boardName, [this](const QString &candidate) { + return deckStateManager->validateNewZoneName(candidate); + }); + if (!zoneName.isEmpty()) { + deckStateManager->createCustomZone(boardName, zoneName); + } + return zoneName; } void DeckEditorDeckDockWidget::refreshShortcuts() diff --git a/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.h b/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.h index fc3b01dc7..1e5f4e677 100644 --- a/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.h +++ b/cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.h @@ -103,8 +103,9 @@ private: [[nodiscard]] QModelIndexList getSelectedCardNodeSourceIndices() const; void offsetCountAtIndex(const QModelIndex &idx, bool isIncrement); - void addMoveToZoneMenu(QMenu *menu, const QModelIndex &sourceCardIndex); + void addMoveToZoneMenu(QMenu *menu, const QModelIndex &sourceCardIndex, const QString ¤tBoardName); void addChangeBoardMenu(QMenu *menu, const QString &zoneName); + QString createNewCustomZone(const QString &initialBoardName = {}); void addNewZoneAction(QMenu *menu, const QString &initialBoardName = {}); private slots: 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 b60685d20..e007e6dd0 100644 --- a/cockatrice/src/interface/widgets/deck_editor/deck_state_manager.cpp +++ b/cockatrice/src/interface/widgets/deck_editor/deck_state_manager.cpp @@ -470,20 +470,13 @@ QString DeckStateManager::validateNewZoneName(const QString &zoneName) const const auto *tree = deckList->getTree(); - // Top-level zones (boards and legacy zones) claim their names too. - for (int i = 0; i < tree->getRoot()->size(); i++) { - if (tree->getRoot()->at(i)->getName() == trimmedZoneName) { - return tr("A zone with this name already exists."); - } - } - - // Custom zone names are unique across the whole deck. - for (const QString &board : InnerDecklistNode::boardZoneNames()) { - for (const auto *customZone : tree->getCustomZones(board)) { - if (customZone->getName() == trimmedZoneName) { - return tr("A zone with this name already exists."); - } - } + // Reuse the tree's own uniqueness contract: any top-level zone and any + // custom zone on *every* board claims the name (hasZoneName also reserves + // the standard board names, which we already rejected with a dedicated + // message above). Scanning only the standard boards here would miss a + // custom zone an imported deck carries under `tokens`. + if (tree->hasZoneName(trimmedZoneName)) { + return tr("A zone with this name already exists."); } return {}; @@ -558,6 +551,7 @@ bool DeckStateManager::modifyTree(const QString &reason, const std::functionsave(memento); deckListModel->rebuildTree(); deckList->refreshDeckHash(); + emit deckListModel->deckHashChanged(); doCardModified(); } diff --git a/cockatrice/src/interface/widgets/deck_editor/deck_zone_dialog.cpp b/cockatrice/src/interface/widgets/deck_editor/deck_zone_dialog.cpp index a14beec0e..9a0be2570 100644 --- a/cockatrice/src/interface/widgets/deck_editor/deck_zone_dialog.cpp +++ b/cockatrice/src/interface/widgets/deck_editor/deck_zone_dialog.cpp @@ -50,6 +50,9 @@ DeckZoneDialog::DeckZoneDialog(QWidget *parent, if (allowBoardSelection) { layout->addWidget(boardLabel); layout->addWidget(boardCombo); + } else { + boardLabel->hide(); + boardCombo->hide(); } layout->addWidget(buttonBox); diff --git a/cockatrice/src/interface/widgets/tabs/visual_deck_editor/tab_deck_editor_visual.cpp b/cockatrice/src/interface/widgets/tabs/visual_deck_editor/tab_deck_editor_visual.cpp index 46d1b08b6..0f43893d3 100644 --- a/cockatrice/src/interface/widgets/tabs/visual_deck_editor/tab_deck_editor_visual.cpp +++ b/cockatrice/src/interface/widgets/tabs/visual_deck_editor/tab_deck_editor_visual.cpp @@ -85,7 +85,7 @@ void TabDeckEditorVisual::createCentralFrame() connect(tabContainer, &TabDeckEditorVisualTabWidget::printingSelectorRequested, this, &TabDeckEditorVisual::showPrintingSelector); connect(tabContainer, &TabDeckEditorVisualTabWidget::cardInfoRequested, this, &TabDeckEditorVisual::updateCardInfo); - connect(tabContainer, &TabDeckEditorVisualTabWidget::newZoneRequested, this, &TabDeckEditorVisual::createNewZone); + tabContainer->visualDatabaseDisplay->setNewZoneCreator([this] { return createNewZone(); }); centralFrame->addWidget(tabContainer); setCentralWidget(centralWidget); @@ -271,8 +271,8 @@ bool TabDeckEditorVisual::actSaveDeckAs() return result; } -/** @brief Prompts for and creates a new custom deck zone. */ -void TabDeckEditorVisual::createNewZone() +/** @brief Prompts for and creates a new custom deck zone. Returns the name of the created zone. */ +QString TabDeckEditorVisual::createNewZone() { QString boardName; const QString zoneName = DeckZoneDialog::promptForNewZone(this, {}, &boardName, [this](const QString &candidate) { @@ -281,6 +281,7 @@ void TabDeckEditorVisual::createNewZone() if (!zoneName.isEmpty()) { deckStateManager->createCustomZone(boardName, zoneName); } + return zoneName; } /** @brief Refreshes keyboard shortcuts for this tab from settings. */ diff --git a/cockatrice/src/interface/widgets/tabs/visual_deck_editor/tab_deck_editor_visual.h b/cockatrice/src/interface/widgets/tabs/visual_deck_editor/tab_deck_editor_visual.h index e0ad6c914..fb09578c4 100644 --- a/cockatrice/src/interface/widgets/tabs/visual_deck_editor/tab_deck_editor_visual.h +++ b/cockatrice/src/interface/widgets/tabs/visual_deck_editor/tab_deck_editor_visual.h @@ -167,8 +167,9 @@ public slots: /** * @brief Prompts for and creates a new custom deck zone. + * @return The name of the created zone, or an empty string if creation was cancelled. */ - void createNewZone(); + QString createNewZone(); private: /** diff --git a/cockatrice/src/interface/widgets/tabs/visual_deck_editor/tab_deck_editor_visual_tab_widget.cpp b/cockatrice/src/interface/widgets/tabs/visual_deck_editor/tab_deck_editor_visual_tab_widget.cpp index 843ddf493..5ccfcc28f 100644 --- a/cockatrice/src/interface/widgets/tabs/visual_deck_editor/tab_deck_editor_visual_tab_widget.cpp +++ b/cockatrice/src/interface/widgets/tabs/visual_deck_editor/tab_deck_editor_visual_tab_widget.cpp @@ -51,8 +51,6 @@ TabDeckEditorVisualTabWidget::TabDeckEditorVisualTabWidget(QWidget *parent, &TabDeckEditorVisualTabWidget::printingSelectorRequested); connect(visualDatabaseDisplay, &VisualDatabaseDisplayWidget::cardInfoRequested, this, &TabDeckEditorVisualTabWidget::cardInfoRequested); - connect(visualDatabaseDisplay, &VisualDatabaseDisplayWidget::newZoneRequested, this, - &TabDeckEditorVisualTabWidget::newZoneRequested); statsAnalyzer = new DeckListStatisticsAnalyzer(this, deckModel); statsAnalyzer->analyze(); diff --git a/cockatrice/src/interface/widgets/tabs/visual_deck_editor/tab_deck_editor_visual_tab_widget.h b/cockatrice/src/interface/widgets/tabs/visual_deck_editor/tab_deck_editor_visual_tab_widget.h index a625aaad7..4f04b51f6 100644 --- a/cockatrice/src/interface/widgets/tabs/visual_deck_editor/tab_deck_editor_visual_tab_widget.h +++ b/cockatrice/src/interface/widgets/tabs/visual_deck_editor/tab_deck_editor_visual_tab_widget.h @@ -133,7 +133,6 @@ signals: void edhrecRequested(const CardInfoPtr &cardInfo, bool isCommander); void printingSelectorRequested(); void cardInfoRequested(const ExactCard &cardName); - void newZoneRequested(); private: QVBoxLayout *layout; ///< Layout for tabs and controls. diff --git a/cockatrice/src/interface/widgets/visual_database_display/visual_database_display_widget.cpp b/cockatrice/src/interface/widgets/visual_database_display/visual_database_display_widget.cpp index 2b58142b7..76bbf344b 100644 --- a/cockatrice/src/interface/widgets/visual_database_display/visual_database_display_widget.cpp +++ b/cockatrice/src/interface/widgets/visual_database_display/visual_database_display_widget.cpp @@ -100,7 +100,7 @@ VisualDatabaseDisplayWidget::VisualDatabaseDisplayWidget(QWidget *parent, } return result; }, - [this] { emit newZoneRequested(); }); + [this] { return newZoneCreator ? newZoneCreator() : QString(); }); } searchEdit->setTreeView(databaseView); @@ -209,6 +209,11 @@ void VisualDatabaseDisplayWidget::showEvent(QShowEvent *event) initializeFilters(); } +void VisualDatabaseDisplayWidget::setNewZoneCreator(const std::function &creator) +{ + newZoneCreator = creator; +} + void VisualDatabaseDisplayWidget::retranslateUi() { databaseLoadIndicator->setText(tr("Loading database ...")); diff --git a/cockatrice/src/interface/widgets/visual_database_display/visual_database_display_widget.h b/cockatrice/src/interface/widgets/visual_database_display/visual_database_display_widget.h index bd6b45fdd..d161ce362 100644 --- a/cockatrice/src/interface/widgets/visual_database_display/visual_database_display_widget.h +++ b/cockatrice/src/interface/widgets/visual_database_display/visual_database_display_widget.h @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -46,6 +47,12 @@ public: void sortCardList(const QStringList &properties, Qt::SortOrder order) const; void setDeckList(const DeckList &new_deck_list_model); + /** + * @brief Sets the callback used to create a custom zone from the add-to-zone menu. + * The callback returns the name of the created zone, or an empty string if creation was cancelled. + */ + void setNewZoneCreator(const std::function &creator); + CardDatabaseDisplayModel *getDatabaseDisplayModel() { return databaseDisplayModel; @@ -78,7 +85,6 @@ signals: void edhrecRequested(const CardInfoPtr &cardInfo, bool isCommander); void printingSelectorRequested(); void cardInfoRequested(const ExactCard &cardName); - void newZoneRequested(); protected slots: void initialize(); @@ -107,6 +113,7 @@ private: VisualDatabaseDisplayFilterToolbarWidget *filterContainer; CardDatabaseDisplayModel *databaseDisplayModel; CardDatabaseView *databaseView; + std::function newZoneCreator; QList *cards; QVBoxLayout *mainLayout; QScrollArea *scrollArea; 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 af1193f26..3c88d6030 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 @@ -115,6 +115,15 @@ public: */ QList getCustomZones(const QString &boardZoneName) const; + /** + * @brief Checks whether a zone name is taken anywhere in the deck. + * + * Covers the standard board names and any top-level or nested custom zone. + * @param zoneName The checked name. + * @return true if the name is reserved or already in use. + */ + bool hasZoneName(const QString &zoneName) const; + /** * @brief Applies a function to every card in the deck tree. This can modify the cards. * @@ -129,7 +138,6 @@ private: InnerDecklistNode *findBoardZone(const QString &boardZoneName) const; InnerDecklistNode *findOrCreateBoardZone(const QString &boardZoneName); InnerDecklistNode *findCustomZoneByName(const QString &zoneName) const; - bool hasZoneName(const QString &zoneName) const; }; #endif // COCKATRICE_DECKLIST_NODE_TREE_H diff --git a/libcockatrice_deck_list/libcockatrice/deck_list/tree/inner_deck_list_node.cpp b/libcockatrice_deck_list/libcockatrice/deck_list/tree/inner_deck_list_node.cpp index 5e7ba403b..d082b3cca 100644 --- a/libcockatrice_deck_list/libcockatrice/deck_list/tree/inner_deck_list_node.cpp +++ b/libcockatrice_deck_list/libcockatrice/deck_list/tree/inner_deck_list_node.cpp @@ -94,6 +94,9 @@ AbstractDecklistNode *InnerDecklistNode::findCardChildByNameProviderIdAndNumber( int InnerDecklistNode::height() const { + if (isEmpty()) { + return 1; + } return at(0)->height() + 1; } 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 ad278c8bf..5e9fb6b4a 100644 --- a/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model.cpp +++ b/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model.cpp @@ -378,7 +378,8 @@ bool DeckListModel::removeRows(int row, int count, const QModelIndex &parent) InnerDecklistNode *DeckListModel::createNodeIfNeeded(const QString &name, InnerDecklistNode *parent) { - auto *newNode = dynamic_cast(parent->findChild(name)); + // Group lookups must not resolve a mirrored custom zone that shares the name. + auto *newNode = DeckListModelCustomZones::findGroupChild(parent, name); if (!newNode) { beginInsertRows(nodeToIndex(parent), parent->size(), parent->size()); newNode = new InnerDecklistNode(name, parent); @@ -401,7 +402,7 @@ DecklistModelCardNode *DeckListModel::findCardNode(const QString &cardName, // nested under the board. if (auto *zoneNode = dynamic_cast(root->findChild(zoneName))) { QString groupCriteria = extractGroupCriteriaValue(info, activeGroupCriteria); - if (auto *groupNode = dynamic_cast(zoneNode->findChild(groupCriteria))) { + if (auto *groupNode = DeckListModelCustomZones::findGroupChild(zoneNode, groupCriteria)) { if (auto *card = dynamic_cast( groupNode->findCardChildByNameProviderIdAndNumber(cardName, providerId, cardNumber))) { return card; @@ -486,6 +487,26 @@ QModelIndex DeckListModel::addCard(const ExactCard &card, const QString &zoneNam // Custom zone: cards live flat inside the zone. cardParent = customZoneNode; } 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 + // card without a rebuild). Check before falling back to creating a + // top-level zone the deck does not actually have. + auto *listRoot = deckList->getTree()->getRoot(); + bool hasDeckZone = false; + for (int i = 0; i < listRoot->size(); ++i) { + if (auto *boardZone = dynamic_cast(listRoot->at(i))) { + if (boardZone->findChild(zoneName)) { + hasDeckZone = true; + break; + } + } + } + + if (hasDeckZone) { + rebuildTree(); + return addCard(card, zoneName); + } + // Unknown zone: create a top-level zone (legacy behavior). QString groupCriteria = extractGroupCriteriaValue(cardInfo, activeGroupCriteria); auto *newZone = createNodeIfNeeded(zoneName, root); diff --git a/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model_custom_zones.cpp b/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model_custom_zones.cpp index f10ee1a72..1dc745e63 100644 --- a/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model_custom_zones.cpp +++ b/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model_custom_zones.cpp @@ -14,26 +14,55 @@ bool isCustomZone(const AbstractDecklistNode *node) return dynamic_cast(node) != nullptr; } +namespace +{ + +/** + * @brief Flattens every card under @p zone into @p shadowZone, preserving order. + * + * Custom zones mirror as a single row level: cards nested in sub-zones of any + * depth are added as direct children of the mirrored zone so no card is left + * without a model row. + */ +void flattenCards(const InnerDecklistNode *zone, InnerDecklistNode *shadowZone) +{ + for (int k = 0; k < zone->size(); k++) { + if (auto *zoneCard = dynamic_cast(zone->at(k))) { + new DecklistModelCardNode(zoneCard, shadowZone); + } else if (auto *subZone = dynamic_cast(zone->at(k))) { + flattenCards(subZone, shadowZone); + } + } +} + +} // namespace + void mirrorCustomZones(const InnerDecklistNode *deckBoardZone, InnerDecklistNode *shadowBoardZone) { for (int j = 0; j < deckBoardZone->size(); j++) { - auto *customCard = dynamic_cast(deckBoardZone->at(j)); - if (customCard) { - continue; - } - auto *customZone = dynamic_cast(deckBoardZone->at(j)); if (!customZone) { continue; } auto *shadowZone = new DecklistModelSubZoneNode(customZone->getName(), shadowBoardZone); - for (int k = 0; k < customZone->size(); k++) { - if (auto *zoneCard = dynamic_cast(customZone->at(k))) { - new DecklistModelCardNode(zoneCard, shadowZone); - } + flattenCards(customZone, shadowZone); + } +} + +InnerDecklistNode *findGroupChild(InnerDecklistNode *parent, const QString &name) +{ + for (int i = 0; i < parent->size(); i++) { + AbstractDecklistNode *child = parent->at(i); + if (isCustomZone(child)) { + continue; + } + auto *group = dynamic_cast(child); + if (group && group->getName() == name) { + return group; } } + return nullptr; } DecklistModelSubZoneNode *findSubZoneByName(InnerDecklistNode *root, const QString &zoneName) diff --git a/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model_custom_zones.h b/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model_custom_zones.h index a973b127e..518a9e1d2 100644 --- a/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model_custom_zones.h +++ b/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model_custom_zones.h @@ -43,6 +43,21 @@ namespace DeckListModelCustomZones */ [[nodiscard]] bool isCustomZone(const AbstractDecklistNode *node); +/** + * @brief Finds a criteria-group child of @p parent by name, skipping custom zones. + * + * The shadow tree keeps criteria groups and mirrored custom zones as siblings + * under a board zone, and `InnerDecklistNode::findChild` matches both by name. + * Group lookups must not resolve a custom zone that happens to share the group + * name (e.g. a zone called "Creature"), so this searches only non-custom + * children. + * + * @param parent The shadow node whose children are searched. + * @param name The group name to find. + * @return The matching group node, or nullptr if none exists. + */ +[[nodiscard]] InnerDecklistNode *findGroupChild(InnerDecklistNode *parent, const QString &name); + /** * @brief Mirrors the custom zones of a deck board zone into its shadow board node. * diff --git a/tests/deck_list_model/CMakeLists.txt b/tests/deck_list_model/CMakeLists.txt index 8f2cfcc2c..e3096c559 100644 --- a/tests/deck_list_model/CMakeLists.txt +++ b/tests/deck_list_model/CMakeLists.txt @@ -1,4 +1,4 @@ -add_executable(deck_list_model_custom_zones_test deck_list_model_custom_zones_test.cpp) +add_executable(deck_list_model_custom_zones_test ${VERSION_STRING_CPP} deck_list_model_custom_zones_test.cpp) if(NOT GTEST_FOUND) add_dependencies(deck_list_model_custom_zones_test gtest) diff --git a/tests/deck_list_model/deck_list_model_custom_zones_test.cpp b/tests/deck_list_model/deck_list_model_custom_zones_test.cpp index c51c72d8f..7f4ab81e1 100644 --- a/tests/deck_list_model/deck_list_model_custom_zones_test.cpp +++ b/tests/deck_list_model/deck_list_model_custom_zones_test.cpp @@ -129,6 +129,51 @@ TEST(DeckListModelCustomZones, MirrorCustomZonesWithNoCustomZonesIsNoop) EXPECT_EQ(shadowBoard->size(), 0); } +TEST(DeckListModelCustomZones, MirrorCustomZonesFlattensNestedSubzones) +{ + // Cards deeper than one level under a custom zone still get a model row. + auto *deckBoard = new InnerDecklistNode(DECK_ZONE_MAIN); + auto *deckZone = new InnerDecklistNode("Removal", deckBoard); + auto *deckCard1 = new DecklistCardNode("Bolt", 1, deckZone); + auto *deeper = new InnerDecklistNode("Deeper", deckZone); + auto *deckCard2 = new DecklistCardNode("Swords", 1, deeper); + + InnerDecklistNode shadowRoot; + auto *shadowBoard = new InnerDecklistNode(DECK_ZONE_MAIN, &shadowRoot); + + DeckListModelCustomZones::mirrorCustomZones(deckBoard, shadowBoard); + + ASSERT_EQ(shadowBoard->size(), 1); + auto *shadowZone = dynamic_cast(shadowBoard->at(0)); + ASSERT_NE(shadowZone, nullptr); + EXPECT_EQ(shadowZone->getName(), QString("Removal")); + + // Both cards are flattened into the mirrored zone, preserving order. + ASSERT_EQ(shadowZone->size(), 2); + auto *shadowCard1 = dynamic_cast(shadowZone->at(0)); + auto *shadowCard2 = dynamic_cast(shadowZone->at(1)); + ASSERT_NE(shadowCard1, nullptr); + ASSERT_NE(shadowCard2, nullptr); + EXPECT_EQ(shadowCard1->getDataNode(), deckCard1); + EXPECT_EQ(shadowCard2->getDataNode(), deckCard2); +} + +// ===================================================================================================================== +// findGroupChild +// ===================================================================================================================== + +TEST(DeckListModelCustomZones, FindGroupChildSkipsCustomZones) +{ + InnerDecklistNode root; + auto *board = new InnerDecklistNode(DECK_ZONE_MAIN, &root); + auto *group = new InnerDecklistNode("Creature", board); + new DecklistModelSubZoneNode("Creature", board); + + EXPECT_EQ(DeckListModelCustomZones::findGroupChild(board, "Creature"), group); + EXPECT_EQ(DeckListModelCustomZones::findGroupChild(board, "Missing"), nullptr); + EXPECT_EQ(DeckListModelCustomZones::findGroupChild(&root, DECK_ZONE_MAIN), board); +} + // ===================================================================================================================== // sortWithCustomZonesLast // ===================================================================================================================== @@ -223,3 +268,9 @@ TEST(DeckListModelCustomZones, SortPlainNodeDoesNotReorderCustomZones) EXPECT_EQ(mapping[1].first, 0); EXPECT_EQ(mapping[1].second, 1); } + +int main(int argc, char **argv) +{ + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} 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 530e1c351..d49d1b443 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 @@ -1,4 +1,8 @@ #include +#include +#include +#include +#include #include #include #include @@ -25,6 +29,32 @@ int totalCustomZoneRows(const DeckListModel &model) return count; } +QModelIndex findBoardIndex(const DeckListModel &model, const QString &boardName) +{ + for (int r = 0; r < model.rowCount(QModelIndex()); ++r) { + const QModelIndex idx = model.index(r, 0, QModelIndex()); + if (idx.data(DeckRoles::IsCardRole).toBool()) { + continue; + } + const QString name = idx.sibling(idx.row(), DeckListModelColumns::CARD_NAME).data(Qt::EditRole).toString(); + if (name == boardName) { + return idx; + } + } + return {}; +} + +QModelIndex findZoneRow(const DeckListModel &model, const QModelIndex &board) +{ + for (int r = 0; r < model.rowCount(board); ++r) { + const QModelIndex child = model.index(r, 0, board); + if (child.data(DeckRoles::IsCustomZoneRole).toBool()) { + return child; + } + } + return {}; +} + } // namespace // The "Add to Zone" combobox/submenu lists getCustomZoneNames(), which reads the @@ -69,3 +99,148 @@ TEST(DeckListModelZoneIntegration, RebuildTreeMirrorsEachZoneOnce) EXPECT_EQ(model.getCustomZoneNames(DECK_ZONE_MAIN), (QStringList{"Removal", "Utility"})); EXPECT_EQ(totalCustomZoneRows(model), 2); } + +// ===================================================================================================================== +// Model behaviour: addCard routing, findCard lookup, removeRows guard, empty-zone survival. +// ===================================================================================================================== + +TEST(DeckListModelZoneIntegration, AddCardRoutesIntoMirroredCustomZone) +{ + QSharedPointer deck(new DeckList()); + DeckListModel model(nullptr, deck); + auto *tree = deck->getTree(); + + ASSERT_NE(tree->addCustomZone(DECK_ZONE_MAIN, "Removal"), nullptr); + model.rebuildTree(); + + QModelIndex added = model.addCard(ExactCard(CardInfo::newInstance("Lightning Bolt")), "Removal"); + ASSERT_TRUE(added.isValid()); + + // The card is a direct child of the mirrored custom zone, not a new top-level zone. + const QModelIndex zoneParent = added.parent(); + ASSERT_TRUE(zoneParent.isValid()); + EXPECT_TRUE(zoneParent.data(DeckRoles::IsCustomZoneRole).toBool()); + EXPECT_EQ(zoneParent.sibling(zoneParent.row(), DeckListModelColumns::CARD_NAME).data(Qt::DisplayRole).toString(), + QString("Removal")); + + // No "Removal" top-level zone appeared in the deck tree. + auto *listRoot = tree->getRoot(); + bool topLevelRemoval = false; + for (int i = 0; i < listRoot->size(); ++i) { + if (auto *zone = dynamic_cast(listRoot->at(i))) { + topLevelRemoval |= zone->getName() == "Removal"; + } + } + EXPECT_FALSE(topLevelRemoval); +} + +TEST(DeckListModelZoneIntegration, AddCardToUnmirroredCustomZoneRebuildsNotCreatesTopLevel) +{ + QSharedPointer deck(new DeckList()); + DeckListModel model(nullptr, deck); + auto *tree = deck->getTree(); + + // The zone exists on the deck tree but the shadow tree has never mirrored it. + ASSERT_NE(tree->addCustomZone(DECK_ZONE_MAIN, "Removal"), nullptr); + + QModelIndex added = model.addCard(ExactCard(CardInfo::newInstance("Lightning Bolt")), "Removal"); + ASSERT_TRUE(added.isValid()); + + const QModelIndex zoneParent = added.parent(); + ASSERT_TRUE(zoneParent.isValid()); + EXPECT_TRUE(zoneParent.data(DeckRoles::IsCustomZoneRole).toBool()); + EXPECT_EQ(zoneParent.sibling(zoneParent.row(), DeckListModelColumns::CARD_NAME).data(Qt::DisplayRole).toString(), + QString("Removal")); +} + +TEST(DeckListModelZoneIntegration, AddCardCreatesGroupSeparatelyFromSameNamedZone) +{ + QSharedPointer deck(new DeckList()); + DeckListModel model(nullptr, deck); + auto *tree = deck->getTree(); + + // A custom zone named exactly like a grouping criterion. + ASSERT_NE(tree->addCustomZone(DECK_ZONE_MAIN, "Creature"), nullptr); + model.rebuildTree(); + + CardInfoPtr bear = CardInfo::newInstance("Grizzly Bears"); + bear->setProperty(Mtg::MainCardType, "Creature"); + + QModelIndex added = model.addCard(ExactCard(bear), DECK_ZONE_MAIN); + ASSERT_TRUE(added.isValid()); + + // The card lands in a *group* node called "Creature", not swallowed by the custom zone. + const QModelIndex groupParent = added.parent(); + ASSERT_TRUE(groupParent.isValid()); + EXPECT_FALSE(groupParent.data(DeckRoles::IsCustomZoneRole).toBool()); + EXPECT_EQ(groupParent.sibling(groupParent.row(), DeckListModelColumns::CARD_NAME).data(Qt::DisplayRole).toString(), + QString("Creature")); + + // The board keeps both rows: the "Creature" group and the "Creature" custom zone. + const QModelIndex boardIndex = groupParent.parent(); + ASSERT_TRUE(boardIndex.isValid()); + EXPECT_EQ(model.rowCount(boardIndex), 2); +} + +TEST(DeckListModelZoneIntegration, FindCardResolvesCardInsideCustomZone) +{ + QSharedPointer deck(new DeckList()); + DeckListModel model(nullptr, deck); + auto *tree = deck->getTree(); + + ASSERT_NE(tree->addCustomZone(DECK_ZONE_MAIN, "Removal"), nullptr); + model.rebuildTree(); + + // findCard resolves through the card database; register the card we add. + const QString cardName = "Swords to Plowshares"; + CardInfoPtr info = CardInfo::newInstance(cardName); + CardDatabaseManager::getInstance()->addCard(info); + + QModelIndex added = model.addCard(ExactCard(info), "Removal"); + ASSERT_TRUE(added.isValid()); + + QModelIndex found = model.findCard(cardName, "Removal"); + EXPECT_TRUE(found.isValid()); + EXPECT_EQ(found, added); +} + +TEST(DeckListModelZoneIntegration, RemoveRowsRefusesCustomZoneRow) +{ + QSharedPointer deck(new DeckList()); + DeckListModel model(nullptr, deck); + auto *tree = deck->getTree(); + + ASSERT_NE(tree->addCustomZone(DECK_ZONE_MAIN, "Removal"), nullptr); + tree->addCard("Lightning Bolt", 2, DECK_ZONE_MAIN, -1); + model.rebuildTree(); + + const QModelIndex mainIndex = findBoardIndex(model, DECK_ZONE_MAIN); + ASSERT_TRUE(mainIndex.isValid()); + const QModelIndex zoneRow = findZoneRow(model, mainIndex); + ASSERT_TRUE(zoneRow.isValid()); + + EXPECT_FALSE(model.removeRow(zoneRow.row(), zoneRow.parent())); + EXPECT_EQ(model.rowCount(mainIndex), 2); // the zone survives, alongside the card group +} + +TEST(DeckListModelZoneIntegration, EmptyCustomZoneSurvivesMirrorAndPruning) +{ + QSharedPointer deck(new DeckList()); + DeckListModel model(nullptr, deck); + auto *tree = deck->getTree(); + + // An empty custom zone must be mirrored (the stack deliberately keeps it alive). + ASSERT_NE(tree->addCustomZone(DECK_ZONE_MAIN, "Removal"), nullptr); + model.rebuildTree(); + + const QModelIndex mainIndex = findBoardIndex(model, DECK_ZONE_MAIN); + ASSERT_TRUE(mainIndex.isValid()); + EXPECT_EQ(model.rowCount(mainIndex), 1); + EXPECT_TRUE(findZoneRow(model, mainIndex).isValid()); +} + +int main(int argc, char **argv) +{ + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +}