[DeckListModel] Refactor: general code cleanup (#6460)

* change one usage

* move method

* move format check code

* make group criteria method static

* move method

* make method private

* more comments

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

@@ -151,9 +151,7 @@ static QModelIndex addAndReplacePrintings(DeckListModel *model,
 
     // 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->offsetCountAtIndex(newCardIndex, extraCopies);
         model->removeRow(existing.row(), existing.parent());
     }
 

libcockatrice_card/libcockatrice/card/card_info.cpp

@@ -75,6 +75,16 @@ QString CardInfo::getCorrectedName() const
     return result.remove(rmrx).replace(spacerx, space);
 }
 
+bool CardInfo::isLegalInFormat(const QString &format) const
+{
+    if (format.isEmpty()) {
+        return true;
+    }
+
+    QString formatLegality = getProperty("format-" + format);
+    return formatLegality == "legal" || formatLegality == "restricted";
+}
+
 void CardInfo::addToSet(const CardSetPtr &_set, const PrintingInfo _info)
 {
     if (!_set->contains(smartThis)) {

libcockatrice_card/libcockatrice/card/card_info.h

@@ -291,6 +291,15 @@ public:
      */
     [[nodiscard]] QString getCorrectedName() const;
 
+    /**
+     * @brief Checks if the card is legal in the given format.
+     * A card is considered legal in a format if its properties map contains an entry for "format-<name>", with value
+     * "legal" or "restricted".
+     * @param format The format's name. If empty, will always return true.
+     * @return Whether the card is legal in the given format.
+     */
+    [[nodiscard]] bool isLegalInFormat(const QString &format) const;
+
     /**
      * @brief Adds a printing to a specific set.
      *

libcockatrice_models/libcockatrice/models/deck_list/deck_list_model.cpp

@@ -18,13 +18,19 @@ DeckListModel::~DeckListModel()
     delete root;
 }
 
-QString DeckListModel::getGroupCriteriaForCard(CardInfoPtr info) const
+/**
+ * @brief Extract the value from the card that is used for the group criteria.
+ * @param info Pointer to card information.
+ * @param criteria The group criteria
+ * @return String representing the value of the criteria.
+ */
+static QString extractGroupCriteriaValue(const CardInfoPtr &info, DeckListModelGroupCriteria::Type criteria)
 {
     if (!info) {
         return "unknown";
     }
 
-    switch (activeGroupCriteria) {
+    switch (criteria) {
         case DeckListModelGroupCriteria::MAIN_TYPE:
             return info->getMainCardType();
         case DeckListModelGroupCriteria::MANA_COST:
@@ -56,7 +62,7 @@ void DeckListModel::rebuildTree()
             }
 
             CardInfoPtr info = CardDatabaseManager::query()->getCardInfo(currentCard->getName());
-            QString groupCriteria = getGroupCriteriaForCard(info);
+            QString groupCriteria = extractGroupCriteriaValue(info, activeGroupCriteria);
 
             auto *groupNode = dynamic_cast<InnerDecklistNode *>(node->findChild(groupCriteria));
 
@@ -353,7 +359,7 @@ DecklistModelCardNode *DeckListModel::findCardNode(const QString &cardName,
         return nullptr;
     }
 
-    QString groupCriteria = getGroupCriteriaForCard(info);
+    QString groupCriteria = extractGroupCriteriaValue(info, activeGroupCriteria);
     InnerDecklistNode *groupNode = dynamic_cast<InnerDecklistNode *>(zoneNode->findChild(groupCriteria));
     if (!groupNode) {
         return nullptr;
@@ -406,7 +412,7 @@ QModelIndex DeckListModel::addCard(const ExactCard &card, const QString &zoneNam
     CardInfoPtr cardInfo = card.getCardPtr();
     PrintingInfo printingInfo = card.getPrinting();
 
-    QString groupCriteria = getGroupCriteriaForCard(cardInfo);
+    QString groupCriteria = extractGroupCriteriaValue(cardInfo, activeGroupCriteria);
     InnerDecklistNode *groupNode = createNodeIfNeeded(groupCriteria, zoneNode);
 
     const QModelIndex parentIndex = nodeToIndex(groupNode);
@@ -420,7 +426,7 @@ QModelIndex DeckListModel::addCard(const ExactCard &card, const QString &zoneNam
 
         auto *decklistCard =
             deckList->addCard(cardInfo->getName(), zoneName, insertRow, cardSetName, printingInfo.getProperty("num"),
-                              printingInfo.getProperty("uuid"), isCardLegalForCurrentFormat(cardInfo));
+                              printingInfo.getProperty("uuid"), cardInfo->isLegalInFormat(deckList->getGameFormat()));
 
         beginInsertRows(parentIndex, insertRow, insertRow);
         cardNode = new DecklistModelCardNode(decklistCard, groupNode, insertRow);
@@ -472,7 +478,7 @@ bool DeckListModel::offsetCountAtIndex(const QModelIndex &idx, int offset)
     return true;
 }
 
-int DeckListModel::findSortedInsertRow(InnerDecklistNode *parent, CardInfoPtr cardInfo) const
+int DeckListModel::findSortedInsertRow(const InnerDecklistNode *parent, const CardInfoPtr &cardInfo) const
 {
     if (!cardInfo) {
         return parent->size(); // fallback: append at end
@@ -661,18 +667,6 @@ QList<QString> DeckListModel::getZones() const
     return zones;
 }
 
-bool DeckListModel::isCardLegalForCurrentFormat(const CardInfoPtr cardInfo)
-{
-    if (!deckList->getGameFormat().isEmpty()) {
-        if (cardInfo->getProperties().contains("format-" + deckList->getGameFormat())) {
-            QString formatLegality = cardInfo->getProperty("format-" + deckList->getGameFormat());
-            return formatLegality == "legal" || formatLegality == "restricted";
-        }
-        return false;
-    }
-    return true;
-}
-
 static int maxAllowedForLegality(const FormatRules &format, const QString &legality)
 {
     for (const AllowedCount &c : format.allowedCounts) {
@@ -683,25 +677,29 @@ static int maxAllowedForLegality(const FormatRules &format, const QString &legal
     return -1; // unknown legality → treat as illegal
 }
 
-bool DeckListModel::isCardQuantityLegalForCurrentFormat(const CardInfoPtr cardInfo, int quantity)
+static bool isCardQuantityLegalForFormat(const QString &format, const CardInfo &cardInfo, int quantity)
 {
-    auto formatRules = CardDatabaseManager::query()->getFormat(deckList->getGameFormat());
+    if (format.isEmpty()) {
+        return true;
+    }
+
+    auto formatRules = CardDatabaseManager::query()->getFormat(format);
 
     if (!formatRules) {
         return true;
     }
 
     // Exceptions always win
-    if (cardHasAnyException(*cardInfo, *formatRules)) {
+    if (cardHasAnyException(cardInfo, *formatRules)) {
         return true;
     }
 
-    const QString legalityProp = "format-" + deckList->getGameFormat();
-    if (!cardInfo->getProperties().contains(legalityProp)) {
+    const QString legalityProp = "format-" + format;
+    if (!cardInfo.getProperties().contains(legalityProp)) {
         return false;
     }
 
-    const QString legality = cardInfo->getProperty(legalityProp);
+    const QString legality = cardInfo.getProperty(legalityProp);
 
     int maxAllowed = maxAllowedForLegality(*formatRules, legality);
 
@@ -735,10 +733,11 @@ void DeckListModel::refreshCardFormatLegalities()
                 continue;
             }
 
-            bool legal = isCardLegalForCurrentFormat(exactCard.getCardPtr());
+            QString format = deckList->getGameFormat();
+            bool legal = exactCard.getInfo().isLegalInFormat(format);
 
             if (legal) {
-                legal = isCardQuantityLegalForCurrentFormat(exactCard.getCardPtr(), currentCard->getNumber());
+                legal = isCardQuantityLegalForFormat(format, exactCard.getInfo(), currentCard->getNumber());
             }
 
             currentCard->setFormatLegality(legal);

libcockatrice_models/libcockatrice/models/deck_list/deck_list_model.h

@@ -217,7 +217,12 @@ public slots:
      */
     void rebuildTree();
 
-public slots:
+    /**
+     * @brief Sets the criteria used to group cards in the model.
+     * @param newCriteria The new grouping criteria.
+     */
+    void setActiveGroupCriteria(DeckListModelGroupCriteria::Type newCriteria);
+
     void setActiveFormat(const QString &_format);
 
 signals:
@@ -251,14 +256,8 @@ public:
         return nodeToIndex(root);
     }
 
-    /**
-     * @brief Returns the value of the grouping category for a card based on the current criteria.
-     * @param info Pointer to card information.
-     * @return String representing the value of the current grouping criteria for the card.
-     */
-    [[nodiscard]] QString getGroupCriteriaForCard(CardInfoPtr info) const;
-
-    // Qt model overrides
+    /// @name Qt model overrides
+    ///@{
     [[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;
@@ -268,6 +267,8 @@ public:
     [[nodiscard]] Qt::ItemFlags flags(const QModelIndex &index) const override;
     bool setData(const QModelIndex &index, const QVariant &value, int role) override;
     bool removeRows(int row, int count, const QModelIndex &parent) override;
+    void sort(int column, Qt::SortOrder order) override;
+    ///@}
 
     /**
      * @brief Finds a card by name, zone, and optional identifiers.
@@ -309,16 +310,6 @@ public:
      */
     bool offsetCountAtIndex(const QModelIndex &idx, int offset);
 
-    /**
-     * @brief Determines the sorted insertion row for a card.
-     * @param parent The parent node where the card will be inserted.
-     * @param cardInfo The card info to insert.
-     * @return Row index where the card should be inserted to maintain sort order.
-     */
-    int findSortedInsertRow(InnerDecklistNode *parent, CardInfoPtr cardInfo) const;
-
-    void sort(int column, Qt::SortOrder order) override;
-
     /**
      * @brief Removes all cards and resets the model.
      */
@@ -359,16 +350,6 @@ public:
      */
     [[nodiscard]] QList<QString> getZones() const;
 
-    bool isCardLegalForCurrentFormat(CardInfoPtr cardInfo);
-    bool isCardQuantityLegalForCurrentFormat(CardInfoPtr cardInfo, int quantity);
-    void refreshCardFormatLegalities();
-
-    /**
-     * @brief Sets the criteria used to group cards in the model.
-     * @param newCriteria The new grouping criteria.
-     */
-    void setActiveGroupCriteria(DeckListModelGroupCriteria::Type newCriteria);
-
 private:
     DeckList *deckList;      /**< Pointer to the deck loader providing the underlying data. */
     InnerDecklistNode *root; /**< Root node of the model tree. */
@@ -383,6 +364,14 @@ private:
                                                       const QString &providerId = "",
                                                       const QString &cardNumber = "") const;
 
+    /**
+     * @brief Determines the sorted insertion row for a card.
+     * @param parent The parent node where the card will be inserted.
+     * @param cardInfo The card info to insert.
+     * @return Row index where the card should be inserted to maintain sort order.
+     */
+    int findSortedInsertRow(const InnerDecklistNode *parent, const CardInfoPtr &cardInfo) const;
+
     /**
      * @brief Recursively emits the dataChanged signal with role as Qt::BackgroundRole for all indices that are children
      * of the given node. This is used to update the background color when changing formats.
@@ -404,6 +393,8 @@ private:
             return dynamic_cast<T>(root);
         return dynamic_cast<T>(static_cast<AbstractDecklistNode *>(index.internalPointer()));
     }
+
+    void refreshCardFormatLegalities();
 };
 
 #endif