Parsing the version files, part IV

Also, start of big refactors.

gui/mainwindow.cpp

@@ -56,6 +56,7 @@
 #include "gameupdatetask.h"
 
 #include "instance.h"
+#include "instanceloader.h"
 #include "minecraftprocess.h"
 
 #include "instancemodel.h"
@@ -182,46 +183,40 @@ void MainWindow::on_actionAddInstance_triggered()
 	}
 	
 	NewInstanceDialog *newInstDlg = new NewInstanceDialog ( this );
-	if (newInstDlg->exec())
+	if (!newInstDlg->exec())
-	{
+		return;
+	
 	Instance *newInstance = NULL;
 	
 	QString instDirName = DirNameFromString(newInstDlg->instName());
-		QString instDir = PathCombine(globalSettings->get("InstanceDir").toString(), 
+	QString instDir = PathCombine(globalSettings->get("InstanceDir").toString(), instDirName);
-									  instDirName);
 	
-		InstanceLoader::InstLoaderError error = InstanceLoader::get().
+	auto &loader = InstanceLoader::get();
-				createInstance(newInstance, instDir);
+	auto error = loader.createInstance(newInstance, instDir);
-		
+	QString errorMsg = QString("Failed to create instance %1: ").arg(instDirName);
-		if (error == InstanceLoader::NoError)
-		{
-			newInstance->setName(newInstDlg->instName());
-			newInstance->setIntendedVersion(newInstDlg->selectedVersion()->descriptor());
-			instList.add(InstancePtr(newInstance));
-		}
-		else
-		{
-			QString errorMsg = QString("Failed to create instance %1: ").
-					arg(instDirName);
 	
 	switch (error)
 	{
+	case InstanceLoader::NoCreateError:
+		newInstance->setName(newInstDlg->instName());
+		newInstance->setIntendedVersion(newInstDlg->selectedVersion()->descriptor());
+		instList.add(InstancePtr(newInstance));
+		return;
+	
 	case InstanceLoader::InstExists:
 		errorMsg += "An instance with the given directory name already exists.";
+		QMessageBox::warning(this, "Error", errorMsg);
 		break;
 		
 	case InstanceLoader::CantCreateDir:
 		errorMsg += "Failed to create the instance directory.";
+		QMessageBox::warning(this, "Error", errorMsg);
 		break;
 		
 	default:
-				errorMsg += QString("Unknown instance loader error %1").
+		errorMsg += QString("Unknown instance loader error %1").arg(error);
-						arg(error);
-				break;
-			}
-			
 		QMessageBox::warning(this, "Error", errorMsg);
-		}
+		break;
 	}
 }
 

gui/newinstancedialog.cpp

@@ -17,7 +17,6 @@
 #include "ui_newinstancedialog.h"
 
 #include "instanceloader.h"
-#include "instancetypeinterface.h"
 
 #include "instversionlist.h"
 #include "instversion.h"
@@ -36,7 +35,6 @@ NewInstanceDialog::NewInstanceDialog(QWidget *parent) :
 	QDialog(parent),
 	ui(new Ui::NewInstanceDialog)
 {
-	m_selectedType = NULL;
 	m_selectedVersion = NULL;
 	
 	ui->setupUi(this);

gui/newinstancedialog.h

@@ -52,7 +52,6 @@ private:
 	Ui::NewInstanceDialog *ui;
 	
 	const InstVersion *m_selectedVersion;
-	const InstanceTypeInterface *m_selectedType;
 };
 
 #endif // NEWINSTANCEDIALOG_H

libmultimc/include/fullversion.h

@@ -6,49 +6,46 @@ class Library;
 class FullVersion
 {
 public:
-	FullVersion()
+	/// the ID - determines which jar to use! ACTUALLY IMPORTANT!
-	{
-		minimumLauncherVersion = 0xDEADBEEF;
-		isLegacy = false;
-	}
-	// the ID - determines which jar to use! ACTUALLY IMPORTANT!
 	QString id;
-	// do we actually care about parsing this?
+	/// Last updated time - as a string
 	QString time;
-	// I don't think we do.
+	/// Release time - as a string
 	QString releaseTime;
-	// eh, not caring - "release" or "snapshot"
+	/// Release type - "release" or "snapshot"
 	QString type;
-	/*
+	/**
 	 * DEPRECATED: Old versions of the new vanilla launcher used this
 	 * ex: "username_session_version"
 	 */
 	QString processArguments; 
-	/*
+	/**
 	 * arguments that should be used for launching minecraft
 	 * 
 	 * ex: "--username ${auth_player_name} --session ${auth_session}
 	 *      --version ${version_name} --gameDir ${game_directory} --assetsDir ${game_assets}"
 	 */
 	QString minecraftArguments;
-	/*
+	/**
 	 * the minimum launcher version required by this version ... current is 4 (at point of writing)
 	 */
 	int minimumLauncherVersion;
-	/*
+	/**
 	 * The main class to load first
 	 */
 	QString mainClass;
 	
-	// the list of libs. just the names for now. expand to full-blown strutures!
+	/// the list of libs - both active and inactive, native and java
 	QList<QSharedPointer<Library> > libraries;
 	
-	// is this actually a legacy version? if so, none of the other stuff here will be ever used.
+	/**
-	// added by FullVersionFactory
+	 * is this actually a legacy version? if so, none of the other stuff here will be ever used.
+	 * added by FullVersionFactory
+	 */
 	bool isLegacy;
 
-/*
+	/*
-FIXME: add support for those rules here? Looks like a pile of quick hacks to me though.
+	FIXME: add support for those rules here? Looks like a pile of quick hacks to me though.
 
 	"rules": [
 		{
@@ -63,7 +60,17 @@ FIXME: add support for those rules here? Looks like a pile of quick hacks to me
 		}
 	],
 	"incompatibilityReason": "There is a bug in LWJGL which makes it incompatible with OSX 10.5.8. Please go to New Profile and use 1.5.2 for now. Sorry!"
-}
+	}
-*/
+	*/
 	// QList<Rule> rules;
+	
+public:
+	FullVersion()
+	{
+		minimumLauncherVersion = 0xDEADBEEF;
+		isLegacy = false;
+	}
+	
+	QList<QSharedPointer<Library> > getActiveNormalLibs();
+	QList<QSharedPointer<Library> > getActiveNativeLibs();
 };

libmultimc/include/instance.h

@@ -22,7 +22,6 @@
 #include <settingsobject.h>
 
 #include "inifile.h"
-#include "instancetypeinterface.h"
 #include "instversionlist.h"
 
 #include "libmmc_config.h"

libmultimc/include/instancelist.h

@@ -27,6 +27,12 @@ class Instance;
 class LIBMULTIMC_EXPORT InstanceList : public QObject
 {
 	Q_OBJECT
+private:
+	/*!
+	 * \brief Get the instance groups
+	 */
+	void loadGroupList(QMap<QString, QString> & groupList);
+	
 public:
 	explicit InstanceList(const QString &instDir, QObject *parent = 0);
 	

libmultimc/include/instanceloader.h

@@ -36,22 +36,18 @@ public:
 	 */
 	static InstanceLoader &get() { return loader; }
 	
-	/*!
+	enum InstLoadError
-	 * \brief Error codes returned by functions in the InstanceLoader and InstanceType classes.
-	 *
-	 * - NoError indicates that no error occurred.
-	 * - OtherError indicates that an unspecified error occurred.
-	 * - InstExists is returned by createInstance() if the given instance directory is already an instance.
-	 * - NotAnInstance is returned by loadInstance() if the given instance directory is not a valid instance.
-	 * - CantCreateDir is returned by createInstance( if the given instance directory can't be created.)
-	 */
-	enum InstLoaderError
 	{
-		NoError = 0,
+		NoLoadError = 0,
-		OtherError,
+		UnknownLoadError,
+		NotAnInstance
+	};
 	
+	enum InstCreateError
+	{
+		NoCreateError = 0,
+		UnknownCreateError,
 		InstExists,
-		NotAnInstance,
 		CantCreateDir
 	};
 	
@@ -61,21 +57,21 @@ public:
 	 * \param inst Pointer to store the created instance in.
 	 * \param type The type of instance to create.
 	 * \param instDir The instance's directory.
-	 * \return An InstLoaderError error code.
+	 * \return An InstCreateError error code.
 	 * - InstExists if the given instance directory is already an instance.
 	 * - CantCreateDir if the given instance directory cannot be created.
 	 */
-	InstLoaderError createInstance(Instance *&inst, const QString &instDir);
+	InstCreateError createInstance(Instance *&inst, const QString &instDir);
 	
 	/*!
 	 * \brief Loads an instance from the given directory.
 	 * Checks the instance's INI file to figure out what the instance's type is first.
 	 * \param inst Pointer to store the loaded instance in.
 	 * \param instDir The instance's directory.
-	 * \return An InstLoaderError error code.
+	 * \return An InstLoadError error code.
 	 * - NotAnInstance if the given instance directory isn't a valid instance.
 	 */
-	InstLoaderError loadInstance(Instance *&inst, const QString &instDir);
+	InstLoadError loadInstance(Instance *&inst, const QString &instDir);
 	
 private:
 	InstanceLoader();

libmultimc/include/instancetypeinterface.h

@@ -1,94 +0,0 @@
-/* Copyright 2013 MultiMC Contributors
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- * 
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#ifndef INSTANCETYPE_H
-#define INSTANCETYPE_H
-
-#include <QObject>
-
-#include "instanceloader.h"
-
-class InstVersionList;
-
-//! The InstanceTypeInterface's interface ID.
-#define InstanceTypeInterface_IID "net.forkk.MultiMC.InstanceTypeInterface/0.1"
-
-/*!
- * \brief The InstanceType class is an interface for all instance types.
- * InstanceTypes are usually provided by plugins.
- * It handles loading and creating instances of a certain type. There should be 
- * one of these for each type of instance and they should be registered with the 
- * InstanceLoader.
- * To create an instance, the InstanceLoader calls the type's createInstance() 
- * function. Loading is done through the loadInstance() function.
- */
-class InstanceTypeInterface
-{
-public:
-	friend class InstanceLoader;
-	
-	/*!
-	 * \brief Gets the ID for this instance type. 
-	 *        The type ID should be unique as it is used to identify the type 
-	 *        of instances when they are loaded.
-	 *        Changing this value at runtime results in undefined behavior.
-	 * \return This instance type's ID string. 
-	 */
-	virtual QString typeID() const = 0;
-	
-	/*!
-	 * \brief Gets the name of this instance type as it is displayed to the user.
-	 * \return The instance type's display name.
-	 */
-	virtual QString displayName() const = 0;
-	
-	/*!
-	 * \brief Gets a longer, more detailed description of this instance type.
-	 * \return The instance type's description.
-	 */
-	virtual QString description() const = 0;
-	
-	/*!
-	 * \brief Gets the version list for this instance type.
-	 * \return A pointer to this instance type's version list.
-	 */
-	virtual InstVersionList *versionList() const = 0;
-	
-protected:
-	/*!
-	 * \brief Creates an instance and stores it in inst.
-	 * \param inst Pointer to store the created instance in.
-	 * \param instDir The instance's directory.
-	 * \return An InstTypeError error code.
-	 *         TypeNotRegistered if the given type is not registered with the InstanceLoader.
-	 *         InstExists if the given instance directory is already an instance.
-	 */
-	virtual InstanceLoader::InstLoaderError createInstance(Instance *&inst, const QString &instDir) const = 0;
-	
-	/*!
-	 * \brief Loads an instance from the given directory.
-	 * \param inst Pointer to store the loaded instance in.
-	 * \param instDir The instance's directory.
-	 * \return An InstTypeError error code.
-	 *         TypeNotRegistered if the given type is not registered with the InstanceLoader.
-	 *         NotAnInstance if the given instance directory isn't a valid instance.
-	 *         WrongInstType if the given instance directory's type isn't an instance of this type.
-	 */
-	virtual InstanceLoader::InstLoaderError loadInstance(Instance *&inst, const QString &instDir) const = 0;
-};
-
-Q_DECLARE_INTERFACE(InstanceTypeInterface, InstanceTypeInterface_IID)
-
-#endif // INSTANCETYPE_H

libmultimc/include/library.h

@@ -133,51 +133,7 @@ public:
 	QStringList extract_excludes;
 	
 public:
-	/// finalize the library, processing the input values into derived values and state
+	/// Constructor
-	void finalize()
-	{
-		QStringList parts = m_name.split(':');
-		QString relative = parts[0];
-		relative.replace('.','/');
-		relative += '/' + parts[1] + '/' + parts[2] + '/' + parts[1] + '-' + parts[2];
-		if(!m_is_native)
-			relative += ".jar";
-		else
-		{
-			if(m_native_suffixes.contains(currentSystem))
-			{
-				relative += "-" + m_native_suffixes[currentSystem] + ".jar";
-			}
-			else
-			{
-				// really, bad.
-				relative += ".jar";
-			}
-		}
-		m_storage_path = relative;
-		m_download_path = m_base_url + relative;
-		
-		if(m_rules.empty())
-		{
-			m_is_active = true;
-		}
-		else
-		{
-			RuleAction result = Disallow;
-			for(auto rule: m_rules)
-			{
-				RuleAction temp = rule->apply( this );
-				if(temp != Defer)
-					result = temp;
-			}
-			m_is_active = (result == Allow);
-		}
-		if(m_is_native)
-		{
-			m_is_active = m_is_active && m_native_suffixes.contains(currentSystem);
-		}
-	};
-	
 	Library(QString name)
 	{
 		m_is_native = false;
@@ -186,34 +142,67 @@ public:
 		m_base_url = "https://s3.amazonaws.com/Minecraft.Download/libraries/";
 	}
 	
+	/**
+	 * finalize the library, processing the input values into derived values and state
+	 * 
+	 * This SHALL be called after all the values are parsed or after any further change.
+	 */
+	void finalize();
+	
+	
+	/**
+	 * Set the library composite name
+	 */
 	void setName(QString name)
 	{
 		m_name = name;
 	}
 	
+	/**
+	 * Set the url base for downloads
+	 */
 	void setBaseUrl(QString base_url)
 	{
 		m_base_url = base_url;
 	}
 	
+	/**
+	 * Call this to mark the library as 'native' (it's a zip archive with DLLs)
+	 */
 	void setIsNative()
 	{
 		m_is_native = true;
 	}
 	
+	/**
+	 * Attach a name suffix to the specified OS native
+	 */
 	void addNative(OpSys os, QString suffix)
 	{
 		m_is_native = true;
 		m_native_suffixes[os] = suffix;
 	}
 	
+	/**
+	 * Set the load rules
+	 */
 	void setRules(QList<QSharedPointer<Rule> > rules)
 	{
 		m_rules = rules;
 	}
 
-	bool applies()
+	/**
+	 * Returns true if the library should be loaded (or extracted, in case of natives)
+	 */
+	bool getIsActive()
 	{
 		return m_is_active;
 	}
+	/**
+	 * Returns true if the library is native
+	 */
+	bool getIsNative()
+	{
+		return m_is_native;
+	}
 };

libmultimc/src/fullversion.cpp

@@ -2,4 +2,28 @@
 #include "fullversion.h"
 #include <library.h>
 
-// ECHO, echo, echo, ....
+QList<QSharedPointer<Library> > FullVersion::getActiveNormalLibs()
+{
+	QList<QSharedPointer<Library> > output;
+	for ( auto lib: libraries )
+	{
+		if (lib->getIsActive() && !lib->getIsNative())
+		{
+			output.append(lib);
+		}
+	}
+	return output;
+}
+
+QList<QSharedPointer<Library> > FullVersion::getActiveNativeLibs()
+{
+	QList<QSharedPointer<Library> > output;
+	for ( auto lib: libraries )
+	{
+		if (lib->getIsActive() && lib->getIsNative())
+		{
+			output.append(lib);
+		}
+	}
+	return output;
+}

libmultimc/src/instancelist.cpp

@@ -37,25 +37,22 @@ InstanceList::InstanceList(const QString &instDir, QObject *parent) :
 	
 }
 
-InstanceList::InstListError InstanceList::loadList()
+void InstanceList::loadGroupList(QMap<QString, QString> & groupMap)
 {
-	QDir dir(m_instDir);
-	QDirIterator iter(dir);
-	
 	QString groupFileName = m_instDir + "/instgroups.json";
-	// temporary map from instance ID to group name
-	QMap<QString, QString> groupMap;
 	
-	// HACK: this is really an if. breaks after one iteration.
+	// if there's no group file, fail
-	while (QFileInfo(groupFileName).exists())
+	if(!QFileInfo(groupFileName).exists())
-	{
+		return;
+	
 	QFile groupFile(groupFileName);
 	
+	// if you can't open the file, fail
 	if (!groupFile.open(QIODevice::ReadOnly))
 	{
 		// An error occurred. Ignore it.
 		qDebug("Failed to read instance group file.");
-			break;
+		return;
 	}
 	
 	QTextStream in(&groupFile);
@@ -65,36 +62,37 @@ InstanceList::InstListError InstanceList::loadList()
 	QJsonParseError error;
 	QJsonDocument jsonDoc = QJsonDocument::fromJson(jsonStr.toUtf8(), &error);
 	
+	// if the json was bad, fail
 	if (error.error != QJsonParseError::NoError)
 	{
 		qWarning(QString("Failed to parse instance group file: %1 at offset %2").
 					arg(error.errorString(), QString::number(error.offset)).toUtf8());
-			break;
+		return;
 	}
 	
+	// if the root of the json wasn't an object, fail
 	if (!jsonDoc.isObject())
 	{
 		qWarning("Invalid group file. Root entry should be an object.");
-			break;
+		return;
 	}
 	
 	QJsonObject rootObj = jsonDoc.object();
 	
-		// Make sure the format version matches.
+	// Make sure the format version matches, otherwise fail.
-		if (rootObj.value("formatVersion").toVariant().toInt() == GROUP_FILE_FORMAT_VERSION)
+	if (rootObj.value("formatVersion").toVariant().toInt() != GROUP_FILE_FORMAT_VERSION)
-		{
+		return;
-			// Get the group list.
+	
+	// Get the groups. if it's not an object, fail
 	if (!rootObj.value("groups").isObject())
 	{
 		qWarning("Invalid group list JSON: 'groups' should be an object.");
-				break;
+		return;
 	}
 		
-			// Iterate through the list.
+	// Iterate through all the groups.
-			QJsonObject groupList = rootObj.value("groups").toObject();
+	QJsonObject groupMapping = rootObj.value("groups").toObject();
-			
+	for (QJsonObject::iterator iter = groupMapping.begin(); iter != groupMapping.end(); iter++)
-			for (QJsonObject::iterator iter = groupList.begin(); 
-				 iter != groupList.end(); iter++)
 	{
 		QString groupName = iter.key();
 		
@@ -107,18 +105,6 @@ InstanceList::InstListError InstanceList::loadList()
 		}
 		
 		QJsonObject groupObj = iter.value().toObject();
-				/*
-				// Create the group object.
-				InstanceGroup *group = new InstanceGroup(groupName, this);
-				groups.push_back(group);
-				
-				// If 'hidden' isn't a bool value, just assume it's false.
-				if (groupObj.value("hidden").isBool() && groupObj.value("hidden").toBool())
-				{
-					group->setHidden(groupObj.value("hidden").toBool());
-				}
-				*/
-				
 		if (!groupObj.value("instances").isArray())
 		{
 			qWarning(QString("Group '%1' in the group list is invalid. "
@@ -136,21 +122,36 @@ InstanceList::InstListError InstanceList::loadList()
 			groupMap[(*iter2).toString()] = groupName;
 		}
 	}
-		}
+}
-		break;
+
-	}
+InstanceList::InstListError InstanceList::loadList()
+{
+	// load the instance groups
+	QMap<QString, QString> groupMap;
+	loadGroupList(groupMap);
+	
 	m_instances.clear();
+	QDir dir(m_instDir);
+	QDirIterator iter(dir);
 	while (iter.hasNext())
 	{
 		QString subDir = iter.next();
-		if (QFileInfo(PathCombine(subDir, "instance.cfg")).exists())
+		if (!QFileInfo(PathCombine(subDir, "instance.cfg")).exists())
-		{
+			continue;
+		
 		Instance *instPtr = NULL;
+		auto &loader = InstanceLoader::get();
+		auto error = loader.loadInstance(instPtr, subDir);
 		
-			InstanceLoader::InstLoaderError error = InstanceLoader::get().
+		switch(error)
-					loadInstance(instPtr, subDir);
+		{
+			case InstanceLoader::NoLoadError:
+				break;
+			case InstanceLoader::NotAnInstance:
+				break;
+		}
 		
-			if (error != InstanceLoader::NoError &&
+		if (error != InstanceLoader::NoLoadError &&
 			error != InstanceLoader::NotAnInstance)
 		{
 			QString errorMsg = QString("Failed to load instance %1: ").
@@ -184,7 +185,6 @@ InstanceList::InstListError InstanceList::loadList()
 			connect(instPtr, SIGNAL(propertiesChanged(Instance*)),this, SLOT(propertiesChanged(Instance*)));
 		}
 	}
-	}
 	emit invalidated();
 	return NoError;
 }

libmultimc/src/instanceloader.cpp

@@ -32,8 +32,7 @@ InstanceLoader::InstanceLoader() :
 	
 }
 
-InstanceLoader::InstLoaderError InstanceLoader::loadInstance(
+InstanceLoader::InstLoadError InstanceLoader::loadInstance(Instance *&inst, const QString &instDir)
-		Instance *&inst, const QString &instDir)
 {
 	Instance *loadedInst = new Instance(instDir, this);
 	
@@ -41,11 +40,11 @@ InstanceLoader::InstLoaderError InstanceLoader::loadInstance(
 	
 	inst = loadedInst;
 	
-	return NoError;
+	return NoLoadError;
 }
 
 
-InstanceLoader::InstLoaderError InstanceLoader::createInstance(Instance *&inst, const QString &instDir)
+InstanceLoader::InstCreateError InstanceLoader::createInstance(Instance *&inst, const QString &instDir)
 {
 	QDir rootDir(instDir);
 	
@@ -57,5 +56,6 @@ InstanceLoader::InstLoaderError InstanceLoader::createInstance(Instance *&inst,
 	
 	inst = new Instance(instDir, this);
 	
-	return InstanceLoader::NoError;
+	//FIXME: really, how do you even know?
+	return InstanceLoader::NoCreateError;
 }

libmultimc/src/library.cpp

@@ -34,4 +34,48 @@ OpSys OpSys_fromString(QString name)
 		return Os_OSX;
 	return Os_Other;
 }
+
+void Library::finalize()
+{
+	QStringList parts = m_name.split ( ':' );
+	QString relative = parts[0];
+	relative.replace ( '.','/' );
+	relative += '/' + parts[1] + '/' + parts[2] + '/' + parts[1] + '-' + parts[2];
+	if ( !m_is_native )
+		relative += ".jar";
+	else
+	{
+		if ( m_native_suffixes.contains ( currentSystem ) )
+		{
+			relative += "-" + m_native_suffixes[currentSystem] + ".jar";
+		}
+		else
+		{
+			// really, bad.
+			relative += ".jar";
+		}
+	}
+	m_storage_path = relative;
+	m_download_path = m_base_url + relative;
+
+	if ( m_rules.empty() )
+	{
+		m_is_active = true;
+	}
+	else
+	{
+		RuleAction result = Disallow;
+		for ( auto rule: m_rules )
+		{
+			RuleAction temp = rule->apply ( this );
+			if ( temp != Defer )
+				result = temp;
+		}
+		m_is_active = ( result == Allow );
+	}
+	if ( m_is_native )
+	{
+		m_is_active = m_is_active && m_native_suffixes.contains ( currentSystem );
+	}
+}
 // default url for lib: