More code butchery related to version files. No end in sight.

6 files changed, 109 insertions, 122 deletions

diff --git a/logic/OneSixInstance.cpp b/logic/OneSixInstance.cpp
index 0e0be4d8..f6fe49f1 100644
--- a/logic/OneSixInstance.cpp
+++ b/logic/OneSixInstance.cpp
@@ -27,6 +27,7 @@
 #include "icons/IconList.h"
 #include "MinecraftProcess.h"
 #include "gui/dialogs/OneSixModEditDialog.h"
+#include <MMCError.h>
 
 OneSixInstance::OneSixInstance(const QString &rootDir, SettingsObject *settings, QObject *parent)
 	: BaseInstance(new OneSixInstancePrivate(), rootDir, settings, parent)
@@ -320,21 +321,23 @@ bool OneSixInstance::reloadVersion()
 {
 	I_D(OneSixInstance);
 
-	bool ret = d->version->reload(false, externalPatches());
-	if (ret)
-	{
-		ret = d->vanillaVersion->reload(true, externalPatches());
-	}
-	if (ret)
+	try
 	{
+		d->version->reload(false, externalPatches());
+		d->vanillaVersion->reload(true, externalPatches());
 		setFlags(flags() & ~VersionBrokenFlag);
 		emit versionReloaded();
+		return true;
 	}
-	else
+	catch(MMCError error)
 	{
+		d->version->clear();
+		d->vanillaVersion->clear();
 		setFlags(flags() | VersionBrokenFlag);
+		//TODO: rethrow to show some error message(s)?
+		emit versionReloaded();
+		return false;
 	}
-	return ret;
 }
 
 void OneSixInstance::clearVersion()
diff --git a/logic/OneSixVersionBuilder.cpp b/logic/OneSixVersionBuilder.cpp
index 752f2d8f..37467aef 100644
--- a/logic/OneSixVersionBuilder.cpp
+++ b/logic/OneSixVersionBuilder.cpp
@@ -30,6 +30,7 @@
 #include "OneSixInstance.h"
 #include "OneSixRule.h"
 #include "VersionFile.h"
+#include "MMCJson.h"
 #include "modutils.h"
 #include "logger/QsLog.h"
 
@@ -37,23 +38,23 @@ OneSixVersionBuilder::OneSixVersionBuilder()
 {
 }
 
-bool OneSixVersionBuilder::build(VersionFinal *version, OneSixInstance *instance, const bool onlyVanilla, const QStringList &external)
+void OneSixVersionBuilder::build(VersionFinal *version, OneSixInstance *instance, const bool onlyVanilla, const QStringList &external)
 {
 	OneSixVersionBuilder builder;
 	builder.m_version = version;
 	builder.m_instance = instance;
-	return builder.buildInternal(onlyVanilla, external);
+	builder.buildInternal(onlyVanilla, external);
 }
 
-bool OneSixVersionBuilder::readJsonAndApplyToVersion(VersionFinal *version, const QJsonObject &obj)
+void OneSixVersionBuilder::readJsonAndApplyToVersion(VersionFinal *version, const QJsonObject &obj)
 {
 	OneSixVersionBuilder builder;
 	builder.m_version = version;
 	builder.m_instance = 0;
-	return builder.readJsonAndApply(obj);
+	builder.readJsonAndApply(obj);
 }
 
-bool OneSixVersionBuilder::buildInternal(const bool onlyVanilla, const QStringList &external)
+void OneSixVersionBuilder::buildInternal(const bool onlyVanilla, const QStringList &external)
 {
 	m_version->clear();
 
@@ -64,11 +65,7 @@ bool OneSixVersionBuilder::buildInternal(const bool onlyVanilla, const QStringLi
 	if(!external.isEmpty()) for (auto fileName : external)
 	{
 		QLOG_INFO() << "Reading" << fileName;
-		VersionFile file;
-		if (!parseJsonFile(QFileInfo(fileName), false, &file, fileName.endsWith("pack.json")))
-		{
-			return false;
-		}
+		VersionFile file = parseJsonFile(QFileInfo(fileName), false, fileName.endsWith("pack.json"));
 		file.name = QFileInfo(fileName).fileName();
 		file.fileId = "org.multimc.external." + file.name;
 		file.version = QString();
@@ -79,11 +76,7 @@ bool OneSixVersionBuilder::buildInternal(const bool onlyVanilla, const QStringLi
 	else if (QFile::exists(root.absoluteFilePath("custom.json")))
 	{
 		QLOG_INFO() << "Reading custom.json";
-		VersionFile file;
-		if (!parseJsonFile(QFileInfo(root.absoluteFilePath("custom.json")), false, &file))
-		{
-			return false;
-		}
+		VersionFile file = parseJsonFile(QFileInfo(root.absoluteFilePath("custom.json")), false);
 		file.name = "custom.json";
 		file.filename = "custom.json";
 		file.fileId = "org.multimc.custom.json";
@@ -98,11 +91,8 @@ bool OneSixVersionBuilder::buildInternal(const bool onlyVanilla, const QStringLi
 		// version.json
 		QLOG_INFO() << "Reading version.json";
 		VersionFile file;
-		if (!parseJsonFile(QFileInfo(root.absoluteFilePath("version.json")), false, &file))
-		{
-			return false;
-		}
-		file.name = "version.json";
+		parseJsonFile(QFileInfo(root.absoluteFilePath("version.json")), false, &file);
+		file.name = "Minecraft";
 		file.fileId = "org.multimc.version.json";
 		file.version = m_instance->intendedVersionId();
 		file.mcVersion = m_instance->intendedVersionId();
@@ -120,19 +110,14 @@ bool OneSixVersionBuilder::buildInternal(const bool onlyVanilla, const QStringLi
 		for (auto info : patches.entryInfoList(QStringList() << "*.json", QDir::Files))
 		{
 			QLOG_INFO() << "Reading" << info.fileName();
-			VersionFile file;
-			if (!parseJsonFile(info, true, &file))
-			{
-				return false;
-			}
+			VersionFile file = parseJsonFile(info, true);
 			if (overrideOrder.contains(file.fileId))
 			{
 				file.order = overrideOrder.value(file.fileId);
 			}
 			if (files.contains(file.order))
 			{
-				QLOG_ERROR() << file.fileId << "has the same order as" << files[file.order].second.fileId;
-				return false;
+				throw VersionBuildError(QObject::tr("%1 has the same order as %2").arg(file.fileId, files[file.order].second.fileId));
 			}
 			files.insert(file.order, qMakePair(info.fileName(), file));
 		}
@@ -141,7 +126,6 @@ bool OneSixVersionBuilder::buildInternal(const bool onlyVanilla, const QStringLi
 			QLOG_DEBUG() << "Applying file with order" << order;
 			auto filePair = files[order];
 			filePair.second.applyTo(m_version);
-			// QObject::tr("Error while applying %1. Please check MultiMC-0.log for more info.").arg(filePair.first));
 		}
 	} while(0);
 
@@ -171,11 +155,9 @@ bool OneSixVersionBuilder::buildInternal(const bool onlyVanilla, const QStringLi
 			}
 		}
 	}
-
-	return true;
 }
 
-bool OneSixVersionBuilder::readJsonAndApply(const QJsonObject &obj)
+void OneSixVersionBuilder::readJsonAndApply(const QJsonObject &obj)
 {
 	m_version->clear();
 
@@ -185,32 +167,25 @@ bool OneSixVersionBuilder::readJsonAndApply(const QJsonObject &obj)
 	file.applyTo(m_version);
 	// QObject::tr("Error while applying. Please check MultiMC-0.log for more info."));
 	// QObject::tr("The version descriptors of this instance are not compatible with the current version of MultiMC"));
-	return true;
 }
 
-bool OneSixVersionBuilder::parseJsonFile(const QFileInfo& fileInfo, const bool requireOrder, VersionFile* out, bool isFTB)
+VersionFile OneSixVersionBuilder::parseJsonFile(const QFileInfo &fileInfo, const bool requireOrder, bool isFTB)
 {
 	QFile file(fileInfo.absoluteFilePath());
 	if (!file.open(QFile::ReadOnly))
 	{
-		// QObject::tr("Unable to open %1: %2").arg(file.fileName(), file.errorString());
-		return false;
+		throw JSONValidationError(QObject::tr("Unable to open %1: %2").arg(file.fileName(), file.errorString()));
 	}
 	QJsonParseError error;
 	QJsonDocument doc = QJsonDocument::fromJson(file.readAll(), &error);
 	if (error.error != QJsonParseError::NoError)
 	{
-		/*
-		QMessageBox::critical(m_widgetParent, QObject::tr("Error"),
-							  QObject::tr("Unable to parse %1: %2 at %3")
+		throw JSONValidationError(QObject::tr("Unable to parse %1: %2 at %3")
 								  .arg(file.fileName(), error.errorString())
 								  .arg(error.offset));
-								  */
-		return false;
 	}
-	*out = VersionFile::fromJson(doc, file.fileName(), requireOrder, isFTB);
+	return VersionFile::fromJson(doc, file.fileName(), requireOrder, isFTB);
 	// QObject::tr("Error while reading %1. Please check MultiMC-0.log for more info.").arg(file.fileName());
-	return true;
 }
 
 QMap<QString, int> OneSixVersionBuilder::readOverrideOrders(OneSixInstance *instance)
diff --git a/logic/OneSixVersionBuilder.h b/logic/OneSixVersionBuilder.h
index 789fe3f1..c5ef83f3 100644
--- a/logic/OneSixVersionBuilder.h
+++ b/logic/OneSixVersionBuilder.h
@@ -17,19 +17,20 @@
 
 #include <QString>
 #include <QMap>
+#include "VersionFile.h"
 
 class VersionFinal;
 class OneSixInstance;
 class QJsonObject;
 class QFileInfo;
-class VersionFile;
 
 class OneSixVersionBuilder
 {
 	OneSixVersionBuilder();
 public:
-	static bool build(VersionFinal *version, OneSixInstance *instance, const bool onlyVanilla, const QStringList &external);
-	static bool readJsonAndApplyToVersion(VersionFinal *version, const QJsonObject &obj);
+	static void build(VersionFinal *version, OneSixInstance *instance, const bool onlyVanilla,
+					  const QStringList &external);
+	static void readJsonAndApplyToVersion(VersionFinal *version, const QJsonObject &obj);
 
 	static QMap<QString, int> readOverrideOrders(OneSixInstance *instance);
 	static bool writeOverrideOrders(const QMap<QString, int> &order, OneSixInstance *instance);
@@ -38,8 +39,8 @@ private:
 	VersionFinal *m_version;
 	OneSixInstance *m_instance;
 
-	bool buildInternal(const bool onlyVanilla, const QStringList &external);
-	bool readJsonAndApply(const QJsonObject &obj);
+	void buildInternal(const bool onlyVanilla, const QStringList &external);
+	void readJsonAndApply(const QJsonObject &obj);
 
-	bool parseJsonFile(const QFileInfo &fileInfo, const bool requireOrder, VersionFile *out, bool isFTB = false);
+	VersionFile parseJsonFile(const QFileInfo &fileInfo, const bool requireOrder, bool isFTB = false);
 };
diff --git a/logic/VersionFile.cpp b/logic/VersionFile.cpp
index 4423e733..b7695907 100644
--- a/logic/VersionFile.cpp
+++ b/logic/VersionFile.cpp
@@ -13,10 +13,10 @@ using namespace MMCJson;
 
 #define CURRENT_MINIMUM_LAUNCHER_VERSION 14
 
-VersionFile::Library VersionFile::Library::fromJson(const QJsonObject &libObj,
+RawLibrary RawLibrary::fromJson(const QJsonObject &libObj,
 													const QString &filename)
 {
-	Library out;
+	RawLibrary out;
 	if (!libObj.contains("name"))
 	{
 		throw JSONValidationError(filename +
@@ -78,7 +78,8 @@ VersionFile::Library VersionFile::Library::fromJson(const QJsonObject &libObj,
 	return out;
 }
 
-VersionFile VersionFile::fromJson(const QJsonDocument &doc, const QString &filename, const bool requireOrder, const bool isFTB)
+VersionFile VersionFile::fromJson(const QJsonDocument &doc, const QString &filename,
+								  const bool requireOrder, const bool isFTB)
 {
 	VersionFile out;
 	if (doc.isEmpty() || doc.isNull())
@@ -173,12 +174,12 @@ VersionFile VersionFile::fromJson(const QJsonDocument &doc, const QString &filen
 		{
 			auto libObj = ensureObject(libVal);
 
-			Library lib = Library::fromJson(libObj, filename);
+			RawLibrary lib = RawLibrary::fromJson(libObj, filename);
 			// FIXME: This should be done when applying.
 			if (isFTB)
 			{
 				lib.hint = "local";
-				lib.insertType = Library::Prepend;
+				lib.insertType = RawLibrary::Prepend;
 				out.addLibs.prepend(lib);
 			}
 			else
@@ -196,7 +197,7 @@ VersionFile VersionFile::fromJson(const QJsonDocument &doc, const QString &filen
 			QJsonValue insertVal = ensureExists(libObj.value("insert"));
 
 			// parse the library
-			Library lib = Library::fromJson(libObj, filename);
+			RawLibrary lib = RawLibrary::fromJson(libObj, filename);
 
 			// TODO: utility functions for handling this case. templates?
 			QString insertString;
@@ -219,19 +220,19 @@ VersionFile VersionFile::fromJson(const QJsonDocument &doc, const QString &filen
 			}
 			if (insertString == "apply")
 			{
-				lib.insertType = Library::Apply;
+				lib.insertType = RawLibrary::Apply;
 			}
 			else if (insertString == "prepend")
 			{
-				lib.insertType = Library::Prepend;
+				lib.insertType = RawLibrary::Prepend;
 			}
 			else if (insertString == "append")
 			{
-				lib.insertType = Library::Prepend;
+				lib.insertType = RawLibrary::Prepend;
 			}
 			else if (insertString == "replace")
 			{
-				lib.insertType = Library::Replace;
+				lib.insertType = RawLibrary::Replace;
 			}
 			else
 			{
@@ -243,11 +244,11 @@ VersionFile VersionFile::fromJson(const QJsonDocument &doc, const QString &filen
 				const QString dependString = ensureString(libObj.value("MMC-depend"));
 				if (dependString == "hard")
 				{
-					lib.dependType = Library::Hard;
+					lib.dependType = RawLibrary::Hard;
 				}
 				else if (dependString == "soft")
 				{
-					lib.dependType = Library::Soft;
+					lib.dependType = RawLibrary::Soft;
 				}
 				else
 				{
@@ -269,7 +270,7 @@ VersionFile VersionFile::fromJson(const QJsonDocument &doc, const QString &filen
 	return out;
 }
 
-std::shared_ptr<OneSixLibrary> VersionFile::createLibrary(const VersionFile::Library &lib)
+std::shared_ptr<OneSixLibrary> VersionFile::createLibrary(const RawLibrary &lib)
 {
 	std::shared_ptr<OneSixLibrary> out(new OneSixLibrary(lib.name));
 	if (!lib.url.isEmpty())
@@ -396,7 +397,7 @@ void VersionFile::applyTo(VersionFinal *version)
 	{
 		switch (lib.insertType)
 		{
-		case Library::Apply:
+		case RawLibrary::Apply:
 		{
 
 			int index = findLibrary(version->libraries, lib.name);
@@ -439,8 +440,8 @@ void VersionFile::applyTo(VersionFinal *version)
 			}
 			break;
 		}
-		case Library::Append:
-		case Library::Prepend:
+		case RawLibrary::Append:
+		case RawLibrary::Prepend:
 		{
 
 			const int startOfVersion = lib.name.lastIndexOf(':') + 1;
@@ -448,7 +449,7 @@ void VersionFile::applyTo(VersionFinal *version)
 				version->libraries, QString(lib.name).replace(startOfVersion, INT_MAX, '*'));
 			if (index < 0)
 			{
-				if (lib.insertType == Library::Append)
+				if (lib.insertType == RawLibrary::Append)
 				{
 					version->libraries.append(createLibrary(lib));
 				}
@@ -469,7 +470,7 @@ void VersionFile::applyTo(VersionFinal *version)
 					// we need a higher version, or we're hard to and the versions aren't
 					// equal
 					if (ourVersion > otherVersion ||
-						(lib.dependType == Library::Hard && ourVersion != otherVersion))
+						(lib.dependType == RawLibrary::Hard && ourVersion != otherVersion))
 					{
 						throw VersionBuildError(
 							QString(
@@ -497,7 +498,7 @@ void VersionFile::applyTo(VersionFinal *version)
 					{
 						// our version is smaller than the existing version, but we require
 						// it: fail
-						if (lib.dependType == Library::Hard)
+						if (lib.dependType == RawLibrary::Hard)
 						{
 							throw VersionBuildError(QString(
 								"Error resolving library dependencies between %1 and %2 in %3.")
@@ -509,7 +510,7 @@ void VersionFile::applyTo(VersionFinal *version)
 			}
 			break;
 		}
-		case Library::Replace:
+		case RawLibrary::Replace:
 		{
 			int index = findLibrary(version->libraries, lib.insertData);
 			if (index >= 0)
diff --git a/logic/VersionFile.h b/logic/VersionFile.h
index 37c8c415..0f183ae8 100644
--- a/logic/VersionFile.h
+++ b/logic/VersionFile.h
@@ -20,8 +20,50 @@ public:
 	virtual ~VersionBuildError() {};
 };
 
+struct RawLibrary
+{
+	QString name;
+	QString url;
+	QString hint;
+	QString absoluteUrl;
+	bool applyExcludes = false;
+	QStringList excludes;
+	bool applyNatives = false;
+	QList<QPair<OpSys, QString>> natives;
+	bool applyRules = false;
+	QList<std::shared_ptr<Rule>> rules;
+
+	// user for '+' libraries
+	enum InsertType
+	{
+		Apply,
+		Append,
+		Prepend,
+		Replace
+	};
+	InsertType insertType = Append;
+	QString insertData;
+	enum DependType
+	{
+		Soft,
+		Hard
+	};
+	DependType dependType = Soft;
+
+	static RawLibrary fromJson(const QJsonObject &libObj, const QString &filename);
+};
+
 struct VersionFile
 {
+public: /* methods */
+	static VersionFile fromJson(const QJsonDocument &doc, const QString &filename,
+								const bool requireOrder, const bool isFTB = false);
+
+	static std::shared_ptr<OneSixLibrary> createLibrary(const RawLibrary &lib);
+	int findLibrary(QList<std::shared_ptr<OneSixLibrary>> haystack, const QString &needle);
+	void applyTo(VersionFinal *version);
+
+public: /* data */
 	int order;
 	QString name;
 	QString fileId;
@@ -48,47 +90,8 @@ struct VersionFile
 	QStringList addTweakers;
 	QStringList removeTweakers;
 
-	struct Library
-	{
-		QString name;
-		QString url;
-		QString hint;
-		QString absoluteUrl;
-		bool applyExcludes = false;
-		QStringList excludes;
-		bool applyNatives = false;
-		QList<QPair<OpSys, QString>> natives;
-		bool applyRules = false;
-		QList<std::shared_ptr<Rule>> rules;
-
-		// user for '+' libraries
-		enum InsertType
-		{
-			Apply,
-			Append,
-			Prepend,
-			Replace
-		};
-		InsertType insertType = Append;
-		QString insertData;
-		enum DependType
-		{
-			Soft,
-			Hard
-		};
-		DependType dependType = Soft;
-
-		static Library fromJson(const QJsonObject &libObj, const QString &filename);
-	};
 	bool shouldOverwriteLibs = false;
-	QList<Library> overwriteLibs;
-	QList<Library> addLibs;
+	QList<RawLibrary> overwriteLibs;
+	QList<RawLibrary> addLibs;
 	QList<QString> removeLibs;
-
-	static VersionFile fromJson(const QJsonDocument &doc, const QString &filename,
-								const bool requireOrder, const bool isFTB = false);
-
-	static std::shared_ptr<OneSixLibrary> createLibrary(const Library &lib);
-	int findLibrary(QList<std::shared_ptr<OneSixLibrary>> haystack, const QString &needle);
-	void applyTo(VersionFinal *version);
 };
\ No newline at end of file
diff --git a/logic/VersionFinal.cpp b/logic/VersionFinal.cpp
index 3aa95ed7..ec450eda 100644
--- a/logic/VersionFinal.cpp
+++ b/logic/VersionFinal.cpp
@@ -28,10 +28,10 @@ VersionFinal::VersionFinal(OneSixInstance *instance, QObject *parent)
 
 bool VersionFinal::reload(const bool onlyVanilla, const QStringList &external)
 {
+	//FIXME: source of epic failure.
 	beginResetModel();
-	bool ret = OneSixVersionBuilder::build(this, m_instance, onlyVanilla, external);
+	OneSixVersionBuilder::build(this, m_instance, onlyVanilla, external);
 	endResetModel();
-	return ret;
 }
 
 void VersionFinal::clear()
@@ -128,11 +128,15 @@ QList<std::shared_ptr<OneSixLibrary> > VersionFinal::getActiveNativeLibs()
 std::shared_ptr<VersionFinal> VersionFinal::fromJson(const QJsonObject &obj)
 {
 	std::shared_ptr<VersionFinal> version(new VersionFinal(0));
-	if (OneSixVersionBuilder::readJsonAndApplyToVersion(version.get(), obj))
+	try
 	{
-		return version;
+		OneSixVersionBuilder::readJsonAndApplyToVersion(version.get(), obj);
 	}
-	return 0;
+	catch(MMCError err)
+	{
+		return 0;
+	}
+	return version;
 }
 
 QVariant VersionFinal::data(const QModelIndex &index, int role) const