From fbc29b6a0626f2ce8521dc74e3171b634d68e9e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Mr=C3=A1zek?= Date: Sun, 30 Mar 2014 20:11:05 +0200 Subject: Fix many memory leaks. --- gui/MainWindow.cpp | 72 ++++++++++++++++++++++++-------------- gui/MainWindow.h | 9 +++-- gui/dialogs/CopyInstanceDialog.cpp | 2 +- gui/dialogs/CopyInstanceDialog.h | 5 +-- gui/groupview/InstanceDelegate.cpp | 1 + 5 files changed, 56 insertions(+), 33 deletions(-) (limited to 'gui') diff --git a/gui/MainWindow.cpp b/gui/MainWindow.cpp index 60b64cb9..480ee3b1 100644 --- a/gui/MainWindow.cpp +++ b/gui/MainWindow.cpp @@ -269,27 +269,32 @@ MainWindow::MainWindow(QWidget *parent) : QMainWindow(parent), ui(new Ui::MainWi auto accounts = MMC->accounts(); - // TODO: Nicer way to iterate? + QList skin_dls; for (int i = 0; i < accounts->count(); i++) { auto account = accounts->at(i); if (account != nullptr) { - auto job = new NetJob("Startup player skins: " + account->username()); - for (auto profile : account->profiles()) { auto meta = MMC->metacache()->resolveEntry("skins", profile.name + ".png"); auto action = CacheDownload::make( QUrl("http://" + URLConstants::SKINS_BASE + profile.name + ".png"), meta); - job->addNetAction(action); + skin_dls.append(action); meta->stale = true; } - - connect(job, SIGNAL(succeeded()), SLOT(activeAccountChanged())); - job->start(); } } + if(!skin_dls.isEmpty()) + { + auto job = new NetJob("Startup player skins download"); + connect(job, SIGNAL(succeeded()), SLOT(skinJobFinished())); + connect(job, SIGNAL(failed()), SLOT(skinJobFinished())); + for(auto action: skin_dls) + job->addNetAction(action); + skin_download_job.reset(job); + job->start(); + } // run the things that load and download other things... FIXME: this is NOT the place // FIXME: invisible actions in the background = NOPE. @@ -338,6 +343,13 @@ MainWindow::~MainWindow() delete proxymodel; } +void MainWindow::skinJobFinished() +{ + activeAccountChanged(); + skin_download_job.reset(); +} + + void MainWindow::showInstanceContextMenu(const QPoint &pos) { if (!view->indexAt(pos).isValid()) @@ -748,7 +760,7 @@ void MainWindow::on_actionAddInstance_triggered() if (!newInstDlg.exec()) return; - BaseInstance *newInstance = NULL; + InstancePtr newInstance; QString instancesDir = MMC->settings()->get("InstanceDir").toString(); QString instDirName = DirNameFromString(newInstDlg.instName(), instancesDir); @@ -825,7 +837,7 @@ void MainWindow::on_actionCopyInstance_triggered() auto &loader = InstanceFactory::get(); - BaseInstance *newInstance = NULL; + InstancePtr newInstance; auto error = loader.copyInstance(newInstance, m_selectedInstance, instDir); QString errorMsg = tr("Failed to create instance %1: ").arg(instDirName); @@ -834,7 +846,7 @@ void MainWindow::on_actionCopyInstance_triggered() case InstanceFactory::NoCreateError: newInstance->setName(copyInstDlg.instName()); newInstance->setIconKey(copyInstDlg.iconKey()); - MMC->instances()->add(InstancePtr(newInstance)); + MMC->instances()->add(newInstance); return; case InstanceFactory::InstExists: @@ -1084,9 +1096,10 @@ void MainWindow::instanceActivated(QModelIndex index) { if (!index.isValid()) return; - - BaseInstance *inst = - (BaseInstance *)index.data(InstanceList::InstancePointerRole).value(); + QString id = index.data(InstanceList::InstanceIDRole).toString(); + InstancePtr inst = MMC->instances()->getInstanceById(id); + if(!inst) + return; NagUtils::checkJVMArgs(inst->settings().get("JvmArgs").toString(), this); @@ -1239,7 +1252,7 @@ void MainWindow::doLaunch(bool online, BaseProfilerFactory *profiler) } } -void MainWindow::updateInstance(BaseInstance *instance, AuthSessionPtr session, BaseProfilerFactory *profiler) +void MainWindow::updateInstance(InstancePtr instance, AuthSessionPtr session, BaseProfilerFactory *profiler) { auto updateTask = instance->doUpdate(); if (!updateTask) @@ -1254,7 +1267,7 @@ void MainWindow::updateInstance(BaseInstance *instance, AuthSessionPtr session, tDialog.exec(updateTask.get()); } -void MainWindow::launchInstance(BaseInstance *instance, AuthSessionPtr session, BaseProfilerFactory *profiler) +void MainWindow::launchInstance(InstancePtr instance, AuthSessionPtr session, BaseProfilerFactory *profiler) { Q_ASSERT_X(instance != NULL, "launchInstance", "instance is NULL"); Q_ASSERT_X(session.get() != nullptr, "launchInstance", "session is NULL"); @@ -1427,8 +1440,9 @@ void MainWindow::on_actionChangeInstLWJGLVersion_triggered() lselect.exec(); if (lselect.result() == QDialog::Accepted) { - LegacyInstance *linst = (LegacyInstance *)m_selectedInstance; - linst->setLWJGLVersion(lselect.selectedVersion()); + auto ptr = std::dynamic_pointer_cast(m_selectedInstance); + if(ptr) + ptr->setLWJGLVersion(lselect.selectedVersion()); } } @@ -1444,10 +1458,15 @@ void MainWindow::on_actionInstanceSettings_triggered() void MainWindow::instanceChanged(const QModelIndex ¤t, const QModelIndex &previous) { - if (current.isValid() && - nullptr != (m_selectedInstance = - (BaseInstance *)current.data(InstanceList::InstancePointerRole) - .value())) + if(!current.isValid()) + { + selectionBad(); + MMC->settings()->set("SelectedInstance", QString()); + return; + } + QString id = current.data(InstanceList::InstanceIDRole).toString(); + m_selectedInstance = MMC->instances()->getInstanceById(id); + if ( m_selectedInstance ) { ui->instanceToolBar->setEnabled(m_selectedInstance->canLaunch()); renameButton->setText(m_selectedInstance->name()); @@ -1466,9 +1485,9 @@ void MainWindow::instanceChanged(const QModelIndex ¤t, const QModelIndex & } else { - selectionBad(); - - MMC->settings()->set("SelectedInstance", QString()); + selectionBad(); + MMC->settings()->set("SelectedInstance", QString()); + return; } } @@ -1490,14 +1509,13 @@ void MainWindow::on_actionEditInstNotes_triggered() { if (!m_selectedInstance) return; - LegacyInstance *linst = (LegacyInstance *)m_selectedInstance; - EditNotesDialog noteedit(linst->notes(), linst->name(), this); + EditNotesDialog noteedit(m_selectedInstance->notes(), m_selectedInstance->name(), this); noteedit.exec(); if (noteedit.result() == QDialog::Accepted) { - linst->setNotes(noteedit.getText()); + m_selectedInstance->setNotes(noteedit.getText()); } } diff --git a/gui/MainWindow.h b/gui/MainWindow.h index 3a2843f8..5ddfef7b 100644 --- a/gui/MainWindow.h +++ b/gui/MainWindow.h @@ -23,6 +23,7 @@ #include "logic/BaseInstance.h" #include "logic/auth/MojangAccount.h" +#include class QToolButton; class LabeledToolButton; @@ -118,12 +119,12 @@ slots: * Launches the given instance with the given account. * This function assumes that the given account has a valid, usable access token. */ - void launchInstance(BaseInstance *instance, AuthSessionPtr session, BaseProfilerFactory *profiler = 0); + void launchInstance(InstancePtr instance, AuthSessionPtr session, BaseProfilerFactory *profiler = 0); /*! * Prepares the given instance for launch with the given account. */ - void updateInstance(BaseInstance *instance, AuthSessionPtr account, BaseProfilerFactory *profiler = 0); + void updateInstance(InstancePtr instance, AuthSessionPtr account, BaseProfilerFactory *profiler = 0); void onGameUpdateError(QString error); @@ -145,6 +146,7 @@ slots: void updateToolsMenu(); + void skinJobFinished(); public slots: void instanceActivated(QModelIndex); @@ -189,13 +191,14 @@ private: Ui::MainWindow *ui; class GroupView *view; InstanceProxyModel *proxymodel; + NetJobPtr skin_download_job; MinecraftProcess *proc; ConsoleWindow *console; LabeledToolButton *renameButton; QToolButton *changeIconButton; QToolButton *newsLabel; - BaseInstance *m_selectedInstance; + InstancePtr m_selectedInstance; QString m_currentInstIcon; Task *m_versionLoadTask; diff --git a/gui/dialogs/CopyInstanceDialog.cpp b/gui/dialogs/CopyInstanceDialog.cpp index 4095408b..71429367 100644 --- a/gui/dialogs/CopyInstanceDialog.cpp +++ b/gui/dialogs/CopyInstanceDialog.cpp @@ -32,7 +32,7 @@ #include "logic/tasks/Task.h" #include "logic/BaseInstance.h" -CopyInstanceDialog::CopyInstanceDialog(BaseInstance *original, QWidget *parent) +CopyInstanceDialog::CopyInstanceDialog(InstancePtr original, QWidget *parent) :QDialog(parent), ui(new Ui::CopyInstanceDialog), m_original(original) { MultiMCPlatform::fixWM_CLASS(this); diff --git a/gui/dialogs/CopyInstanceDialog.h b/gui/dialogs/CopyInstanceDialog.h index 7ab366e2..95f4bf6c 100644 --- a/gui/dialogs/CopyInstanceDialog.h +++ b/gui/dialogs/CopyInstanceDialog.h @@ -17,6 +17,7 @@ #include #include "logic/BaseVersion.h" +#include class BaseInstance; @@ -30,7 +31,7 @@ class CopyInstanceDialog : public QDialog Q_OBJECT public: - explicit CopyInstanceDialog(BaseInstance *original, QWidget *parent = 0); + explicit CopyInstanceDialog(InstancePtr original, QWidget *parent = 0); ~CopyInstanceDialog(); void updateDialogState(); @@ -46,5 +47,5 @@ slots: private: Ui::CopyInstanceDialog *ui; QString InstIconKey; - BaseInstance *m_original; + InstancePtr m_original; }; diff --git a/gui/groupview/InstanceDelegate.cpp b/gui/groupview/InstanceDelegate.cpp index 7ca4d7b4..cd26ddaa 100644 --- a/gui/groupview/InstanceDelegate.cpp +++ b/gui/groupview/InstanceDelegate.cpp @@ -317,6 +317,7 @@ void ListViewDelegate::paint(QPainter *painter, const QStyleOptionViewItem &opti line.draw(painter, position); } + // FIXME: this really has no business of being here. Make generic. auto instance = (BaseInstance*)index.data(InstanceList::InstancePointerRole) .value(); if (instance) -- cgit v1.2.3