[DeckList] Disable copy constructor (#6339)

* [DeckList] Disable copy constructor Took 1 hour 44 minutes Took 1 minute # Commit time for manual adjustment: # Took 28 seconds Took 33 seconds * Revert member to pointer. Took 19 minutes * Revert pulling up setters/getters now that getDeckList is no longer const. Took 6 minutes * Revert more. Took 2 minutes * One more fix. Took 1 minute * Update cockatrice/src/interface/deck_loader/deck_loader.cpp Co-authored-by: RickyRister <42636155+RickyRister@users.noreply.github.com> --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de> Co-authored-by: RickyRister <42636155+RickyRister@users.noreply.github.com>
2025-12-12 15:49:28 -08:00 · 2025-11-20 13:20:09 +01:00
parent 9957cb20e2
commit ab5d6db8a2
18 changed files with 49 additions and 69 deletions
--- a/cockatrice/src/game/deckview/deck_view.cpp
+++ b/cockatrice/src/game/deckview/deck_view.cpp
@@ -330,7 +330,7 @@ void DeckViewScene::setDeck(const DeckList &_deck)
    if (deck)
        delete deck;

-    deck = new DeckList(_deck);
+    deck = new DeckList(_deck.writeToString_Native());
    rebuildTree();
    applySideboardPlan(deck->getCurrentSideboardPlan());
    rearrangeItems();
--- a/cockatrice/src/game/player/player.cpp
+++ b/cockatrice/src/game/player/player.cpp
@@ -263,7 +263,8 @@ void Player::deleteCard(CardItem *card)
    }
 }

-void Player::setDeck(const DeckLoader &_deck)
+// TODO: Does a player need a DeckLoader?
+void Player::setDeck(DeckLoader &_deck)
 {
    deck = new DeckLoader(this, _deck.getDeckList());

--- a/cockatrice/src/game/player/player.h
+++ b/cockatrice/src/game/player/player.h
@@ -130,7 +130,7 @@ public:
        return playerMenu;
    }

-    void setDeck(const DeckLoader &_deck);
+    void setDeck(DeckLoader &_deck);

    [[nodiscard]] DeckLoader *getDeck() const
    {
--- a/cockatrice/src/interface/deck_loader/deck_loader.cpp
+++ b/cockatrice/src/interface/deck_loader/deck_loader.cpp
@@ -33,13 +33,6 @@ DeckLoader::DeckLoader(QObject *parent)
 DeckLoader::DeckLoader(QObject *parent, DeckList *_deckList)
    : QObject(parent), deckList(_deckList), lastFileFormat(CockatriceFormat), lastRemoteDeckId(-1)
 {
-    deckList->setParent(this);
-}
-
-void DeckLoader::setDeckList(DeckList *_deckList)
-{
-    deckList = _deckList;
-    deckList->setParent(this);
 }

 bool DeckLoader::loadFromFile(const QString &fileName, FileFormat fmt, bool userRequest)
@@ -159,12 +152,14 @@ bool DeckLoader::saveToFile(const QString &fileName, FileFormat fmt)
            break;
        case CockatriceFormat:
            result = deckList->saveToFile_Native(&file);
+            qCInfo(DeckLoaderLog) << "Saving to " << fileName << "-" << result;
            break;
    }

    if (result) {
        lastFileName = fileName;
        lastFileFormat = fmt;
+        qCInfo(DeckLoaderLog) << "Deck was saved -" << result;
    }

    file.flush();
--- a/cockatrice/src/interface/deck_loader/deck_loader.h
+++ b/cockatrice/src/interface/deck_loader/deck_loader.h
@@ -56,8 +56,6 @@ public:
    DeckLoader(const DeckLoader &) = delete;
    DeckLoader &operator=(const DeckLoader &) = delete;

-    void setDeckList(DeckList *_deckList);
-
    const QString &getLastFileName() const
    {
        return lastFileName;
@@ -109,7 +107,7 @@ public:

    bool convertToCockatriceFormat(QString fileName);

-    DeckList *getDeckList() const
+    DeckList *getDeckList()
    {
        return deckList;
    }
--- 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
@@ -109,7 +109,7 @@ void DeckEditorDeckDockWidget::createDeckDock()
            &DeckEditorDeckDockWidget::setBannerCard);
    bannerCardComboBox->setHidden(!SettingsCache::instance().getDeckEditorBannerCardComboBoxVisible());

-    deckTagsDisplayWidget = new DeckPreviewDeckTagsDisplayWidget(this, deckModel->getDeckList());
+    deckTagsDisplayWidget = new DeckPreviewDeckTagsDisplayWidget(this, deckLoader);
    deckTagsDisplayWidget->setHidden(!SettingsCache::instance().getDeckEditorTagsWidgetVisible());

    activeGroupCriteriaLabel = new QLabel(this);
@@ -382,7 +382,7 @@ void DeckEditorDeckDockWidget::setDeck(DeckLoader *_deck)
    deckView->expandAll();
    deckView->expandAll();

-    deckTagsDisplayWidget->connectDeckList(deckModel->getDeckList());
+    deckTagsDisplayWidget->connectDeckList();

    emit deckChanged();
 }
@@ -412,7 +412,7 @@ void DeckEditorDeckDockWidget::cleanDeck()
    emit deckModified();
    emit deckChanged();
    updateBannerCardComboBox();
-    deckTagsDisplayWidget->connectDeckList(deckModel->getDeckList());
+    deckTagsDisplayWidget->connectDeckList();
 }

 void DeckEditorDeckDockWidget::recursiveExpand(const QModelIndex &index)
--- a/cockatrice/src/interface/widgets/dialogs/dlg_load_deck_from_clipboard.cpp
+++ b/cockatrice/src/interface/widgets/dialogs/dlg_load_deck_from_clipboard.cpp
@@ -133,16 +133,16 @@ void DlgLoadDeckFromClipboard::actOK()
 /**
 * Creates the dialog window for the "Edit deck in clipboard" action
 *
- * @param deckList The existing deck in the deck editor. Copies the instance
+ * @param _deckLoader The existing deck in the deck editor. Copies the instance
 * @param _annotated Whether to add annotations to the text that is loaded from the deck
 * @param parent The parent widget
 */
-DlgEditDeckInClipboard::DlgEditDeckInClipboard(const DeckLoader &deckList, bool _annotated, QWidget *parent)
+DlgEditDeckInClipboard::DlgEditDeckInClipboard(DeckLoader *_deckLoader, bool _annotated, QWidget *parent)
    : AbstractDlgDeckTextEdit(parent), annotated(_annotated)
 {
    setWindowTitle(tr("Edit deck in clipboard"));

-    deckLoader = new DeckLoader(this, deckList.getDeckList());
+    deckLoader = new DeckLoader(this, _deckLoader->getDeckList());
    deckLoader->setParent(this);

    DlgEditDeckInClipboard::actRefresh();
--- a/cockatrice/src/interface/widgets/dialogs/dlg_load_deck_from_clipboard.h
+++ b/cockatrice/src/interface/widgets/dialogs/dlg_load_deck_from_clipboard.h
@@ -88,7 +88,7 @@ private:
    bool annotated;

 public:
-    explicit DlgEditDeckInClipboard(const DeckLoader &deckList, bool _annotated, QWidget *parent = nullptr);
+    explicit DlgEditDeckInClipboard(DeckLoader *_deckLoader, bool _annotated, QWidget *parent = nullptr);

    [[nodiscard]] DeckLoader *getDeckList() const override
    {
--- a/cockatrice/src/interface/widgets/tabs/abstract_tab_deck_editor.cpp
+++ b/cockatrice/src/interface/widgets/tabs/abstract_tab_deck_editor.cpp
@@ -486,7 +486,7 @@ void AbstractTabDeckEditor::actLoadDeckFromClipboard()
 */
 void AbstractTabDeckEditor::editDeckInClipboard(bool annotated)
 {
-    DlgEditDeckInClipboard dlg(*getDeckLoader(), annotated, this);
+    DlgEditDeckInClipboard dlg(getDeckLoader(), annotated, this);
    if (!dlg.exec())
        return;

--- a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp
+++ b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp
@@ -242,11 +242,11 @@ void TabDeckStorage::actOpenLocalDeck()
            continue;
        QString filePath = localDirModel->filePath(curLeft);

-        DeckLoader deckLoader(this);
-        if (!deckLoader.loadFromFile(filePath, DeckLoader::CockatriceFormat, true))
+        auto deckLoader = new DeckLoader(this);
+        if (!deckLoader->loadFromFile(filePath, DeckLoader::CockatriceFormat, true))
            continue;

-        emit openDeckEditor(&deckLoader);
+        emit openDeckEditor(deckLoader);
    }
 }

--- a/cockatrice/src/interface/widgets/tabs/tab_supervisor.cpp
+++ b/cockatrice/src/interface/widgets/tabs/tab_supervisor.cpp
@@ -869,7 +869,7 @@ TabDeckEditor *TabSupervisor::addDeckEditorTab(DeckLoader *deckToOpen)
 {
    auto *tab = new TabDeckEditor(this);
    if (deckToOpen)
-        tab->openDeck(new DeckLoader(this, deckToOpen->getDeckList()));
+        tab->openDeck(deckToOpen);
    connect(tab, &AbstractTabDeckEditor::deckEditorClosing, this, &TabSupervisor::deckEditorClosed);
    connect(tab, &AbstractTabDeckEditor::openDeckEditor, this, &TabSupervisor::addDeckEditorTab);
    myAddTab(tab);
@@ -882,7 +882,7 @@ TabDeckEditorVisual *TabSupervisor::addVisualDeckEditorTab(DeckLoader *deckToOpe
 {
    auto *tab = new TabDeckEditorVisual(this);
    if (deckToOpen)
-        tab->openDeck(new DeckLoader(this, deckToOpen->getDeckList()));
+        tab->openDeck(deckToOpen);
    connect(tab, &AbstractTabDeckEditor::deckEditorClosing, this, &TabSupervisor::deckEditorClosed);
    connect(tab, &AbstractTabDeckEditor::openDeckEditor, this, &TabSupervisor::addVisualDeckEditorTab);
    myAddTab(tab);
--- a/cockatrice/src/interface/widgets/tabs/visual_deck_storage/tab_deck_storage_visual.cpp
+++ b/cockatrice/src/interface/widgets/tabs/visual_deck_storage/tab_deck_storage_visual.cpp
@@ -27,11 +27,11 @@ TabDeckStorageVisual::TabDeckStorageVisual(TabSupervisor *_tabSupervisor)

 void TabDeckStorageVisual::actOpenLocalDeck(const QString &filePath)
 {
-    DeckLoader deckLoader(this);
-    if (!deckLoader.loadFromFile(filePath, DeckLoader::getFormatFromName(filePath), true)) {
+    auto deckLoader = new DeckLoader(this);
+    if (!deckLoader->loadFromFile(filePath, DeckLoader::getFormatFromName(filePath), true)) {
        QMessageBox::critical(this, tr("Error"), tr("Could not open deck at %1").arg(filePath));
        return;
    }

-    emit openDeckEditor(&deckLoader);
+    emit openDeckEditor(deckLoader);
 }
--- 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
@@ -14,8 +14,8 @@
 #include <QLabel>
 #include <QMessageBox>

-DeckPreviewDeckTagsDisplayWidget::DeckPreviewDeckTagsDisplayWidget(QWidget *_parent, DeckList *_deckList)
-    : QWidget(_parent), deckList(nullptr)
+DeckPreviewDeckTagsDisplayWidget::DeckPreviewDeckTagsDisplayWidget(QWidget *_parent, DeckLoader *_deckLoader)
+    : QWidget(_parent), deckLoader(_deckLoader)
 {

    setSizePolicy(QSizePolicy::Minimum, QSizePolicy::Minimum);
@@ -28,21 +28,20 @@ DeckPreviewDeckTagsDisplayWidget::DeckPreviewDeckTagsDisplayWidget(QWidget *_par

    flowWidget = new FlowWidget(this, Qt::Horizontal, Qt::ScrollBarAlwaysOff, Qt::ScrollBarAsNeeded);

-    if (_deckList) {
-        connectDeckList(_deckList);
-    }
+    connectDeckList();

    layout->addWidget(flowWidget);
 }

-void DeckPreviewDeckTagsDisplayWidget::connectDeckList(DeckList *_deckList)
+void DeckPreviewDeckTagsDisplayWidget::connectDeckList()
 {
-    if (deckList) {
-        disconnect(deckList, &DeckList::deckTagsChanged, this, &DeckPreviewDeckTagsDisplayWidget::refreshTags);
+    if (deckLoader) {
+        disconnect(deckLoader->getDeckList(), &DeckList::deckTagsChanged, this,
+                   &DeckPreviewDeckTagsDisplayWidget::refreshTags);
    }

-    deckList = _deckList;
-    connect(deckList, &DeckList::deckTagsChanged, this, &DeckPreviewDeckTagsDisplayWidget::refreshTags);
+    connect(deckLoader->getDeckList(), &DeckList::deckTagsChanged, this,
+            &DeckPreviewDeckTagsDisplayWidget::refreshTags);

    refreshTags();
 }
@@ -51,7 +50,7 @@ void DeckPreviewDeckTagsDisplayWidget::refreshTags()
 {
    flowWidget->clearLayout();

-    for (const QString &tag : deckList->getTags()) {
+    for (const QString &tag : deckLoader->getDeckList()->getTags()) {
        flowWidget->addWidget(new DeckPreviewTagDisplayWidget(this, tag));
    }

@@ -98,7 +97,7 @@ void DeckPreviewDeckTagsDisplayWidget::openTagEditDlg()
    if (qobject_cast<DeckPreviewWidget *>(parentWidget())) {
        auto *deckPreviewWidget = qobject_cast<DeckPreviewWidget *>(parentWidget());
        QStringList knownTags = deckPreviewWidget->visualDeckStorageWidget->tagFilterWidget->getAllKnownTags();
-        QStringList activeTags = deckList->getTags();
+        QStringList activeTags = deckLoader->getDeckList()->getTags();

        bool canAddTags = true;

@@ -149,7 +148,7 @@ void DeckPreviewDeckTagsDisplayWidget::openTagEditDlg()
            DeckPreviewTagDialog dialog(knownTags, activeTags);
            if (dialog.exec() == QDialog::Accepted) {
                QStringList updatedTags = dialog.getActiveTags();
-                deckList->setTags(updatedTags);
+                deckLoader->getDeckList()->setTags(updatedTags);
                deckPreviewWidget->deckLoader->saveToFile(deckPreviewWidget->filePath, DeckLoader::CockatriceFormat);
            }
        }
@@ -175,12 +174,12 @@ void DeckPreviewDeckTagsDisplayWidget::openTagEditDlg()
                knownTags.removeDuplicates();
            }

-            QStringList activeTags = deckList->getTags();
+            QStringList activeTags = deckLoader->getDeckList()->getTags();

            DeckPreviewTagDialog dialog(knownTags, activeTags);
            if (dialog.exec() == QDialog::Accepted) {
                QStringList updatedTags = dialog.getActiveTags();
-                deckList->setTags(updatedTags);
+                deckLoader->getDeckList()->setTags(updatedTags);
                deckEditor->setModified(true);
            }
        }
--- 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
@@ -20,10 +20,10 @@ class DeckPreviewDeckTagsDisplayWidget : public QWidget
    Q_OBJECT

 public:
-    explicit DeckPreviewDeckTagsDisplayWidget(QWidget *_parent, DeckList *_deckList);
-    void connectDeckList(DeckList *_deckList);
+    explicit DeckPreviewDeckTagsDisplayWidget(QWidget *_parent, DeckLoader *_deckLoader);
+    void connectDeckList();
    void refreshTags();
-    DeckList *deckList;
+    DeckLoader *deckLoader;
    FlowWidget *flowWidget;

 public slots:
--- a/cockatrice/src/interface/widgets/visual_deck_storage/deck_preview/deck_preview_widget.cpp
+++ b/cockatrice/src/interface/widgets/visual_deck_storage/deck_preview/deck_preview_widget.cpp
@@ -82,7 +82,7 @@ void DeckPreviewWidget::initializeUi(const bool deckLoadSuccess)
    setFilePath(deckLoader->getLastFileName());

    colorIdentityWidget = new ColorIdentityWidget(this, getColorIdentity());
-    deckTagsDisplayWidget = new DeckPreviewDeckTagsDisplayWidget(this, deckLoader->getDeckList());
+    deckTagsDisplayWidget = new DeckPreviewDeckTagsDisplayWidget(this, deckLoader);

    bannerCardLabel = new QLabel(this);
    bannerCardLabel->setObjectName("bannerCardLabel");
--- a/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp
+++ b/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp
@@ -80,20 +80,6 @@ DeckList::DeckList()
    root = new InnerDecklistNode;
 }

-// TODO: https://qt-project.org/doc/qt-4.8/qobject.html#no-copy-constructor-or-assignment-operator
-DeckList::DeckList(const DeckList &other)
-    : QObject(), name(other.name), comments(other.comments), bannerCard(other.bannerCard),
-      lastLoadedTimestamp(other.lastLoadedTimestamp), tags(other.tags), cachedDeckHash(other.cachedDeckHash)
-{
-    root = new InnerDecklistNode(other.getRoot());
-
-    QMapIterator<QString, SideboardPlan *> spIterator(other.getSideboardPlans());
-    while (spIterator.hasNext()) {
-        spIterator.next();
-        sideboardPlans.insert(spIterator.key(), new SideboardPlan(spIterator.key(), spIterator.value()->getMoveList()));
-    }
-}
-
 DeckList::DeckList(const QString &nativeString)
 {
    root = new InnerDecklistNode;
--- a/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.h
+++ b/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.h
@@ -215,8 +215,9 @@ public slots:
 public:
    /// @brief Construct an empty deck.
    explicit DeckList();
-    /// @brief Deep-copy constructor.
-    DeckList(const DeckList &other);
+    /// @brief Delete copy constructor.
+    DeckList(const DeckList &) = delete;
+    DeckList &operator=(const DeckList &) = delete;
    /// @brief Construct from a serialized native-format string.
    explicit DeckList(const QString &nativeString);
    ~DeckList() override;
--- a/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model.cpp
+++ b/libcockatrice_models/libcockatrice/models/deck_list/deck_list_model.cpp
@@ -533,14 +533,14 @@ void DeckListModel::cleanList()
 }

 /**
- * @param _deck The deck. Takes ownership of the object
+ * @param _deck The deck.
 */
 void DeckListModel::setDeckList(DeckList *_deck)
 {
-    deckList->deleteLater();
-    deckList = _deck;
-    deckList->setParent(this);
-    rebuildTree();
+    if (deckList != _deck) {
+        deckList = _deck;
+        rebuildTree();
+    }
 }

 QList<ExactCard> DeckListModel::getCards() const