Merge pull request #689 from kdmcser/develop

fix bug: may load mod before dependencies

lib/CModHandler.cpp

@@ -716,46 +716,68 @@ bool CModHandler::checkDependencies(const std::vector <TModID> & input) const
 	return true;
 }
 
-std::vector <TModID> CModHandler::resolveDependencies(std::vector <TModID> modsToResolve) const
-{
-	std::vector<TModID> brokenMods;
-
-	auto looksValid = [&](const CModInfo & mod) -> bool
+// Returned vector affects the resource loaders call order (see CFilesystemList::load).
+// The loaders call order matters when dependent mod overrides resources in its dependencies.
+std::vector <TModID> CModHandler::validateAndSortDependencies(std::vector <TModID> modsToResolve) const
+{
+	// Topological sort algorithm.
+	// TODO: Investigate possible ways to improve performance.
+	boost::range::sort(modsToResolve); // Sort mods per name
+	std::vector <TModID> sortedValidMods; // Vector keeps order of elements (LIFO) 
+	sortedValidMods.reserve(modsToResolve.size()); // push_back calls won't cause memory reallocation
+	std::set <TModID> resolvedModIDs; // Use a set for validation for performance reason, but set does not keep order of elements
+
+	// Mod is resolved if it has not dependencies or all its dependencies are already resolved
+	auto isResolved = [&](const CModInfo & mod) -> CModInfo::EValidationStatus
 	{
-		auto res = true;
+		if(mod.dependencies.size() > resolvedModIDs.size())
+			return CModInfo::PENDING;
+
 		for(const TModID & dependency : mod.dependencies)
 		{
-			if(!vstd::contains(modsToResolve, dependency))
-			{
-				logMod->error("Mod '%s' will not work: it depends on mod '%s', which is not installed.", mod.name, dependency);
-				res = false; //continue iterations, since we should show all errors for the current mod.
-			}
+			if(!vstd::contains(resolvedModIDs, dependency))
+				return CModInfo::PENDING;
 		}
-		return res;
+		return CModInfo::PASSED;
 	};
 
 	while(true)
 	{
-		for(auto mod : modsToResolve)
+		std::set <TModID> resolvedOnCurrentTreeLevel;
+		for(auto it = modsToResolve.begin(); it != modsToResolve.end();) // One iteration - one level of mods tree
 		{
-			if(!looksValid(this->allMods.at(mod)))
-				brokenMods.push_back(mod);
+			if(isResolved(allMods.at(*it)) == CModInfo::PASSED)
+			{
+				resolvedOnCurrentTreeLevel.insert(*it); // Not to the resolvedModIDs, so current node childs will be resolved on the next iteration
+				sortedValidMods.push_back(*it);
+				it = modsToResolve.erase(it);
+				continue;
+			}
+			it++;
 		}
-		if(!brokenMods.empty())
+		if(resolvedOnCurrentTreeLevel.size())
 		{
-			vstd::erase_if(modsToResolve, [&](TModID mid) 
-			{
-				return brokenMods.end() != std::find(brokenMods.begin(), brokenMods.end(), mid);
-			});
-			brokenMods.clear();
-			continue;
+			resolvedModIDs.insert(resolvedOnCurrentTreeLevel.begin(), resolvedOnCurrentTreeLevel.end());
+		            continue;
 		}
+		// If there're no valid mods on the current mods tree level, no more mod can be resolved, should be end.
 		break;
 	}
-	boost::range::sort(modsToResolve);
-	return modsToResolve;
+
+	// Left mods have unresolved dependencies, output all to log.
+	for(const auto & brokenModID : modsToResolve)
+	{
+		const CModInfo & brokenMod = allMods.at(brokenModID);
+		for(const TModID & dependency : brokenMod.dependencies)
+		{
+			if(!vstd::contains(resolvedModIDs, dependency))
+				logMod->error("Mod '%s' will not work: it depends on mod '%s', which is not installed.", brokenMod.name, dependency);
+		}
+	}
+	return sortedValidMods;
 }
 
+
 std::vector<std::string> CModHandler::getModList(std::string path)
 {
 	std::string modDir = boost::to_upper_copy(path + "MODS/");
@@ -895,7 +917,7 @@ static ui32 calculateModChecksum(const std::string modName, ISimpleResourceLoade
 
 void CModHandler::loadModFilesystems()
 {
-	activeMods = resolveDependencies(activeMods);
+	activeMods = validateAndSortDependencies(activeMods);
 
 	coreMod.updateChecksum(calculateModChecksum("core", CResourceHandler::get("core")));
 

lib/CModHandler.h

@@ -242,9 +242,15 @@ class DLL_LINKAGE CModHandler
 	// - circular dependencies
 	bool checkDependencies(const std::vector <TModID> & input) const;
 
-	// returns load order in which all dependencies are resolved, e.g. loaded after required mods
-	// function assumes that input list is valid (checkDependencies returned true)
-	std::vector <TModID> resolveDependencies(std::vector<TModID> input) const;
+	/**
+	* 1. Set apart mods with resolved dependencies from mods which have unresolved dependencies
+	* 2. Sort resolved mods using topological algorithm
+	* 3. Log all problem mods and their unresolved dependencies
+	*
+	* @param modsToResolve list of valid mod IDs (checkDependencies returned true - TODO: Clarify it.)
+	* @return a vector of the topologically sorted resolved mods: child nodes (dependent mods) have greater index than parents
+	*/
+	std::vector <TModID> validateAndSortDependencies(std::vector <TModID> modsToResolve) const;
 
 	std::vector<std::string> getModList(std::string path);
 	void loadMods(std::string path, std::string parent, const JsonNode & modSettings, bool enableMods);