From 654157096985d0cec0d3b7bb53f39a5c0157206e Mon Sep 17 00:00:00 2001 From: flow Date: Thu, 28 Jul 2022 15:58:04 -0300 Subject: [PATCH] fix: simplify abort handling and add missing emits Signed-off-by: flow --- launcher/InstanceImportTask.cpp | 11 ++++++++--- launcher/InstanceImportTask.h | 1 + launcher/InstanceList.cpp | 17 ++++++++++------- .../atlauncher/ATLPackInstallTask.cpp | 18 ++++++++++++++++++ .../atlauncher/ATLPackInstallTask.h | 1 + .../modplatform/legacy_ftb/PackFetchTask.cpp | 14 ++++++++++++++ .../modplatform/legacy_ftb/PackFetchTask.h | 2 ++ .../modplatform/legacy_ftb/PackInstallTask.cpp | 6 ++++++ .../modplatform/legacy_ftb/PackInstallTask.h | 1 + .../modpacksch/FTBPackInstallTask.cpp | 3 +-- .../technic/SolderPackInstallTask.cpp | 8 ++++++++ .../technic/SolderPackInstallTask.h | 1 + launcher/ui/MainWindow.cpp | 4 ++++ launcher/ui/dialogs/ProgressDialog.cpp | 5 ++--- .../ui/pages/modplatform/legacy_ftb/Page.cpp | 8 +++++++- .../ui/pages/modplatform/legacy_ftb/Page.h | 1 + 16 files changed, 85 insertions(+), 16 deletions(-) diff --git a/launcher/InstanceImportTask.cpp b/launcher/InstanceImportTask.cpp index 09e65064..56aabb2d 100644 --- a/launcher/InstanceImportTask.cpp +++ b/launcher/InstanceImportTask.cpp @@ -91,6 +91,7 @@ void InstanceImportTask::executeTask() connect(m_filesNetJob.get(), &NetJob::succeeded, this, &InstanceImportTask::downloadSucceeded); connect(m_filesNetJob.get(), &NetJob::progress, this, &InstanceImportTask::downloadProgressChanged); connect(m_filesNetJob.get(), &NetJob::failed, this, &InstanceImportTask::downloadFailed); + connect(m_filesNetJob.get(), &NetJob::aborted, this, &InstanceImportTask::downloadAborted); m_filesNetJob->start(); } @@ -113,6 +114,12 @@ void InstanceImportTask::downloadProgressChanged(qint64 current, qint64 total) setProgress(current, total); } +void InstanceImportTask::downloadAborted() +{ + emitAborted(); + m_filesNetJob.reset(); +} + void InstanceImportTask::processZipPack() { setStatus(tr("Extracting modpack")); @@ -246,9 +253,7 @@ void InstanceImportTask::extractFinished() void InstanceImportTask::extractAborted() { - emitFailed(tr("Instance import has been aborted.")); - emit aborted(); - return; + emitAborted(); } void InstanceImportTask::processFlame() diff --git a/launcher/InstanceImportTask.h b/launcher/InstanceImportTask.h index 48ba2161..acb1f9d6 100644 --- a/launcher/InstanceImportTask.h +++ b/launcher/InstanceImportTask.h @@ -80,6 +80,7 @@ private slots: void downloadSucceeded(); void downloadFailed(QString reason); void downloadProgressChanged(qint64 current, qint64 total); + void downloadAborted(); void extractFinished(); void extractAborted(); diff --git a/launcher/InstanceList.cpp b/launcher/InstanceList.cpp index 0e59150e..5f604f9a 100644 --- a/launcher/InstanceList.cpp +++ b/launcher/InstanceList.cpp @@ -784,6 +784,7 @@ class InstanceStaging : public Task { m_child.reset(child); connect(child, &Task::succeeded, this, &InstanceStaging::childSucceded); connect(child, &Task::failed, this, &InstanceStaging::childFailed); + connect(child, &Task::aborted, this, &InstanceStaging::childAborted); connect(child, &Task::status, this, &InstanceStaging::setStatus); connect(child, &Task::progress, this, &InstanceStaging::setProgress); connect(&m_backoffTimer, &QTimer::timeout, this, &InstanceStaging::childSucceded); @@ -794,17 +795,14 @@ class InstanceStaging : public Task { // FIXME/TODO: add ability to abort during instance commit retries bool abort() override { - if (m_child && m_child->canAbort()) { + if (m_child && m_child->canAbort()) return m_child->abort(); - } + return false; } bool canAbort() const override { - if (m_child && m_child->canAbort()) { - return true; - } - return false; + return (m_child && m_child->canAbort()); } protected: @@ -834,7 +832,12 @@ class InstanceStaging : public Task { emitFailed(reason); } - private: + void childAborted() + { + emitAborted(); + } + +private: InstanceList * m_parent; /* * WHY: the whole reason why this uses an exponential backoff retry scheme is antivirus on Windows. diff --git a/launcher/modplatform/atlauncher/ATLPackInstallTask.cpp b/launcher/modplatform/atlauncher/ATLPackInstallTask.cpp index 13cab256..a553eafd 100644 --- a/launcher/modplatform/atlauncher/ATLPackInstallTask.cpp +++ b/launcher/modplatform/atlauncher/ATLPackInstallTask.cpp @@ -90,6 +90,7 @@ void PackInstallTask::executeTask() QObject::connect(netJob, &NetJob::succeeded, this, &PackInstallTask::onDownloadSucceeded); QObject::connect(netJob, &NetJob::failed, this, &PackInstallTask::onDownloadFailed); + QObject::connect(netJob, &NetJob::aborted, this, &PackInstallTask::onDownloadAborted); } void PackInstallTask::onDownloadSucceeded() @@ -169,6 +170,12 @@ void PackInstallTask::onDownloadFailed(QString reason) emitFailed(reason); } +void PackInstallTask::onDownloadAborted() +{ + jobPtr.reset(); + emitAborted(); +} + void PackInstallTask::deleteExistingFiles() { setStatus(tr("Deleting existing files...")); @@ -675,6 +682,11 @@ void PackInstallTask::installConfigs() abortable = true; setProgress(current, total); }); + connect(jobPtr.get(), &NetJob::aborted, [&]{ + abortable = false; + jobPtr.reset(); + emitAborted(); + }); jobPtr->start(); } @@ -831,6 +843,12 @@ void PackInstallTask::downloadMods() abortable = true; setProgress(current, total); }); + connect(jobPtr.get(), &NetJob::aborted, [&] + { + abortable = false; + jobPtr.reset(); + emitAborted(); + }); jobPtr->start(); } diff --git a/launcher/modplatform/atlauncher/ATLPackInstallTask.h b/launcher/modplatform/atlauncher/ATLPackInstallTask.h index a7124d59..ed4436f0 100644 --- a/launcher/modplatform/atlauncher/ATLPackInstallTask.h +++ b/launcher/modplatform/atlauncher/ATLPackInstallTask.h @@ -93,6 +93,7 @@ protected: private slots: void onDownloadSucceeded(); void onDownloadFailed(QString reason); + void onDownloadAborted(); void onModsDownloaded(); void onModsExtracted(); diff --git a/launcher/modplatform/legacy_ftb/PackFetchTask.cpp b/launcher/modplatform/legacy_ftb/PackFetchTask.cpp index 4da6a866..36aa60c7 100644 --- a/launcher/modplatform/legacy_ftb/PackFetchTask.cpp +++ b/launcher/modplatform/legacy_ftb/PackFetchTask.cpp @@ -59,6 +59,7 @@ void PackFetchTask::fetch() QObject::connect(jobPtr.get(), &NetJob::succeeded, this, &PackFetchTask::fileDownloadFinished); QObject::connect(jobPtr.get(), &NetJob::failed, this, &PackFetchTask::fileDownloadFailed); + QObject::connect(jobPtr.get(), &NetJob::aborted, this, &PackFetchTask::fileDownloadAborted); jobPtr->start(); } @@ -98,6 +99,14 @@ void PackFetchTask::fetchPrivate(const QStringList & toFetch) delete data; }); + QObject::connect(job, &NetJob::aborted, this, [this, job, data]{ + emit aborted(); + job->deleteLater(); + + data->clear(); + delete data; + }); + job->start(); } } @@ -204,4 +213,9 @@ void PackFetchTask::fileDownloadFailed(QString reason) emit failed(reason); } +void PackFetchTask::fileDownloadAborted() +{ + emit aborted(); +} + } diff --git a/launcher/modplatform/legacy_ftb/PackFetchTask.h b/launcher/modplatform/legacy_ftb/PackFetchTask.h index f1667e90..8f3c4f3b 100644 --- a/launcher/modplatform/legacy_ftb/PackFetchTask.h +++ b/launcher/modplatform/legacy_ftb/PackFetchTask.h @@ -33,10 +33,12 @@ private: protected slots: void fileDownloadFinished(); void fileDownloadFailed(QString reason); + void fileDownloadAborted(); signals: void finished(ModpackList publicPacks, ModpackList thirdPartyPacks); void failed(QString reason); + void aborted(); void privateFileDownloadFinished(Modpack modpack); void privateFileDownloadFailed(QString reason, QString packCode); diff --git a/launcher/modplatform/legacy_ftb/PackInstallTask.cpp b/launcher/modplatform/legacy_ftb/PackInstallTask.cpp index e2e61a7c..209ad884 100644 --- a/launcher/modplatform/legacy_ftb/PackInstallTask.cpp +++ b/launcher/modplatform/legacy_ftb/PackInstallTask.cpp @@ -86,6 +86,7 @@ void PackInstallTask::downloadPack() connect(netJobContainer.get(), &NetJob::succeeded, this, &PackInstallTask::onDownloadSucceeded); connect(netJobContainer.get(), &NetJob::failed, this, &PackInstallTask::onDownloadFailed); connect(netJobContainer.get(), &NetJob::progress, this, &PackInstallTask::onDownloadProgress); + connect(netJobContainer.get(), &NetJob::aborted, this, &PackInstallTask::onDownloadAborted); netJobContainer->start(); progress(1, 4); @@ -110,6 +111,11 @@ void PackInstallTask::onDownloadProgress(qint64 current, qint64 total) setStatus(tr("Downloading zip for %1 (%2%)").arg(m_pack.name).arg(current / 10)); } +void PackInstallTask::onDownloadAborted() +{ + emitAborted(); +} + void PackInstallTask::unzip() { progress(2, 4); diff --git a/launcher/modplatform/legacy_ftb/PackInstallTask.h b/launcher/modplatform/legacy_ftb/PackInstallTask.h index da4c0da5..da791e06 100644 --- a/launcher/modplatform/legacy_ftb/PackInstallTask.h +++ b/launcher/modplatform/legacy_ftb/PackInstallTask.h @@ -38,6 +38,7 @@ private slots: void onDownloadSucceeded(); void onDownloadFailed(QString reason); void onDownloadProgress(qint64 current, qint64 total); + void onDownloadAborted(); void onUnzipFinished(); void onUnzipCanceled(); diff --git a/launcher/modplatform/modpacksch/FTBPackInstallTask.cpp b/launcher/modplatform/modpacksch/FTBPackInstallTask.cpp index f17a311a..97ce1dc6 100644 --- a/launcher/modplatform/modpacksch/FTBPackInstallTask.cpp +++ b/launcher/modplatform/modpacksch/FTBPackInstallTask.cpp @@ -65,9 +65,8 @@ bool PackInstallTask::abort() if (m_mod_id_resolver_task) aborted &= m_mod_id_resolver_task->abort(); - // FIXME: This should be 'emitAborted()', but InstanceStaging doesn't connect to the abort signal yet... if (aborted) - emitFailed(tr("Aborted")); + emitAborted(); return aborted; } diff --git a/launcher/modplatform/technic/SolderPackInstallTask.cpp b/launcher/modplatform/technic/SolderPackInstallTask.cpp index 65b6ca51..19731b38 100644 --- a/launcher/modplatform/technic/SolderPackInstallTask.cpp +++ b/launcher/modplatform/technic/SolderPackInstallTask.cpp @@ -77,6 +77,7 @@ void Technic::SolderPackInstallTask::executeTask() auto job = m_filesNetJob.get(); connect(job, &NetJob::succeeded, this, &Technic::SolderPackInstallTask::fileListSucceeded); connect(job, &NetJob::failed, this, &Technic::SolderPackInstallTask::downloadFailed); + connect(job, &NetJob::aborted, this, &Technic::SolderPackInstallTask::downloadAborted); m_filesNetJob->start(); } @@ -127,6 +128,7 @@ void Technic::SolderPackInstallTask::fileListSucceeded() connect(m_filesNetJob.get(), &NetJob::succeeded, this, &Technic::SolderPackInstallTask::downloadSucceeded); connect(m_filesNetJob.get(), &NetJob::progress, this, &Technic::SolderPackInstallTask::downloadProgressChanged); connect(m_filesNetJob.get(), &NetJob::failed, this, &Technic::SolderPackInstallTask::downloadFailed); + connect(m_filesNetJob.get(), &NetJob::aborted, this, &Technic::SolderPackInstallTask::downloadAborted); m_filesNetJob->start(); } @@ -171,6 +173,12 @@ void Technic::SolderPackInstallTask::downloadProgressChanged(qint64 current, qin setProgress(current / 2, total); } +void Technic::SolderPackInstallTask::downloadAborted() +{ + emitAborted(); + m_filesNetJob.reset(); +} + void Technic::SolderPackInstallTask::extractFinished() { if (!m_extractFuture.result()) diff --git a/launcher/modplatform/technic/SolderPackInstallTask.h b/launcher/modplatform/technic/SolderPackInstallTask.h index 117a7bd6..aa14ce88 100644 --- a/launcher/modplatform/technic/SolderPackInstallTask.h +++ b/launcher/modplatform/technic/SolderPackInstallTask.h @@ -61,6 +61,7 @@ namespace Technic void downloadSucceeded(); void downloadFailed(QString reason); void downloadProgressChanged(qint64 current, qint64 total); + void downloadAborted(); void extractFinished(); void extractAborted(); diff --git a/launcher/ui/MainWindow.cpp b/launcher/ui/MainWindow.cpp index 58b1ae80..5729b44d 100644 --- a/launcher/ui/MainWindow.cpp +++ b/launcher/ui/MainWindow.cpp @@ -1656,6 +1656,10 @@ void MainWindow::runModalTask(Task *task) CustomMessageBox::selectable(this, tr("Warnings"), warnings.join('\n'), QMessageBox::Warning)->show(); } }); + connect(task, &Task::aborted, [this] + { + CustomMessageBox::selectable(this, tr("Task aborted"), tr("The task has been aborted by the user."), QMessageBox::Information)->show(); + }); ProgressDialog loadDialog(this); loadDialog.setSkipButton(true, tr("Abort")); loadDialog.execWithTask(task); diff --git a/launcher/ui/dialogs/ProgressDialog.cpp b/launcher/ui/dialogs/ProgressDialog.cpp index 3c7f53d3..6f9cc8e0 100644 --- a/launcher/ui/dialogs/ProgressDialog.cpp +++ b/launcher/ui/dialogs/ProgressDialog.cpp @@ -43,8 +43,7 @@ void ProgressDialog::setSkipButton(bool present, QString label) void ProgressDialog::on_skipButton_clicked(bool checked) { Q_UNUSED(checked); - if (task->abort()) - QDialog::reject(); + task->abort(); } ProgressDialog::~ProgressDialog() @@ -81,7 +80,7 @@ int ProgressDialog::execWithTask(Task* task) connect(task, &Task::stepStatus, this, &ProgressDialog::changeStatus); connect(task, &Task::progress, this, &ProgressDialog::changeProgress); - connect(task, &Task::aborted, [this] { onTaskFailed(tr("Aborted by user")); }); + connect(task, &Task::aborted, [this] { QDialog::reject(); }); m_is_multi_step = task->isMultiStep(); if (!m_is_multi_step) { diff --git a/launcher/ui/pages/modplatform/legacy_ftb/Page.cpp b/launcher/ui/pages/modplatform/legacy_ftb/Page.cpp index 0208b36b..98ab8799 100644 --- a/launcher/ui/pages/modplatform/legacy_ftb/Page.cpp +++ b/launcher/ui/pages/modplatform/legacy_ftb/Page.cpp @@ -146,6 +146,7 @@ void Page::openedImpl() { connect(ftbFetchTask.get(), &PackFetchTask::finished, this, &Page::ftbPackDataDownloadSuccessfully); connect(ftbFetchTask.get(), &PackFetchTask::failed, this, &Page::ftbPackDataDownloadFailed); + connect(ftbFetchTask.get(), &PackFetchTask::aborted, this, &Page::ftbPackDataDownloadAborted); connect(ftbFetchTask.get(), &PackFetchTask::privateFileDownloadFinished, this, &Page::ftbPrivatePackDataDownloadSuccessfully); connect(ftbFetchTask.get(), &PackFetchTask::privateFileDownloadFailed, this, &Page::ftbPrivatePackDataDownloadFailed); @@ -220,7 +221,12 @@ void Page::ftbPackDataDownloadSuccessfully(ModpackList publicPacks, ModpackList void Page::ftbPackDataDownloadFailed(QString reason) { - //TODO: Display the error + CustomMessageBox::selectable(this, tr("Error"), reason, QMessageBox::Critical)->show(); +} + +void Page::ftbPackDataDownloadAborted() +{ + CustomMessageBox::selectable(this, tr("Task aborted"), tr("The task has been aborted by the user."), QMessageBox::Information)->show(); } void Page::ftbPrivatePackDataDownloadSuccessfully(Modpack pack) diff --git a/launcher/ui/pages/modplatform/legacy_ftb/Page.h b/launcher/ui/pages/modplatform/legacy_ftb/Page.h index 52db7d91..1de8b40a 100644 --- a/launcher/ui/pages/modplatform/legacy_ftb/Page.h +++ b/launcher/ui/pages/modplatform/legacy_ftb/Page.h @@ -95,6 +95,7 @@ private: private slots: void ftbPackDataDownloadSuccessfully(ModpackList publicPacks, ModpackList thirdPartyPacks); void ftbPackDataDownloadFailed(QString reason); + void ftbPackDataDownloadAborted(); void ftbPrivatePackDataDownloadSuccessfully(Modpack pack); void ftbPrivatePackDataDownloadFailed(QString reason, QString packCode);