From 658ae83157d0d9566683da3a477cbd8875911f08 Mon Sep 17 00:00:00 2001 From: RickyRister <42636155+RickyRister@users.noreply.github.com> Date: Tue, 2 Dec 2025 21:18:46 -0800 Subject: [PATCH] [DeckList] Make DeckList not a QObject (#6383) --- .../deck_editor_deck_dock_widget.cpp | 5 +- .../deck_preview_deck_tags_display_widget.cpp | 12 ++-- .../deck_preview_deck_tags_display_widget.h | 2 +- .../libcockatrice/deck_list/deck_list.cpp | 14 +++-- .../libcockatrice/deck_list/deck_list.h | 55 +++---------------- .../models/deck_list/deck_list_model.cpp | 7 ++- 6 files changed, 28 insertions(+), 67 deletions(-) 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 4db7b8774..075b73216 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 @@ -424,7 +424,6 @@ void DeckEditorDeckDockWidget::setDeck(DeckLoader *_deck) deckLoader->setParent(this); deckModel->setDeckList(deckLoader->getDeckList()); connect(deckLoader, &DeckLoader::deckLoaded, deckModel, &DeckListModel::rebuildTree); - connect(deckLoader->getDeckList(), &DeckList::deckHashChanged, deckModel, &DeckListModel::deckHashChanged); emit requestDeckHistoryClear(); historyManagerWidget->setDeckListModel(deckModel); @@ -452,7 +451,7 @@ void DeckEditorDeckDockWidget::syncDisplayWidgetsToModel() sortDeckModelToDeckView(); expandAll(); - deckTagsDisplayWidget->connectDeckList(deckModel->getDeckList()); + deckTagsDisplayWidget->setDeckList(deckModel->getDeckList()); } void DeckEditorDeckDockWidget::sortDeckModelToDeckView() @@ -485,7 +484,7 @@ void DeckEditorDeckDockWidget::cleanDeck() emit deckModified(); emit deckChanged(); updateBannerCardComboBox(); - deckTagsDisplayWidget->connectDeckList(deckModel->getDeckList()); + deckTagsDisplayWidget->setDeckList(deckModel->getDeckList()); } void DeckEditorDeckDockWidget::recursiveExpand(const QModelIndex &index) diff --git a/cockatrice/src/interface/widgets/visual_deck_storage/deck_preview/deck_preview_deck_tags_display_widget.cpp b/cockatrice/src/interface/widgets/visual_deck_storage/deck_preview/deck_preview_deck_tags_display_widget.cpp index 706444545..a29f3bfc4 100644 --- a/cockatrice/src/interface/widgets/visual_deck_storage/deck_preview/deck_preview_deck_tags_display_widget.cpp +++ b/cockatrice/src/interface/widgets/visual_deck_storage/deck_preview/deck_preview_deck_tags_display_widget.cpp @@ -28,21 +28,15 @@ DeckPreviewDeckTagsDisplayWidget::DeckPreviewDeckTagsDisplayWidget(QWidget *_par flowWidget = new FlowWidget(this, Qt::Horizontal, Qt::ScrollBarAlwaysOff, Qt::ScrollBarAsNeeded); if (_deckList) { - connectDeckList(_deckList); + setDeckList(_deckList); } layout->addWidget(flowWidget); } -void DeckPreviewDeckTagsDisplayWidget::connectDeckList(DeckList *_deckList) +void DeckPreviewDeckTagsDisplayWidget::setDeckList(DeckList *_deckList) { - if (deckList) { - disconnect(deckList, &DeckList::deckTagsChanged, this, &DeckPreviewDeckTagsDisplayWidget::refreshTags); - } - deckList = _deckList; - connect(deckList, &DeckList::deckTagsChanged, this, &DeckPreviewDeckTagsDisplayWidget::refreshTags); - refreshTags(); } @@ -150,6 +144,7 @@ void DeckPreviewDeckTagsDisplayWidget::openTagEditDlg() QStringList updatedTags = dialog.getActiveTags(); deckList->setTags(updatedTags); deckPreviewWidget->deckLoader->saveToFile(deckPreviewWidget->filePath, DeckLoader::CockatriceFormat); + refreshTags(); } } } else if (parentWidget()) { @@ -181,6 +176,7 @@ void DeckPreviewDeckTagsDisplayWidget::openTagEditDlg() QStringList updatedTags = dialog.getActiveTags(); deckList->setTags(updatedTags); deckEditor->setModified(true); + refreshTags(); } } } diff --git a/cockatrice/src/interface/widgets/visual_deck_storage/deck_preview/deck_preview_deck_tags_display_widget.h b/cockatrice/src/interface/widgets/visual_deck_storage/deck_preview/deck_preview_deck_tags_display_widget.h index 4270e38e5..b1b1117ae 100644 --- a/cockatrice/src/interface/widgets/visual_deck_storage/deck_preview/deck_preview_deck_tags_display_widget.h +++ b/cockatrice/src/interface/widgets/visual_deck_storage/deck_preview/deck_preview_deck_tags_display_widget.h @@ -21,7 +21,7 @@ class DeckPreviewDeckTagsDisplayWidget : public QWidget public: explicit DeckPreviewDeckTagsDisplayWidget(QWidget *_parent, DeckList *_deckList); - void connectDeckList(DeckList *_deckList); + void setDeckList(DeckList *_deckList); void refreshTags(); DeckList *deckList; FlowWidget *flowWidget; diff --git a/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp b/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp index 37f537249..cea759428 100644 --- a/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp +++ b/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp @@ -87,6 +87,12 @@ DeckList::DeckList() root = new InnerDecklistNode; } +DeckList::DeckList(const DeckList &other) + : metadata(other.metadata), sideboardPlans(other.sideboardPlans), root(new InnerDecklistNode(other.getRoot())), + cachedDeckHash(other.cachedDeckHash) +{ +} + DeckList::DeckList(const QString &nativeString) { root = new InnerDecklistNode; @@ -443,11 +449,8 @@ bool DeckList::loadFromStream_Plain(QTextStream &in, bool preserveMetadata) cardName.replace(diff.key(), diff.value()); } - // Resolve complete card name, this function does nothing if the name is not found - cardName = getCompleteCardName(cardName); - // Determine the zone (mainboard/sideboard) - QString zoneName = getCardZoneFromName(cardName, sideboard ? DECK_ZONE_SIDE : DECK_ZONE_MAIN); + QString zoneName = sideboard ? DECK_ZONE_SIDE : DECK_ZONE_MAIN; // make new entry in decklist new DecklistCardNode(cardName, amount, getZoneObjFromName(zoneName), -1, setCode, collectorNumber); @@ -708,12 +711,11 @@ QString DeckList::getDeckHash() const } /** - * Invalidates the cached deckHash and emits the deckHashChanged signal. + * Invalidates the cached deckHash. */ void DeckList::refreshDeckHash() { cachedDeckHash = QString(); - emit deckHashChanged(); } /** diff --git a/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.h b/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.h index 5340d69fe..d4d1204e6 100644 --- a/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.h +++ b/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.h @@ -1,5 +1,5 @@ /** - * @file decklist.h + * @file deck_list.h * @brief Defines the DeckList class and supporting types for managing a full * deck structure including cards, zones, sideboard plans, and * serialization to/from multiple formats. This is a logic class which @@ -93,7 +93,7 @@ public: * @brief Represents a complete deck, including metadata, zones, cards, * and sideboard plans. * - * A DeckList is a QObject wrapper around an `InnerDecklistNode` tree, + * A DeckList is a wrapper around an `InnerDecklistNode` tree, * enriched with metadata like deck name, comments, tags, banner card, * and multiple sideboard plans. * @@ -110,10 +110,6 @@ public: * - Owns the root `InnerDecklistNode` tree. * - Owns `SideboardPlan` instances stored in `sideboardPlans`. * - * ### Signals: - * - @c deckHashChanged() — emitted when the deck contents change. - * - @c deckTagsChanged() — emitted when tags are added/removed. - * * ### Example workflow: * ``` * DeckList deck; @@ -123,10 +119,8 @@ public: * deck.saveToFile_Native(device); * ``` */ -class DeckList : public QObject +class DeckList { - Q_OBJECT - public: struct Metadata { @@ -158,37 +152,7 @@ private: static void getCardRefListHelper(InnerDecklistNode *item, QList &result); InnerDecklistNode *getZoneObjFromName(const QString &zoneName); -protected: - /** - * @brief Map a card name to its zone. - * Override in subclasses for format-specific logic. - * @param cardName Card being placed. - * @param currentZoneName Zone candidate. - * @return Zone name to use. - */ - virtual QString getCardZoneFromName(const QString /*cardName*/, QString currentZoneName) - { - return currentZoneName; - } - - /** - * @brief Produce the complete display name of a card. - * Override in subclasses to add set suffixes or annotations. - * @param cardName Base name. - * @return Full display name. - */ - virtual QString getCompleteCardName(const QString &cardName) const - { - return cardName; - } - -signals: - /// Emitted when the deck hash changes. - void deckHashChanged(); - /// Emitted when the deck tags are modified. - void deckTagsChanged(); - -public slots: +public: /// @name Metadata setters ///@{ void setName(const QString &_name = QString()) @@ -202,17 +166,14 @@ public slots: void setTags(const QStringList &_tags = QStringList()) { metadata.tags = _tags; - emit deckTagsChanged(); } void addTag(const QString &_tag) { metadata.tags.append(_tag); - emit deckTagsChanged(); } void clearTags() { metadata.tags.clear(); - emit deckTagsChanged(); } void setBannerCard(const CardRef &_bannerCard = {}) { @@ -224,15 +185,13 @@ public slots: } ///@} -public: /// @brief Construct an empty deck. explicit DeckList(); - /// @brief Delete copy constructor. - DeckList(const DeckList &) = delete; - DeckList &operator=(const DeckList &) = delete; + /// @brief Copy constructor (deep copies the node tree) + DeckList(const DeckList &other); /// @brief Construct from a serialized native-format string. explicit DeckList(const QString &nativeString); - ~DeckList() override; + virtual ~DeckList(); /// @name Metadata getters /// The individual metadata getters still exist for backwards compatibility. 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 e3e2de58d..bd16ab3d2 100644 --- a/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model.cpp +++ b/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model.cpp @@ -5,8 +5,11 @@ DeckListModel::DeckListModel(QObject *parent) : QAbstractItemModel(parent), lastKnownColumn(1), lastKnownOrder(Qt::AscendingOrder) { + // This class will leak the decklist object. We cannot safely delete it in the dtor because the deckList field is a + // non-owning pointer and another deckList might have been assigned to it. + // `DeckListModel::cleanList` also leaks for the same reason. + // TODO: fix the leak deckList = new DeckList; - deckList->setParent(this); root = new InnerDecklistNode; } @@ -284,6 +287,7 @@ bool DeckListModel::setData(const QModelIndex &index, const QVariant &value, con emitRecursiveUpdates(index); deckList->refreshDeckHash(); + emit deckHashChanged(); emit dataChanged(index, index); return true; @@ -422,6 +426,7 @@ QModelIndex DeckListModel::addCard(const ExactCard &card, const QString &zoneNam cardNode->setCardCollectorNumber(printingInfo.getProperty("num")); cardNode->setCardProviderId(printingInfo.getProperty("uuid")); deckList->refreshDeckHash(); + emit deckHashChanged(); } sort(lastKnownColumn, lastKnownOrder); emitRecursiveUpdates(parentIndex);