[DeckDockWidget] Correctly handle auto-expanding tree (#6446)

* move method

* remove expandAll calls

* update recursiveExpand

* Refactor DeckModel access

* [DeckDockWidget] Correctly handle auto-expand

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

@@ -156,6 +156,9 @@ void DeckEditorDeckDockWidget::createDeckDock()
         // Delay the update to avoid race conditions
         QTimer::singleShot(100, this, &DeckEditorDeckDockWidget::updateBannerCardComboBox);
     });
+    connect(deckModel, &DeckListModel::cardAddedAt, this, &DeckEditorDeckDockWidget::recursiveExpand);
+    connect(deckModel, &DeckListModel::deckReplaced, this, &DeckEditorDeckDockWidget::expandAll);
+
     connect(bannerCardComboBox, QOverload<int>::of(&QComboBox::currentIndexChanged), this,
             &DeckEditorDeckDockWidget::setBannerCard);
     bannerCardComboBox->setHidden(!SettingsCache::instance().getDeckEditorBannerCardComboBoxVisible());
@@ -175,8 +178,6 @@ void DeckEditorDeckDockWidget::createDeckDock()
         deckModel->setActiveGroupCriteria(static_cast<DeckListModelGroupCriteria::Type>(
             activeGroupCriteriaComboBox->currentData(Qt::UserRole).toInt()));
         deckModel->sort(deckView->header()->sortIndicatorSection(), deckView->header()->sortIndicatorOrder());
-        deckView->expandAll();
-        deckView->expandAll();
     });
 
     aIncrement = new QAction(QString(), this);
@@ -506,7 +507,6 @@ void DeckEditorDeckDockWidget::syncDisplayWidgetsToModel()
     bannerCardComboBox->blockSignals(false);
     updateHash();
     sortDeckModelToDeckView();
-    expandAll();
 
     deckTagsDisplayWidget->setTags(deckModel->getDeckList()->getTags());
 }
@@ -516,8 +516,6 @@ void DeckEditorDeckDockWidget::sortDeckModelToDeckView()
     deckModel->sort(deckView->header()->sortIndicatorSection(), deckView->header()->sortIndicatorOrder());
     deckModel->setActiveFormat(deckModel->getDeckList()->getGameFormat());
     formatComboBox->setCurrentIndex(formatComboBox->findData(deckModel->getDeckList()->getGameFormat()));
-    deckView->expandAll();
-    deckView->expandAll();
 
     emit deckChanged();
 }
@@ -550,17 +548,26 @@ void DeckEditorDeckDockWidget::cleanDeck()
     deckTagsDisplayWidget->setTags(deckModel->getDeckList()->getTags());
 }
 
-void DeckEditorDeckDockWidget::recursiveExpand(const QModelIndex &index)
+/**
+ * @brief Expands all parents of the given index.
+ * @param sourceIndex The index to expand (model source index)
+ */
+void DeckEditorDeckDockWidget::recursiveExpand(const QModelIndex &sourceIndex)
 {
-    if (index.parent().isValid())
-        recursiveExpand(index.parent());
-    deckView->expand(index);
+    auto index = proxy->mapFromSource(sourceIndex);
+
+    while (index.parent().isValid()) {
+        index = index.parent();
+        deckView->expand(index);
+    }
 }
 
+/**
+ * @brief Fully expands all levels of the deck view
+ */
 void DeckEditorDeckDockWidget::expandAll()
 {
-    deckView->expandAll();
-    deckView->expandAll();
+    deckView->expandRecursively(deckView->rootIndex());
 }
 
 /**
@@ -600,7 +607,6 @@ void DeckEditorDeckDockWidget::actAddCard(const ExactCard &card, const QString &
         return;
     }
 
-    expandAll();
     deckView->clearSelection();
     deckView->setCurrentIndex(newCardIndex);
 
@@ -612,7 +618,7 @@ void DeckEditorDeckDockWidget::actIncrementSelection()
     auto selectedRows = getSelectedCardNodes();
 
     for (const auto &index : selectedRows) {
-        offsetCountAtIndex(index, 1);
+        offsetCountAtIndex(index, true);
     }
 }
 
@@ -674,14 +680,15 @@ bool DeckEditorDeckDockWidget::swapCard(const QModelIndex &currentIndex)
         return false;
 
     const QString zoneName = gparent.siblingAtColumn(DeckListModelColumns::CARD_NAME).data(Qt::EditRole).toString();
-    offsetCountAtIndex(currentIndex, -1);
+    offsetCountAtIndex(currentIndex, false);
     const QString otherZoneName = zoneName == DECK_ZONE_MAIN ? DECK_ZONE_SIDE : DECK_ZONE_MAIN;
 
-    ExactCard card = CardDatabaseManager::query()->getCard({cardName, cardProviderID});
-    QModelIndex newCardIndex = card ? deckModel->addCard(card, otherZoneName)
-                                    // Third argument (true) says create the card no matter what, even if not in DB
-                                    : deckModel->addPreferredPrintingCard(cardName, otherZoneName, true);
-    recursiveExpand(proxy->mapFromSource(newCardIndex));
+    if (ExactCard card = CardDatabaseManager::query()->getCard({cardName, cardProviderID})) {
+        deckModel->addCard(card, otherZoneName);
+    } else {
+        // Third argument (true) says create the card no matter what, even if not in DB
+        deckModel->addPreferredPrintingCard(cardName, otherZoneName, true);
+    }
 
     return true;
 }
@@ -703,7 +710,7 @@ void DeckEditorDeckDockWidget::actDecrementCard(const ExactCard &card, QString z
 
     deckView->clearSelection();
     deckView->setCurrentIndex(proxy->mapToSource(idx));
-    offsetCountAtIndex(idx, -1);
+    offsetCountAtIndex(idx, false);
 }
 
 void DeckEditorDeckDockWidget::actDecrementSelection()
@@ -717,7 +724,7 @@ void DeckEditorDeckDockWidget::actDecrementSelection()
     }
 
     for (const auto &index : selectedRows) {
-        offsetCountAtIndex(index, -1);
+        offsetCountAtIndex(index, false);
     }
 
     deckView->setSelectionMode(QAbstractItemView::ExtendedSelection);
@@ -754,7 +761,12 @@ void DeckEditorDeckDockWidget::actRemoveCard()
     }
 }
 
-void DeckEditorDeckDockWidget::offsetCountAtIndex(const QModelIndex &idx, int offset)
+/**
+ * @brief Increments or decrements the amount of the card node at the index by 1.
+ * @param idx The proxy index
+ * @param isIncrement If true, increments the count. If false, decrements the count
+ */
+void DeckEditorDeckDockWidget::offsetCountAtIndex(const QModelIndex &idx, bool isIncrement)
 {
     if (!idx.isValid() || deckModel->hasChildren(idx)) {
         return;
@@ -762,26 +774,22 @@ void DeckEditorDeckDockWidget::offsetCountAtIndex(const QModelIndex &idx, int of
 
     QModelIndex sourceIndex = proxy->mapToSource(idx);
 
-    const QModelIndex numberIndex = sourceIndex.siblingAtColumn(DeckListModelColumns::CARD_AMOUNT);
-    const QModelIndex nameIndex = sourceIndex.siblingAtColumn(DeckListModelColumns::CARD_NAME);
+    QString cardName = sourceIndex.siblingAtColumn(DeckListModelColumns::CARD_NAME).data(Qt::EditRole).toString();
+    QString providerId =
+        sourceIndex.siblingAtColumn(DeckListModelColumns::CARD_PROVIDER_ID).data(Qt::DisplayRole).toString();
 
-    const QString cardName = nameIndex.data(Qt::EditRole).toString();
-    const int count = numberIndex.data(Qt::EditRole).toInt();
-    const int new_count = count + offset;
-
-    const auto reason =
-        QString(tr("%1 %2 × \"%3\" (%4)"))
-            .arg(offset > 0 ? tr("Added") : tr("Removed"))
-            .arg(qAbs(offset))
-            .arg(cardName)
-            .arg(sourceIndex.siblingAtColumn(DeckListModelColumns::CARD_PROVIDER_ID).data(Qt::DisplayRole).toString());
+    const auto reason = QString(tr("%1 %2 × \"%3\" (%4)"))
+                            .arg(isIncrement ? tr("Added") : tr("Removed"))
+                            .arg(1)
+                            .arg(cardName)
+                            .arg(providerId);
 
     emit requestDeckHistorySave(reason);
 
-    if (new_count <= 0) {
-        deckModel->removeRow(sourceIndex.row(), sourceIndex.parent());
+    if (isIncrement) {
+        deckModel->incrementAmountAtIndex(sourceIndex);
     } else {
-        deckModel->setData(numberIndex, new_count, Qt::EditRole);
+        deckModel->decrementAmountAtIndex(sourceIndex);
     }
 
     emit deckModified();

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

@@ -70,7 +70,6 @@ public slots:
     void actSwapSelection();
     void actRemoveCard();
     void initializeFormats();
-    void expandAll();
 
 signals:
     void nameChanged();
@@ -106,9 +105,8 @@ private:
 
     QAction *aRemoveCard, *aIncrement, *aDecrement, *aSwapCard;
 
-    void recursiveExpand(const QModelIndex &index);
     [[nodiscard]] QModelIndexList getSelectedCardNodes() const;
-    void offsetCountAtIndex(const QModelIndex &idx, int offset);
+    void offsetCountAtIndex(const QModelIndex &idx, bool isIncrement);
 
 private slots:
     void decklistCustomMenu(QPoint point);
@@ -124,6 +122,8 @@ private slots:
     void updateShowBannerCardComboBox(bool visible);
     void updateShowTagsWidget(bool visible);
     void syncBannerCardComboBoxSelectionWithDeck();
+    void recursiveExpand(const QModelIndex &parent);
+    void expandAll();
 };
 
 #endif // DECK_EDITOR_DECK_DOCK_WIDGET_H

cockatrice/src/interface/widgets/printing_selector/card_amount_widget.cpp

@@ -175,7 +175,6 @@ void CardAmountWidget::addPrinting(const QString &zone)
 
     // Add the card and expand the list UI
     auto newCardIndex = deckModel->addCard(rootCard, zone);
-    recursiveExpand(newCardIndex);
 
     // Check if a card without a providerId already exists in the deckModel and replace it, if so.
     QString foundProviderId =
@@ -229,46 +228,6 @@ void CardAmountWidget::removePrintingSideboard()
     decrementCardHelper(DECK_ZONE_SIDE);
 }
 
-/**
- * @brief Recursively expands the card in the deck view starting from the given index.
- *
- * @param index The model index of the card to expand.
- */
-void CardAmountWidget::recursiveExpand(const QModelIndex &index)
-{
-    if (index.parent().isValid()) {
-        recursiveExpand(index.parent());
-    }
-    deckView->expand(index);
-}
-
-/**
- * @brief Offsets the card count at the specified index by the given amount.
- *
- * @param idx The model index of the card.
- * @param offset The amount to add or subtract from the card count.
- */
-void CardAmountWidget::offsetCountAtIndex(const QModelIndex &idx, int offset)
-{
-    if (!idx.isValid() || offset == 0) {
-        return;
-    }
-
-    const QModelIndex numberIndex = idx.siblingAtColumn(DeckListModelColumns::CARD_AMOUNT);
-    const int count = numberIndex.data(Qt::EditRole).toInt();
-    const int new_count = count + offset;
-
-    deckView->setCurrentIndex(numberIndex);
-
-    if (new_count <= 0) {
-        deckModel->removeRow(idx.row(), idx.parent());
-    } else {
-        deckModel->setData(numberIndex, new_count, Qt::EditRole);
-    }
-
-    deckEditor->setModified(true);
-}
-
 /**
  * @brief Helper function to decrement the card count for a given zone.
  *
@@ -288,7 +247,7 @@ void CardAmountWidget::decrementCardHelper(const QString &zone)
     QModelIndex idx = deckModel->findCard(rootCard.getName(), zone, rootCard.getPrinting().getUuid(),
                                           rootCard.getPrinting().getProperty("num"));
 
-    offsetCountAtIndex(idx, -1);
+    deckModel->decrementAmountAtIndex(idx);
     deckEditor->setModified(true);
 }
 

cockatrice/src/interface/widgets/printing_selector/card_amount_widget.h

@@ -57,9 +57,7 @@ private:
 
     bool hovered;
 
-    void offsetCountAtIndex(const QModelIndex &idx, int offset);
     void decrementCardHelper(const QString &zoneName);
-    void recursiveExpand(const QModelIndex &index);
 
 private slots:
     void addPrintingMainboard();

libcockatrice_models/libcockatrice/models/deck_list/deck_list_model.cpp

@@ -436,7 +436,51 @@ QModelIndex DeckListModel::addCard(const ExactCard &card, const QString &zoneNam
     }
     sort(lastKnownColumn, lastKnownOrder);
     emitRecursiveUpdates(parentIndex);
-    return nodeToIndex(cardNode);
+    auto index = nodeToIndex(cardNode);
+
+    emit cardAddedAt(index);
+
+    return index;
+}
+
+bool DeckListModel::incrementAmountAtIndex(const QModelIndex &idx)
+{
+    return offsetAmountAtIndex(idx, 1);
+}
+
+bool DeckListModel::decrementAmountAtIndex(const QModelIndex &idx)
+{
+    return offsetAmountAtIndex(idx, -1);
+}
+
+bool DeckListModel::offsetAmountAtIndex(const QModelIndex &idx, int offset)
+{
+    if (!idx.isValid()) {
+        return false;
+    }
+
+    auto *node = static_cast<AbstractDecklistNode *>(idx.internalPointer());
+    auto *card = dynamic_cast<DecklistModelCardNode *>(node);
+
+    if (!card) {
+        return false;
+    }
+
+    const QModelIndex numberIndex = idx.siblingAtColumn(DeckListModelColumns::CARD_AMOUNT);
+    const int count = numberIndex.data(Qt::EditRole).toInt();
+    const int newCount = count + offset;
+
+    if (newCount <= 0) {
+        removeRow(idx.row(), idx.parent());
+    } else {
+        setData(numberIndex, newCount, Qt::EditRole);
+    }
+
+    if (offset > 0) {
+        emit cardAddedAt(idx);
+    }
+
+    return true;
 }
 
 int DeckListModel::findSortedInsertRow(InnerDecklistNode *parent, CardInfoPtr cardInfo) const
@@ -559,6 +603,7 @@ void DeckListModel::setDeckList(DeckList *_deck)
         deckList = _deck;
     }
     rebuildTree();
+    emit deckReplaced();
 }
 
 void DeckListModel::forEachCard(const std::function<void(InnerDecklistNode *, DecklistCardNode *)> &func)

libcockatrice_models/libcockatrice/models/deck_list/deck_list_model.h

@@ -226,6 +226,18 @@ signals:
      */
     void deckHashChanged();
 
+    /**
+     * @brief Emitted whenever a card is added to the deck, regardless of whether it's an entirely new card or an
+     * existing card that got incremented.
+     * @param index The index of the card that got added.
+     */
+    void cardAddedAt(const QModelIndex &index);
+
+    /**
+     * @brief Emitted whenever the deck in the model has been replaced with a new one
+     */
+    void deckReplaced();
+
 public:
     explicit DeckListModel(QObject *parent = nullptr);
     ~DeckListModel() override;
@@ -250,7 +262,6 @@ public:
     [[nodiscard]] int rowCount(const QModelIndex &parent) const override;
     [[nodiscard]] int columnCount(const QModelIndex & /*parent*/ = QModelIndex()) const override;
     [[nodiscard]] QVariant data(const QModelIndex &index, int role) const override;
-    void emitBackgroundUpdates(const QModelIndex &parent);
     [[nodiscard]] QVariant headerData(int section, Qt::Orientation orientation, int role) const override;
     [[nodiscard]] QModelIndex index(int row, int column, const QModelIndex &parent) const override;
     [[nodiscard]] QModelIndex parent(const QModelIndex &index) const override;
@@ -258,6 +269,12 @@ public:
     bool setData(const QModelIndex &index, const QVariant &value, int role) override;
     bool removeRows(int row, int count, const QModelIndex &parent) override;
 
+    /**
+     * Recursively emits the dataChanged signal for all child nodes.
+     * @param parent The parent node
+     */
+    void emitBackgroundUpdates(const QModelIndex &parent);
+
     /**
      * @brief Finds a card by name, zone, and optional identifiers.
      * @param cardName The card's name.
@@ -289,6 +306,21 @@ public:
      */
     QModelIndex addCard(const ExactCard &card, const QString &zoneName);
 
+    /**
+     * @brief Increments the `amount` field of the card node at the index by 1.
+     * @param idx The index of a card node. No-ops if the index is invalid or not a card node
+     * @return Whether the operation was successful
+     */
+    bool incrementAmountAtIndex(const QModelIndex &idx);
+
+    /**
+     * @brief Decrements the `amount` field of the card node at the index by 1.
+     * Removes the node if it causes the amount to fall to 0.
+     * @param idx The index of a card node. No-ops if the index is invalid or not a card node
+     * @return Whether the operation was successful
+     */
+    bool decrementAmountAtIndex(const QModelIndex &idx);
+
     /**
      * @brief Determines the sorted insertion row for a card.
      * @param parent The parent node where the card will be inserted.
@@ -362,6 +394,9 @@ private:
                                                       const QString &zoneName,
                                                       const QString &providerId = "",
                                                       const QString &cardNumber = "") const;
+
+    bool offsetAmountAtIndex(const QModelIndex &idx, int offset);
+
     void emitRecursiveUpdates(const QModelIndex &index);
     void sortHelper(InnerDecklistNode *node, Qt::SortOrder order);