From 192dac0396b4851b6ba74ab7c9f7a148d7b3a977 Mon Sep 17 00:00:00 2001 From: RickyRister <42636155+RickyRister@users.noreply.github.com> Date: Mon, 5 Jan 2026 09:28:59 -0800 Subject: [PATCH] [DeckListModel] Consolidate methods and signals for card change (#6466) --- .../deck_list_statistics_analyzer.cpp | 2 +- .../deck_editor_deck_dock_widget.cpp | 2 +- .../deck_editor/deck_state_manager.cpp | 3 +- .../dialogs/dlg_select_set_for_cards.cpp | 2 +- .../printing_selector/card_amount_widget.cpp | 2 +- .../models/deck_list/deck_list_model.cpp | 41 +++++++++++++++++++ .../models/deck_list/deck_list_model.h | 40 ++++++++++++++++++ 7 files changed, 86 insertions(+), 6 deletions(-) diff --git a/cockatrice/src/interface/widgets/deck_analytics/deck_list_statistics_analyzer.cpp b/cockatrice/src/interface/widgets/deck_analytics/deck_list_statistics_analyzer.cpp index a1dc1ab55..ad8afb766 100644 --- a/cockatrice/src/interface/widgets/deck_analytics/deck_list_statistics_analyzer.cpp +++ b/cockatrice/src/interface/widgets/deck_analytics/deck_list_statistics_analyzer.cpp @@ -12,7 +12,7 @@ DeckListStatisticsAnalyzer::DeckListStatisticsAnalyzer(QObject *parent, DeckListStatisticsAnalyzerConfig _config) : QObject(parent), model(_model), config(_config) { - connect(model, &DeckListModel::dataChanged, this, &DeckListStatisticsAnalyzer::analyze); + connect(model, &DeckListModel::cardsChanged, this, &DeckListStatisticsAnalyzer::analyze); } void DeckListStatisticsAnalyzer::analyze() 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 afc1f41b2..bf7688bb0 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 @@ -154,7 +154,7 @@ void DeckEditorDeckDockWidget::createDeckDock() bannerCardLabel->setText(tr("Banner Card")); bannerCardLabel->setHidden(!SettingsCache::instance().getDeckEditorBannerCardComboBoxVisible()); bannerCardComboBox = new QComboBox(this); - connect(getModel(), &DeckListModel::dataChanged, this, [this]() { + connect(getModel(), &DeckListModel::cardNodesChanged, this, [this]() { // Delay the update to avoid race conditions QTimer::singleShot(100, this, &DeckEditorDeckDockWidget::updateBannerCardComboBox); }); 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 7b99e8c6a..d45416b17 100644 --- a/cockatrice/src/interface/widgets/deck_editor/deck_state_manager.cpp +++ b/cockatrice/src/interface/widgets/deck_editor/deck_state_manager.cpp @@ -11,8 +11,7 @@ DeckStateManager::DeckStateManager(QObject *parent) setModified(true); emit historyChanged(); }); - connect(deckListModel, &DeckListModel::rowsInserted, this, &DeckStateManager::uniqueCardsChanged); - connect(deckListModel, &DeckListModel::rowsRemoved, this, &DeckStateManager::uniqueCardsChanged); + connect(deckListModel, &DeckListModel::cardNodesChanged, this, &DeckStateManager::uniqueCardsChanged); } const DeckList &DeckStateManager::getDeckList() const diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_select_set_for_cards.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_select_set_for_cards.cpp index 6ed6b67a4..5d9e2679c 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_select_set_for_cards.cpp +++ b/cockatrice/src/interface/widgets/dialogs/dlg_select_set_for_cards.cpp @@ -152,7 +152,7 @@ static bool swapPrinting(DeckListModel *model, const QString &modifiedSet, const return false; } int amount = model->data(idx.siblingAtColumn(DeckListModelColumns::CARD_AMOUNT), Qt::DisplayRole).toInt(); - model->removeRow(idx.row(), idx.parent()); + model->removeCardAtIndex(idx); CardInfoPtr cardInfo = CardDatabaseManager::query()->getCardInfo(cardName); PrintingInfo printing = CardDatabaseManager::query()->getSpecificPrinting(cardName, modifiedSet, ""); for (int i = 0; i < amount; i++) { diff --git a/cockatrice/src/interface/widgets/printing_selector/card_amount_widget.cpp b/cockatrice/src/interface/widgets/printing_selector/card_amount_widget.cpp index f8002eb89..1c09a42c1 100644 --- a/cockatrice/src/interface/widgets/printing_selector/card_amount_widget.cpp +++ b/cockatrice/src/interface/widgets/printing_selector/card_amount_widget.cpp @@ -152,7 +152,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) { model->offsetCountAtIndex(newCardIndex, extraCopies); - model->removeRow(existing.row(), existing.parent()); + model->removeCardAtIndex(existing); } // Set Index and Focus as if the user had just clicked the new card and modify the deckEditor saveState 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 499208a6d..447d868a8 100644 --- a/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model.cpp +++ b/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model.cpp @@ -12,6 +12,15 @@ DeckListModel::DeckListModel(QObject *parent) DeckListModel::DeckListModel(QObject *parent, const QSharedPointer &deckList) : DeckListModel(parent) { setDeckList(deckList); + + // forward change signals + connect(this, &DeckListModel::cardAddedAt, this, &DeckListModel::cardsChanged); + connect(this, &DeckListModel::cardRemoved, this, &DeckListModel::cardsChanged); + connect(this, &DeckListModel::deckReplaced, this, &DeckListModel::cardsChanged); + + connect(this, &DeckListModel::cardNodeAddedAt, this, &DeckListModel::cardNodesChanged); + connect(this, &DeckListModel::cardNodeRemoved, this, &DeckListModel::cardNodesChanged); + connect(this, &DeckListModel::deckReplaced, this, &DeckListModel::cardNodesChanged); } DeckListModel::~DeckListModel() @@ -421,6 +430,7 @@ QModelIndex DeckListModel::addCard(const ExactCard &card, const QString &zoneNam card.getName(), printingInfo.getUuid(), printingInfo.getProperty("num"))); const auto cardSetName = printingInfo.getSet().isNull() ? "" : printingInfo.getSet()->getCorrectedShortName(); + bool cardNodeAdded = false; if (!cardNode) { // Determine the correct index int insertRow = findSortedInsertRow(groupNode, cardInfo); @@ -432,6 +442,8 @@ QModelIndex DeckListModel::addCard(const ExactCard &card, const QString &zoneNam beginInsertRows(parentIndex, insertRow, insertRow); cardNode = new DecklistModelCardNode(decklistCard, groupNode, insertRow); endInsertRows(); + + cardNodeAdded = true; } else { cardNode->setNumber(cardNode->getNumber() + 1); cardNode->setCardSetShortName(cardSetName); @@ -444,6 +456,10 @@ QModelIndex DeckListModel::addCard(const ExactCard &card, const QString &zoneNam emitRecursiveUpdates(parentIndex); auto index = nodeToIndex(cardNode); + if (cardNodeAdded) { + emit cardNodeAddedAt(index); + } + emit cardAddedAt(index); return index; @@ -468,17 +484,42 @@ bool DeckListModel::offsetCountAtIndex(const QModelIndex &idx, int offset) if (newCount <= 0) { removeRow(idx.row(), idx.parent()); + emit cardNodeRemoved(); } else { setData(numberIndex, newCount, Qt::EditRole); } if (offset > 0) { emit cardAddedAt(idx); + } else if (offset < 0) { + emit cardRemoved(); } return true; } +bool DeckListModel::removeCardAtIndex(const QModelIndex &idx) +{ + if (!idx.isValid()) { + return false; + } + + auto *node = static_cast(idx.internalPointer()); + auto *card = dynamic_cast(node); + + if (!card) { + return false; + } + + bool success = removeRow(idx.row(), idx.parent()); + + if (success) { + emit cardRemoved(); + } + + return success; +} + int DeckListModel::findSortedInsertRow(const InnerDecklistNode *parent, const CardInfoPtr &cardInfo) const { if (!cardInfo) { diff --git a/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model.h b/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model.h index 133e7a947..86d36b7f9 100644 --- a/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model.h +++ b/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model.h @@ -197,6 +197,12 @@ public: * InnerDecklistNode containers and supports grouping, sorting, adding/removing * cards, and printing decklists. * + * Outside code should refrain from modifying the model with methods inherited from QAbstractItemModel, such as with + * `setData` or `removeRow`. + * Instead, use the custom methods on this class to modify the model, such as `addCard`, `offsetCountAtIndex`, or + * `removeCardAtIndex`. + * This ensures the custom signals for this class are correctly emitted. + * * Signals: * - deckHashChanged(): emitted when the deck contents change in a way that * affects its hash. @@ -231,6 +237,11 @@ signals: */ void deckHashChanged(); + /** + * @brief Emitted whenever the cards in the deck changes. This includes when the deck is replaced. + */ + void cardsChanged(); + /** * @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. @@ -238,6 +249,28 @@ signals: */ void cardAddedAt(const QModelIndex &index); + /** + * @brief Emitted whenever a card is removed from the deck, regardless of whether a card node was removed or an + * existing card got decremented. + */ + void cardRemoved(); + + /** + * @brief Emitted whenever a card node is added or removed. This includes when the deck is replaced. + */ + void cardNodesChanged(); + + /** + * @brief Emitted whenever a new card node is added. + * @param index The index of the card node that got added. + */ + void cardNodeAddedAt(const QModelIndex &index); + + /** + * @brief Emitted whenever a card node is removed. + */ + void cardNodeRemoved(); + /** * @brief Emitted whenever the deck in the model has been replaced with a new one */ @@ -311,6 +344,13 @@ public: */ bool offsetCountAtIndex(const QModelIndex &idx, int offset); + /** + * @brief Removes the card node at the index + * @param idx The index of a card node. No-ops if the index is invalid or not a card node. + * @return Whether the node was removed. + */ + bool removeCardAtIndex(const QModelIndex &idx); + /** * @brief Removes all cards and resets the model. */