From e061f105935debc03b7922299847160ab274d423 Mon Sep 17 00:00:00 2001 From: MrD Date: Sat, 27 Jun 2015 21:32:22 +0200 Subject: [PATCH] [7.2.0]LoadOrder::CheckValidity() drop master/children order check: Closes #7. Dropping out of order warn (for now at least): The library is too low level (Wrye Bash allows out of order masters for instance) + GetMasters is extra file IO. Note also that ActivePlugins did not check (asymmetry). Moreover lo_fix_plugin_lists did not fix this, leading to the stack overflows described in: https://github.com/WrinklyNinja/libloadorder/issues/16 This should be re-added as a weak warning - again will be addressed in liblo 8+ on unifying LO and active loading. LoadOrder::CheckValidity() changed to check as much as possible instead of throwing at first error Not sure about my way of deleting the file pointer - my C++ is rusty. Under #3: cache masters and existence checks !!! Current cache is a monkey patch, but the obvious cache key (Name, size, modtime) would fail for instance on esm flip by wrye bash. --- src/api/libloadorder.cpp | 2 +- src/api/loadorder.h | 11 +++----- src/backend/plugins.cpp | 55 ++++++++++++++++++++++------------------ src/backend/plugins.h | 7 +++-- 4 files changed, 41 insertions(+), 34 deletions(-) diff --git a/src/api/libloadorder.cpp b/src/api/libloadorder.cpp index 2ef8439..58f51d5 100644 --- a/src/api/libloadorder.cpp +++ b/src/api/libloadorder.cpp @@ -38,7 +38,7 @@ using namespace liblo; ------------------------------*/ const unsigned int LIBLO_VERSION_MAJOR = 7; -const unsigned int LIBLO_VERSION_MINOR = 1; +const unsigned int LIBLO_VERSION_MINOR = 2; const unsigned int LIBLO_VERSION_PATCH = 0; /* Returns whether this version of libloadorder is compatible with the given diff --git a/src/api/loadorder.h b/src/api/loadorder.h index f72ed54..94e8861 100644 --- a/src/api/loadorder.h +++ b/src/api/loadorder.h @@ -36,14 +36,11 @@ * - The first plugin in the load order must be the game's main master file. * - Loads all master files before all plugin files. Master bit flag value, * rather than file extension, is checked. - * - Loads each plugin after all its masters. * - * Note that due to the complexity of satisfying the final condition above, - * libloadorder does not attempt to satisfy it when lo_fix_plugin_lists() is - * run. In addition, if the load order passed to lo_set_load_order() does - * not contain an entry for all installed plugins, then libloadorder must - * provide load order positions for any missing plugins itself, and these - * positions may not satisfy the final condition above. + * Note that libloadorder does not attempt to load each plugin after all + * its masters. Note also that if the load order passed to lo_set_load_order() + * does not contain an entry for all installed plugins, then libloadorder must + * provide load order positions for any missing plugins itself. */ #ifndef __LIBLO_LOAD_ORDER__ diff --git a/src/backend/plugins.cpp b/src/backend/plugins.cpp index d23597e..1594964 100644 --- a/src/backend/plugins.cpp +++ b/src/backend/plugins.cpp @@ -74,15 +74,17 @@ namespace liblo { } bool Plugin::IsMasterFile(const _lo_game_handle_int& parentGame) const { + espm::File * file = nullptr; try { - espm::File * file = ReadHeader(parentGame); - + file = ReadHeader(parentGame); bool ret = file->isMaster(parentGame.espm_settings); - + isEsm = ret; delete file; return ret; } catch (std::exception&) { + // TODO(ut): log it ! + if (file != nullptr) delete file; return false; } } @@ -92,7 +94,9 @@ namespace liblo { } bool Plugin::Exists(const _lo_game_handle_int& parentGame) const { - return (fs::exists(parentGame.PluginsFolder() / name) || fs::exists(parentGame.PluginsFolder() / fs::path(name + ".ghost"))); + exist = fs::exists(parentGame.PluginsFolder() / name) || + fs::exists(parentGame.PluginsFolder() / fs::path(name + ".ghost")); + return exist; } time_t Plugin::GetModTime(const _lo_game_handle_int& parentGame) const { @@ -175,6 +179,9 @@ namespace liblo { } } + bool Plugin::esm() const { return isEsm; } + bool Plugin::exists() const { return exist; } + ///////////////////////// // LoadOrder Members ///////////////////////// @@ -341,26 +348,32 @@ namespace liblo { if (empty()) return; - if (at(0) != Plugin(parentGame.MasterFile())) - throw error(LIBLO_WARN_INVALID_LIST, "\"" + parentGame.MasterFile() + "\" is not the first plugin in the load order. " + at(0).Name() + " is first."); + std::string msg = ""; + + Plugin masterEsm = Plugin(parentGame.MasterFile()); + if (at(0) != masterEsm) + msg += "\"" + masterEsm.Name() + "\" is not the first plugin in the load order. " + + at(0).Name() + " is first.\n"; - bool wasMaster = true; - unordered_set hashset; + bool wasMaster = at(0).IsMasterFile(parentGame); + unordered_set hashset; // check for duplicates for (const auto plugin : *this) { - if (!plugin.Exists(parentGame)) - throw error(LIBLO_WARN_INVALID_LIST, "\"" + plugin.Name() + "\" is not installed."); + if (hashset.find(plugin) != hashset.end()) { + msg += "\"" + plugin.Name() + "\" is in the load order twice.\n"; + if (plugin.exists()) wasMaster = plugin.esm(); + continue; + } else hashset.insert(plugin); + if (!plugin.Exists(parentGame)){ + msg += "\"" + plugin.Name() + "\" is not installed.\n"; + continue; + } + // plugin exists bool isMaster = plugin.IsMasterFile(parentGame); if (isMaster && !wasMaster) - throw error(LIBLO_WARN_INVALID_LIST, "Master plugin \"" + plugin.Name() + "\" loaded after a non-master plugin."); - if (hashset.find(plugin) != hashset.end()) - throw error(LIBLO_WARN_INVALID_LIST, "\"" + plugin.Name() + "\" is in the load order twice."); - for (const auto &master : plugin.GetMasters(parentGame)) { - if (hashset.find(master) == hashset.end() && this->Find(master) != this->end()) //Only complain about masters loading after the plugin if the master is installed (so that Filter patches do not cause false positives). This means libloadorder doesn't check to ensure all a plugin's masters are present, but I don't think it should get mixed up with Bash Tag detection. - throw error(LIBLO_WARN_INVALID_LIST, "\"" + plugin.Name() + "\" is loaded before one of its masters (\"" + master.Name() + "\")."); - } - hashset.insert(plugin); + msg += "Master plugin \"" + plugin.Name() + "\" loaded after a non-master plugin.\n"; wasMaster = isMaster; } + if (msg != "") throw error(LIBLO_WARN_INVALID_LIST, msg); } bool LoadOrder::HasChanged(const _lo_game_handle_int& parentGame) const { @@ -604,12 +617,6 @@ namespace liblo { throw error(LIBLO_WARN_INVALID_LIST, "\"" + plugin.Name() + "\" is not installed."); else if (!plugin.IsValid(parentGame)) throw error(LIBLO_WARN_INVALID_LIST, "\"" + plugin.Name() + "\" is not a valid plugin file."); - /*vector masters = plugin.GetMasters(parentGame); - //Disabled because it causes false positives for Filter patches. This means libloadorder doesn't check to ensure all a plugin's masters are active, but I don't think it should get mixed up with Bash Tag detection. - for (const auto& master: masters) { - if (this->find(master) == this->end()) - throw error(LIBLO_ERROR_INVALID_ARGS, "\"" + plugin.Name() + "\" has a master (\"" + master.Name() + "\") which isn't active."); - }*/ } if (size() > 255) diff --git a/src/backend/plugins.h b/src/backend/plugins.h index e72c651..f0095e1 100644 --- a/src/backend/plugins.h +++ b/src/backend/plugins.h @@ -56,9 +56,12 @@ namespace liblo { bool operator == (const Plugin& rhs) const; bool operator != (const Plugin& rhs) const; + bool esm() const; + bool exists() const; private: std::string name; - + mutable bool isEsm = false; + mutable bool exist = false; espm::File * ReadHeader(const _lo_game_handle_int& parentGame) const; }; @@ -67,7 +70,7 @@ namespace liblo { void Load(const _lo_game_handle_int& parentGame); void Save(_lo_game_handle_int& parentGame); //Also updates mtime and active plugins list. - void CheckValidity(const _lo_game_handle_int& parentGame); //Game master first, masters before plugins, plugins all exist. + void CheckValidity(const _lo_game_handle_int& parentGame); //Game master first, plugins all exist. bool HasChanged(const _lo_game_handle_int& parentGame) const; //Checks timestamp and also if LoadOrder is empty.