diff --git a/src/backend/metadata/plugin_metadata.cpp b/src/backend/metadata/plugin_metadata.cpp index 44ce201d..a9a7d239 100644 --- a/src/backend/metadata/plugin_metadata.cpp +++ b/src/backend/metadata/plugin_metadata.cpp @@ -25,6 +25,7 @@ #include "plugin_metadata.h" #include "../game/game.h" #include "../helpers/helpers.h" +#include "../error.h" #include #include @@ -36,9 +37,9 @@ using namespace std; namespace loot { - PluginMetadata::PluginMetadata() : enabled(true), _isPriorityExplicit(false), priority(0) {} + PluginMetadata::PluginMetadata() : enabled(true), _isPriorityExplicit(false), isPriorityGlobal(false), priority(0) {} - PluginMetadata::PluginMetadata(const std::string& n) : name(n), enabled(true), _isPriorityExplicit(false), priority(0) { + PluginMetadata::PluginMetadata(const std::string& n) : name(n), enabled(true), _isPriorityExplicit(false), isPriorityGlobal(false), priority(0) { //If the name passed ends in '.ghost', that should be trimmed. if (boost::iends_with(name, ".ghost")) name = name.substr(0, name.length() - 6); @@ -52,7 +53,8 @@ namespace loot { //For 'enabled' and 'priority' metadata, use the given plugin's values, but if the 'priority' user value is not explicit, ignore it. enabled = plugin.Enabled(); if (plugin.IsPriorityExplicit()) { - priority = plugin.Priority(); + Priority(plugin.Priority()); + SetPriorityGlobal(plugin.IsPriorityGlobal()); _isPriorityExplicit = true; } @@ -87,8 +89,9 @@ namespace loot { BOOST_LOG_TRIVIAL(trace) << "Calculating metadata difference for: " << name; PluginMetadata p(*this); - if (priority == plugin.Priority()) { + if (Priority() == plugin.Priority() && IsPriorityGlobal() == plugin.IsPriorityGlobal()) { p.Priority(0); + p.SetPriorityGlobal(false); p.SetPriorityExplicit(false); } @@ -192,6 +195,14 @@ namespace loot { return priority; } + bool PluginMetadata::IsPriorityExplicit() const { + return priority != 0 || _isPriorityExplicit; + } + + bool PluginMetadata::IsPriorityGlobal() const { + return isPriorityGlobal; + } + std::set PluginMetadata::LoadAfter() const { return loadAfter; } @@ -224,12 +235,19 @@ namespace loot { enabled = e; } + void PluginMetadata::Priority(const int p) { + if (abs(p) >= yamlGlobalPriorityDivisor) + throw error(error::invalid_args, "Cannot set priority that has an absolute value greater than or equal to " + to_string(yamlGlobalPriorityDivisor)); + + priority = p; + } + void PluginMetadata::SetPriorityExplicit(bool state) { _isPriorityExplicit = state; } - void PluginMetadata::Priority(const int p) { - priority = p; + void PluginMetadata::SetPriorityGlobal(bool state) { + isPriorityGlobal = state; } void PluginMetadata::LoadAfter(const std::set& l) { @@ -321,10 +339,6 @@ namespace loot { return strpbrk(name.c_str(), ":\\*?|") != nullptr; } - bool PluginMetadata::IsPriorityExplicit() const { - return priority != 0 || _isPriorityExplicit; - } - bool PluginMetadata::operator == (const PluginMetadata& rhs) const { if (IsRegexPlugin() == rhs.IsRegexPlugin()) return boost::iequals(name, rhs.Name()); @@ -349,6 +363,17 @@ namespace loot { bool PluginMetadata::operator != (const std::string& rhs) const { return !(*this == rhs); } + + int PluginMetadata::GetYamlPriorityValue() const { + int priorityValue = Priority(); + if (IsPriorityGlobal()) { + if (priorityValue < 0) + priorityValue -= loot::yamlGlobalPriorityDivisor; + else + priorityValue += loot::yamlGlobalPriorityDivisor; + } + return priorityValue; + } } namespace YAML { @@ -357,8 +382,9 @@ namespace YAML { out << BeginMap << Key << "name" << Value << YAML::SingleQuoted << rhs.Name(); - if (rhs.IsPriorityExplicit()) - out << Key << "priority" << Value << rhs.Priority(); + if (rhs.IsPriorityExplicit()) { + out << Key << "priority" << Value << rhs.GetYamlPriorityValue(); + } if (!rhs.Enabled()) out << Key << "enabled" << Value << rhs.Enabled(); diff --git a/src/backend/metadata/plugin_metadata.h b/src/backend/metadata/plugin_metadata.h index 90a1a73b..5348f4b0 100644 --- a/src/backend/metadata/plugin_metadata.h +++ b/src/backend/metadata/plugin_metadata.h @@ -43,7 +43,7 @@ #include namespace loot { - const int max_priority = 1000000; + const int yamlGlobalPriorityDivisor = 1000000; class Game; @@ -69,6 +69,8 @@ namespace loot { std::string Name() const; bool Enabled() const; int Priority() const; + bool IsPriorityExplicit() const; + bool IsPriorityGlobal() const; std::set LoadAfter() const; std::set Reqs() const; std::set Incs() const; @@ -78,8 +80,9 @@ namespace loot { std::set Locations() const; void Enabled(const bool enabled); - void SetPriorityExplicit(bool state); void Priority(const int priority); + void SetPriorityExplicit(bool state); + void SetPriorityGlobal(bool state); void LoadAfter(const std::set& after); void Reqs(const std::set& reqs); void Incs(const std::set& incs); @@ -91,7 +94,6 @@ namespace loot { PluginMetadata& EvalAllConditions(Game& game, const unsigned int language); bool HasNameOnly() const; bool IsRegexPlugin() const; - bool IsPriorityExplicit() const; //Compare name strings. bool operator == (const PluginMetadata& rhs) const; @@ -100,11 +102,14 @@ namespace loot { //Compare name string. bool operator == (const std::string& rhs) const; bool operator != (const std::string& rhs) const; + + int GetYamlPriorityValue() const; private: std::string name; bool enabled; //Default to true. - bool _isPriorityExplicit; //If false and priority is 0, then priority was not explicitly set as such. int priority; //Default to 0 : >0 is lower down in load order, <0 is higher up. + bool _isPriorityExplicit; //If false and priority is 0, then priority was not explicitly set as such. + bool isPriorityGlobal; std::set loadAfter; std::set requirements; std::set incompatibilities; @@ -136,7 +141,7 @@ namespace YAML { node["enabled"] = rhs.Enabled(); if (rhs.IsPriorityExplicit()) - node["priority"] = rhs.Priority(); + node["priority"] = rhs.GetYamlPriorityValue(); if (!rhs.LoadAfter().empty()) node["after"] = rhs.LoadAfter(); @@ -178,8 +183,10 @@ namespace YAML { rhs.Enabled(node["enabled"].as()); if (node["priority"]) { - rhs.Priority(node["priority"].as()); + int priority = node["priority"].as(); + rhs.Priority(priority % loot::yamlGlobalPriorityDivisor); rhs.SetPriorityExplicit(true); + rhs.SetPriorityGlobal(abs(priority) >= loot::yamlGlobalPriorityDivisor); } if (node["after"]) diff --git a/src/backend/plugin_sorter.cpp b/src/backend/plugin_sorter.cpp index 8564e551..a1448d73 100644 --- a/src/backend/plugin_sorter.cpp +++ b/src/backend/plugin_sorter.cpp @@ -352,23 +352,22 @@ namespace loot { //as they are for loading BSAs, and in Skyrim that means the resources they load can //be affected by load order. + // If the plugin does not have a global priority and doesn't load + // an archive and has no override records, skip it. + if (!graph[*vit].IsPriorityGlobal() && graph[*vit].NumOverrideFormIDs() == 0 && !graph[*vit].LoadsBSA()) + continue; + loot::vertex_it vit2, vitend2; for (boost::tie(vit2, vitend2) = boost::vertices(graph); vit2 != vitend2; ++vit2) { + // If the plugins have equal priority, or have non-global + // priorities but don't conflict, don't add a priority edge. if (graph[*vit].Priority() == graph[*vit2].Priority() - || (abs(graph[*vit].Priority()) < max_priority && abs(graph[*vit2].Priority()) < max_priority - && !graph[*vit].getFormIds().empty() && !graph[*vit2].getFormIds().empty() && !graph[*vit].DoFormIDsOverlap(graph[*vit2]) - ) - ) { + || !graph[*vit].IsPriorityGlobal() && !graph[*vit2].IsPriorityGlobal() && !graph[*vit].DoFormIDsOverlap(graph[*vit2])) { continue; } - //BOOST_LOG_TRIVIAL(trace) << "Checking priority difference between \"" << graph[*vit].Name() << "\" and \"" << graph[*vit2].Name() << "\"."; - vertex_t vertex, parentVertex; - //Modulo operator is not consistently defined for negative numbers except in C++11, so use function. - int p1 = graph[*vit].Priority() % max_priority; - int p2 = graph[*vit2].Priority() % max_priority; - if (p1 < p2) { + if (graph[*vit].Priority() < graph[*vit2].Priority()) { parentVertex = *vit; vertex = *vit2; } diff --git a/src/gui/handler.cpp b/src/gui/handler.cpp index 5213ad26..0b4c7e53 100644 --- a/src/gui/handler.cpp +++ b/src/gui/handler.cpp @@ -491,18 +491,9 @@ namespace loot { if (pluginMetadata["modPriority"] && pluginMetadata["isGlobalPriority"]) { BOOST_LOG_TRIVIAL(trace) << "Priority value was changed, recalculating..."; // Priority value was changed, so add it to the userlist data. - int priority = pluginMetadata["modPriority"].as(); - - if (pluginMetadata["isGlobalPriority"].as()) { - if (priority >= 0) { - priority += max_priority; - } - else { - priority -= max_priority; - } - } - newUserlistEntry.Priority(priority); + newUserlistEntry.Priority(pluginMetadata["modPriority"].as()); newUserlistEntry.SetPriorityExplicit(true); + newUserlistEntry.SetPriorityGlobal(pluginMetadata["isGlobalPriority"].as()); } else { // Priority value wasn't changed, use the existing userlist value. @@ -510,6 +501,7 @@ namespace loot { if (!ulistPlugin.HasNameOnly()) { newUserlistEntry.Priority(ulistPlugin.Priority()); newUserlistEntry.SetPriorityExplicit(ulistPlugin.IsPriorityExplicit()); + newUserlistEntry.SetPriorityGlobal(ulistPlugin.IsPriorityGlobal()); } } @@ -1064,8 +1056,8 @@ namespace loot { // Now add to pluginNode. YAML::Node pluginNode; pluginNode["name"] = tempPlugin.Name(); - pluginNode["modPriority"] = tempPlugin.Priority() % max_priority; - pluginNode["isGlobalPriority"] = (abs(tempPlugin.Priority()) >= max_priority); + pluginNode["modPriority"] = tempPlugin.Priority(); + pluginNode["isGlobalPriority"] = tempPlugin.IsPriorityGlobal(); pluginNode["messages"] = tempPlugin.Messages(); pluginNode["tags"] = tempPlugin.Tags(); pluginNode["isDirty"] = isDirty; diff --git a/src/tests/backend/metadata/test_plugin_metadata.h b/src/tests/backend/metadata/test_plugin_metadata.h index 9fb03fc2..b797a535 100644 --- a/src/tests/backend/metadata/test_plugin_metadata.h +++ b/src/tests/backend/metadata/test_plugin_metadata.h @@ -34,12 +34,14 @@ TEST_F(PluginMetadata, Constructors) { loot::PluginMetadata pm; EXPECT_TRUE(pm.Enabled()); EXPECT_FALSE(pm.IsPriorityExplicit()); + EXPECT_FALSE(pm.IsPriorityGlobal()); EXPECT_EQ(0, pm.Priority()); pm = loot::PluginMetadata("Blank.esm"); EXPECT_EQ("Blank.esm", pm.Name()); EXPECT_TRUE(pm.Enabled()); EXPECT_FALSE(pm.IsPriorityExplicit()); + EXPECT_FALSE(pm.IsPriorityGlobal()); EXPECT_EQ(0, pm.Priority()); } @@ -134,6 +136,7 @@ TEST_F(PluginMetadata, MergeMetadata) { EXPECT_TRUE(pm1.Enabled()); EXPECT_EQ(5, pm1.Priority()); EXPECT_TRUE(pm1.IsPriorityExplicit()); + EXPECT_FALSE(pm1.IsPriorityGlobal()); EXPECT_EQ(std::set({ loot::File("Blank.esm"), loot::File("Blank.esp"), @@ -168,6 +171,7 @@ TEST_F(PluginMetadata, MergeMetadata) { EXPECT_NO_THROW(pm1.MergeMetadata(pm2)); EXPECT_EQ(0, pm1.Priority()); EXPECT_TRUE(pm1.IsPriorityExplicit()); + EXPECT_FALSE(pm1.IsPriorityGlobal()); } TEST_F(PluginMetadata, DiffMetadata) { @@ -244,6 +248,7 @@ TEST_F(PluginMetadata, DiffMetadata) { EXPECT_FALSE(result.Enabled()); EXPECT_EQ(0, result.Priority()); EXPECT_FALSE(result.IsPriorityExplicit()); + EXPECT_FALSE(result.IsPriorityGlobal()); EXPECT_EQ(std::set({ loot::File("Blank.esp"), loot::File("Blank - Different.esm"), @@ -279,6 +284,7 @@ TEST_F(PluginMetadata, DiffMetadata) { EXPECT_EQ(5, result.Priority()); EXPECT_TRUE(result.IsPriorityExplicit()); + EXPECT_FALSE(result.IsPriorityGlobal()); } TEST_F(PluginMetadata, NewMetadata) { @@ -355,6 +361,7 @@ TEST_F(PluginMetadata, NewMetadata) { EXPECT_TRUE(result.Enabled()); EXPECT_EQ(0, result.Priority()); EXPECT_FALSE(result.IsPriorityExplicit()); + EXPECT_FALSE(result.IsPriorityGlobal()); EXPECT_EQ(std::set({ loot::File("Blank.esp") }), result.LoadAfter()); @@ -383,6 +390,7 @@ TEST_F(PluginMetadata, NewMetadata) { EXPECT_EQ(0, result.Priority()); EXPECT_FALSE(result.IsPriorityExplicit()); + EXPECT_FALSE(result.IsPriorityGlobal()); } TEST_F(PluginMetadata, Enabled) { @@ -406,6 +414,48 @@ TEST_F(PluginMetadata, Priority) { EXPECT_EQ(10, pm.Priority()); } +TEST_F(PluginMetadata, settingPriorityWithAbsoluteValueGreaterOrEqualToGlobalPriorityDivisorShouldThrow) { + loot::PluginMetadata pm; + EXPECT_ANY_THROW(pm.Priority(loot::yamlGlobalPriorityDivisor)); + EXPECT_ANY_THROW(pm.Priority(loot::yamlGlobalPriorityDivisor + 1)); + EXPECT_ANY_THROW(pm.Priority(-loot::yamlGlobalPriorityDivisor)); + EXPECT_ANY_THROW(pm.Priority(-loot::yamlGlobalPriorityDivisor - 1)); +} + +TEST_F(PluginMetadata, settingPriorityAsGlobalShouldSucceed) { + loot::PluginMetadata pm; + ASSERT_FALSE(pm.IsPriorityGlobal()); + pm.SetPriorityGlobal(true); + EXPECT_TRUE(pm.IsPriorityGlobal()); +} + +TEST_F(PluginMetadata, settingPriorityAsNotGlobalShouldSucceed) { + loot::PluginMetadata pm; + pm.SetPriorityGlobal(true); + ASSERT_TRUE(pm.IsPriorityGlobal()); + pm.SetPriorityGlobal(false); + EXPECT_FALSE(pm.IsPriorityGlobal()); +} + +TEST_F(PluginMetadata, gettingYamlPriorityValueForAGlobalPriorityShouldReturnValueWithGlobalFlagSet) { + loot::PluginMetadata pm; + pm.Priority(10); + pm.SetPriorityGlobal(true); + EXPECT_EQ(1000010, pm.GetYamlPriorityValue()); + + pm.Priority(-20); + EXPECT_EQ(-1000020, pm.GetYamlPriorityValue()); +} + +TEST_F(PluginMetadata, gettingYamlPriorityValueForANonGlobalPriorityShouldReturnValueWithGlobalFlagNotSet) { + loot::PluginMetadata pm; + pm.Priority(10); + EXPECT_EQ(10, pm.GetYamlPriorityValue()); + + pm.Priority(-20); + EXPECT_EQ(-20, pm.GetYamlPriorityValue()); +} + TEST_F(PluginMetadata, LoadAfter) { loot::PluginMetadata pm; ASSERT_TRUE(pm.LoadAfter().empty()); @@ -798,6 +848,7 @@ TEST_F(PluginMetadata, YamlDecode) { EXPECT_EQ("Blank.esp", pm.Name()); EXPECT_EQ(0, pm.Priority()); EXPECT_FALSE(pm.IsPriorityExplicit()); + EXPECT_FALSE(pm.IsPriorityGlobal()); node = YAML::Load("name: 'Blank.esp'\n" "priority: 5\n" @@ -824,6 +875,7 @@ TEST_F(PluginMetadata, YamlDecode) { EXPECT_EQ("Blank.esp", pm.Name()); EXPECT_EQ(5, pm.Priority()); EXPECT_TRUE(pm.IsPriorityExplicit()); + EXPECT_FALSE(pm.IsPriorityGlobal()); EXPECT_FALSE(pm.Enabled()); EXPECT_EQ(std::set({ loot::File("Blank.esm") diff --git a/src/tests/backend/test_plugin_sorter.h b/src/tests/backend/test_plugin_sorter.h index 9d25dc3f..7bdd04e8 100644 --- a/src/tests/backend/test_plugin_sorter.h +++ b/src/tests/backend/test_plugin_sorter.h @@ -93,7 +93,8 @@ TEST_F(PluginSorter, Sort_HeadersOnly) { TEST_F(PluginSorter, Sort_WithPriority) { ASSERT_NO_THROW(game.LoadPlugins(false)); loot::PluginMetadata plugin("Blank - Different Master Dependent.esp"); - plugin.Priority(-1100000); + plugin.Priority(-100000); + plugin.SetPriorityGlobal(true); game.GetUserlist().AddPlugin(plugin); loot::PluginSorter ps;