[DeckEditor] Address custom-zone menu and export review feedback

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<DecklistCardNode *>(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<const InnerDecklistNode *>(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<const AbstractDecklistCardNode *> cards;
+    QVector<const InnerDecklistNode *> subZones;
+    for (int i = 0; i < node->size(); i++) {
+        if (auto *card = dynamic_cast<const AbstractDecklistCardNode *>(node->at(i))) {
+            cards.append(card);
+        } else if (auto *zone = dynamic_cast<const InnerDecklistNode *>(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);
+        QTextTable *table = cursor->insertTable(cards.size() + 1, totalColumns, tableFormat);
-        for (int i = 0; i < node->size(); i++) {
+        for (int i = 0; i < cards.size(); i++) {
-            auto *card = dynamic_cast<AbstractDecklistCardNode *>(node->at(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, 0).firstCursorPosition();
-            QTextCursor cellCursor = table->cellAt(0, (i * totalColumns) / node->size()).lastCursorPosition();
+        printDeckListNode(&cellCursor, subZone);
-            printDeckListNode(&cellCursor, dynamic_cast<InnerDecklistNode *>(node->at(i)));
-        }
     }
 
     cursor->movePosition(QTextCursor::End);

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<QList<QPair<QString, QStringList>>()> &provider,
-                                           const std::function<void()> &newZoneHandler)
+                                           const std::function<QString()> &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"));
+        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();
+            }();
+            if (customZones.isEmpty()) {
                 QAction *action = addToZoneMenu->addAction(InnerDecklistNode::visibleNameFromName(boardName));
                 connect(action, &QAction::triggered, this,
                         [this, card, boardName] { emit cardAdded(card->getName(), boardName); });
-        }
+            } else {
-
-        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()) {
-                continue;
-            }
-            anyCustomZone = true;
                 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) {
-            addToZoneMenu->addSeparator();
         }
 
+        if (newZoneHandler) {
+            addToZoneMenu->addSeparator();
+
             QAction *newZoneAction = addToZoneMenu->addAction(tr("Create &new zone..."));
-        connect(newZoneAction, &QAction::triggered, this, [this] { newZoneHandler(); });
+            connect(newZoneAction, &QAction::triggered, this, [this, card] {
+                const QString zoneName = newZoneHandler();
+                if (!zoneName.isEmpty()) {
+                    emit cardAdded(card->getName(), zoneName);
+                }
+            });
+        }
     }
 
     if (canBeCommander(*card)) {

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<QList<QPair<QString, QStringList>>()> zoneMenuProvider;
     /// Handler invoked when the user picks "New zone..." from the add-to-zone menu.
-    std::function<void()> newZoneHandler;
+    /// Returns the name of the created zone, or an empty string if creation was cancelled.
+    std::function<QString()> 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<QList<QPair<QString, QStringList>>()> &provider,
-                             const std::function<void()> &newZoneHandler);
+                             const std::function<QString()> &newZoneHandler);
 
 signals:
     void cardChanged(const QString &cardName);

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;

cockatrice/src/interface/widgets/deck_editor/deck_editor_deck_dock_widget.cpp

@@ -784,17 +784,33 @@ 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);
+        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) {
+            // The unchanged name must not validate as a duplicate.
-                return deckStateManager->validateNewZoneName(candidate);
+            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));
+        const bool zoneHasCards = getModel()->hasChildren(sourceIndex);
-        deleteAction->setStatusTip(tr("Move or remove all cards first."));
+        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 &currentBoardName)
 {
-    const auto moveToZone = [this, sourceCardIndex](const QString &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<const InnerDecklistNode *> customZones = tree->getCustomZones(boardName);
+        const QString boardLabel = InnerDecklistNode::visibleNameFromName(boardName);
-        if (customZones.isEmpty()) {
+        const auto customZones = tree->getCustomZones(boardName);
-            continue;
+
-        }
+        // Boards with zones nest their children so no two menu entries share a
-        anyCustomZone = true;
+        // visible name: "Maindeck ▸ { Maindeck (whole board), Removal, … }".
-        QMenu *boardSubmenu = menu->addMenu(InnerDecklistNode::visibleNameFromName(boardName));
+        // 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) {
-            QAction *action = boardSubmenu->addAction(customZone->getName());
+                addMoveAction(boardSubmenu, customZone->getName(), customZone->getName(), true);
-            connect(action, &QAction::triggered, this, [moveToZone, customZone] { moveToZone(customZone->getName()); });
+            }
+        } else {
+            addMoveAction(moveMenu, boardName, boardLabel, boardName != currentBoardName);
         }
     }
 
-    if (anyCustomZone) {
+    moveMenu->addSeparator();
-        menu->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,7 +935,12 @@ 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] {
+    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) {
@@ -909,7 +949,7 @@ void DeckEditorDeckDockWidget::addNewZoneAction(QMenu *menu, const QString &init
     if (!zoneName.isEmpty()) {
         deckStateManager->createCustomZone(boardName, zoneName);
     }
-    });
+    return zoneName;
 }
 
 void DeckEditorDeckDockWidget::refreshShortcuts()

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 &currentBoardName);
     void addChangeBoardMenu(QMenu *menu, const QString &zoneName);
+    QString createNewCustomZone(const QString &initialBoardName = {});
     void addNewZoneAction(QMenu *menu, const QString &initialBoardName = {});
 
 private slots:

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. */
+/** @brief Prompts for and creates a new custom deck zone. Returns the name of the created zone. */
-void TabDeckEditorVisual::createNewZone()
+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. */

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:
     /**

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();

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.

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<QString()> &creator)
+{
+    newZoneCreator = creator;
+}
+
 void VisualDatabaseDisplayWidget::retranslateUi()
 {
     databaseLoadIndicator->setText(tr("Loading database ..."));

cockatrice/src/interface/widgets/visual_database_display/visual_database_display_widget.h

@@ -22,6 +22,7 @@
 #include <QVBoxLayout>
 #include <QWheelEvent>
 #include <QWidget>
+#include <functional>
 #include <libcockatrice/models/database/card_database_model.h>
 #include <libcockatrice/models/deck_list/deck_list_model.h>
 #include <qscrollarea.h>
@@ -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<QString()> &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<QString()> newZoneCreator;
     QList<ExactCard> *cards;
     QVBoxLayout *mainLayout;
     QScrollArea *scrollArea;

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;
 }