GH-1927 Add more specific task status logging

* Tasks are now described by class name and object name (or memory address).
* Tasks starts are logged.
* Aborted tasks are now treated just as the other cases.

13 files changed, 181 insertions, 109 deletions

diff --git a/api/logic/minecraft/SkinUpload.h b/api/logic/minecraft/SkinUpload.h
index 86944b82..5b331fa9 100644
--- a/api/logic/minecraft/SkinUpload.h
+++ b/api/logic/minecraft/SkinUpload.h
@@ -9,9 +9,9 @@
 
 typedef std::shared_ptr<class SkinUpload> SkinUploadPtr;
 
-class MULTIMC_LOGIC_EXPORT SkinUpload : public Task\
+class MULTIMC_LOGIC_EXPORT SkinUpload : public Task
 {
-Q_OBJECT
+	Q_OBJECT
 public:
 	enum Model
 	{
diff --git a/api/logic/minecraft/legacy/LegacyInstance.cpp b/api/logic/minecraft/legacy/LegacyInstance.cpp
index 0987d56f..9382ec21 100644
--- a/api/logic/minecraft/legacy/LegacyInstance.cpp
+++ b/api/logic/minecraft/legacy/LegacyInstance.cpp
@@ -95,85 +95,87 @@ shared_qobject_ptr<Task> LegacyInstance::createUpdateTask()
 	return shared_qobject_ptr<Task>(new LegacyUpdate(this, this));
 }
 
-std::shared_ptr<Task> LegacyInstance::createJarModdingTask()
+class LegacyJarModTask : public Task
 {
-	class JarModTask : public Task
+	Q_OBJECT
+public:
+	explicit LegacyJarModTask(std::shared_ptr<LegacyInstance> inst) : Task(nullptr), m_inst(inst)
+	{
+	}
+	virtual void executeTask()
 	{
-	public:
-		explicit JarModTask(std::shared_ptr<LegacyInstance> inst) : Task(nullptr), m_inst(inst)
+		if (!m_inst->shouldRebuild())
 		{
+			emitSucceeded();
+			return;
 		}
-		virtual void executeTask()
-		{
-			if (!m_inst->shouldRebuild())
-			{
-				emitSucceeded();
-				return;
-			}
 
-			// Get the mod list
-			auto modList = m_inst->getJarMods();
+		// Get the mod list
+		auto modList = m_inst->getJarMods();
 
-			QFileInfo runnableJar(m_inst->runnableJar());
-			QFileInfo baseJar(m_inst->baseJar());
-			bool base_is_custom = m_inst->shouldUseCustomBaseJar();
+		QFileInfo runnableJar(m_inst->runnableJar());
+		QFileInfo baseJar(m_inst->baseJar());
+		bool base_is_custom = m_inst->shouldUseCustomBaseJar();
 
-			// Nothing to do if there are no jar mods to install, no backup and just the mc jar
-			if (base_is_custom)
-			{
-				// yes, this can happen if the instance only has the runnable jar and not the base jar
-				// it *could* be assumed that such an instance is vanilla, but that wouldn't be safe
-				// because that's not something mmc4 guarantees
-				if (runnableJar.isFile() && !baseJar.exists() && modList.empty())
-				{
-					m_inst->setShouldRebuild(false);
-					emitSucceeded();
-					return;
-				}
-
-				setStatus(tr("Installing mods: Backing up minecraft.jar ..."));
-				if (!baseJar.exists() && !QFile::copy(runnableJar.filePath(), baseJar.filePath()))
-				{
-					emitFailed("It seems both the active and base jar are gone. A fresh base jar will "
-							"be used on next run.");
-					m_inst->setShouldRebuild(true);
-					m_inst->setShouldUpdate(true);
-					m_inst->setShouldUseCustomBaseJar(false);
-					return;
-				}
-			}
-
-			if (!baseJar.exists())
+		// Nothing to do if there are no jar mods to install, no backup and just the mc jar
+		if (base_is_custom)
+		{
+			// yes, this can happen if the instance only has the runnable jar and not the base jar
+			// it *could* be assumed that such an instance is vanilla, but that wouldn't be safe
+			// because that's not something mmc4 guarantees
+			if (runnableJar.isFile() && !baseJar.exists() && modList.empty())
 			{
-				emitFailed("The base jar " + baseJar.filePath() + " does not exist");
+				m_inst->setShouldRebuild(false);
+				emitSucceeded();
 				return;
 			}
 
-			if (runnableJar.exists() && !QFile::remove(runnableJar.filePath()))
+			setStatus(tr("Installing mods: Backing up minecraft.jar ..."));
+			if (!baseJar.exists() && !QFile::copy(runnableJar.filePath(), baseJar.filePath()))
 			{
-				emitFailed("Failed to delete old minecraft.jar");
+				emitFailed("It seems both the active and base jar are gone. A fresh base jar will "
+						"be used on next run.");
+				m_inst->setShouldRebuild(true);
+				m_inst->setShouldUpdate(true);
+				m_inst->setShouldUseCustomBaseJar(false);
 				return;
 			}
+		}
 
-			setStatus(tr("Installing mods: Opening minecraft.jar ..."));
-
-			QString outputJarPath = runnableJar.filePath();
-			QString inputJarPath = baseJar.filePath();
+		if (!baseJar.exists())
+		{
+			emitFailed("The base jar " + baseJar.filePath() + " does not exist");
+			return;
+		}
 
-			if(!MMCZip::createModdedJar(inputJarPath, outputJarPath, modList))
-			{
-				emitFailed(tr("Failed to create the custom Minecraft jar file."));
-				return;
-			}
-			m_inst->setShouldRebuild(false);
-			// inst->UpdateVersion(true);
-			emitSucceeded();
+		if (runnableJar.exists() && !QFile::remove(runnableJar.filePath()))
+		{
+			emitFailed("Failed to delete old minecraft.jar");
 			return;
+		}
+
+		setStatus(tr("Installing mods: Opening minecraft.jar ..."));
+
+		QString outputJarPath = runnableJar.filePath();
+		QString inputJarPath = baseJar.filePath();
 
+		if(!MMCZip::createModdedJar(inputJarPath, outputJarPath, modList))
+		{
+			emitFailed(tr("Failed to create the custom Minecraft jar file."));
+			return;
 		}
-		std::shared_ptr<LegacyInstance> m_inst;
-	};
-	return std::make_shared<JarModTask>(std::dynamic_pointer_cast<LegacyInstance>(shared_from_this()));
+		m_inst->setShouldRebuild(false);
+		// inst->UpdateVersion(true);
+		emitSucceeded();
+		return;
+
+	}
+	std::shared_ptr<LegacyInstance> m_inst;
+};
+
+std::shared_ptr<Task> LegacyInstance::createJarModdingTask()
+{
+	return std::make_shared<LegacyJarModTask>(std::dynamic_pointer_cast<LegacyInstance>(shared_from_this()));
 }
 
 QString LegacyInstance::createLaunchScript(AuthSessionPtr session)
@@ -515,3 +517,5 @@ QStringList LegacyInstance::processMinecraftArgs(AuthSessionPtr account) const
 	out.append(account->session);
 	return out;
 }
+
+#include "LegacyInstance.moc"
diff --git a/api/logic/minecraft/onesix/OneSixInstance.cpp b/api/logic/minecraft/onesix/OneSixInstance.cpp
index ecfd0647..107da04f 100644
--- a/api/logic/minecraft/onesix/OneSixInstance.cpp
+++ b/api/logic/minecraft/onesix/OneSixInstance.cpp
@@ -368,53 +368,54 @@ std::shared_ptr<LaunchStep> OneSixInstance::createMainLaunchStep(LaunchTask * pa
 	return nullptr;
 }
 
-
-std::shared_ptr<Task> OneSixInstance::createJarModdingTask()
+class JarModTask : public Task
 {
-	class JarModTask : public Task
+	Q_OBJECT
+public:
+	explicit JarModTask(std::shared_ptr<OneSixInstance> inst) : Task(nullptr), m_inst(inst)
+	{
+	}
+	virtual void executeTask()
 	{
-	public:
-		explicit JarModTask(std::shared_ptr<OneSixInstance> inst) : Task(nullptr), m_inst(inst)
+		auto profile = m_inst->getMinecraftProfile();
+		// nuke obsolete stripped jar(s) if needed
+		QString version_id = profile->getMinecraftVersion();
+		if(!FS::ensureFolderPathExists(m_inst->binRoot()))
 		{
+			emitFailed(tr("Couldn't create the bin folder for Minecraft.jar"));
 		}
-		virtual void executeTask()
+		auto finalJarPath = QDir(m_inst->binRoot()).absoluteFilePath("minecraft.jar");
+		QFile finalJar(finalJarPath);
+		if(finalJar.exists())
 		{
-			auto profile = m_inst->getMinecraftProfile();
-			// nuke obsolete stripped jar(s) if needed
-			QString version_id = profile->getMinecraftVersion();
-			if(!FS::ensureFolderPathExists(m_inst->binRoot()))
+			if(!finalJar.remove())
 			{
-				emitFailed(tr("Couldn't create the bin folder for Minecraft.jar"));
-			}
-			auto finalJarPath = QDir(m_inst->binRoot()).absoluteFilePath("minecraft.jar");
-			QFile finalJar(finalJarPath);
-			if(finalJar.exists())
-			{
-				if(!finalJar.remove())
-				{
-					emitFailed(tr("Couldn't remove stale jar file: %1").arg(finalJarPath));
-					return;
-				}
+				emitFailed(tr("Couldn't remove stale jar file: %1").arg(finalJarPath));
+				return;
 			}
+		}
 
-			// create temporary modded jar, if needed
-			auto jarMods = m_inst->getJarMods();
-			if(jarMods.size())
+		// create temporary modded jar, if needed
+		auto jarMods = m_inst->getJarMods();
+		if(jarMods.size())
+		{
+			auto mainJar = profile->getMainJar();
+			QStringList jars, temp1, temp2, temp3, temp4;
+			mainJar->getApplicableFiles(currentSystem, jars, temp1, temp2, temp3, m_inst->getLocalLibraryPath());
+			auto sourceJarPath = jars[0];
+			if(!MMCZip::createModdedJar(sourceJarPath, finalJarPath, jarMods))
 			{
-				auto mainJar = profile->getMainJar();
-				QStringList jars, temp1, temp2, temp3, temp4;
-				mainJar->getApplicableFiles(currentSystem, jars, temp1, temp2, temp3, m_inst->getLocalLibraryPath());
-				auto sourceJarPath = jars[0];
-				if(!MMCZip::createModdedJar(sourceJarPath, finalJarPath, jarMods))
-				{
-					emitFailed(tr("Failed to create the custom Minecraft jar file."));
-					return;
-				}
+				emitFailed(tr("Failed to create the custom Minecraft jar file."));
+				return;
 			}
-			emitSucceeded();
 		}
-		std::shared_ptr<OneSixInstance> m_inst;
-	};
+		emitSucceeded();
+	}
+	std::shared_ptr<OneSixInstance> m_inst;
+};
+
+std::shared_ptr<Task> OneSixInstance::createJarModdingTask()
+{
 	return std::make_shared<JarModTask>(std::dynamic_pointer_cast<OneSixInstance>(shared_from_this()));
 }
 
@@ -696,3 +697,5 @@ QStringList OneSixInstance::getNativeJars() const
 	m_profile->getLibraryFiles(javaArchitecture, jars, nativeJars, getLocalLibraryPath(), binRoot());
 	return nativeJars;
 }
+
+#include "OneSixInstance.moc"
diff --git a/api/logic/minecraft/onesix/OneSixUpdate.cpp b/api/logic/minecraft/onesix/OneSixUpdate.cpp
index e0027032..07ca5fb2 100644
--- a/api/logic/minecraft/onesix/OneSixUpdate.cpp
+++ b/api/logic/minecraft/onesix/OneSixUpdate.cpp
@@ -142,11 +142,21 @@ void OneSixUpdate::next()
 
 void OneSixUpdate::subtaskSucceeded()
 {
+	if(isFinished())
+	{
+		qCritical() << "OneSixUpdate: Subtask" << sender() << "succeeded, but work was already done!";
+		return;
+	}
 	next();
 }
 
 void OneSixUpdate::subtaskFailed(QString error)
 {
+	if(isFinished())
+	{
+		qCritical() << "OneSixUpdate: Subtask" << sender() << "failed, but work was already done!";
+		return;
+	}
 	emitFailed(error);
 }
 
diff --git a/api/logic/minecraft/onesix/update/AssetUpdateTask.h b/api/logic/minecraft/onesix/update/AssetUpdateTask.h
index dff72571..3b500189 100644
--- a/api/logic/minecraft/onesix/update/AssetUpdateTask.h
+++ b/api/logic/minecraft/onesix/update/AssetUpdateTask.h
@@ -5,6 +5,7 @@ class OneSixInstance;
 
 class AssetUpdateTask : public Task
 {
+	Q_OBJECT
 public:
 	AssetUpdateTask(OneSixInstance * inst);
 	void executeTask() override;
diff --git a/api/logic/minecraft/onesix/update/FMLLibrariesTask.h b/api/logic/minecraft/onesix/update/FMLLibrariesTask.h
index d1c250e4..616053d7 100644
--- a/api/logic/minecraft/onesix/update/FMLLibrariesTask.h
+++ b/api/logic/minecraft/onesix/update/FMLLibrariesTask.h
@@ -5,6 +5,7 @@ class OneSixInstance;
 
 class FMLLibrariesTask : public Task
 {
+	Q_OBJECT
 public:
 	FMLLibrariesTask(OneSixInstance * inst);
 
diff --git a/api/logic/minecraft/onesix/update/FoldersTask.cpp b/api/logic/minecraft/onesix/update/FoldersTask.cpp
index 239a2675..d478560c 100644
--- a/api/logic/minecraft/onesix/update/FoldersTask.cpp
+++ b/api/logic/minecraft/onesix/update/FoldersTask.cpp
@@ -3,6 +3,7 @@
 #include <QDir>
 
 FoldersTask::FoldersTask(OneSixInstance * inst)
+	:Task()
 {
 	m_inst = inst;
 }
diff --git a/api/logic/minecraft/onesix/update/FoldersTask.h b/api/logic/minecraft/onesix/update/FoldersTask.h
index 552d3098..7cdc5a93 100644
--- a/api/logic/minecraft/onesix/update/FoldersTask.h
+++ b/api/logic/minecraft/onesix/update/FoldersTask.h
@@ -5,6 +5,7 @@
 class OneSixInstance;
 class FoldersTask : public Task
 {
+	Q_OBJECT
 public:
 	FoldersTask(OneSixInstance * inst);
 	void executeTask() override;
diff --git a/api/logic/minecraft/onesix/update/LibrariesTask.h b/api/logic/minecraft/onesix/update/LibrariesTask.h
index 80cf0d2a..a84975e5 100644
--- a/api/logic/minecraft/onesix/update/LibrariesTask.h
+++ b/api/logic/minecraft/onesix/update/LibrariesTask.h
@@ -5,6 +5,7 @@ class OneSixInstance;
 
 class LibrariesTask : public Task
 {
+	Q_OBJECT
 public:
 	LibrariesTask(OneSixInstance * inst);
 
diff --git a/api/logic/net/NetJob.cpp b/api/logic/net/NetJob.cpp
index 65d649f5..9a8277b0 100644
--- a/api/logic/net/NetJob.cpp
+++ b/api/logic/net/NetJob.cpp
@@ -94,7 +94,6 @@ void NetJob::partProgress(int index, qint64 bytesReceived, qint64 bytesTotal)
 
 void NetJob::executeTask()
 {
-	qDebug() << m_job_name.toLocal8Bit() << " started.";
 	// hack that delays early failures so they can be caught easier
 	QMetaObject::invokeMethod(this, "startMoreParts", Qt::QueuedConnection);
 }
@@ -114,18 +113,15 @@ void NetJob::startMoreParts()
 		{
 			if(!m_failed.size())
 			{
-				qDebug() << m_job_name << "succeeded.";
 				emitSucceeded();
 			}
 			else if(m_aborted)
 			{
-				qDebug() << m_job_name << "aborted.";
-				emitFailed(tr("Job '%1' aborted.").arg(m_job_name));
+				emitAborted();
 			}
 			else
 			{
-				qCritical() << m_job_name << "failed.";
-				emitFailed(tr("Job '%1' failed to process:\n%2").arg(m_job_name).arg(getFailedFiles().join("\n")));
+				emitFailed(tr("Job '%1' failed to process:\n%2").arg(objectName()).arg(getFailedFiles().join("\n")));
 			}
 		}
 		return;
diff --git a/api/logic/net/NetJob.h b/api/logic/net/NetJob.h
index 2b5c3d9a..6ae3a73f 100644
--- a/api/logic/net/NetJob.h
+++ b/api/logic/net/NetJob.h
@@ -30,7 +30,10 @@ class MULTIMC_LOGIC_EXPORT NetJob : public Task
 {
 	Q_OBJECT
 public:
-	explicit NetJob(QString job_name) : Task(), m_job_name(job_name) {}
+	explicit NetJob(QString job_name) : Task()
+	{
+		setObjectName(job_name);
+	}
 	virtual ~NetJob() {}
 
 	bool addNetAction(NetActionPtr action);
@@ -77,7 +80,6 @@ private:
 		qint64 total_progress = 1;
 		int failures = 0;
 	};
-	QString m_job_name;
 	QList<NetActionPtr> downloads;
 	QList<part_info> parts_progress;
 	QQueue<int> m_todo;
diff --git a/api/logic/tasks/Task.cpp b/api/logic/tasks/Task.cpp
index 94a4d428..bc48e902 100644
--- a/api/logic/tasks/Task.cpp
+++ b/api/logic/tasks/Task.cpp
@@ -41,31 +41,79 @@ void Task::start()
 {
 	m_running = true;
 	emit started();
+	qDebug() << "Task" << describe() << "started";
 	executeTask();
 }
 
 void Task::emitFailed(QString reason)
 {
+	// Don't fail twice.
+	if (!m_running)
+	{
+		qCritical() << "Task" << describe() << "failed while not running!!!!: " << reason;
+		return;
+	}
 	m_running = false;
 	m_finished = true;
 	m_succeeded = false;
 	m_failReason = reason;
-	qCritical() << "Task failed: " << reason;
+	qCritical() << "Task" << describe() << "failed: " << reason;
 	emit failed(reason);
 	emit finished();
 }
 
+void Task::emitAborted()
+{
+	// Don't abort twice.
+	if (!m_running)
+	{
+		qCritical() << "Task" << describe() << "aborted while not running!!!!";
+		return;
+	}
+	m_running = false;
+	m_finished = true;
+	m_succeeded = false;
+	m_failReason = "Aborted.";
+	qDebug() << "Task" << describe() << "aborted.";
+	emit failed(m_failReason);
+	emit finished();
+}
+
 void Task::emitSucceeded()
 {
-	if (!m_running) { return; } // Don't succeed twice.
+	// Don't succeed twice.
+	if (!m_running)
+	{
+		qCritical() << "Task" << describe() << "succeeded while not running!!!!";
+		return;
+	}
 	m_running = false;
 	m_finished = true;
 	m_succeeded = true;
-	qDebug() << "Task succeeded";
+	qDebug() << "Task" << describe() << "succeeded";
 	emit succeeded();
 	emit finished();
 }
 
+QString Task::describe()
+{
+	QString outStr;
+	QTextStream out(&outStr);
+	out << metaObject()->className() << QChar('(');
+	auto name = objectName();
+	if(name.isEmpty())
+	{
+		out << QString("0x%1").arg((quintptr)this, 0, 16);
+	}
+	else
+	{
+		out << name;
+	}
+	out << QChar(')');
+	out.flush();
+	return outStr;
+}
+
 bool Task::isRunning() const
 {
 	return m_running;
diff --git a/api/logic/tasks/Task.h b/api/logic/tasks/Task.h
index 3654ed24..f54996c1 100644
--- a/api/logic/tasks/Task.h
+++ b/api/logic/tasks/Task.h
@@ -54,6 +54,9 @@ public:
 		return m_progressTotal;
 	}
 
+private:
+	QString describe();
+
 signals:
 	void started();
 	void progress(qint64 current, qint64 total);
@@ -71,6 +74,7 @@ protected:
 
 protected slots:
 	virtual void emitSucceeded();
+	virtual void emitAborted();
 	virtual void emitFailed(QString reason);
 
 public slots: