[Refactor] Clean up some PrintingSelector widgets (#6451)

* remove currentZone from PrintingSelector

* don't store constructor args in fields if they're just passed

* simplify some methods

* refactor

* clean up initializeFormats

* more refactoring in CardAmountWidget

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

@@ -284,16 +284,15 @@ void DeckEditorDeckDockWidget::createDeckDock()
 
 void DeckEditorDeckDockWidget::initializeFormats()
 {
-    QMap<QString, int> allFormats = CardDatabaseManager::query()->getAllFormatsWithCount();
+    QStringList allFormats = CardDatabaseManager::query()->getAllFormatsWithCount().keys();
 
     formatComboBox->clear(); // Remove "Loading Database..."
     formatComboBox->setEnabled(true);
 
     // Populate with formats
     formatComboBox->addItem("", "");
-    for (auto it = allFormats.constBegin(); it != allFormats.constEnd(); ++it) {
+    for (auto formatName : allFormats) {
-        QString displayText = QString("%1").arg(it.key());
+        formatComboBox->addItem(formatName, formatName); // store the raw key in itemData
-        formatComboBox->addItem(displayText, it.key()); // store the raw key in itemData
     }
 
     if (!deckModel->getDeckList()->getGameFormat().isEmpty()) {

cockatrice/src/interface/widgets/dialogs/dlg_select_set_for_cards.cpp

@@ -143,6 +143,22 @@ void DlgSelectSetForCards::retranslateUi()
     setAllToPreferredButton->setText(tr("Set all to preferred"));
 }
 
+static bool swapPrinting(DeckListModel *model, const QString &modifiedSet, const QString &cardName)
+{
+    QModelIndex idx = model->findCard(cardName, DECK_ZONE_MAIN);
+    if (!idx.isValid()) {
+        return false;
+    }
+    int amount = model->data(idx.siblingAtColumn(DeckListModelColumns::CARD_AMOUNT), Qt::DisplayRole).toInt();
+    model->removeRow(idx.row(), idx.parent());
+    CardInfoPtr cardInfo = CardDatabaseManager::query()->getCardInfo(cardName);
+    PrintingInfo printing = CardDatabaseManager::query()->getSpecificPrinting(cardName, modifiedSet, "");
+    for (int i = 0; i < amount; i++) {
+        model->addCard(ExactCard(cardInfo, printing), DECK_ZONE_MAIN);
+    }
+    return true;
+}
+
 void DlgSelectSetForCards::actOK()
 {
     QMap<QString, QStringList> modifiedSetsAndCardsMap = getModifiedCards();
@@ -155,20 +171,10 @@ void DlgSelectSetForCards::actOK()
 
     for (QString modifiedSet : modifiedSetsAndCardsMap.keys()) {
         for (QString card : modifiedSetsAndCardsMap.value(modifiedSet)) {
-            QModelIndex find_card = model->findCard(card, DECK_ZONE_MAIN);
+            swapPrinting(model, modifiedSet, card);
-            if (!find_card.isValid()) {
-                continue;
-            }
-            int amount =
-                model->data(find_card.siblingAtColumn(DeckListModelColumns::CARD_AMOUNT), Qt::DisplayRole).toInt();
-            model->removeRow(find_card.row(), find_card.parent());
-            CardInfoPtr cardInfo = CardDatabaseManager::query()->getCardInfo(card);
-            PrintingInfo printing = CardDatabaseManager::query()->getSpecificPrinting(card, modifiedSet, "");
-            for (int i = 0; i < amount; i++) {
-                model->addCard(ExactCard(cardInfo, printing), DECK_ZONE_MAIN);
-            }
         }
     }
+
     if (!modifiedSetsAndCardsMap.isEmpty()) {
         emit deckModified();
     }

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

@@ -23,8 +23,7 @@ AllZonesCardAmountWidget::AllZonesCardAmountWidget(QWidget *parent,
                                                    QTreeView *deckView,
                                                    QSlider *cardSizeSlider,
                                                    const ExactCard &rootCard)
-    : QWidget(parent), deckEditor(deckEditor), deckModel(deckModel), deckView(deckView), cardSizeSlider(cardSizeSlider),
+    : QWidget(parent), cardSizeSlider(cardSizeSlider)
-      rootCard(rootCard)
 {
     layout = new QVBoxLayout(this);
     layout->setAlignment(Qt::AlignHCenter);

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

@@ -36,11 +36,7 @@ public slots:
 
 private:
     QVBoxLayout *layout;
-    AbstractTabDeckEditor *deckEditor;
-    DeckListModel *deckModel;
-    QTreeView *deckView;
     QSlider *cardSizeSlider;
-    ExactCard rootCard;
     QLabel *zoneLabelMainboard;
     CardAmountWidget *buttonBoxMainboard;
     QLabel *zoneLabelSideboard;

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

@@ -137,6 +137,31 @@ void CardAmountWidget::updateCardCount()
     layout->activate();
 }
 
+static QModelIndex addAndReplacePrintings(DeckListModel *model,
+                                          const QModelIndex &existing,
+                                          const ExactCard &rootCard,
+                                          const QString &zone,
+                                          int extraCopies,
+                                          bool replaceProviderless)
+{
+    auto newCardIndex = model->addCard(rootCard, zone);
+    if (!newCardIndex.isValid()) {
+        return {};
+    }
+
+    // Check if a card without a providerId already exists in the deckModel and replace it, if so.
+    if (existing.isValid() && existing != newCardIndex && replaceProviderless) {
+        for (int i = 0; i < extraCopies; i++) {
+            model->addCard(rootCard, zone);
+        }
+        model->removeRow(existing.row(), existing.parent());
+    }
+
+    // Set Index and Focus as if the user had just clicked the new card and modify the deckEditor saveState
+    return model->findCard(rootCard.getName(), zone, rootCard.getPrinting().getUuid(),
+                           rootCard.getPrinting().getProperty("num"));
+}
+
 /**
  * @brief Adds a printing of the card to the specified zone (Mainboard or Sideboard).
  *
@@ -144,26 +169,24 @@ void CardAmountWidget::updateCardCount()
  */
 void CardAmountWidget::addPrinting(const QString &zone)
 {
-    int addedCount = 1;
     // Check if we will need to add extra copies due to replacing copies without providerIds
     QModelIndex existing = deckModel->findCard(rootCard.getName(), zone);
+
     int extraCopies = 0;
     bool replacingProviderless = false;
 
     if (existing.isValid()) {
-        QString providerId =
+        QString foundProviderId =
             existing.siblingAtColumn(DeckListModelColumns::CARD_PROVIDER_ID).data(Qt::DisplayRole).toString();
-        if (providerId.isEmpty()) {
+        if (foundProviderId.isEmpty()) {
             int amount = existing.data(Qt::DisplayRole).toInt();
             extraCopies = amount - 1; // One less because we *always* add one
             replacingProviderless = true;
         }
     }
 
-    addedCount += extraCopies;
-
     QString reason = QString("Added %1 copies of '%2 (%3) %4' to %5 [ProviderID: %6]%7")
-                         .arg(addedCount)
+                         .arg(1 + extraCopies)
                          .arg(rootCard.getName())
                          .arg(rootCard.getPrinting().getSet()->getShortName())
                          .arg(rootCard.getPrinting().getProperty("num"))
@@ -174,26 +197,13 @@ void CardAmountWidget::addPrinting(const QString &zone)
     emit deckModified(reason);
 
     // Add the card and expand the list UI
-    auto newCardIndex = deckModel->addCard(rootCard, zone);
+    auto newCardIndex = addAndReplacePrintings(deckModel, existing, rootCard, zone, extraCopies, replacingProviderless);
 
-    // Check if a card without a providerId already exists in the deckModel and replace it, if so.
+    if (newCardIndex.isValid()) {
-    QString foundProviderId =
+        deckView->setCurrentIndex(newCardIndex);
-        existing.siblingAtColumn(DeckListModelColumns::CARD_PROVIDER_ID).data(Qt::DisplayRole).toString();
+        deckView->setFocus(Qt::FocusReason::MouseFocusReason);
-    if (existing.isValid() && existing != newCardIndex && foundProviderId == "") {
+        deckEditor->setModified(true);
-        auto amount = existing.data(Qt::DisplayRole);
-        for (int i = 0; i < amount.toInt() - 1; i++) {
-            deckModel->addCard(rootCard, zone);
-        }
-        deckModel->removeRow(existing.row(), existing.parent());
     }
-
-    // Set Index and Focus as if the user had just clicked the new card and modify the deckEditor saveState
-    newCardIndex = deckModel->findCard(rootCard.getName(), zone, rootCard.getPrinting().getUuid(),
-                                       rootCard.getPrinting().getProperty("num"));
-
-    deckView->setCurrentIndex(newCardIndex);
-    deckView->setFocus(Qt::FocusReason::MouseFocusReason);
-    deckEditor->setModified(true);
 }
 
 /**
@@ -259,22 +269,14 @@ void CardAmountWidget::decrementCardHelper(const QString &zone)
  */
 int CardAmountWidget::countCardsInZone(const QString &deckZone)
 {
-    if (rootCard.getPrinting().getUuid().isEmpty()) {
+    QString uuid = rootCard.getPrinting().getUuid();
-        return 0; // Cards without uuids/providerIds CANNOT match another card, they are undefined for us.
-    }
 
-    if (!deckModel) {
+    if (uuid.isEmpty()) {
-        return -1;
+        return 0; // Cards without uuids/providerIds CANNOT match another card, they are undefined for us.
     }
 
     QList<ExactCard> cards = deckModel->getCardsForZone(deckZone);
 
-    int count = 0;
+    return std::count_if(cards.cbegin(), cards.cend(),
-    for (auto currentCard : cards) {
+                         [&uuid](const ExactCard &card) { return card.getPrinting().getUuid() == uuid; });
-        if (currentCard.getPrinting().getUuid() == rootCard.getPrinting().getProperty("uuid")) {
-            count++;
-        }
-    }
-
-    return count;
 }

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

@@ -115,9 +115,8 @@ void PrintingSelector::updateDisplay()
  * @brief Sets the current card for the selector and updates the display.
  *
  * @param newCard The new card to set.
- * @param _currentZone The current zone the card is in.
  */
-void PrintingSelector::setCard(const CardInfoPtr &newCard, const QString &_currentZone)
+void PrintingSelector::setCard(const CardInfoPtr &newCard)
 {
     if (newCard.isNull()) {
         return;
@@ -129,7 +128,6 @@ void PrintingSelector::setCard(const CardInfoPtr &newCard, const QString &_curre
     }
 
     selectedCard = newCard;
-    currentZone = _currentZone;
     if (isVisible()) {
         updateDisplay();
     }
@@ -166,8 +164,8 @@ void PrintingSelector::getAllSetsForCurrentCard()
     connect(widgetLoadingBufferTimer, &QTimer::timeout, this, [=, this]() mutable {
         for (int i = 0; i < BATCH_SIZE && currentIndex < printingsToUse.size(); ++i, ++currentIndex) {
             auto card = ExactCard(selectedCard, printingsToUse[currentIndex]);
-            auto *cardDisplayWidget = new PrintingSelectorCardDisplayWidget(
+            auto *cardDisplayWidget = new PrintingSelectorCardDisplayWidget(this, deckEditor, deckModel, deckView,
-                this, deckEditor, deckModel, deckView, cardSizeWidget->getSlider(), card, currentZone);
+                                                                            cardSizeWidget->getSlider(), card);
             flowWidget->addWidget(cardDisplayWidget);
             cardDisplayWidget->clampSetNameToPicture();
             connect(cardDisplayWidget, &PrintingSelectorCardDisplayWidget::cardPreferenceChanged, this,

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

@@ -33,7 +33,7 @@ class PrintingSelector : public QWidget
 public:
     PrintingSelector(QWidget *parent, AbstractTabDeckEditor *deckEditor);
 
-    void setCard(const CardInfoPtr &newCard, const QString &_currentZone);
+    void setCard(const CardInfoPtr &newCard);
     void getAllSetsForCurrentCard();
     [[nodiscard]] DeckListModel *getDeckModel() const
     {
@@ -78,7 +78,6 @@ private:
     DeckListModel *deckModel;
     QTreeView *deckView;
     CardInfoPtr selectedCard;
-    QString currentZone;
     QTimer *widgetLoadingBufferTimer;
     int currentIndex = 0;
 };

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

@@ -17,22 +17,19 @@
  * display.
  *
  * @param parent The parent widget for this display.
- * @param _deckEditor The TabDeckEditor instance for deck management.
+ * @param deckEditor The TabDeckEditor instance for deck management.
- * @param _deckModel The DeckListModel instance providing deck data.
+ * @param deckModel The DeckListModel instance providing deck data.
- * @param _deckView The QTreeView instance displaying the deck.
+ * @param deckView The QTreeView instance displaying the deck.
- * @param _cardSizeSlider The slider controlling the size of the displayed card.
+ * @param cardSizeSlider The slider controlling the size of the displayed card.
- * @param _rootCard The root card object, representing the card to be displayed.
+ * @param rootCard The root card object, representing the card to be displayed.
- * @param _currentZone The current zone in which the card is located.
  */
 PrintingSelectorCardDisplayWidget::PrintingSelectorCardDisplayWidget(QWidget *parent,
-                                                                     AbstractTabDeckEditor *_deckEditor,
+                                                                     AbstractTabDeckEditor *deckEditor,
-                                                                     DeckListModel *_deckModel,
+                                                                     DeckListModel *deckModel,
-                                                                     QTreeView *_deckView,
+                                                                     QTreeView *deckView,
-                                                                     QSlider *_cardSizeSlider,
+                                                                     QSlider *cardSizeSlider,
-                                                                     const ExactCard &_rootCard,
+                                                                     const ExactCard &rootCard)
-                                                                     QString &_currentZone)
+    : QWidget(parent)
-    : QWidget(parent), deckEditor(_deckEditor), deckModel(_deckModel), deckView(_deckView),
-      cardSizeSlider(_cardSizeSlider), rootCard(_rootCard), currentZone(_currentZone)
 {
     layout = new QVBoxLayout(this);
     setLayout(layout);

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

@@ -20,12 +20,11 @@ class PrintingSelectorCardDisplayWidget : public QWidget
 
 public:
     PrintingSelectorCardDisplayWidget(QWidget *parent,
-                                      AbstractTabDeckEditor *_deckEditor,
+                                      AbstractTabDeckEditor *deckEditor,
-                                      DeckListModel *_deckModel,
+                                      DeckListModel *deckModel,
-                                      QTreeView *_deckView,
+                                      QTreeView *deckView,
-                                      QSlider *_cardSizeSlider,
+                                      QSlider *cardSizeSlider,
-                                      const ExactCard &_rootCard,
+                                      const ExactCard &rootCard);
-                                      QString &_currentZone);
 
 public slots:
     void clampSetNameToPicture();
@@ -36,12 +35,6 @@ signals:
 private:
     QVBoxLayout *layout;
     SetNameAndCollectorsNumberDisplayWidget *setNameAndCollectorsNumberDisplayWidget;
-    AbstractTabDeckEditor *deckEditor;
-    DeckListModel *deckModel;
-    QTreeView *deckView;
-    QSlider *cardSizeSlider;
-    ExactCard rootCard;
-    QString currentZone;
     PrintingSelectorCardOverlayWidget *overlayWidget;
 };
 

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

@@ -22,19 +22,18 @@
  *
  * @param parent The parent widget for this overlay.
  * @param _deckEditor The TabDeckEditor instance for deck management.
- * @param _deckModel The DeckListModel instance providing deck data.
+ * @param deckModel The DeckListModel instance providing deck data.
- * @param _deckView The QTreeView instance displaying the deck.
+ * @param deckView The QTreeView instance displaying the deck.
- * @param _cardSizeSlider The slider controlling the size of the card.
+ * @param cardSizeSlider The slider controlling the size of the card.
  * @param _rootCard The root card object that contains information about the card.
  */
 PrintingSelectorCardOverlayWidget::PrintingSelectorCardOverlayWidget(QWidget *parent,
                                                                      AbstractTabDeckEditor *_deckEditor,
-                                                                     DeckListModel *_deckModel,
+                                                                     DeckListModel *deckModel,
-                                                                     QTreeView *_deckView,
+                                                                     QTreeView *deckView,
-                                                                     QSlider *_cardSizeSlider,
+                                                                     QSlider *cardSizeSlider,
                                                                      const ExactCard &_rootCard)
-    : QWidget(parent), deckEditor(_deckEditor), deckModel(_deckModel), deckView(_deckView),
+    : QWidget(parent), deckEditor(_deckEditor), rootCard(_rootCard)
-      cardSizeSlider(_cardSizeSlider), rootCard(_rootCard)
 {
     // Set up the main layout
     auto *mainLayout = new QVBoxLayout(this);

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

@@ -48,9 +48,6 @@ private:
     AllZonesCardAmountWidget *allZonesCardAmountWidget;
     QLabel *pinBadge = nullptr;
     AbstractTabDeckEditor *deckEditor;
-    DeckListModel *deckModel;
-    QTreeView *deckView;
-    QSlider *cardSizeSlider;
     ExactCard rootCard;
 };
 

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

@@ -13,7 +13,7 @@ SetNameAndCollectorsNumberDisplayWidget::SetNameAndCollectorsNumberDisplayWidget
                                                                                  const QString &_setName,
                                                                                  const QString &_collectorsNumber,
                                                                                  QSlider *_cardSizeSlider)
-    : QWidget(parent)
+    : QWidget(parent), cardSizeSlider(_cardSizeSlider)
 {
     // Set up the layout for the widget
     layout = new QVBoxLayout(this);
@@ -35,7 +35,6 @@ SetNameAndCollectorsNumberDisplayWidget::SetNameAndCollectorsNumberDisplayWidget
     collectorsNumber->setSizePolicy(QSizePolicy::Minimum, QSizePolicy::Minimum);
 
     // Store the card size slider and connect its signal to the font size adjustment slot
-    cardSizeSlider = _cardSizeSlider;
     connect(cardSizeSlider, &QSlider::valueChanged, this, &SetNameAndCollectorsNumberDisplayWidget::adjustFontSize);
 
     // Add labels to the layout

cockatrice/src/interface/widgets/tabs/abstract_tab_deck_editor.cpp

@@ -101,7 +101,7 @@ AbstractTabDeckEditor::AbstractTabDeckEditor(TabSupervisor *_tabSupervisor) : Ta
 void AbstractTabDeckEditor::updateCard(const ExactCard &card)
 {
     cardInfoDockWidget->updateCard(card);
-    printingSelectorDockWidget->printingSelector->setCard(card.getCardPtr(), DECK_ZONE_MAIN);
+    printingSelectorDockWidget->printingSelector->setCard(card.getCardPtr());
 }
 
 /** @brief Placeholder: called when the deck changes. */

cockatrice/src/interface/widgets/tabs/tab_deck_editor.cpp

@@ -158,8 +158,7 @@ void TabDeckEditor::refreshShortcuts()
  */
 void TabDeckEditor::showPrintingSelector()
 {
-    printingSelectorDockWidget->printingSelector->setCard(cardInfoDockWidget->cardInfo->getCard().getCardPtr(),
+    printingSelectorDockWidget->printingSelector->setCard(cardInfoDockWidget->cardInfo->getCard().getCardPtr());
-                                                          DECK_ZONE_MAIN);
     printingSelectorDockWidget->printingSelector->updateDisplay();
     aPrintingSelectorDockVisible->setChecked(true);
     printingSelectorDockWidget->setVisible(true);

cockatrice/src/interface/widgets/tabs/visual_deck_editor/tab_deck_editor_visual.cpp

@@ -266,8 +266,7 @@ bool TabDeckEditorVisual::actSaveDeckAs()
 /** @brief Shows the printing selector dock and updates it with the current card. */
 void TabDeckEditorVisual::showPrintingSelector()
 {
-    printingSelectorDockWidget->printingSelector->setCard(cardInfoDockWidget->cardInfo->getCard().getCardPtr(),
+    printingSelectorDockWidget->printingSelector->setCard(cardInfoDockWidget->cardInfo->getCard().getCardPtr());
-                                                          DECK_ZONE_MAIN);
     printingSelectorDockWidget->printingSelector->updateDisplay();
     aPrintingSelectorDockVisible->setChecked(true);
     printingSelectorDockWidget->setVisible(true);