From 125abf502769c2ef092a2f5516d303f0333ae802 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Mr=C3=A1zek?= Date: Tue, 20 Oct 2015 17:18:53 +0200 Subject: [PATCH] NOISSUE rename QObjectPtr to shared_qobject_ptr, introduce unique_qobject_ptr, refactor MainWindow to match --- application/MainWindow.cpp | 270 ++++++++-------------- application/MainWindow.h | 68 +++--- application/pages/InstanceSettingsPage.h | 2 +- application/pages/global/JavaPage.h | 2 +- application/resources/multimc/multimc.qrc | 7 + logic/QObjectPtr.h | 29 ++- logic/forge/ForgeVersionList.cpp | 4 +- logic/net/NetAction.h | 2 +- logic/net/NetJob.h | 2 +- 9 files changed, 170 insertions(+), 216 deletions(-) diff --git a/application/MainWindow.cpp b/application/MainWindow.cpp index 2469f983..ced644e4 100644 --- a/application/MainWindow.cpp +++ b/application/MainWindow.cpp @@ -21,8 +21,14 @@ #include "MainWindow.h" - #include +#include +#include +#include + +#include +#include + #include #include #include @@ -32,8 +38,61 @@ #include #include #include +#include +#include +#include +#include +#include +#include +#include +#include -class Ui_MainWindow +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "InstancePageProvider.h" +#include "InstanceProxyModel.h" +#include "JavaCommon.h" +#include "LaunchInteraction.h" +#include "SettingsUI.h" +#include "groupview/GroupView.h" +#include "groupview/InstanceDelegate.h" +#include "widgets/LabeledToolButton.h" +#include "widgets/ServerStatus.h" +#include "dialogs/NewInstanceDialog.h" +#include "dialogs/ProgressDialog.h" +#include "dialogs/AboutDialog.h" +#include "dialogs/VersionSelectDialog.h" +#include "dialogs/CustomMessageBox.h" +#include "dialogs/IconPickerDialog.h" +#include "dialogs/CopyInstanceDialog.h" +#include "dialogs/AccountSelectDialog.h" +#include "dialogs/UpdateDialog.h" +#include "dialogs/EditAccountDialog.h" +#include "dialogs/NotificationDialog.h" +#include "dialogs/ExportInstanceDialog.h" + +class MainWindow::Ui { public: QAction *actionAddInstance; @@ -310,91 +369,15 @@ public: }; -namespace Ui { - class MainWindow: public Ui_MainWindow {}; -} // namespace Ui - -#include -#include -#include - -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#include - -#include "groupview/GroupView.h" -#include "groupview/InstanceDelegate.h" -#include "InstanceProxyModel.h" - -#include "widgets/LabeledToolButton.h" -#include "widgets/ServerStatus.h" - -#include "dialogs/NewInstanceDialog.h" -#include "dialogs/ProgressDialog.h" -#include "dialogs/AboutDialog.h" -#include "dialogs/VersionSelectDialog.h" -#include "dialogs/CustomMessageBox.h" -#include "dialogs/IconPickerDialog.h" -#include "dialogs/CopyInstanceDialog.h" -#include "dialogs/AccountSelectDialog.h" -#include "dialogs/UpdateDialog.h" -#include "dialogs/EditAccountDialog.h" -#include "dialogs/NotificationDialog.h" -#include "dialogs/ExportInstanceDialog.h" - -#include "InstanceList.h" -#include "minecraft/MinecraftVersionList.h" -#include "minecraft/LwjglVersionList.h" -#include "icons/IconList.h" -#include "java/JavaVersionList.h" - -#include "auth/flows/AuthenticateTask.h" -#include "auth/flows/RefreshTask.h" - -#include "updater/DownloadTask.h" - -#include "news/NewsChecker.h" - -#include "net/URLConstants.h" -#include "net/NetJob.h" -#include "Env.h" - -#include "BaseInstance.h" -#include "launch/LaunchTask.h" -#include "java/JavaUtils.h" -#include "JavaCommon.h" -#include "InstancePageProvider.h" -#include "LaunchInteraction.h" -#include "SettingsUI.h" -#include "minecraft/SkinUtils.h" -#include "resources/Resource.h" - -#include -#include -#include - -#include "tools/BaseProfiler.h" - -MainWindow::MainWindow(QWidget *parent) : QMainWindow(parent), ui(new Ui::MainWindow) +MainWindow::MainWindow(QWidget *parent) : QMainWindow(parent), ui(new MainWindow::Ui) { ui->setupUi(this); - // initialize the news checker - m_newsChecker.reset(new NewsChecker(BuildConfig.NEWS_RSS_URL)); - - QString winTitle = - QString("MultiMC 5 - Version %1").arg(BuildConfig.printableVersionString()); + QString winTitle = tr("MultiMC 5 - Version %1").arg(BuildConfig.printableVersionString()); if (!BuildConfig.BUILD_PLATFORM.isEmpty()) - winTitle += " on " + BuildConfig.BUILD_PLATFORM; + { + winTitle += tr(" on ") + BuildConfig.BUILD_PLATFORM; + } setWindowTitle(winTitle); // OSX magic. @@ -424,6 +407,7 @@ MainWindow::MainWindow(QWidget *parent) : QMainWindow(parent), ui(new Ui::MainWi // Add the news label to the news toolbar. { + m_newsChecker.reset(new NewsChecker(BuildConfig.NEWS_RSS_URL)); newsLabel = new QToolButton(); newsLabel->setIcon(MMC->getThemedIcon("news")); newsLabel->setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Preferred); @@ -441,41 +425,21 @@ MainWindow::MainWindow(QWidget *parent) : QMainWindow(parent), ui(new Ui::MainWi view = new GroupView(ui->centralWidget); view->setSelectionMode(QAbstractItemView::SingleSelection); - // view->setCategoryDrawer(drawer); - // view->setCollapsibleBlocks(true); - // view->setViewMode(QListView::IconMode); - // view->setFlow(QListView::LeftToRight); - // view->setWordWrap(true); - // view->setMouseTracking(true); - // view->viewport()->setAttribute(Qt::WA_Hover); - auto delegate = new ListViewDelegate(); - view->setItemDelegate(delegate); - // view->setSpacing(10); - // view->setUniformItemWidths(true); - + view->setItemDelegate(new ListViewDelegate()); + view->setFrameShape(QFrame::NoFrame); // do not show ugly blue border on the mac view->setAttribute(Qt::WA_MacShowFocusRect, false); view->installEventFilter(this); + view->setContextMenuPolicy(Qt::CustomContextMenu); + connect(view, &QWidget::customContextMenuRequested, this, &MainWindow::showInstanceContextMenu); proxymodel = new InstanceProxyModel(this); - // proxymodel->setSortRole(KCategorizedSortFilterProxyModel::CategorySortRole); - // proxymodel->setFilterRole(KCategorizedSortFilterProxyModel::CategorySortRole); - // proxymodel->setDynamicSortFilter ( true ); - - // FIXME: instList should be global-ish, or at least not tied to the main window... - // maybe the application itself? proxymodel->setSourceModel(MMC->instances().get()); proxymodel->sort(0); - view->setFrameShape(QFrame::NoFrame); + connect(proxymodel, &InstanceProxyModel::dataChanged, this, &MainWindow::instanceDataChanged); + view->setModel(proxymodel); - - connect(proxymodel, SIGNAL(dataChanged(QModelIndex,QModelIndex,QVector)), SLOT(instanceDataChanged(QModelIndex,QModelIndex))); - - view->setContextMenuPolicy(Qt::CustomContextMenu); - connect(view, SIGNAL(customContextMenuRequested(const QPoint &)), this, - SLOT(showInstanceContextMenu(const QPoint &))); - ui->horizontalLayout->addWidget(view); } // The cat background @@ -486,19 +450,16 @@ MainWindow::MainWindow(QWidget *parent) : QMainWindow(parent), ui(new Ui::MainWi setCatBackground(cat_enable); } // start instance when double-clicked - connect(view, SIGNAL(doubleClicked(const QModelIndex &)), this, - SLOT(instanceActivated(const QModelIndex &))); + connect(view, &GroupView::doubleClicked, this, &MainWindow::instanceActivated); + // track the selection -- update the instance toolbar - connect(view->selectionModel(), - SIGNAL(currentChanged(const QModelIndex &, const QModelIndex &)), this, - SLOT(instanceChanged(const QModelIndex &, const QModelIndex &))); + connect(view->selectionModel(), &QItemSelectionModel::currentChanged, this, &MainWindow::instanceChanged); // track icon changes and update the toolbar! - connect(ENV.icons().get(), SIGNAL(iconUpdated(QString)), SLOT(iconUpdated(QString))); + connect(ENV.icons().get(), &IconList::iconUpdated, this, &MainWindow::iconUpdated); // model reset -> selection is invalid. All the instance pointers are wrong. - // FIXME: stop using POINTERS everywhere - connect(MMC->instances().get(), SIGNAL(dataIsInvalid()), SLOT(selectionBad())); + connect(MMC->instances().get(), &InstanceList::dataIsInvalid, this, &MainWindow::selectionBad); m_statusLeft = new QLabel(tr("No instance selected"), this); m_statusRight = new ServerStatus(this); @@ -513,6 +474,7 @@ MainWindow::MainWindow(QWidget *parent) : QMainWindow(parent), ui(new Ui::MainWi accountMenu = new QMenu(this); manageAccountsAction = new QAction(tr("Manage Accounts"), this); manageAccountsAction->setCheckable(false); + manageAccountsAction->setIcon(MMC->getThemedIcon("accounts")); connect(manageAccountsAction, SIGNAL(triggered(bool)), this, SLOT(on_actionManageAccounts_triggered())); @@ -547,16 +509,17 @@ MainWindow::MainWindow(QWidget *parent) : QMainWindow(parent), ui(new Ui::MainWi for (int i = 0; i < accounts->count(); i++) { auto account = accounts->at(i); - if (account != nullptr) + if (!account) { - for (auto profile : account->profiles()) - { - auto meta = Env::getInstance().metacache()->resolveEntry("skins", profile.id + ".png"); - auto action = CacheDownload::make( - QUrl("https://" + URLConstants::SKINS_BASE + profile.id + ".png"), meta); - skin_dls.append(action); - meta->stale = true; - } + qWarning() << "Null account at index" << i; + continue; + } + for (auto profile : account->profiles()) + { + auto meta = Env::getInstance().metacache()->resolveEntry("skins", profile.id + ".png"); + auto action = CacheDownload::make(QUrl("https://" + URLConstants::SKINS_BASE + profile.id + ".png"), meta); + skin_dls.append(action); + meta->stale = true; } } if (!skin_dls.isEmpty()) @@ -620,8 +583,6 @@ MainWindow::MainWindow(QWidget *parent) : QMainWindow(parent), ui(new Ui::MainWi MainWindow::~MainWindow() { - delete ui; - delete proxymodel; } void MainWindow::skinJobFinished() @@ -760,14 +721,6 @@ void MainWindow::repopulateAccountsMenu() for (int i = 0; i < accounts->count(); i++) { MojangAccountPtr account = accounts->at(i); - - // Styling hack - /* - QAction *section = new QAction(account->username(), this); - section->setEnabled(false); - accountMenu->addAction(section); - */ - for (auto profile : account->profiles()) { QAction *action = new QAction(profile.name, this); @@ -1459,15 +1412,18 @@ void MainWindow::on_mainToolBar_visibilityChanged(bool) void MainWindow::on_actionDeleteInstance_triggered() { - if (m_selectedInstance) + if (!m_selectedInstance) { - auto response = CustomMessageBox::selectable( - this, tr("CAREFUL!"), tr("About to delete: %1\nThis is permanent and will completely erase all data, even for tracked instances!\nAre you sure?").arg(m_selectedInstance->name()), - QMessageBox::Warning, QMessageBox::Yes | QMessageBox::No)->exec(); - if (response == QMessageBox::Yes) - { - m_selectedInstance->nuke(); - } + return; + } + auto response = CustomMessageBox::selectable(this, tr("CAREFUL!"), tr("About to delete: %1\nThis is permanent and will completely erase " + "all data, even for tracked instances!\nAre you sure?") + .arg(m_selectedInstance->name()), + QMessageBox::Warning, QMessageBox::Yes | QMessageBox::No) + ->exec(); + if (response == QMessageBox::Yes) + { + m_selectedInstance->nuke(); } } @@ -1520,17 +1476,7 @@ void MainWindow::closeEvent(QCloseEvent *event) QMainWindow::closeEvent(event); QApplication::exit(); } -/* -void MainWindow::on_instanceView_customContextMenuRequested(const QPoint &pos) -{ - QMenu *instContextMenu = new QMenu("Instance", this); - // Add the actions from the toolbar to the context menu. - instContextMenu->addActions(ui->instanceToolBar->actions()); - - instContextMenu->exec(view->mapToGlobal(pos)); -} -*/ void MainWindow::instanceActivated(QModelIndex index) { if (!index.isValid()) @@ -1561,7 +1507,7 @@ void MainWindow::on_actionLaunchInstanceOffline_triggered() void MainWindow::launch(InstancePtr instance, bool online, BaseProfilerFactory* profiler) { - m_launchController = std::make_shared(); + m_launchController.reset(new LaunchController()); m_launchController->setInstance(instance); m_launchController->setOnline(online); m_launchController->setParentWidget(this); @@ -1569,19 +1515,6 @@ void MainWindow::launch(InstancePtr instance, bool online, BaseProfilerFactory* m_launchController->launch(); } - -/* -void MainWindow::onGameUpdateError(QString error) -{ - CustomMessageBox::selectable(this, tr("Error updating instance"), error, - QMessageBox::Warning)->show(); -} -*/ -void MainWindow::taskStart() -{ - // Nothing to do here yet. -} - void MainWindow::taskEnd() { QObject *sender = QObject::sender(); @@ -1593,7 +1526,6 @@ void MainWindow::taskEnd() void MainWindow::startTask(Task *task) { - connect(task, SIGNAL(started()), SLOT(taskStart())); connect(task, SIGNAL(succeeded()), SLOT(taskEnd())); connect(task, SIGNAL(failed(QString)), SLOT(taskEnd())); task->start(); diff --git a/application/MainWindow.h b/application/MainWindow.h index 7f3f4c63..0685c9f8 100644 --- a/application/MainWindow.h +++ b/application/MainWindow.h @@ -15,6 +15,8 @@ #pragma once +#include + #include #include #include @@ -33,32 +35,26 @@ class LabeledToolButton; class QLabel; class MinecraftLauncher; class BaseProfilerFactory; - -namespace Ui -{ -class MainWindow; -} +class GroupView; +class ServerStatus; class MainWindow : public QMainWindow { Q_OBJECT + class Ui; + public: explicit MainWindow(QWidget *parent = 0); ~MainWindow(); - void closeEvent(QCloseEvent *event); - - // Browser Dialog - void openWebPage(QUrl url); + virtual bool eventFilter(QObject *obj, QEvent *ev) override; + virtual void closeEvent(QCloseEvent *event) override; void checkSetDefaultJava(); void checkInstancePathForProblems(); -private -slots: - void setSelectedInstanceById(const QString &id); - +private slots: void onCatToggled(bool); void on_actionAbout_triggered(); @@ -101,8 +97,6 @@ slots: void on_mainToolBar_visibilityChanged(bool); - // void on_instanceView_customContextMenuRequested(const QPoint &pos); - void on_actionLaunchInstance_triggered(); void on_actionLaunchInstanceOffline_triggered(); @@ -119,19 +113,19 @@ slots: void on_actionScreenshots_triggered(); - void taskStart(); void taskEnd(); - // called when an icon is changed in the icon model. + /** + * called when an icon is changed in the icon model. + */ void iconUpdated(QString); void showInstanceContextMenu(const QPoint &); void updateToolsMenu(); - void skinJobFinished(); -public -slots: + void skinJobFinished(); + void instanceActivated(QModelIndex); void instanceChanged(const QModelIndex ¤t, const QModelIndex &previous); @@ -161,10 +155,10 @@ slots: */ void downloadUpdates(GoUpdate::Status status); -protected: - bool eventFilter(QObject *obj, QEvent *ev); +private: void setCatBackground(bool enabled); void updateInstanceToolIcon(QString new_icon); + void setSelectedInstanceById(const QString &id); void waitForMinecraftVersions(); InstancePtr instanceFromVersion(QString instName, QString instGroup, QString instIcon, BaseVersionPtr version); @@ -172,28 +166,32 @@ protected: void finalizeInstance(InstancePtr inst); void launch(InstancePtr instance, bool online = true, BaseProfilerFactory *profiler = nullptr); + /// open a web page in the default browser + void openWebPage(QUrl url); + private: - Ui::MainWindow *ui; - class GroupView *view; + std::unique_ptr ui; + + // these are managed by Qt's memory management model! + GroupView *view; InstanceProxyModel *proxymodel; - NetJobPtr skin_download_job; LabeledToolButton *renameButton; QToolButton *changeIconButton; QToolButton *newsLabel; + QLabel *m_statusLeft; + ServerStatus *m_statusRight; + QMenu *accountMenu; + QToolButton *accountMenuButton; + QAction *manageAccountsAction; - std::shared_ptr m_newsChecker; - std::shared_ptr m_notificationChecker; - std::shared_ptr m_launchController; + unique_qobject_ptr skin_download_job; + unique_qobject_ptr m_newsChecker; + unique_qobject_ptr m_notificationChecker; + unique_qobject_ptr m_launchController; InstancePtr m_selectedInstance; QString m_currentInstIcon; + // managed by the application object Task *m_versionLoadTask; - - QLabel *m_statusLeft; - class ServerStatus *m_statusRight; - - QMenu *accountMenu; - QToolButton *accountMenuButton; - QAction *manageAccountsAction; }; diff --git a/application/pages/InstanceSettingsPage.h b/application/pages/InstanceSettingsPage.h index 1fc0bdfe..33efe6c3 100644 --- a/application/pages/InstanceSettingsPage.h +++ b/application/pages/InstanceSettingsPage.h @@ -70,5 +70,5 @@ private: Ui::InstanceSettingsPage *ui; BaseInstance *m_instance; SettingsObjectPtr m_settings; - QObjectPtr checker; + unique_qobject_ptr checker; }; diff --git a/application/pages/global/JavaPage.h b/application/pages/global/JavaPage.h index 4dd2b306..7ae068ab 100644 --- a/application/pages/global/JavaPage.h +++ b/application/pages/global/JavaPage.h @@ -68,5 +68,5 @@ slots: private: Ui::JavaPage *ui; - QObjectPtr checker; + unique_qobject_ptr checker; }; diff --git a/application/resources/multimc/multimc.qrc b/application/resources/multimc/multimc.qrc index 76559282..1300bdd7 100644 --- a/application/resources/multimc/multimc.qrc +++ b/application/resources/multimc/multimc.qrc @@ -210,6 +210,13 @@ 32x32/noaccount.png 48x48/noaccount.png + + 8x8/noaccount.png + 16x16/noaccount.png + 24x24/noaccount.png + 32x32/noaccount.png + 48x48/noaccount.png + 16x16/log.png 24x24/log.png diff --git a/logic/QObjectPtr.h b/logic/QObjectPtr.h index 32e59bd9..b81b3234 100644 --- a/logic/QObjectPtr.h +++ b/logic/QObjectPtr.h @@ -1,26 +1,43 @@ #pragma once #include +#include + +namespace details +{ +struct DeleteQObjectLater +{ + void operator()(QObject *obj) const + { + obj->deleteLater(); + } +}; +} +/** + * A unique pointer class with unique pointer semantics intended for derivates of QObject + * Calls deleteLater() instead of destroying the contained object immediately + */ +template using unique_qobject_ptr = std::unique_ptr; /** - * A pointer class with the usual shared pointer semantics intended for derivates of QObject + * A shared pointer class with shared pointer semantics intended for derivates of QObject * Calls deleteLater() instead of destroying the contained object immediately */ template -class QObjectPtr +class shared_qobject_ptr { public: - QObjectPtr(){} - QObjectPtr(T * wrap) + shared_qobject_ptr(){} + shared_qobject_ptr(T * wrap) { reset(wrap); } - QObjectPtr(const QObjectPtr& other) + shared_qobject_ptr(const shared_qobject_ptr& other) { m_ptr = other.m_ptr; } template - QObjectPtr(const QObjectPtr &other) + shared_qobject_ptr(const shared_qobject_ptr &other) { m_ptr = other.unwrap(); } diff --git a/logic/forge/ForgeVersionList.cpp b/logic/forge/ForgeVersionList.cpp index 7f2176fd..a05ced99 100644 --- a/logic/forge/ForgeVersionList.cpp +++ b/logic/forge/ForgeVersionList.cpp @@ -424,7 +424,7 @@ void ForgeListLoadTask::listDownloaded() void ForgeListLoadTask::listFailed() { - auto reply = listDownload->m_reply; + auto &reply = listDownload->m_reply; if (reply) { qCritical() << "Getting forge version list failed: " << reply->errorString(); @@ -437,7 +437,7 @@ void ForgeListLoadTask::listFailed() void ForgeListLoadTask::gradleListFailed() { - auto reply = gradleListDownload->m_reply; + auto &reply = gradleListDownload->m_reply; if (reply) { qCritical() << "Getting forge version list failed: " << reply->errorString(); diff --git a/logic/net/NetAction.h b/logic/net/NetAction.h index faf4dbe0..1d7eb94b 100644 --- a/logic/net/NetAction.h +++ b/logic/net/NetAction.h @@ -61,7 +61,7 @@ public: public: /// the network reply - QObjectPtr m_reply; + unique_qobject_ptr m_reply; /// the content of the content-type header QString m_content_type; diff --git a/logic/net/NetJob.h b/logic/net/NetJob.h index 85a8bf83..45f6dacf 100644 --- a/logic/net/NetJob.h +++ b/logic/net/NetJob.h @@ -27,7 +27,7 @@ #include "multimc_logic_export.h" class NetJob; -typedef QObjectPtr NetJobPtr; +typedef shared_qobject_ptr NetJobPtr; class MULTIMC_LOGIC_EXPORT NetJob : public Task {