NOISSUE some safe refactors and changes of the task subsystem

Possibly also some bug fixes.

14 files changed, 89 insertions, 162 deletions

diff --git a/api/logic/CMakeLists.txt b/api/logic/CMakeLists.txt
index 2eda34fe..bb999619 100644
--- a/api/logic/CMakeLists.txt
+++ b/api/logic/CMakeLists.txt
@@ -350,8 +350,6 @@ set(TASKS_SOURCES
 	# Tasks
 	tasks/Task.h
 	tasks/Task.cpp
-	tasks/ThreadTask.h
-	tasks/ThreadTask.cpp
 	tasks/SequentialTask.h
 	tasks/SequentialTask.cpp
 )
diff --git a/api/logic/java/JavaCheckerJob.cpp b/api/logic/java/JavaCheckerJob.cpp
index 01b5b28d..a111af20 100644
--- a/api/logic/java/JavaCheckerJob.cpp
+++ b/api/logic/java/JavaCheckerJob.cpp
@@ -22,7 +22,7 @@ void JavaCheckerJob::partFinished(JavaCheckResult result)
 	num_finished++;
 	qDebug() << m_job_name.toLocal8Bit() << "progress:" << num_finished << "/"
 				<< javacheckers.size();
-	emit progress(num_finished, javacheckers.size());
+	setProgress(num_finished, javacheckers.size());
 
 	javaresults.replace(result.id, result);
 
@@ -35,7 +35,6 @@ void JavaCheckerJob::partFinished(JavaCheckResult result)
 void JavaCheckerJob::executeTask()
 {
 	qDebug() << m_job_name.toLocal8Bit() << " started.";
-	m_running = true;
 	for (auto iter : javacheckers)
 	{
 		javaresults.append(JavaCheckResult());
diff --git a/api/logic/java/JavaCheckerJob.h b/api/logic/java/JavaCheckerJob.h
index 58a98190..c27a50c3 100644
--- a/api/logic/java/JavaCheckerJob.h
+++ b/api/logic/java/JavaCheckerJob.h
@@ -58,10 +58,6 @@ public:
 	{
 		return javacheckers.size();
 	}
-	virtual bool isRunning() const override
-	{
-		return m_running;
-	}
 
 signals:
 	void started();
@@ -80,5 +76,4 @@ private:
 	qint64 current_progress = 0;
 	qint64 total_progress = 0;
 	int num_finished = 0;
-	bool m_running = false;
 };
diff --git a/api/logic/launch/LaunchTask.cpp b/api/logic/launch/LaunchTask.cpp
index 23c28f50..e1794cae 100644
--- a/api/logic/launch/LaunchTask.cpp
+++ b/api/logic/launch/LaunchTask.cpp
@@ -83,7 +83,7 @@ void LaunchTask::onStepFinished()
 	}
 
 	auto step = m_steps[currentStep];
-	if(step->successful())
+	if(step->wasSuccessful())
 	{
 		// end?
 		if(currentStep == m_steps.size() - 1)
diff --git a/api/logic/launch/steps/Update.cpp b/api/logic/launch/steps/Update.cpp
index 956230f4..f06c39eb 100644
--- a/api/logic/launch/steps/Update.cpp
+++ b/api/logic/launch/steps/Update.cpp
@@ -42,7 +42,7 @@ void Update::proceed()
 
 void Update::updateFinished()
 {
-	if(m_updateTask->successful())
+	if(m_updateTask->wasSuccessful())
 	{
 		m_updateTask.reset();
 		emitSucceeded();
diff --git a/api/logic/minecraft/launch/ModMinecraftJar.cpp b/api/logic/minecraft/launch/ModMinecraftJar.cpp
index dad41a98..b56f6fca 100644
--- a/api/logic/minecraft/launch/ModMinecraftJar.cpp
+++ b/api/logic/minecraft/launch/ModMinecraftJar.cpp
@@ -31,7 +31,7 @@ void ModMinecraftJar::executeTask()
 
 void ModMinecraftJar::jarModdingFinished()
 {
-	if(m_jarModTask->successful())
+	if(m_jarModTask->wasSuccessful())
 	{
 		emitSucceeded();
 	}
diff --git a/api/logic/net/NetAction.h b/api/logic/net/NetAction.h
index a533c317..f164dda3 100644
--- a/api/logic/net/NetAction.h
+++ b/api/logic/net/NetAction.h
@@ -43,18 +43,26 @@ protected:
 public:
 	virtual ~NetAction() {};
 
-public:
-	virtual qint64 totalProgress() const
+	bool isRunning() const
 	{
-		return m_total_progress;
+		return m_status == Job_InProgress;
 	}
-	virtual qint64 currentProgress() const
+	bool isFinished() const
 	{
-		return m_progress;
+		return m_status >= Job_Finished;
 	}
-	virtual qint64 numberOfFailures() const
+	bool wasSuccessful() const
 	{
-		return m_failures;
+		return m_status == Job_Finished || m_status == Job_Failed_Proceed;
+	}
+
+	qint64 totalProgress() const
+	{
+		return m_total_progress;
+	}
+	qint64 currentProgress() const
+	{
+		return m_progress;
 	}
 	virtual bool abort()
 	{
@@ -64,25 +72,10 @@ public:
 	{
 		return false;
 	}
-
-public:
-	/// the network reply
-	unique_qobject_ptr<QNetworkReply> m_reply;
-
-	/// source URL
-	QUrl m_url;
-
-	/// The file's status
-	JobStatus m_status = Job_NotStarted;
-
-	/// index within the parent job
-	int m_index_within_job = 0;
-
-	qint64 m_progress = 0;
-	qint64 m_total_progress = 1;
-
-	/// number of failures up to this point
-	int m_failures = 0;
+	QUrl url()
+	{
+		return m_url;
+	}
 
 signals:
 	void started(int index);
@@ -91,14 +84,28 @@ signals:
 	void failed(int index);
 	void aborted(int index);
 
-protected
-slots:
+protected slots:
 	virtual void downloadProgress(qint64 bytesReceived, qint64 bytesTotal) = 0;
 	virtual void downloadError(QNetworkReply::NetworkError error) = 0;
 	virtual void downloadFinished() = 0;
 	virtual void downloadReadyRead() = 0;
 
-public
-slots:
+public slots:
 	virtual void start() = 0;
+
+public:
+	/// index within the parent job, FIXME: nuke
+	int m_index_within_job = 0;
+
+	/// the network reply
+	unique_qobject_ptr<QNetworkReply> m_reply;
+
+	/// source URL
+	QUrl m_url;
+
+	qint64 m_progress = 0;
+	qint64 m_total_progress = 1;
+
+protected:
+	JobStatus m_status = Job_NotStarted;
 };
diff --git a/api/logic/net/NetJob.cpp b/api/logic/net/NetJob.cpp
index 275da749..693a0934 100644
--- a/api/logic/net/NetJob.cpp
+++ b/api/logic/net/NetJob.cpp
@@ -73,18 +73,19 @@ void NetJob::partProgress(int index, qint64 bytesReceived, qint64 bytesTotal)
 void NetJob::executeTask()
 {
 	qDebug() << m_job_name.toLocal8Bit() << " started.";
-	m_running = true;
-	for (int i = 0; i < downloads.size(); i++)
-	{
-		m_todo.enqueue(i);
-	}
 	// hack that delays early failures so they can be caught easier
 	QMetaObject::invokeMethod(this, "startMoreParts", Qt::QueuedConnection);
 }
 
 void NetJob::startMoreParts()
 {
-	// check for final conditions if there's nothing in the queue
+	if(!isRunning())
+	{
+		// this actually makes sense. You can put running downloads into a NetJob and then not start it until much later.
+		return;
+	}
+	// OK. We are actively processing tasks, proceed.
+	// Check for final conditions if there's nothing in the queue.
 	if(!m_todo.size())
 	{
 		if(!m_doing.size())
@@ -107,7 +108,7 @@ void NetJob::startMoreParts()
 		}
 		return;
 	}
-	// otherwise try to start more parts
+	// There's work to do, try to start more parts.
 	while (m_doing.size() < 6)
 	{
 		if(!m_todo.size())
@@ -131,7 +132,7 @@ QStringList NetJob::getFailedFiles()
 	QStringList failed;
 	for (auto index: m_failed)
 	{
-		failed.push_back(downloads[index]->m_url.toString());
+		failed.push_back(downloads[index]->url().toString());
 	}
 	failed.sort();
 	return failed;
@@ -170,3 +171,31 @@ bool NetJob::abort()
 	}
 	return fullyAborted;
 }
+
+bool NetJob::addNetAction(NetActionPtr action)
+{
+	action->m_index_within_job = downloads.size();
+	downloads.append(action);
+	part_info pi;
+	{
+		pi.current_progress = action->currentProgress();
+		pi.total_progress = action->totalProgress();
+		pi.failures = 0;
+	}
+	parts_progress.append(pi);
+	total_progress += pi.total_progress;
+
+	// FIXME: detect if the action is already running, put it in m_doing if it is!
+	setProgress(current_progress, total_progress);
+	if(action->isRunning())
+	{
+		connect(action.get(), SIGNAL(succeeded(int)), SLOT(partSucceeded(int)));
+		connect(action.get(), SIGNAL(failed(int)), SLOT(partFailed(int)));
+		connect(action.get(), SIGNAL(netActionProgress(int, qint64, qint64)), SLOT(partProgress(int, qint64, qint64)));
+	}
+	else
+	{
+		m_todo.append(parts_progress.size() - 1);
+	}
+	return true;
+}
diff --git a/api/logic/net/NetJob.h b/api/logic/net/NetJob.h
index ca4f5df1..cd576664 100644
--- a/api/logic/net/NetJob.h
+++ b/api/logic/net/NetJob.h
@@ -32,30 +32,8 @@ class MULTIMC_LOGIC_EXPORT NetJob : public Task
 public:
 	explicit NetJob(QString job_name) : Task(), m_job_name(job_name) {}
 	virtual ~NetJob() {}
-	bool addNetAction(NetActionPtr action)
-	{
-		action->m_index_within_job = downloads.size();
-		downloads.append(action);
-		part_info pi;
-		{
-			pi.current_progress = action->currentProgress();
-			pi.total_progress = action->totalProgress();
-			pi.failures = action->numberOfFailures();
-		}
-		parts_progress.append(pi);
-		total_progress += pi.total_progress;
-		// if this is already running, the action needs to be started right away!
-		if (isRunning())
-		{
-			setProgress(current_progress, total_progress);
-			connect(action.get(), SIGNAL(succeeded(int)), SLOT(partSucceeded(int)));
-			connect(action.get(), SIGNAL(failed(int)), SLOT(partFailed(int)));
-			connect(action.get(), SIGNAL(netActionProgress(int, qint64, qint64)),
-					SLOT(partProgress(int, qint64, qint64)));
-			action->start();
-		}
-		return true;
-	}
+
+	bool addNetAction(NetActionPtr action);
 
 	NetActionPtr operator[](int index)
 	{
@@ -75,10 +53,6 @@ public:
 	{
 		return downloads.size();
 	}
-	virtual bool isRunning() const override
-	{
-		return m_running;
-	}
 	QStringList getFailedFiles();
 
 	bool canAbort() const override;
@@ -113,6 +87,5 @@ private:
 	QSet<int> m_failed;
 	qint64 current_progress = 0;
 	qint64 total_progress = 0;
-	bool m_running = false;
 	bool m_aborted = false;
 };
diff --git a/api/logic/tasks/Task.cpp b/api/logic/tasks/Task.cpp
index 23ee08e4..94a4d428 100644
--- a/api/logic/tasks/Task.cpp
+++ b/api/logic/tasks/Task.cpp
@@ -76,7 +76,7 @@ bool Task::isFinished() const
 	return m_finished;
 }
 
-bool Task::successful() const
+bool Task::wasSuccessful() const
 {
 	return m_succeeded;
 }
diff --git a/api/logic/tasks/Task.h b/api/logic/tasks/Task.h
index 47c4a13e..3654ed24 100644
--- a/api/logic/tasks/Task.h
+++ b/api/logic/tasks/Task.h
@@ -27,21 +27,15 @@ public:
 	explicit Task(QObject *parent = 0);
 	virtual ~Task() {};
 
-	virtual bool isRunning() const;
-
-	virtual bool isFinished() const;
-
-	/*!
-	 * True if this task was successful.
-	 * If the task failed or is still running, returns false.
-	 */
-	virtual bool successful() const;
+	bool isRunning() const;
+	bool isFinished() const;
+	bool wasSuccessful() const;
 
 	/*!
 	 * Returns the string that was passed to emitFailed as the error message when the task failed.
 	 * If the task hasn't failed, returns an empty string.
 	 */
-	virtual QString failReason() const;
+	QString failReason() const;
 
 	virtual bool canAbort() const { return false; }
 
@@ -68,8 +62,7 @@ signals:
 	void failed(QString reason);
 	void status(QString status);
 
-public
-slots:
+public slots:
 	virtual void start();
 	virtual bool abort() { return false; };
 
@@ -84,7 +77,7 @@ public slots:
 	void setStatus(const QString &status);
 	void setProgress(qint64 current, qint64 total);
 
-protected:
+private:
 	bool m_running = false;
 	bool m_finished = false;
 	bool m_succeeded = false;
diff --git a/api/logic/tasks/ThreadTask.cpp b/api/logic/tasks/ThreadTask.cpp
deleted file mode 100644
index ddd1dee5..00000000
--- a/api/logic/tasks/ThreadTask.cpp
+++ /dev/null
@@ -1,41 +0,0 @@
-#include "ThreadTask.h"
-#include <QtConcurrentRun>
-ThreadTask::ThreadTask(Task * internal, QObject *parent) : Task(parent), m_internal(internal)
-{
-}
-
-void ThreadTask::start()
-{
-	connect(m_internal, SIGNAL(failed(QString)), SLOT(iternal_failed(QString)));
-	connect(m_internal, SIGNAL(progress(qint64,qint64)), SLOT(iternal_progress(qint64,qint64)));
-	connect(m_internal, SIGNAL(started()), SLOT(iternal_started()));
-	connect(m_internal, SIGNAL(status(QString)), SLOT(iternal_status(QString)));
-	connect(m_internal, SIGNAL(succeeded()), SLOT(iternal_succeeded()));
-	m_running = true;
-	QtConcurrent::run(m_internal, &Task::start);
-}
-
-void ThreadTask::iternal_failed(QString reason)
-{
-	emitFailed(reason);
-}
-
-void ThreadTask::iternal_progress(qint64 current, qint64 total)
-{
-	progress(current, total);
-}
-
-void ThreadTask::iternal_started()
-{
-	emit started();
-}
-
-void ThreadTask::iternal_status(QString status)
-{
-	setStatus(status);
-}
-
-void ThreadTask::iternal_succeeded()
-{
-	emitSucceeded();
-}
diff --git a/api/logic/tasks/ThreadTask.h b/api/logic/tasks/ThreadTask.h
deleted file mode 100644
index 46ce3a36..00000000
--- a/api/logic/tasks/ThreadTask.h
+++ /dev/null
@@ -1,26 +0,0 @@
-#pragma once
-
-#include "Task.h"
-#include "multimc_logic_export.h"
-
-class MULTIMC_LOGIC_EXPORT ThreadTask : public Task
-{
-	Q_OBJECT
-public:
-	explicit ThreadTask(Task * internal, QObject * parent = nullptr);
-
-protected:
-	void executeTask() {};
-
-public slots:
-	virtual void start();
-
-private slots:
-	void iternal_started();
-	void iternal_progress(qint64 current, qint64 total);
-	void iternal_succeeded();
-	void iternal_failed(QString reason);
-	void iternal_status(QString status);
-private:
-	Task * m_internal;
-};
\ No newline at end of file
diff --git a/api/logic/updater/DownloadTask.cpp b/api/logic/updater/DownloadTask.cpp
index 0d40f78a..f08839b5 100644
--- a/api/logic/updater/DownloadTask.cpp
+++ b/api/logic/updater/DownloadTask.cpp
@@ -70,7 +70,7 @@ void DownloadTask::vinfoDownloadFailed()
 {
 	// Something failed. We really need the second download (current version info), so parse
 	// downloads anyways as long as the first one succeeded.
-	if (m_newVersionFileListDownload->m_status != Job_Failed)
+	if (m_newVersionFileListDownload->wasSuccessful())
 	{
 		processDownloadedVersionInfo();
 		return;
@@ -97,7 +97,7 @@ void DownloadTask::processDownloadedVersionInfo()
 	}
 
 	// if we have the current version info, use it.
-	if (m_currentVersionFileListDownload && m_currentVersionFileListDownload->m_status != Job_Failed)
+	if (m_currentVersionFileListDownload && m_currentVersionFileListDownload->wasSuccessful())
 	{
 		setStatus(tr("Reading file list for current version..."));
 		qDebug() << "Reading file list for current version...";