From ab5d6db8a2ac6ec6273ae4c8299f94541ee64c79 Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Thu, 20 Nov 2025 13:20:09 +0100 Subject: [PATCH] [DeckList] Disable copy constructor (#6339) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [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 Co-authored-by: RickyRister <42636155+RickyRister@users.noreply.github.com> --- cockatrice/src/game/deckview/deck_view.cpp | 2 +- cockatrice/src/game/player/player.cpp | 3 +- cockatrice/src/game/player/player.h | 2 +- .../src/interface/deck_loader/deck_loader.cpp | 9 ++---- .../src/interface/deck_loader/deck_loader.h | 4 +-- .../deck_editor_deck_dock_widget.cpp | 6 ++-- .../dialogs/dlg_load_deck_from_clipboard.cpp | 6 ++-- .../dialogs/dlg_load_deck_from_clipboard.h | 2 +- .../widgets/tabs/abstract_tab_deck_editor.cpp | 2 +- .../widgets/tabs/tab_deck_storage.cpp | 6 ++-- .../interface/widgets/tabs/tab_supervisor.cpp | 4 +-- .../tab_deck_storage_visual.cpp | 6 ++-- .../deck_preview_deck_tags_display_widget.cpp | 29 +++++++++---------- .../deck_preview_deck_tags_display_widget.h | 6 ++-- .../deck_preview/deck_preview_widget.cpp | 2 +- .../libcockatrice/deck_list/deck_list.cpp | 14 --------- .../libcockatrice/deck_list/deck_list.h | 5 ++-- .../models/deck_list/deck_list_model.cpp | 10 +++---- 18 files changed, 49 insertions(+), 69 deletions(-) diff --git a/cockatrice/src/game/deckview/deck_view.cpp b/cockatrice/src/game/deckview/deck_view.cpp index 48c906100..64e9abc46 100644 --- 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(); diff --git a/cockatrice/src/game/player/player.cpp b/cockatrice/src/game/player/player.cpp index 1ff2eb381..ffb26c26e 100644 --- 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()); diff --git a/cockatrice/src/game/player/player.h b/cockatrice/src/game/player/player.h index 151a7d30c..0751abb14 100644 --- 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 { diff --git a/cockatrice/src/interface/deck_loader/deck_loader.cpp b/cockatrice/src/interface/deck_loader/deck_loader.cpp index 3d739cade..320302142 100644 --- 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(); diff --git a/cockatrice/src/interface/deck_loader/deck_loader.h b/cockatrice/src/interface/deck_loader/deck_loader.h index be4601cd3..57305b7de 100644 --- 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; } 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 7a4820dd5..11818a912 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 @@ -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) diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_load_deck_from_clipboard.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_load_deck_from_clipboard.cpp index 6ba05f0b0..2a276479f 100644 --- 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(); diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_load_deck_from_clipboard.h b/cockatrice/src/interface/widgets/dialogs/dlg_load_deck_from_clipboard.h index d93a020cc..982840228 100644 --- 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 { diff --git a/cockatrice/src/interface/widgets/tabs/abstract_tab_deck_editor.cpp b/cockatrice/src/interface/widgets/tabs/abstract_tab_deck_editor.cpp index 65e1e4e71..8a26b3a6a 100644 --- 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; diff --git a/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp b/cockatrice/src/interface/widgets/tabs/tab_deck_storage.cpp index bf74b73fb..b483f0616 100644 --- 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); } } diff --git a/cockatrice/src/interface/widgets/tabs/tab_supervisor.cpp b/cockatrice/src/interface/widgets/tabs/tab_supervisor.cpp index 6d550be01..902b1f589 100644 --- 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); diff --git 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 index b4749f408..bfb498909 100644 --- 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); } 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 eb105697f..e5cbc0adb 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 @@ -14,8 +14,8 @@ #include #include -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(parentWidget())) { auto *deckPreviewWidget = qobject_cast(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); } } 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..645c769bd 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 @@ -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: diff --git 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 index 8b2bec10b..ba111100d 100644 --- 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"); diff --git a/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp b/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.cpp index a12c6fe0e..be26c51a3 100644 --- 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 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; diff --git a/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.h b/libcockatrice_deck_list/libcockatrice/deck_list/deck_list.h index b724efc0f..824d62f45 100644 --- 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; 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 52e3b678c..ed77b55ca 100644 --- 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 DeckListModel::getCards() const