From 3049c4b70e971f686d27d05242fa07a709bcd69b Mon Sep 17 00:00:00 2001 From: kdmcser Date: Sun, 4 Apr 2021 21:54:00 +0800 Subject: [PATCH 1/2] fix bug: may load mod before dependeny --- lib/CModHandler.cpp | 71 +++++++++++++++++++++++++++++---------------- 1 file changed, 46 insertions(+), 25 deletions(-) diff --git a/lib/CModHandler.cpp b/lib/CModHandler.cpp index b530e870c..83fc90369 100644 --- a/lib/CModHandler.cpp +++ b/lib/CModHandler.cpp @@ -716,46 +716,67 @@ bool CModHandler::checkDependencies(const std::vector & input) const return true; } -std::vector CModHandler::resolveDependencies(std::vector modsToResolve) const +std::vector CModHandler::resolveDependencies(std::vector input) const { - std::vector brokenMods; + // Topological sort algorithm + // May not be the fastest one but VCMI does not needs any speed here + // Unless user have dozens of mods with complex dependencies this code should be fine - auto looksValid = [&](const CModInfo & mod) -> bool + // first - sort input to have input strictly based on name (and not on hashmap or anything else) + boost::range::sort(input); + + std::vector output; + output.reserve(input.size()); + + std::set resolvedMods; + + // Check if all mod dependencies are resolved (moved to resolvedMods) + auto isResolved = [&](const CModInfo & mod) -> bool { - auto res = true; 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(resolvedMods, dependency)) + return false; } - return res; + return true; }; - while(true) + while (true) { - for(auto mod : modsToResolve) + size_t lastResolvedCount = resolvedMods.size(); + std::set toResolve; // list of mods resolved on this iteration + + for (auto it = input.begin(); it != input.end();) { - if(!looksValid(this->allMods.at(mod))) - brokenMods.push_back(mod); - } - if(!brokenMods.empty()) - { - vstd::erase_if(modsToResolve, [&](TModID mid) + if (isResolved(allMods.at(*it))) { - return brokenMods.end() != std::find(brokenMods.begin(), brokenMods.end(), mid); - }); - brokenMods.clear(); - continue; + toResolve.insert(*it); + output.push_back(*it); + it = input.erase(it); + continue; + } + it++; } - break; + resolvedMods.insert(toResolve.begin(), toResolve.end()); + + if (lastResolvedCount == resolvedMods.size()) + break; // No more mod can be resolved, should be end. } - boost::range::sort(modsToResolve); - return modsToResolve; + + + // Left mods are broken, output all to log. + for (auto it = input.begin(); it != input.end(); it++) + { + const CModInfo & mod = allMods.at(*it); + for(const TModID & dependency : mod.dependencies) + if(!vstd::contains(resolvedMods, dependency)) + logMod->error("Mod '%s' will not work: it depends on mod '%s', which is not installed.", mod.name, dependency); + } + + return output; } + std::vector CModHandler::getModList(std::string path) { std::string modDir = boost::to_upper_copy(path + "MODS/"); From 10dc6922de5c76bf1a942d48689162db23eac8d3 Mon Sep 17 00:00:00 2001 From: kdmcser Date: Sat, 10 Apr 2021 23:23:58 +0800 Subject: [PATCH 2/2] refactor code: change code style and add some comments --- lib/CModHandler.cpp | 81 +++++++++++++++++++++++---------------------- lib/CModHandler.h | 12 +++++-- 2 files changed, 50 insertions(+), 43 deletions(-) diff --git a/lib/CModHandler.cpp b/lib/CModHandler.cpp index 83fc90369..59b575056 100644 --- a/lib/CModHandler.cpp +++ b/lib/CModHandler.cpp @@ -716,64 +716,65 @@ bool CModHandler::checkDependencies(const std::vector & input) const return true; } -std::vector CModHandler::resolveDependencies(std::vector input) const +// 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 CModHandler::validateAndSortDependencies(std::vector modsToResolve) const { - // Topological sort algorithm - // May not be the fastest one but VCMI does not needs any speed here - // Unless user have dozens of mods with complex dependencies this code should be fine + // Topological sort algorithm. + // TODO: Investigate possible ways to improve performance. + boost::range::sort(modsToResolve); // Sort mods per name + std::vector sortedValidMods; // Vector keeps order of elements (LIFO) + sortedValidMods.reserve(modsToResolve.size()); // push_back calls won't cause memory reallocation + std::set resolvedModIDs; // Use a set for validation for performance reason, but set does not keep order of elements - // first - sort input to have input strictly based on name (and not on hashmap or anything else) - boost::range::sort(input); - - std::vector output; - output.reserve(input.size()); - - std::set resolvedMods; - - // Check if all mod dependencies are resolved (moved to resolvedMods) - auto isResolved = [&](const CModInfo & mod) -> bool + // Mod is resolved if it has not dependencies or all its dependencies are already resolved + auto isResolved = [&](const CModInfo & mod) -> CModInfo::EValidationStatus { + if(mod.dependencies.size() > resolvedModIDs.size()) + return CModInfo::PENDING; + for(const TModID & dependency : mod.dependencies) { - if (!vstd::contains(resolvedMods, dependency)) - return false; + if(!vstd::contains(resolvedModIDs, dependency)) + return CModInfo::PENDING; } - return true; + return CModInfo::PASSED; }; - while (true) + while(true) { - size_t lastResolvedCount = resolvedMods.size(); - std::set toResolve; // list of mods resolved on this iteration - - for (auto it = input.begin(); it != input.end();) + std::set resolvedOnCurrentTreeLevel; + for(auto it = modsToResolve.begin(); it != modsToResolve.end();) // One iteration - one level of mods tree { - if (isResolved(allMods.at(*it))) + if(isResolved(allMods.at(*it)) == CModInfo::PASSED) { - toResolve.insert(*it); - output.push_back(*it); - it = input.erase(it); + 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++; } - resolvedMods.insert(toResolve.begin(), toResolve.end()); - - if (lastResolvedCount == resolvedMods.size()) - break; // No more mod can be resolved, should be end. + if(resolvedOnCurrentTreeLevel.size()) + { + 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; } - - // Left mods are broken, output all to log. - for (auto it = input.begin(); it != input.end(); it++) + // Left mods have unresolved dependencies, output all to log. + for(const auto & brokenModID : modsToResolve) { - const CModInfo & mod = allMods.at(*it); - for(const TModID & dependency : mod.dependencies) - if(!vstd::contains(resolvedMods, dependency)) - logMod->error("Mod '%s' will not work: it depends on mod '%s', which is not installed.", mod.name, dependency); + 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 output; + return sortedValidMods; } @@ -916,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"))); diff --git a/lib/CModHandler.h b/lib/CModHandler.h index 6be65e728..6bc7373db 100644 --- a/lib/CModHandler.h +++ b/lib/CModHandler.h @@ -242,9 +242,15 @@ class DLL_LINKAGE CModHandler // - circular dependencies bool checkDependencies(const std::vector & 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 resolveDependencies(std::vector 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 validateAndSortDependencies(std::vector modsToResolve) const; std::vector getModList(std::string path); void loadMods(std::string path, std::string parent, const JsonNode & modSettings, bool enableMods);