Skip to content

Commit

Permalink
[7.2.0]LoadOrder::CheckValidity() drop master/children order check:
Browse files Browse the repository at this point in the history
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:
Ortham#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.
  • Loading branch information
Utumno committed Jun 27, 2015
1 parent c34973d commit e061f10
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 34 deletions.
2 changes: 1 addition & 1 deletion src/api/libloadorder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 4 additions & 7 deletions src/api/loadorder.h
Original file line number Diff line number Diff line change
Expand Up @@ -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__
Expand Down
55 changes: 31 additions & 24 deletions src/backend/plugins.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand All @@ -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 {
Expand Down Expand Up @@ -175,6 +179,9 @@ namespace liblo {
}
}

bool Plugin::esm() const { return isEsm; }
bool Plugin::exists() const { return exist; }

/////////////////////////
// LoadOrder Members
/////////////////////////
Expand Down Expand Up @@ -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<Plugin> hashset;
bool wasMaster = at(0).IsMasterFile(parentGame);
unordered_set<Plugin> 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 {
Expand Down Expand Up @@ -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<Plugin> 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)
Expand Down
7 changes: 5 additions & 2 deletions src/backend/plugins.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand All @@ -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.

Expand Down

0 comments on commit e061f10

Please sign in to comment.