diff --git a/cockatrice/src/interface/deck_loader/deck_loader.cpp b/cockatrice/src/interface/deck_loader/deck_loader.cpp index a406b6976..39a0c1071 100644 --- a/cockatrice/src/interface/deck_loader/deck_loader.cpp +++ b/cockatrice/src/interface/deck_loader/deck_loader.cpp @@ -384,14 +384,6 @@ 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"; @@ -518,26 +510,9 @@ bool DeckLoader::convertToCockatriceFormat(LoadedDeck &deck) void DeckLoader::printDeckListNode(QTextCursor *cursor, const InnerDecklistNode *node) { - if (!node || node->isEmpty()) { - return; - } - const int totalColumns = 2; - // 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()) { + if (node->height() == 1) { QTextBlockFormat blockFormat; QTextCharFormat charFormat; charFormat.setFontPointSize(11); @@ -548,9 +523,9 @@ void DeckLoader::printDeckListNode(QTextCursor *cursor, const InnerDecklistNode tableFormat.setCellPadding(0); tableFormat.setCellSpacing(0); tableFormat.setBorder(0); - QTextTable *table = cursor->insertTable(cards.size() + 1, totalColumns, tableFormat); - for (int i = 0; i < cards.size(); i++) { - const AbstractDecklistCardNode *card = cards[i]; + QTextTable *table = cursor->insertTable(node->size() + 1, totalColumns, tableFormat); + for (int i = 0; i < node->size(); i++) { + auto *card = dynamic_cast(node->at(i)); QTextCharFormat cellCharFormat; cellCharFormat.setFontPointSize(9); @@ -565,13 +540,7 @@ void DeckLoader::printDeckListNode(QTextCursor *cursor, const InnerDecklistNode cellCursor = cell.firstCursorPosition(); cellCursor.insertText(card->getName()); } - } - - for (const InnerDecklistNode *subZone : subZones) { - if (subZone->isEmpty()) { - continue; - } - + } else if (node->height() == 2) { QTextBlockFormat blockFormat; QTextCharFormat charFormat; charFormat.setFontPointSize(14); @@ -590,8 +559,10 @@ void DeckLoader::printDeckListNode(QTextCursor *cursor, const InnerDecklistNode tableFormat.setColumnWidthConstraints(constraints); QTextTable *table = cursor->insertTable(1, totalColumns, tableFormat); - QTextCursor cellCursor = table->cellAt(0, 0).firstCursorPosition(); - printDeckListNode(&cellCursor, subZone); + 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))); + } } 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 00388a3cd..c51830cf0 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,46 +151,35 @@ void CardDatabaseView::openCustomMenu(QPoint point) if (zoneMenuProvider) { QMenu *addToZoneMenu = menu.addMenu(tr("Add to Zone")); - const auto zoneBoards = zoneMenuProvider(); 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(); - }(); + 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; if (customZones.isEmpty()) { - QAction *action = addToZoneMenu->addAction(InnerDecklistNode::visibleNameFromName(boardName)); + continue; + } + anyCustomZone = true; + QMenu *boardSubmenu = addToZoneMenu->addMenu(InnerDecklistNode::visibleNameFromName(boardName)); + for (const QString &zoneName : customZones) { + QAction *action = boardSubmenu->addAction(zoneName); connect(action, &QAction::triggered, this, - [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); }); - } + [this, card, zoneName] { emit cardAdded(card->getName(), zoneName); }); } } - if (newZoneHandler) { + if (anyCustomZone) { addToZoneMenu->addSeparator(); - - 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); - } - }); } + + QAction *newZoneAction = addToZoneMenu->addAction(tr("Create &new zone...")); + connect(newZoneAction, &QAction::triggered, this, [this] { newZoneHandler(); }); } 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 668444199..72040b97e 100644 --- a/cockatrice/src/interface/widgets/deck_editor/card_database_view.h +++ b/cockatrice/src/interface/widgets/deck_editor/card_database_view.h @@ -24,8 +24,7 @@ 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. - /// Returns the name of the created zone, or an empty string if creation was cancelled. - std::function newZoneHandler; + std::function newZoneHandler; public: explicit CardDatabaseView(QWidget *parent, CardDatabaseDisplayModel *model); @@ -46,11 +45,10 @@ 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 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. + * @param newZoneHandler Invoked when the user chooses "New zone..." in the submenu */ 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 6269f0323..d78758375 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]() -> QString { + [this, deckEditor] { QString boardName; const QString zoneName = DeckZoneDialog::promptForNewZone(this, {}, &boardName, [deckEditor](const QString &candidate) { @@ -40,7 +40,6 @@ 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 6f3fff5ae..9296f8697 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,34 +784,18 @@ 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) { - if (currentBoardName != DECK_ZONE_TOKENS) { - addMoveToZoneMenu(&menu, sourceIndex, currentBoardName); - menu.addSeparator(); - } + addMoveToZoneMenu(&menu, sourceIndex); + 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] { - // 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); - }); + const QString newName = DeckZoneDialog::promptForRename(this, zoneName, [this](const QString &candidate) { + return deckStateManager->validateNewZoneName(candidate); + }); if (!newName.isEmpty() && newName != zoneName) { deckStateManager->renameCustomZone(zoneName, newName); } @@ -821,12 +805,8 @@ void DeckEditorDeckDockWidget::decklistCustomMenu(QPoint point) addChangeBoardMenu(boardMenu, zoneName); QAction *deleteAction = menu.addAction(tr("&Delete zone")); - const bool zoneHasCards = getModel()->hasChildren(sourceIndex); - deleteAction->setEnabled(!zoneHasCards); - if (zoneHasCards) { - deleteAction->setToolTip(tr("Move or remove all cards first.")); - menu.setToolTipsVisible(true); - } + deleteAction->setEnabled(!getModel()->hasChildren(sourceIndex)); + deleteAction->setStatusTip(tr("Move or remove all cards first.")); connect(deleteAction, &QAction::triggered, this, [this, zoneName] { const auto result = QMessageBox::warning(this, tr("Delete zone"), tr("Delete the zone \"%1\"?").arg(zoneName), @@ -857,52 +837,37 @@ void DeckEditorDeckDockWidget::decklistCustomMenu(QPoint point) menu.exec(deckView->mapToGlobal(point)); } -void DeckEditorDeckDockWidget::addMoveToZoneMenu(QMenu *menu, - const QModelIndex &sourceCardIndex, - const QString ¤tBoardName) +void DeckEditorDeckDockWidget::addMoveToZoneMenu(QMenu *menu, const QModelIndex &sourceCardIndex) { - 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); - }); - } + const auto moveToZone = [this, sourceCardIndex](const QString &targetZoneName) { + deckStateManager->moveCardToZone(sourceCardIndex, targetZoneName); }; - const auto tree = deckStateManager->getDeckListShared()->getTree(); - - QMenu *moveMenu = menu->addMenu(tr("Move to &zone")); - for (const QString &boardName : InnerDecklistNode::boardZoneNames()) { - const QString boardLabel = InnerDecklistNode::visibleNameFromName(boardName); - const auto customZones = tree->getCustomZones(boardName); + QAction *action = menu->addAction(InnerDecklistNode::visibleNameFromName(boardName)); + connect(action, &QAction::triggered, this, [moveToZone, boardName] { moveToZone(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); + const auto tree = deckStateManager->getDeckListShared()->getTree(); + bool anyCustomZone = false; + 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()); }); } } - moveMenu->addSeparator(); + if (anyCustomZone) { + menu->addSeparator(); + } - 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); - } - }); + addNewZoneAction(menu); } void DeckEditorDeckDockWidget::addChangeBoardMenu(QMenu *menu, const QString &zoneName) @@ -935,21 +900,16 @@ 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] { 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; + 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); + } + }); } 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 1e5f4e677..fc3b01dc7 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,9 +103,8 @@ private: [[nodiscard]] QModelIndexList getSelectedCardNodeSourceIndices() const; void offsetCountAtIndex(const QModelIndex &idx, bool isIncrement); - void addMoveToZoneMenu(QMenu *menu, const QModelIndex &sourceCardIndex, const QString ¤tBoardName); + void addMoveToZoneMenu(QMenu *menu, const QModelIndex &sourceCardIndex); 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 e007e6dd0..b60685d20 100644 --- a/cockatrice/src/interface/widgets/deck_editor/deck_state_manager.cpp +++ b/cockatrice/src/interface/widgets/deck_editor/deck_state_manager.cpp @@ -470,13 +470,20 @@ QString DeckStateManager::validateNewZoneName(const QString &zoneName) const const auto *tree = deckList->getTree(); - // 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."); + // 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."); + } + } } return {}; @@ -551,7 +558,6 @@ 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 9a0be2570..a14beec0e 100644 --- a/cockatrice/src/interface/widgets/deck_editor/deck_zone_dialog.cpp +++ b/cockatrice/src/interface/widgets/deck_editor/deck_zone_dialog.cpp @@ -50,9 +50,6 @@ 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 0f43893d3..46d1b08b6 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); - tabContainer->visualDatabaseDisplay->setNewZoneCreator([this] { return createNewZone(); }); + connect(tabContainer, &TabDeckEditorVisualTabWidget::newZoneRequested, this, &TabDeckEditorVisual::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. Returns the name of the created zone. */ -QString TabDeckEditorVisual::createNewZone() +/** @brief Prompts for and creates a new custom deck zone. */ +void TabDeckEditorVisual::createNewZone() { QString boardName; const QString zoneName = DeckZoneDialog::promptForNewZone(this, {}, &boardName, [this](const QString &candidate) { @@ -281,7 +281,6 @@ QString 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 fb09578c4..e0ad6c914 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,9 +167,8 @@ 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. */ - QString createNewZone(); + void 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 5ccfcc28f..843ddf493 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,6 +51,8 @@ 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 4f04b51f6..a625aaad7 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,6 +133,7 @@ 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 76bbf344b..2b58142b7 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] { return newZoneCreator ? newZoneCreator() : QString(); }); + [this] { emit newZoneRequested(); }); } searchEdit->setTreeView(databaseView); @@ -209,11 +209,6 @@ 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 d161ce362..bd6b45fdd 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,7 +22,6 @@ #include #include #include -#include #include #include #include @@ -47,12 +46,6 @@ 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; @@ -85,6 +78,7 @@ signals: void edhrecRequested(const CardInfoPtr &cardInfo, bool isCommander); void printingSelectorRequested(); void cardInfoRequested(const ExactCard &cardName); + void newZoneRequested(); protected slots: void initialize(); @@ -113,7 +107,6 @@ 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 3c88d6030..af1193f26 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,15 +115,6 @@ 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. * @@ -138,6 +129,7 @@ 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 d082b3cca..5e7ba403b 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,9 +94,6 @@ 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 5e9fb6b4a..ad278c8bf 100644 --- a/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model.cpp +++ b/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model.cpp @@ -378,8 +378,7 @@ bool DeckListModel::removeRows(int row, int count, const QModelIndex &parent) InnerDecklistNode *DeckListModel::createNodeIfNeeded(const QString &name, InnerDecklistNode *parent) { - // Group lookups must not resolve a mirrored custom zone that shares the name. - auto *newNode = DeckListModelCustomZones::findGroupChild(parent, name); + auto *newNode = dynamic_cast(parent->findChild(name)); if (!newNode) { beginInsertRows(nodeToIndex(parent), parent->size(), parent->size()); newNode = new InnerDecklistNode(name, parent); @@ -402,7 +401,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 = DeckListModelCustomZones::findGroupChild(zoneNode, groupCriteria)) { + if (auto *groupNode = dynamic_cast(zoneNode->findChild(groupCriteria))) { if (auto *card = dynamic_cast( groupNode->findCardChildByNameProviderIdAndNumber(cardName, providerId, cardNumber))) { return card; @@ -487,26 +486,6 @@ 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 1dc745e63..f10ee1a72 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,55 +14,26 @@ 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); - 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; + for (int k = 0; k < customZone->size(); k++) { + if (auto *zoneCard = dynamic_cast(customZone->at(k))) { + new DecklistModelCardNode(zoneCard, shadowZone); + } } } - 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 518a9e1d2..a973b127e 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,21 +43,6 @@ 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 e3096c559..8f2cfcc2c 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 ${VERSION_STRING_CPP} deck_list_model_custom_zones_test.cpp) +add_executable(deck_list_model_custom_zones_test 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 7f4ab81e1..c51c72d8f 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,51 +129,6 @@ 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 // ===================================================================================================================== @@ -268,9 +223,3 @@ 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 d49d1b443..530e1c351 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,8 +1,4 @@ #include -#include -#include -#include -#include #include #include #include @@ -29,32 +25,6 @@ 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 @@ -99,148 +69,3 @@ 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(); -}