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

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());
+	try
-	if (ret)
-	{
-		ret = d->vanillaVersion->reload(true, externalPatches());
-	}
-	if (ret)
 	{
+		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()

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;
+		VersionFile file = parseJsonFile(QFileInfo(fileName), false, fileName.endsWith("pack.json"));
-		if (!parseJsonFile(QFileInfo(fileName), false, &file, fileName.endsWith("pack.json")))
-		{
-			return false;
-		}
 		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;
+		VersionFile file = parseJsonFile(QFileInfo(root.absoluteFilePath("custom.json")), false);
-		if (!parseJsonFile(QFileInfo(root.absoluteFilePath("custom.json")), false, &file))
-		{
-			return 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))
+		parseJsonFile(QFileInfo(root.absoluteFilePath("version.json")), false, &file);
-		{
+		file.name = "Minecraft";
-			return false;
-		}
-		file.name = "version.json";
 		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;
+			VersionFile file = parseJsonFile(info, true);
-			if (!parseJsonFile(info, true, &file))
-			{
-				return false;
-			}
 			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;
+				throw VersionBuildError(QObject::tr("%1 has the same order as %2").arg(file.fileId, files[file.order].second.fileId));
-				return false;
 			}
 			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());
+		throw JSONValidationError(QObject::tr("Unable to open %1: %2").arg(file.fileName(), file.errorString()));
-		return false;
 	}
 	QJsonParseError error;
 	QJsonDocument doc = QJsonDocument::fromJson(file.readAll(), &error);
 	if (error.error != QJsonParseError::NoError)
 	{
-		/*
+		throw JSONValidationError(QObject::tr("Unable to parse %1: %2 at %3")
-		QMessageBox::critical(m_widgetParent, QObject::tr("Error"),
-							  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)

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 void build(VersionFinal *version, OneSixInstance *instance, const bool onlyVanilla,
-	static bool readJsonAndApplyToVersion(VersionFinal *version, const QJsonObject &obj);
+					  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);
+	void buildInternal(const bool onlyVanilla, const QStringList &external);
-	bool readJsonAndApply(const QJsonObject &obj);
+	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);
 };

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 RawLibrary::Append:
-		case Library::Prepend:
+		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)

logic/VersionFile.h

@@ -20,35 +20,7 @@ public:
 	virtual ~VersionBuildError() {};
 };
 
-struct VersionFile
+struct RawLibrary
-{
-	int order;
-	QString name;
-	QString fileId;
-	QString version;
-	// TODO use the mcVersion to determine if a version file should be removed on update
-	QString mcVersion;
-	QString filename;
-	// TODO requirements
-	// QMap<QString, QString> requirements;
-	QString id;
-	QString mainClass;
-	QString overwriteMinecraftArguments;
-	QString addMinecraftArguments;
-	QString removeMinecraftArguments;
-	QString processArguments;
-	QString type;
-	QString releaseTime;
-	QString time;
-	QString assets;
-	int minimumLauncherVersion = -1;
-
-	bool shouldOverwriteTweakers = false;
-	QStringList overwriteTweakers;
-	QStringList addTweakers;
-	QStringList removeTweakers;
-
-	struct Library
 {
 	QString name;
 	QString url;
@@ -78,17 +50,48 @@ struct VersionFile
 	};
 	DependType dependType = Soft;
 
-		static Library fromJson(const QJsonObject &libObj, const QString &filename);
+	static RawLibrary fromJson(const QJsonObject &libObj, const QString &filename);
 };
-	bool shouldOverwriteLibs = false;
-	QList<Library> overwriteLibs;
-	QList<Library> addLibs;
-	QList<QString> removeLibs;
 
+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 Library &lib);
+	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;
+	QString version;
+	// TODO use the mcVersion to determine if a version file should be removed on update
+	QString mcVersion;
+	QString filename;
+	// TODO requirements
+	// QMap<QString, QString> requirements;
+	QString id;
+	QString mainClass;
+	QString overwriteMinecraftArguments;
+	QString addMinecraftArguments;
+	QString removeMinecraftArguments;
+	QString processArguments;
+	QString type;
+	QString releaseTime;
+	QString time;
+	QString assets;
+	int minimumLauncherVersion = -1;
+
+	bool shouldOverwriteTweakers = false;
+	QStringList overwriteTweakers;
+	QStringList addTweakers;
+	QStringList removeTweakers;
+
+	bool shouldOverwriteLibs = false;
+	QList<RawLibrary> overwriteLibs;
+	QList<RawLibrary> addLibs;
+	QList<QString> removeLibs;
 };

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,12 +128,16 @@ 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);
 	}
+	catch(MMCError err)
+	{
 		return 0;
 	}
+	return version;
+}
 
 QVariant VersionFinal::data(const QModelIndex &index, int role) const
 {