From 4baad635b1d892c1b89e59eb385c8f34d398e8ff Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Fri, 15 Jul 2016 18:22:57 +0100 Subject: [PATCH] Rework priority metadata The priority field now stores values between -127 and 127 inclusive, and global priorities are given by a separate global_priority field with the same value range. Sorting compares global priorities first before falling back to priorities if the former are equal. --- CMakeLists.txt | 3 + docs/LOOT Metadata Syntax.html | 12 +- src/backend/metadata/plugin_metadata.cpp | 78 +++--- src/backend/metadata/plugin_metadata.h | 38 +-- src/backend/metadata/priority.cpp | 68 +++++ src/backend/metadata/priority.h | 55 ++++ src/backend/plugin/plugin_sorter.cpp | 52 +++- src/gui/html/elements/loot-plugin-editor.html | 21 +- src/gui/html/elements/loot-plugin-item.html | 32 ++- src/gui/html/index.html | 2 +- src/gui/html/js/game.js | 2 +- src/gui/html/js/plugin.js | 14 +- src/gui/html/js/translateStaticText.js | 8 +- src/gui/query_handler.cpp | 38 ++- src/tests/backend/main.cpp | 1 + .../backend/metadata/plugin_metadata_test.h | 258 ++++++++++-------- src/tests/backend/metadata/priority_test.h | 112 ++++++++ src/tests/backend/metadata_list_test.h | 10 +- src/tests/backend/plugin/plugin_sorter_test.h | 38 ++- src/tests/gui/html/js/test_game.js | 6 +- src/tests/gui/html/js/test_plugin.js | 36 +-- 21 files changed, 578 insertions(+), 306 deletions(-) create mode 100644 src/backend/metadata/priority.cpp create mode 100644 src/backend/metadata/priority.h create mode 100644 src/tests/backend/metadata/priority_test.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 7e956dd6..61922108 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -163,6 +163,7 @@ set (LOOT_SRC "${CMAKE_SOURCE_DIR}/src/backend/app/loot_paths.cpp" "${CMAKE_SOURCE_DIR}/src/backend/metadata/message_content.cpp" "${CMAKE_SOURCE_DIR}/src/backend/metadata/plugin_cleaning_data.cpp" "${CMAKE_SOURCE_DIR}/src/backend/metadata/plugin_metadata.cpp" + "${CMAKE_SOURCE_DIR}/src/backend/metadata/priority.cpp" "${CMAKE_SOURCE_DIR}/src/backend/metadata/tag.cpp" "${CMAKE_SOURCE_DIR}/src/backend/game/game.cpp" "${CMAKE_SOURCE_DIR}/src/backend/game/game_cache.cpp" @@ -189,6 +190,7 @@ set (LOOT_HEADERS "${CMAKE_SOURCE_DIR}/src/backend/app/loot_paths.h" "${CMAKE_SOURCE_DIR}/src/backend/metadata/message_content.h" "${CMAKE_SOURCE_DIR}/src/backend/metadata/plugin_cleaning_data.h" "${CMAKE_SOURCE_DIR}/src/backend/metadata/plugin_metadata.h" + "${CMAKE_SOURCE_DIR}/src/backend/metadata/priority.h" "${CMAKE_SOURCE_DIR}/src/backend/metadata/tag.h" "${CMAKE_SOURCE_DIR}/src/backend/game/game.h" "${CMAKE_SOURCE_DIR}/src/backend/game/game_cache.h" @@ -256,6 +258,7 @@ set (LOOT_TESTS_HEADERS # Testing this here rather than as part of the API tests "${CMAKE_SOURCE_DIR}/src/tests/backend/metadata/message_content_test.h" "${CMAKE_SOURCE_DIR}/src/tests/backend/metadata/plugin_cleaning_data_test.h" "${CMAKE_SOURCE_DIR}/src/tests/backend/metadata/plugin_metadata_test.h" + "${CMAKE_SOURCE_DIR}/src/tests/backend/metadata/priority_test.h" "${CMAKE_SOURCE_DIR}/src/tests/backend/metadata/tag_test.h" "${CMAKE_SOURCE_DIR}/src/tests/backend/plugin/plugin_test.h" "${CMAKE_SOURCE_DIR}/src/tests/backend/plugin/plugin_sorter_test.h" diff --git a/docs/LOOT Metadata Syntax.html b/docs/LOOT Metadata Syntax.html index f1fd95a3..4b5920ac 100644 --- a/docs/LOOT Metadata Syntax.html +++ b/docs/LOOT Metadata Syntax.html @@ -457,13 +457,12 @@ udr: 160 namestring✓Can be an exact plugin filename or a regular expression plugin filename. If the filename contains any of the characters :\*?|, the string will be treated as a regular expression, otherwise it will be treated as an exact filename. For example, Example\.esm will be treated as a regular expression, as it contains a \ character. enabledboolean✗Enables or disables use of the plugin object. Used for user rules, but no reason to use it in the masterlist. If unspecified, defaults to true. - priorityinteger✗Modifies plugin position relative to others that change one or more of the same records, but which are otherwise unrelated (ie. neither plugin lists the other as a master, requirement, or in its after list). Plugins that don't change any of the same records are not compared, unless: - -

For example, priority: 153000352 and priority: 352 have equal values when compared, but a plugin with the first priority will be compared against all others, while a (non-empty) plugin with the second priority will be compared against only those it conflicts with. -

A plugin with a higher priority value will load after a plugin with a lower priority value. The value can be anything in the range -2000000000 to 2000000000, and if unspecified defaults to 0. +

A plugin with a higher priority value will load after a plugin with a lower priority value. The value can be anything in the range -127 to 127 inclusive, and if unspecified defaults to 0. + global_priorityinteger✗Modifies plugin position relative to all unrelated plugins (ie. neither plugin lists the other as a master, requirement, or in its after list). +

A plugin with a higher global_priority value will load after a plugin with a lower priority value. The value can be anything in the range -127 to 127 inclusive, and if unspecified defaults to 0. +

global_priority takes precedence over priority when comparing two plugins' priorities: the priority value is only compared if the two plugins have the same global_priority value. afterfile set✗An unordered set of plugins that this plugin must load after, but which are not dependencies. Used for resolving specific compatibility issues and by user rules for specifying custom plugin positions. reqfile set✗An unordered set of files that this plugin requires to be present. This plugin will load after any plugins listed. If any of these files are missing, an error message will be displayed. Intended for use specifying implicit dependencies, as LOOT will detect a plugin's explicit masters itself. incfile set✗An unordered set of files that this plugin is incompatible with. If any of these files are present, an error message will be displayed. @@ -489,6 +488,7 @@ udr: 160 nameNot merged. enabledReplaced by B's value. priorityReplaced by B's value, unless that value is 0 and it was not explicitly set. + global_priorityReplaced by B's value, unless that value is 0 and it was not explicitly set. afterMerged. If B's file set contains an item that is equal to one already present in A's file set, B's item is discarded. reqMerged. If B's file set contains an item that is equal to one already present in A's file set, B's item is discarded. incMerged. If B's file set contains an item that is equal to one already present in A's file set, B's item is discarded. diff --git a/src/backend/metadata/plugin_metadata.cpp b/src/backend/metadata/plugin_metadata.cpp index 49e7175d..3c7f0ab3 100644 --- a/src/backend/metadata/plugin_metadata.cpp +++ b/src/backend/metadata/plugin_metadata.cpp @@ -43,9 +43,9 @@ using std::regex_match; using std::set; namespace loot { -PluginMetadata::PluginMetadata() : enabled_(true), isPriorityExplicit_(false), isPriorityGlobal_(false), priority_(0) {} +PluginMetadata::PluginMetadata() : enabled_(true) {} -PluginMetadata::PluginMetadata(const std::string& n) : name_(n), enabled_(true), isPriorityExplicit_(false), isPriorityGlobal_(false), priority_(0) { +PluginMetadata::PluginMetadata(const std::string& n) : name_(n), enabled_(true) { //If the name passed ends in '.ghost', that should be trimmed. if (boost::iends_with(name_, ".ghost")) name_ = name_.substr(0, name_.length() - 6); @@ -59,10 +59,13 @@ void PluginMetadata::MergeMetadata(const PluginMetadata& plugin) { // 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()); - SetPriorityGlobal(plugin.IsPriorityGlobal()); - isPriorityExplicit_ = true; + + if (plugin.localPriority_.isExplicit()) { + LocalPriority(plugin.localPriority_); + } + + if (plugin.globalPriority_.isExplicit()) { + GlobalPriority(plugin.globalPriority_); } // Merge the following. If any files in the source already exist in the @@ -94,11 +97,11 @@ PluginMetadata PluginMetadata::DiffMetadata(const PluginMetadata& plugin) const BOOST_LOG_TRIVIAL(trace) << "Calculating metadata difference for: " << name_; PluginMetadata p(*this); - if (Priority() == plugin.Priority() && IsPriorityGlobal() == plugin.IsPriorityGlobal()) { - p.Priority(0); - p.SetPriorityGlobal(false); - p.SetPriorityExplicit(false); - } + if (localPriority_ == plugin.localPriority_) + p.localPriority_ = Priority(); + + if (globalPriority_ == plugin.globalPriority_) + p.globalPriority_ = Priority(); //Compare this plugin against the given plugin. set filesDiff; @@ -258,16 +261,12 @@ bool PluginMetadata::Enabled() const { return enabled_; } -int PluginMetadata::Priority() const { - return priority_; +Priority PluginMetadata::LocalPriority() const { + return localPriority_; } -bool PluginMetadata::IsPriorityExplicit() const { - return priority_ != 0 || isPriorityExplicit_; -} - -bool PluginMetadata::IsPriorityGlobal() const { - return isPriorityGlobal_; +Priority PluginMetadata::GlobalPriority() const { + return globalPriority_; } std::set PluginMetadata::LoadAfter() const { @@ -306,19 +305,12 @@ void PluginMetadata::Enabled(const bool e) { enabled_ = e; } -void PluginMetadata::Priority(const int p) { - if (abs(p) >= yamlGlobalPriorityDivisor) - throw Error(Error::Code::invalid_args, "Cannot set priority that has an absolute value greater than or equal to " + std::to_string(yamlGlobalPriorityDivisor)); - - priority_ = p; +void PluginMetadata::LocalPriority(const Priority& priority) { + localPriority_ = priority; } -void PluginMetadata::SetPriorityExplicit(bool state) { - isPriorityExplicit_ = state; -} - -void PluginMetadata::SetPriorityGlobal(bool state) { - isPriorityGlobal_ = state; +void PluginMetadata::GlobalPriority(const Priority& priority) { + globalPriority_ = priority; } void PluginMetadata::LoadAfter(const std::set& l) { @@ -411,7 +403,8 @@ PluginMetadata& PluginMetadata::EvalAllConditions(Game& game, const Language::Co } bool PluginMetadata::HasNameOnly() const { - return !IsPriorityExplicit() + return !localPriority_.isExplicit() + && !globalPriority_.isExplicit() && loadAfter_.empty() && requirements_.empty() && incompatibilities_.empty() @@ -453,17 +446,6 @@ bool PluginMetadata::operator == (const std::string& rhs) const { bool PluginMetadata::operator != (const std::string& rhs) const { return !(*this == rhs); } - -int PluginMetadata::GetYamlPriorityValue() const { - int priorityValue = Priority(); - if (IsPriorityGlobal()) { - if (priorityValue < 0) - priorityValue -= yamlGlobalPriorityDivisor; - else - priorityValue += yamlGlobalPriorityDivisor; - } - return priorityValue; -} } namespace YAML { @@ -472,13 +454,17 @@ Emitter& operator << (Emitter& out, const loot::PluginMetadata& rhs) { out << BeginMap << Key << "name" << Value << YAML::SingleQuoted << rhs.Name(); - if (rhs.IsPriorityExplicit()) { - out << Key << "priority" << Value << rhs.GetYamlPriorityValue(); - } - if (!rhs.Enabled()) out << Key << "enabled" << Value << rhs.Enabled(); + if (rhs.LocalPriority().isExplicit()) { + out << Key << "priority" << Value << rhs.LocalPriority().getValue(); + } + + if (rhs.GlobalPriority().isExplicit()) { + out << Key << "global_priority" << Value << rhs.GlobalPriority().getValue(); + } + if (!rhs.LoadAfter().empty()) out << Key << "after" << Value << rhs.LoadAfter(); diff --git a/src/backend/metadata/plugin_metadata.h b/src/backend/metadata/plugin_metadata.h index cb75fcdc..3a8127c4 100644 --- a/src/backend/metadata/plugin_metadata.h +++ b/src/backend/metadata/plugin_metadata.h @@ -39,13 +39,12 @@ #include "backend/metadata/location.h" #include "backend/metadata/message.h" #include "backend/metadata/plugin_cleaning_data.h" +#include "backend/metadata/priority.h" #include "backend/metadata/tag.h" namespace loot { class Game; -const int yamlGlobalPriorityDivisor = 1000000; - class PluginMetadata { public: PluginMetadata(); @@ -67,9 +66,8 @@ public: std::string Name() const; bool Enabled() const; - int Priority() const; - bool IsPriorityExplicit() const; - bool IsPriorityGlobal() const; + Priority LocalPriority() const; + Priority GlobalPriority() const; std::set LoadAfter() const; std::set Reqs() const; std::set Incs() const; @@ -80,9 +78,8 @@ public: std::set Locations() const; void Enabled(const bool enabled); - void Priority(const int priority); - void SetPriorityExplicit(bool state); - void SetPriorityGlobal(bool state); + void LocalPriority(const Priority& priority); + void GlobalPriority(const Priority& priority); void LoadAfter(const std::set& after); void Reqs(const std::set& reqs); void Incs(const std::set& incs); @@ -103,17 +100,14 @@ public: //Compare name string. bool operator == (const std::string& rhs) const; bool operator != (const std::string& rhs) const; - - int GetYamlPriorityValue() const; protected: std::list messages_; std::set tags_; private: std::string name_; bool enabled_; //Default to true. - 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_; + Priority localPriority_; + Priority globalPriority_; std::set loadAfter_; std::set requirements_; std::set incompatibilities_; @@ -142,8 +136,11 @@ struct convert { if (!rhs.Enabled()) node["enabled"] = rhs.Enabled(); - if (rhs.IsPriorityExplicit()) - node["priority"] = rhs.GetYamlPriorityValue(); + if (rhs.LocalPriority().isExplicit()) + node["priority"] = rhs.LocalPriority().getValue(); + + if (rhs.GlobalPriority().isExplicit()) + node["global_priority"] = rhs.GlobalPriority().getValue(); if (!rhs.LoadAfter().empty()) node["after"] = rhs.LoadAfter(); @@ -185,11 +182,14 @@ struct convert { if (node["enabled"]) rhs.Enabled(node["enabled"].as()); + // Read priority values as int to prevent values that are too large from + // being converted to -128. if (node["priority"]) { - int priority = node["priority"].as(); - rhs.Priority(priority % loot::yamlGlobalPriorityDivisor); - rhs.SetPriorityExplicit(true); - rhs.SetPriorityGlobal(abs(priority) >= loot::yamlGlobalPriorityDivisor); + rhs.LocalPriority(loot::Priority(node["priority"].as())); + } + + if (node["global_priority"]) { + rhs.GlobalPriority(loot::Priority(node["global_priority"].as())); } if (node["after"]) diff --git a/src/backend/metadata/priority.cpp b/src/backend/metadata/priority.cpp new file mode 100644 index 00000000..6b7a5aec --- /dev/null +++ b/src/backend/metadata/priority.cpp @@ -0,0 +1,68 @@ +/* LOOT + + A load order optimisation tool for Oblivion, Skyrim, Fallout 3 and + Fallout: New Vegas. + + Copyright (C) 2012-2016 WrinklyNinja + + This file is part of LOOT. + + LOOT is free software: you can redistribute + it and/or modify it under the terms of the GNU General Public License + as published by the Free Software Foundation, either version 3 of + the License, or (at your option) any later version. + + LOOT is distributed in the hope that it will + be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with LOOT. If not, see + . + */ + +#include "backend/metadata/priority.h" + +namespace loot { +Priority::Priority() : value_(0), isExplicitZeroValue_(false) {} + +Priority::Priority(const int value) + : isExplicitZeroValue_(true) { + if (value > 127) { + value_ = 127; + } else if (value < -127) { + value_ = -127; + } else { + value_ = value; + } +} + +short Priority::getValue() const { + return value_; +} + +bool Priority::isExplicit() const { + return value_ != 0 || isExplicitZeroValue_; +} + +bool Priority::operator < (const Priority& rhs) const { + return value_ < rhs.value_; +} + +bool Priority::operator > (const Priority& rhs) const { + return value_ > rhs.value_; +} + +bool Priority::operator >= (const Priority& rhs) const { + return value_ >= rhs.value_; +} + +bool Priority::operator == (const Priority& rhs) const { + return value_ == rhs.value_; +} + +bool Priority::operator > (const uint8_t rhs) const { + return value_ > rhs; +} +} diff --git a/src/backend/metadata/priority.h b/src/backend/metadata/priority.h new file mode 100644 index 00000000..0b6f131f --- /dev/null +++ b/src/backend/metadata/priority.h @@ -0,0 +1,55 @@ +/* LOOT + + A load order optimisation tool for Oblivion, Skyrim, Fallout 3 and + Fallout: New Vegas. + + Copyright (C) 2012-2016 WrinklyNinja + + This file is part of LOOT. + + LOOT is free software: you can redistribute + it and/or modify it under the terms of the GNU General Public License + as published by the Free Software Foundation, either version 3 of + the License, or (at your option) any later version. + + LOOT is distributed in the hope that it will + be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with LOOT. If not, see + . + */ +#ifndef LOOT_BACKEND_METADATA_PRIORITY +#define LOOT_BACKEND_METADATA_PRIORITY + +#include + +namespace loot { +class Priority { +public: + Priority(); + // Take an int to prevent literals that are too large for one byte from + // wrapping around to negative values. + explicit Priority(const int value); + + // Doesn't return an int8_t because it is commonly signed char, which + // yaml-cpp interprets as a character rather than an integer. + short getValue() const; + bool isExplicit() const; + + bool operator < (const Priority& rhs) const; + bool operator > (const Priority& rhs) const; + bool operator >= (const Priority& rhs) const; + bool operator == (const Priority& rhs) const; + + bool operator > (const uint8_t rhs) const; + +private: + bool isExplicitZeroValue_; + int8_t value_; +}; +} + +#endif diff --git a/src/backend/plugin/plugin_sorter.cpp b/src/backend/plugin/plugin_sorter.cpp index 285c1e36..e6cbdc3c 100644 --- a/src/backend/plugin/plugin_sorter.cpp +++ b/src/backend/plugin/plugin_sorter.cpp @@ -267,7 +267,8 @@ void PluginSorter::PropagatePriorities() { vitend, std::back_inserter(positivePriorityVertices), [&](const vertex_t& vertex) { - return graph_[vertex].Priority() > 0; + return graph_[vertex].LocalPriority() > 0 + || graph_[vertex].GlobalPriority() > 0; }); // To reduce the number of priorities that will need setting, @@ -275,7 +276,8 @@ void PluginSorter::PropagatePriorities() { std::sort(begin(positivePriorityVertices), end(positivePriorityVertices), [&](const vertex_t& lhs, const vertex_t& rhs) { - return graph_[lhs].Priority() > graph_[rhs].Priority(); + return graph_[lhs].LocalPriority() > graph_[rhs].LocalPriority() + || graph_[lhs].GlobalPriority() > graph_[rhs].GlobalPriority(); }); // Create a color map. @@ -286,24 +288,39 @@ void PluginSorter::PropagatePriorities() { // search, setting priorities until an equal or larger value is // encountered. for (const vertex_t& vertex : positivePriorityVertices) { - BOOST_LOG_TRIVIAL(trace) << "Doing DFS for " << graph_[vertex].Name() << " which has priority " << graph_[vertex].Priority(); + BOOST_LOG_TRIVIAL(trace) << "Doing DFS for " << graph_[vertex].Name() + << " which has local priority " << graph_[vertex].LocalPriority().getValue() + << " and global priority " << graph_[vertex].GlobalPriority().getValue(); boost::dfs_visitor<> visitor; boost::depth_first_visit(graph_, vertex, visitor, colorMap, [&vertex](const vertex_t& currentVertex, const PluginGraph& graph) { - if (graph[currentVertex].Priority() < graph[vertex].Priority()) { - BOOST_LOG_TRIVIAL(trace) << "Overriding priority for " << graph[currentVertex].Name() << " from " << graph[currentVertex].Priority() << " to " << graph[vertex].Priority(); - // const_cast is necessary because depth_first_search - // takes a const graph. - const_cast(graph)[currentVertex].Priority(graph[vertex].Priority()); + // depth_first_search takes a const graph, so cast it if modifying a vertex. + if (graph[currentVertex].LocalPriority() < graph[vertex].LocalPriority()) { + BOOST_LOG_TRIVIAL(trace) << "Overriding local priority for " + << graph[currentVertex].Name() + << " from " << graph[currentVertex].LocalPriority().getValue() + << " to " << graph[vertex].LocalPriority().getValue(); + const_cast(graph)[currentVertex].LocalPriority(graph[vertex].LocalPriority()); + + return false; + } + + if (graph[currentVertex].GlobalPriority() < graph[vertex].GlobalPriority()) { + BOOST_LOG_TRIVIAL(trace) << "Overriding global priority for " + << graph[currentVertex].Name() + << " from " << graph[currentVertex].GlobalPriority().getValue() + << " to " << graph[vertex].GlobalPriority().getValue(); + const_cast(graph)[currentVertex].GlobalPriority(graph[vertex].GlobalPriority()); return false; } return currentVertex != vertex - && graph[currentVertex].Priority() >= graph[vertex].Priority(); + && graph[currentVertex].LocalPriority() >= graph[vertex].LocalPriority() + && graph[currentVertex].GlobalPriority() >= graph[vertex].GlobalPriority(); }); } } @@ -363,24 +380,31 @@ void PluginSorter::AddSpecificEdges() { void PluginSorter::AddPriorityEdges() { for (const auto& vertex : boost::make_iterator_range(boost::vertices(graph_))) { BOOST_LOG_TRIVIAL(trace) << "Adding priority difference edges to vertex for \"" << graph_[vertex].Name() << "\"."; - // If the plugin does not have a global priority and doesn't load + // If the plugin has a global priority of zero and doesn't load // an archive and has no override records, skip it. Plugins without // override records can only conflict with plugins that override // the records they add, so any edge necessary will be added when // evaluating that plugin. - if (!graph_[vertex].IsPriorityGlobal() && graph_[vertex].NumOverrideFormIDs() == 0 && !graph_[vertex].LoadsArchive()) + if (graph_[vertex].GlobalPriority().getValue() == 0 + && graph_[vertex].NumOverrideFormIDs() == 0 + && !graph_[vertex].LoadsArchive()) { continue; + } for (const auto& otherVertex : boost::make_iterator_range(boost::vertices(graph_))) { // If the plugins have equal priority, or have non-global // priorities but don't conflict, don't add a priority edge. - if (graph_[vertex].Priority() == graph_[otherVertex].Priority() - || !graph_[vertex].IsPriorityGlobal() && !graph_[otherVertex].IsPriorityGlobal() && !graph_[vertex].DoFormIDsOverlap(graph_[otherVertex])) { + if ((graph_[vertex].LocalPriority() == graph_[otherVertex].LocalPriority() && graph_[vertex].GlobalPriority() == graph_[otherVertex].GlobalPriority()) + || (graph_[vertex].GlobalPriority().getValue() == 0 + && graph_[otherVertex].GlobalPriority().getValue() == 0 + && !graph_[vertex].DoFormIDsOverlap(graph_[otherVertex]))) { continue; } vertex_t toVertex, fromVertex; - if (graph_[vertex].Priority() < graph_[otherVertex].Priority()) { + if (graph_[vertex].GlobalPriority() < graph_[otherVertex].GlobalPriority() + || (graph_[vertex].GlobalPriority() == graph_[otherVertex].GlobalPriority() + && graph_[vertex].LocalPriority() < graph_[otherVertex].LocalPriority())) { fromVertex = vertex; toVertex = otherVertex; } else { diff --git a/src/gui/html/elements/loot-plugin-editor.html b/src/gui/html/elements/loot-plugin-editor.html index 411103bc..4e53cf6b 100644 --- a/src/gui/html/elements/loot-plugin-editor.html +++ b/src/gui/html/elements/loot-plugin-editor.html @@ -138,13 +138,12 @@ loot-editor-close

-
Global Priority
- - Compare against all other plugins, not just conflicting plugins. +
Priority
+
-
Priority Value
- +
Global Priority
+
@@ -298,13 +297,11 @@ loot-editor-close /* If either of the priority values have been changed, the base priority value they're derived from will have changed, so record both. */ - if (this.$.isPriorityGlobal.checked !== oldData.isPriorityGlobal - || this.$.priorityValue.value !== oldData.priority) { - plugin.isPriorityGlobal = this.$.isPriorityGlobal.checked; + if (this.$.priorityValue.value !== oldData.priority) { plugin.priority = this.$.priorityValue.value; - - /* Also mark the priority as being explicitly set in the userlist. */ - plugin.userlist.hasExplicitPriority = true; + } + if (this.$.globalPriorityValue.value !== oldData.globalPriority) { + plugin.globalPriority = this.$.globalPriorityValue.value; } plugin.userlist.enabled = this.$.enableEdits.checked; @@ -388,8 +385,8 @@ loot-editor-close } else { this.$.enableEdits.checked = true; } - this.$.isPriorityGlobal.checked = newData.isPriorityGlobal; this.$.priorityValue.value = newData.priority; + this.$.globalPriorityValue.value = newData.globalPriority; /* Clear then fill in editor table data. Masterlist-originated rows should have their contents made read-only. */ diff --git a/src/gui/html/elements/loot-plugin-item.html b/src/gui/html/elements/loot-plugin-item.html index 84378d90..d6184107 100644 --- a/src/gui/html/elements/loot-plugin-item.html +++ b/src/gui/html/elements/loot-plugin-item.html @@ -75,14 +75,15 @@ :host-context(body[data-editors]) #primary { line-height: normal; } - #priority { + #secondary > span { font-size: 0.857rem; color: var(--secondary-text-color); height: 13px; overflow: visible; } - #globalPriorityIcon { - height: 13px; + #secondary iron-icon { + height: 13px; + vertical-align: text-bottom; } iron-icon { color: var(--secondary-text-color); @@ -96,10 +97,6 @@ #hasUserEdits[hidden] { display: none; } - #globalPriorityIcon[hidden] { - visibility: hidden; - display: inline-flex; - } :host { cursor: pointer; } @@ -113,9 +110,16 @@
- - Global Priority - [[priority]] + + + [[globalPriority]] + Global Priority + + + + [[priority]] + Priority +
Editor Is Open @@ -139,9 +143,9 @@ type: String, value: '', }, - isPriorityGlobal: { - type: Boolean, - value: false, + globalPriority: { + type: String, + value: '', }, isEditorOpen: { type: Boolean, @@ -185,7 +189,7 @@ updateStyling(pluginData) { this.priority = pluginData.priority; - this.isPriorityGlobal = pluginData.isPriorityGlobal; + this.globalPriority = pluginData.globalPriority; this.isEditorOpen = pluginData.isEditorOpen; this.hasUserEdits = pluginData.hasUserEdits; }, diff --git a/src/gui/html/index.html b/src/gui/html/index.html index 0e6e6ece..c88368f0 100644 --- a/src/gui/html/index.html +++ b/src/gui/html/index.html @@ -162,7 +162,7 @@ data-index$="[[index]]" load-order-index="[[item.loadOrderIndex]]" priority="[[item.priority]]" - is-priority-global="[[item.isPriorityGlobal]]" + global-priority="[[item.globalPriority]]" is-editor-open="[[item.isEditorOpen]]" has-user-edits="[[item.hasUserEdits]]">[[item.name]] diff --git a/src/gui/html/js/game.js b/src/gui/html/js/game.js index b8c67853..be156614 100644 --- a/src/gui/html/js/game.js +++ b/src/gui/html/js/game.js @@ -273,7 +273,7 @@ loadsArchive: plugin.loadsArchive, priority: plugin.priority, - isPriorityGlobal: plugin.isPriorityGlobal, + globalPriority: plugin.globalPriority, messages: plugin.messages, tags: plugin.tags, isDirty: plugin.isDirty, diff --git a/src/gui/html/js/plugin.js b/src/gui/html/js/plugin.js index 0cd7fec6..4c9c6e7a 100644 --- a/src/gui/html/js/plugin.js +++ b/src/gui/html/js/plugin.js @@ -177,7 +177,7 @@ this._userlist = obj.userlist; this._priority = obj.priority || 0; - this._isPriorityGlobal = obj.isPriorityGlobal || false; + this._globalPriority = obj.globalPriority || 0; this._messages = obj.messages || []; this._tags = obj.tags || []; this._isDirty = obj.isDirty || false; @@ -263,7 +263,7 @@ detail: { pluginId: this.id, priority: this.priority, - isPriorityGlobal: this.isPriorityGlobal, + globalPriority: this.globalPriority, isEditorOpen: this.isEditorOpen, hasUserEdits: this.hasUserEdits, }, @@ -408,13 +408,13 @@ } } - get isPriorityGlobal() { - return this._isPriorityGlobal; + get globalPriority() { + return this._globalPriority; } - set isPriorityGlobal(isPriorityGlobal) { - if (this._isPriorityGlobal !== isPriorityGlobal) { - this._isPriorityGlobal = isPriorityGlobal; + set globalPriority(globalPriority) { + if (this._globalPriority !== globalPriority) { + this._globalPriority = globalPriority; this._dispatchItemContentChangeEvent(); } diff --git a/src/gui/html/js/translateStaticText.js b/src/gui/html/js/translateStaticText.js index da1c7f96..ff3efd3b 100644 --- a/src/gui/html/js/translateStaticText.js +++ b/src/gui/html/js/translateStaticText.js @@ -30,9 +30,8 @@ const pluginEditor = document.getElementById('loot-plugin-editor').querySelector('template').content; pluginEditor.getElementById('enableEdits').previousElementSibling.textContent = l10n.translate('Enable Edits'); - pluginEditor.getElementById('isPriorityGlobal').previousElementSibling.textContent = l10n.translate('Global Priority'); - pluginEditor.querySelector('paper-tooltip[for=isPriorityGlobal]').textContent = l10n.translate('Global priorities are compared against all other plugins. Normal priorities are compared against only conflicting plugins.'); - pluginEditor.getElementById('priorityValue').previousElementSibling.textContent = l10n.translate('Priority Value'); + pluginEditor.getElementById('priorityValue').previousElementSibling.textContent = l10n.translate('Priority'); + pluginEditor.getElementById('globalPriorityValue').previousElementSibling.textContent = l10n.translate('Global Priority'); pluginEditor.getElementById('tableTabs').querySelector('[data-for=main]').textContent = l10n.translate('Main'); pluginEditor.getElementById('tableTabs').querySelector('[data-for=after]').textContent = l10n.translate('Load After'); @@ -85,7 +84,8 @@ /* Plugin List Item Template */ const pluginItem = document.getElementById('loot-plugin-item').querySelector('template').content; - pluginItem.querySelector('paper-tooltip[for=globalPriorityIcon]').textContent = l10n.translate('Global Priority'); + pluginItem.querySelector('#globalPriorityTooltip').textContent = l10n.translate('Global Priority'); + pluginItem.querySelector('#localPriorityTooltip').textContent = l10n.translate('Priority'); pluginItem.querySelector('paper-tooltip[for=hasUserEdits]').textContent = l10n.translate('Has User Metadata'); pluginItem.querySelector('paper-tooltip[for=editorIsOpen]').textContent = l10n.translate('Editor Is Open'); } diff --git a/src/gui/query_handler.cpp b/src/gui/query_handler.cpp index b6e1a5e0..29e56475 100644 --- a/src/gui/query_handler.cpp +++ b/src/gui/query_handler.cpp @@ -429,21 +429,24 @@ std::string QueryHandler::ApplyUserEdits(const YAML::Node& pluginMetadata) { PluginMetadata ulistPlugin = lootState_.getCurrentGame().GetUserlist().FindPlugin(newUserlistEntry); // First sort out the priority value. This is only given if it was changed. - BOOST_LOG_TRIVIAL(trace) << "Calculating userlist metadata priority value from Javascript variables."; - if (pluginMetadata["priority"] && pluginMetadata["isPriorityGlobal"]) { - BOOST_LOG_TRIVIAL(trace) << "Priority value was changed, recalculating..."; + BOOST_LOG_TRIVIAL(trace) << "Calculating userlist metadata local priority value from Javascript variables."; + if (pluginMetadata["priority"]) { + BOOST_LOG_TRIVIAL(trace) << "Local priority value was changed, recalculating..."; // Priority value was changed, so add it to the userlist data. - newUserlistEntry.Priority(pluginMetadata["priority"].as()); - newUserlistEntry.SetPriorityExplicit(true); - newUserlistEntry.SetPriorityGlobal(pluginMetadata["isPriorityGlobal"].as()); + newUserlistEntry.LocalPriority(Priority(pluginMetadata["priority"].as())); } else { - // Priority value wasn't changed, use the existing userlist value. - BOOST_LOG_TRIVIAL(trace) << "Priority value is unchanged, using existing userlist value (if it exists)."; - if (!ulistPlugin.HasNameOnly()) { - newUserlistEntry.Priority(ulistPlugin.Priority()); - newUserlistEntry.SetPriorityExplicit(ulistPlugin.IsPriorityExplicit()); - newUserlistEntry.SetPriorityGlobal(ulistPlugin.IsPriorityGlobal()); - } + // Priority value wasn't changed, use the existing userlist value. + BOOST_LOG_TRIVIAL(trace) << "Local priority value is unchanged, using existing userlist value (if it exists)."; + newUserlistEntry.LocalPriority(ulistPlugin.LocalPriority()); + } + + if (pluginMetadata["globalPriority"]) { + BOOST_LOG_TRIVIAL(trace) << "Global priority value was changed, recalculating..."; + // Priority value was changed, so add it to the userlist data. + newUserlistEntry.GlobalPriority(Priority(pluginMetadata["globalPriority"].as())); + } else { + BOOST_LOG_TRIVIAL(trace) << "Global priority value is unchanged, using existing userlist value (if it exists)."; + newUserlistEntry.GlobalPriority(ulistPlugin.GlobalPriority()); } // Now the enabled flag. @@ -698,11 +701,6 @@ void QueryHandler::GetGameData(CefRefPtr frame, CefRefPtr ca pluginNode["userlist"]["dirty"] = ulistPlugin.DirtyInfo(); pluginNode["userlist"]["clean"] = ulistPlugin.CleanInfo(); pluginNode["userlist"]["url"] = ulistPlugin.Locations(); - // The raw priority data isn't used, but should be set - // that LOOT knows it exists. - if (ulistPlugin.IsPriorityExplicit()) { - pluginNode["userlist"]["hasExplicitPriority"] = true; - } } // Now merge masterlist and userlist metadata and evaluate, @@ -970,8 +968,8 @@ YAML::Node QueryHandler::GenerateDerivedMetadata(const Plugin& file, const Plugi // Now add to pluginNode. YAML::Node pluginNode; pluginNode["name"] = tempPlugin.Name(); - pluginNode["priority"] = tempPlugin.Priority(); - pluginNode["isPriorityGlobal"] = tempPlugin.IsPriorityGlobal(); + pluginNode["priority"] = tempPlugin.LocalPriority().getValue(); + pluginNode["globalPriority"] = tempPlugin.GlobalPriority().getValue(); pluginNode["messages"] = tempPlugin.Messages(); pluginNode["tags"] = tempPlugin.Tags(); pluginNode["isDirty"] = isDirty; diff --git a/src/tests/backend/main.cpp b/src/tests/backend/main.cpp index 990c8371..d017dc70 100644 --- a/src/tests/backend/main.cpp +++ b/src/tests/backend/main.cpp @@ -45,6 +45,7 @@ #include "tests/backend/metadata/message_content_test.h" #include "tests/backend/metadata/plugin_cleaning_data_test.h" #include "tests/backend/metadata/plugin_metadata_test.h" +#include "tests/backend/metadata/priority_test.h" #include "tests/backend/metadata/tag_test.h" #include "tests/backend/plugin/plugin_test.h" #include "tests/backend/plugin/plugin_sorter_test.h" diff --git a/src/tests/backend/metadata/plugin_metadata_test.h b/src/tests/backend/metadata/plugin_metadata_test.h index 7f997b43..d32b0929 100644 --- a/src/tests/backend/metadata/plugin_metadata_test.h +++ b/src/tests/backend/metadata/plugin_metadata_test.h @@ -45,9 +45,6 @@ TEST_P(PluginMetadataTest, defaultConstructorShouldLeaveNameEmptyAndEnableMetada EXPECT_TRUE(plugin.Name().empty()); EXPECT_TRUE(plugin.Enabled()); - EXPECT_FALSE(plugin.IsPriorityExplicit()); - EXPECT_FALSE(plugin.IsPriorityGlobal()); - EXPECT_EQ(0, plugin.Priority()); } TEST_P(PluginMetadataTest, stringConstructorShouldSetNameToGivenStringAndEnableMetadataAndLeaveAllOtherFieldsAtTheirDefaults) { @@ -55,9 +52,6 @@ TEST_P(PluginMetadataTest, stringConstructorShouldSetNameToGivenStringAndEnableM EXPECT_EQ(blankEsm, plugin.Name()); EXPECT_TRUE(plugin.Enabled()); - EXPECT_FALSE(plugin.IsPriorityExplicit()); - EXPECT_FALSE(plugin.IsPriorityGlobal()); - EXPECT_EQ(0, plugin.Priority()); } TEST_P(PluginMetadataTest, equalityOperatorShouldUseCaseInsensitiveNameComparisonForNonRegexNames) { @@ -119,74 +113,77 @@ TEST_P(PluginMetadataTest, mergeMetadataShouldUseMergedEnabledStateIfMergedMetad PluginMetadata plugin2; plugin2.Enabled(false); - plugin2.SetPriorityExplicit(true); + plugin2.LocalPriority(Priority(5)); ASSERT_FALSE(plugin2.HasNameOnly()); plugin1.MergeMetadata(plugin2); EXPECT_FALSE(plugin1.Enabled()); } -TEST_P(PluginMetadataTest, mergeMetadataShouldUseMergedNonZeroPriorityValue) { +TEST_P(PluginMetadataTest, mergeMetadataShouldUseMergedNonZeroLocalPriorityValue) { PluginMetadata plugin1; PluginMetadata plugin2; - plugin1.Priority(5); - plugin2.Priority(3); + plugin1.LocalPriority(Priority(5)); + plugin2.LocalPriority(Priority(3)); plugin1.MergeMetadata(plugin2); - EXPECT_EQ(3, plugin1.Priority()); + EXPECT_EQ(3, plugin1.LocalPriority().getValue()); } -TEST_P(PluginMetadataTest, mergeMetadataShouldMergeAnExplicitPriorityValueOfZero) { +TEST_P(PluginMetadataTest, mergeMetadataShouldUseMergedNonZeroGlobalPriorityValue) { PluginMetadata plugin1; PluginMetadata plugin2; - plugin1.Priority(5); - plugin2.SetPriorityExplicit(true); + plugin1.GlobalPriority(Priority(5)); + plugin2.GlobalPriority(Priority(3)); plugin1.MergeMetadata(plugin2); - EXPECT_EQ(0, plugin1.Priority()); + EXPECT_EQ(3, plugin1.GlobalPriority().getValue()); } -TEST_P(PluginMetadataTest, mergeMetadataShouldNotMergeNonExplicitGlobalPriorityState) { +TEST_P(PluginMetadataTest, mergeMetadataShouldNotUseImplicitZeroLocalPriorityValue) { PluginMetadata plugin1; PluginMetadata plugin2; - plugin2.SetPriorityGlobal(true); + plugin1.LocalPriority(Priority(5)); plugin1.MergeMetadata(plugin2); - EXPECT_FALSE(plugin1.IsPriorityGlobal()); + EXPECT_EQ(5, plugin1.LocalPriority().getValue()); } -TEST_P(PluginMetadataTest, mergeMetadataShouldMergeExplicitGlobalPriorityState) { +TEST_P(PluginMetadataTest, mergeMetadataShouldNotUseImplicitZeroGlobalPriorityValue) { PluginMetadata plugin1; PluginMetadata plugin2; - plugin2.SetPriorityExplicit(true); - plugin2.SetPriorityGlobal(true); + plugin1.GlobalPriority(Priority(5)); plugin1.MergeMetadata(plugin2); - EXPECT_TRUE(plugin1.IsPriorityGlobal()); + EXPECT_EQ(5, plugin1.GlobalPriority().getValue()); } -TEST_P(PluginMetadataTest, mergeMetadataShouldMergeTrueExplicitPriorityState) { +TEST_P(PluginMetadataTest, mergeMetadataShouldMergeAnExplicitLocalPriorityValueOfZero) { PluginMetadata plugin1; PluginMetadata plugin2; - plugin2.SetPriorityExplicit(true); + plugin1.LocalPriority(Priority(5)); + plugin2.LocalPriority(Priority(0)); plugin1.MergeMetadata(plugin2); - EXPECT_TRUE(plugin1.IsPriorityExplicit()); + EXPECT_EQ(0, plugin1.LocalPriority().getValue()); + EXPECT_TRUE(plugin1.LocalPriority().isExplicit()); } -TEST_P(PluginMetadataTest, mergeMetadataShouldNotMergeFalseExplicitPriorityState) { +TEST_P(PluginMetadataTest, mergeMetadataShouldMergeAnExplicitGlobalPriorityValueOfZero) { PluginMetadata plugin1; PluginMetadata plugin2; - plugin1.SetPriorityExplicit(true); + plugin1.GlobalPriority(Priority(5)); + plugin2.GlobalPriority(Priority(0)); plugin1.MergeMetadata(plugin2); - EXPECT_TRUE(plugin1.IsPriorityExplicit()); + EXPECT_EQ(0, plugin1.GlobalPriority().getValue()); + EXPECT_TRUE(plugin1.GlobalPriority().isExplicit()); } TEST_P(PluginMetadataTest, mergeMetadataShouldMergeLoadAfterData) { @@ -316,39 +313,50 @@ TEST_P(PluginMetadataTest, diffMetadataShouldUseSourcePluginEnabledState) { EXPECT_FALSE(diff.Enabled()); } -TEST_P(PluginMetadataTest, diffMetadataShouldUseSourcePluginPriorityDataIfItDiffersFromTheTargetPluginPriorityData) { +TEST_P(PluginMetadataTest, diffMetadataShouldUseSourcePluginLocalPriorityIfItDiffersFromTheTargetPluginLocalPriority) { PluginMetadata plugin1; PluginMetadata plugin2; - plugin1.SetPriorityGlobal(true); + plugin2.LocalPriority(Priority(5)); PluginMetadata diff = plugin1.DiffMetadata(plugin2); - EXPECT_EQ(0, diff.Priority()); - EXPECT_FALSE(diff.IsPriorityExplicit()); - EXPECT_TRUE(diff.IsPriorityGlobal()); - - plugin1.Priority(5); - plugin1.SetPriorityGlobal(false); - diff = plugin1.DiffMetadata(plugin2); - - EXPECT_EQ(5, diff.Priority()); - EXPECT_TRUE(diff.IsPriorityExplicit()); - EXPECT_FALSE(diff.IsPriorityGlobal()); + EXPECT_EQ(0, diff.LocalPriority().getValue()); + EXPECT_FALSE(diff.LocalPriority().isExplicit()); } -TEST_P(PluginMetadataTest, diffMetadataShouldOutputDefaultPriorityDataIfTheSourceAndTargetPluginPriorityDataMatch) { +TEST_P(PluginMetadataTest, diffMetadataShouldUseSourcePluginGlobalPriorityIfItDiffersFromTheTargetPluginGlobalPriority) { PluginMetadata plugin1; PluginMetadata plugin2; - plugin1.Priority(5); - plugin1.SetPriorityGlobal(true); - plugin2.Priority(5); - plugin2.SetPriorityGlobal(true); + plugin2.GlobalPriority(Priority(5)); PluginMetadata diff = plugin1.DiffMetadata(plugin2); - EXPECT_EQ(0, diff.Priority()); - EXPECT_FALSE(diff.IsPriorityExplicit()); - EXPECT_FALSE(diff.IsPriorityGlobal()); + EXPECT_EQ(0, diff.GlobalPriority().getValue()); + EXPECT_FALSE(diff.GlobalPriority().isExplicit()); +} + +TEST_P(PluginMetadataTest, diffMetadataShouldOutputDefaultLocalPriorityIfTheSourceAndTargetPluginLocalPrioritiesMatch) { + PluginMetadata plugin1; + PluginMetadata plugin2; + + plugin1.LocalPriority(Priority(5)); + plugin2.LocalPriority(Priority(5)); + PluginMetadata diff = plugin1.DiffMetadata(plugin2); + + EXPECT_EQ(0, diff.LocalPriority().getValue()); + EXPECT_FALSE(diff.LocalPriority().isExplicit()); +} + +TEST_P(PluginMetadataTest, diffMetadataShouldOutputDefaultGlobalPriorityIfTheSourceAndTargetPluginGlobalPrioritiesMatch) { + PluginMetadata plugin1; + PluginMetadata plugin2; + + plugin1.GlobalPriority(Priority(5)); + plugin2.GlobalPriority(Priority(5)); + PluginMetadata diff = plugin1.DiffMetadata(plugin2); + + EXPECT_EQ(0, diff.GlobalPriority().getValue()); + EXPECT_FALSE(diff.GlobalPriority().isExplicit()); } TEST_P(PluginMetadataTest, diffMetadataShouldOutputLoadAfterDataThatAreNotCommonToBothInputPlugins) { @@ -487,17 +495,24 @@ TEST_P(PluginMetadataTest, newMetadataShouldUseSourcePluginEnabledState) { EXPECT_FALSE(newMetadata.Enabled()); } -TEST_P(PluginMetadataTest, newMetadataShouldUseSourcePluginPriorityData) { +TEST_P(PluginMetadataTest, newMetadataShouldUseSourcePluginLocalPriority) { PluginMetadata plugin1; PluginMetadata plugin2; - plugin1.Priority(5); - plugin1.SetPriorityGlobal(true); + plugin1.LocalPriority(Priority(5)); PluginMetadata newMetadata = plugin1.NewMetadata(plugin2); - EXPECT_EQ(5, newMetadata.Priority()); - EXPECT_TRUE(newMetadata.IsPriorityExplicit()); - EXPECT_TRUE(newMetadata.IsPriorityGlobal()); + EXPECT_EQ(5, newMetadata.LocalPriority().getValue()); +} + +TEST_P(PluginMetadataTest, newMetadataShouldUseSourcePluginGlobalPriority) { + PluginMetadata plugin1; + PluginMetadata plugin2; + + plugin1.GlobalPriority(Priority(5)); + PluginMetadata newMetadata = plugin1.NewMetadata(plugin2); + + EXPECT_EQ(5, newMetadata.GlobalPriority().getValue()); } TEST_P(PluginMetadataTest, newMetadataShouldOutputLoadAfterDataThatAreNotCommonToBothInputPlugins) { @@ -612,48 +627,6 @@ TEST_P(PluginMetadataTest, newMetadataShouldOutputLocationsThatAreNotCommonToBot EXPECT_EQ(std::set({location2}), newMetadata.Locations()); } -TEST_P(PluginMetadataTest, settingPriorityWithAbsoluteValueGreaterOrEqualToGlobalPriorityDivisorShouldThrow) { - PluginMetadata plugin; - EXPECT_ANY_THROW(plugin.Priority(yamlGlobalPriorityDivisor)); - EXPECT_ANY_THROW(plugin.Priority(yamlGlobalPriorityDivisor + 1)); - EXPECT_ANY_THROW(plugin.Priority(-yamlGlobalPriorityDivisor)); - EXPECT_ANY_THROW(plugin.Priority(-yamlGlobalPriorityDivisor - 1)); -} - -TEST_P(PluginMetadataTest, settingPriorityAsGlobalShouldSucceed) { - PluginMetadata plugin; - ASSERT_FALSE(plugin.IsPriorityGlobal()); - plugin.SetPriorityGlobal(true); - EXPECT_TRUE(plugin.IsPriorityGlobal()); -} - -TEST_P(PluginMetadataTest, settingPriorityAsNotGlobalShouldSucceed) { - PluginMetadata plugin; - plugin.SetPriorityGlobal(true); - ASSERT_TRUE(plugin.IsPriorityGlobal()); - plugin.SetPriorityGlobal(false); - EXPECT_FALSE(plugin.IsPriorityGlobal()); -} - -TEST_P(PluginMetadataTest, gettingYamlPriorityValueForAGlobalPriorityShouldReturnValueWithGlobalFlagSet) { - PluginMetadata plugin; - plugin.Priority(10); - plugin.SetPriorityGlobal(true); - EXPECT_EQ(1000010, plugin.GetYamlPriorityValue()); - - plugin.Priority(-20); - EXPECT_EQ(-1000020, plugin.GetYamlPriorityValue()); -} - -TEST_P(PluginMetadataTest, gettingYamlPriorityValueForANonGlobalPriorityShouldReturnValueWithGlobalFlagNotSet) { - PluginMetadata plugin; - plugin.Priority(10); - EXPECT_EQ(10, plugin.GetYamlPriorityValue()); - - plugin.Priority(-20); - EXPECT_EQ(-20, plugin.GetYamlPriorityValue()); -} - TEST_P(PluginMetadataTest, evalAllConditionsShouldEvaluateAllMetadataConditions) { Game game(GetParam()); game.SetGamePath(dataPath.parent_path()); @@ -710,9 +683,16 @@ TEST_P(PluginMetadataTest, hasNameOnlyShouldBeTrueIfThePluginMetadataIsDisabled) EXPECT_TRUE(plugin.HasNameOnly()); } -TEST_P(PluginMetadataTest, hasNameOnlyShouldBeFalseIfThePriorityValueIsExplicit) { +TEST_P(PluginMetadataTest, hasNameOnlyShouldBeFalseIfTheLocalPriorityIsExplicit) { PluginMetadata plugin(blankEsp); - plugin.SetPriorityExplicit(true); + plugin.LocalPriority(Priority(0)); + + EXPECT_FALSE(plugin.HasNameOnly()); +} + +TEST_P(PluginMetadataTest, hasNameOnlyShouldBeFalseIfTheGlobalPriorityIsExplicit) { + PluginMetadata plugin(blankEsp); + plugin.GlobalPriority(Priority(0)); EXPECT_FALSE(plugin.HasNameOnly()); } @@ -823,9 +803,9 @@ TEST_P(PluginMetadataTest, emittingAsYamlShouldOutputAPluginWithNoMetadataAsABla EXPECT_STREQ("", emitter.c_str()); } -TEST_P(PluginMetadataTest, emittingAsYamlShouldOutputAPluginWithAnExplicitPriorityCorrectly) { +TEST_P(PluginMetadataTest, emittingAsYamlShouldOutputAPluginWithAnExplicitLocalPriorityCorrectly) { PluginMetadata plugin(blankEsm); - plugin.SetPriorityExplicit(true); + plugin.LocalPriority(Priority(0)); YAML::Emitter emitter; emitter << plugin; @@ -834,17 +814,38 @@ TEST_P(PluginMetadataTest, emittingAsYamlShouldOutputAPluginWithAnExplicitPriori "priority: 0", emitter.c_str()); } -TEST_P(PluginMetadataTest, emittingAsYamlShouldOutputAPluginWithAnExplicitPriorityThatIsDisabledCorrectly) { +TEST_P(PluginMetadataTest, emittingAsYamlShouldOutputAPluginWithAnExplicitGlobalPriorityCorrectly) { PluginMetadata plugin(blankEsm); - plugin.SetPriorityExplicit(true); + plugin.GlobalPriority(Priority(0)); + + YAML::Emitter emitter; + emitter << plugin; + + EXPECT_STREQ("name: 'Blank.esm'\n" + "global_priority: 0", emitter.c_str()); +} + +TEST_P(PluginMetadataTest, emittingAsYamlShouldOutputAPluginThatIsDisabledAndIsNotNameOnlyCorrectly) { + PluginMetadata plugin(blankEsm); + plugin.GlobalPriority(Priority(0)); plugin.Enabled(false); YAML::Emitter emitter; emitter << plugin; EXPECT_STREQ("name: 'Blank.esm'\n" - "priority: 0\n" - "enabled: false", emitter.c_str()); + "enabled: false\n" + "global_priority: 0", emitter.c_str()); +} + +TEST_P(PluginMetadataTest, emittingAsYamlShouldOutputAPluginThatIsDisabledAndIsNameOnlyAsAnEmptyString) { + PluginMetadata plugin(blankEsm); + plugin.Enabled(false); + + YAML::Emitter emitter; + emitter << plugin; + + EXPECT_STREQ("", emitter.c_str()); } TEST_P(PluginMetadataTest, emittingAsYamlShouldOutputAPluginWithLoadAfterMetadataCorrectly) { @@ -966,13 +967,38 @@ TEST_P(PluginMetadataTest, encodingAsYamlShouldOmitAllUnsetFields) { EXPECT_FALSE(node["url"]); } -TEST_P(PluginMetadataTest, encodingAsYamlShouldSetPriorityFieldIfPriorityIsExplicit) { +TEST_P(PluginMetadataTest, encodingAsYamlShouldSetPriorityFieldIfLocalPriorityIsExplicit) { PluginMetadata plugin(blankEsp); - plugin.SetPriorityExplicit(true); + plugin.LocalPriority(Priority(0)); YAML::Node node; node = plugin; - EXPECT_EQ(0, node["priority"].as()); + EXPECT_EQ(0, node["priority"].as()); +} + +TEST_P(PluginMetadataTest, encodingAsYamlShouldSetGlobalPriorityFieldIfGlobalPriorityIsExplicit) { + PluginMetadata plugin(blankEsp); + plugin.GlobalPriority(Priority(0)); + YAML::Node node; + node = plugin; + + EXPECT_EQ(0, node["global_priority"].as()); +} + +TEST_P(PluginMetadataTest, encodingAsYamlShouldNotSetPriorityFieldIfLocalPriorityIsImplicit) { + PluginMetadata plugin(blankEsp); + YAML::Node node; + node = plugin; + + EXPECT_FALSE(node["priority"]); +} + +TEST_P(PluginMetadataTest, encodingAsYamlShouldNotSetPriorityFieldIfGlobalPriorityIsImplicit) { + PluginMetadata plugin(blankEsp); + YAML::Node node; + node = plugin; + + EXPECT_FALSE(node["global_priority"]); } TEST_P(PluginMetadataTest, encodingAsYamlShouldSetEnabledFieldIfItIsFalse) { @@ -1061,15 +1087,17 @@ TEST_P(PluginMetadataTest, decodingFromYamlShouldSetDefaultPriorityValuesIfNoneA PluginMetadata plugin = node.as(); EXPECT_EQ(blankEsp, plugin.Name()); - EXPECT_EQ(0, plugin.Priority()); - EXPECT_FALSE(plugin.IsPriorityExplicit()); - EXPECT_FALSE(plugin.IsPriorityGlobal()); + EXPECT_EQ(0, plugin.LocalPriority().getValue()); + EXPECT_FALSE(plugin.LocalPriority().isExplicit()); + EXPECT_EQ(0, plugin.GlobalPriority().getValue()); + EXPECT_FALSE(plugin.GlobalPriority().isExplicit()); } TEST_P(PluginMetadataTest, decodingFromYamlShouldStoreAllGivenData) { YAML::Node node = YAML::Load("name: 'Blank.esp'\n" - "priority: 5\n" "enabled: false\n" + "priority: 5\n" + "global_priority: 3\n" "after:\n" " - 'Blank.esm'\n" "req:\n" @@ -1094,10 +1122,8 @@ TEST_P(PluginMetadataTest, decodingFromYamlShouldStoreAllGivenData) { PluginMetadata plugin = node.as(); EXPECT_EQ("Blank.esp", plugin.Name()); - EXPECT_EQ(5, plugin.Priority()); - EXPECT_TRUE(plugin.IsPriorityExplicit()); - EXPECT_FALSE(plugin.IsPriorityGlobal()); - EXPECT_FALSE(plugin.Enabled()); + EXPECT_EQ(5, plugin.LocalPriority().getValue()); + EXPECT_EQ(3, plugin.GlobalPriority().getValue()); EXPECT_EQ(std::set({ File("Blank.esm") }), plugin.LoadAfter()); diff --git a/src/tests/backend/metadata/priority_test.h b/src/tests/backend/metadata/priority_test.h new file mode 100644 index 00000000..418365cd --- /dev/null +++ b/src/tests/backend/metadata/priority_test.h @@ -0,0 +1,112 @@ +/* LOOT + +A load order optimisation tool for Oblivion, Skyrim, Fallout 3 and +Fallout: New Vegas. + +Copyright (C) 2014-2016 WrinklyNinja + +This file is part of LOOT. + +LOOT is free software: you can redistribute +it and/or modify it under the terms of the GNU General Public License +as published by the Free Software Foundation, either version 3 of +the License, or (at your option) any later version. + +LOOT is distributed in the hope that it will +be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with LOOT. If not, see +. +*/ + +#ifndef LOOT_TESTS_BACKEND_METADATA_PRIORITY_TEST +#define LOOT_TESTS_BACKEND_METADATA_PRIORITY_TEST + +#include "backend/metadata/priority.h" + +#include + +namespace loot { +namespace test { +TEST(Priority, defaultConstructorShouldInitialiseAnImplicitZeroPriority) { + Priority priority; + + EXPECT_EQ(0, priority.getValue()); + EXPECT_FALSE(priority.isExplicit()); +} + +TEST(Priority, valueConstructorShouldInitialseAnExplicitValue) { + Priority priority(0); + + EXPECT_EQ(0, priority.getValue()); + EXPECT_TRUE(priority.isExplicit()); + + priority = Priority(5); + + EXPECT_EQ(5, priority.getValue()); + EXPECT_TRUE(priority.isExplicit()); +} + +TEST(Priority, valueConstructorTruncatesOutOfRangeValues) { + EXPECT_EQ(127, Priority(127).getValue()); + EXPECT_EQ(127, Priority(128).getValue()); + + EXPECT_EQ(-127, Priority(-127).getValue()); + EXPECT_EQ(-127, Priority(-128).getValue()); +} + +TEST(Priority, lessThanOperatorShouldCompareValues) { + Priority priority1(1); + Priority priority2(2); + + EXPECT_TRUE(priority1 < priority2); + EXPECT_FALSE(priority2 < priority1); + + EXPECT_FALSE(priority1 < priority1); +} + +TEST(Priority, greaterThanOperatorShouldCompareValues) { + Priority priority1(1); + Priority priority2(2); + + EXPECT_FALSE(priority1 > priority2); + EXPECT_TRUE(priority2 > priority1); + + EXPECT_FALSE(priority1 > priority1); +} + +TEST(Priority, greaterThanOrEqualToOperatorShouldCompareValues) { + Priority priority1(1); + Priority priority2(2); + + EXPECT_FALSE(priority1 >= priority2); + EXPECT_TRUE(priority2 >= priority1); + + EXPECT_TRUE(priority1 >= priority1); +} + +TEST(Priority, equalityOperatorShouldCompareValues) { + Priority priority1(1); + Priority priority2(2); + + EXPECT_FALSE(priority1 == priority2); + EXPECT_FALSE(priority2 == priority1); + + EXPECT_TRUE(priority1 == priority1); +} + +TEST(Priority, greaterThanUint8OperatorShouldCompareValue) { + Priority priority1(1); + Priority priority2(3); + uint8_t value = 2; + + EXPECT_FALSE(priority1 > value); + EXPECT_TRUE(priority2 > value); +} +} +} + +#endif diff --git a/src/tests/backend/metadata_list_test.h b/src/tests/backend/metadata_list_test.h index 9ef8e578..0d449253 100644 --- a/src/tests/backend/metadata_list_test.h +++ b/src/tests/backend/metadata_list_test.h @@ -233,13 +233,13 @@ TEST_P(MetadataListTest, addPluginShouldStoreGivenSpecificPluginMetadata) { ASSERT_TRUE(metadataList.FindPlugin(PluginMetadata(blankDifferentEsm)).HasNameOnly()); PluginMetadata plugin(blankDifferentEsm); - plugin.Priority(1000); + plugin.LocalPriority(Priority(100)); metadataList.AddPlugin(plugin); plugin = metadataList.FindPlugin(plugin); EXPECT_EQ(blankDifferentEsm, plugin.Name()); - EXPECT_EQ(1000, plugin.Priority()); + EXPECT_EQ(100, plugin.LocalPriority().getValue()); } TEST_P(MetadataListTest, addPluginShouldStoreGivenRegexPluginMetadata) { @@ -247,12 +247,12 @@ TEST_P(MetadataListTest, addPluginShouldStoreGivenRegexPluginMetadata) { ASSERT_NO_THROW(metadataList.Load(metadataPath)); PluginMetadata plugin(".+Dependent\\.esp"); - plugin.Priority(-10); + plugin.LocalPriority(Priority(-10)); metadataList.AddPlugin(plugin); plugin = metadataList.FindPlugin(PluginMetadata(blankPluginDependentEsp)); - EXPECT_EQ(-10, plugin.Priority()); + EXPECT_EQ(-10, plugin.LocalPriority().getValue()); } TEST_P(MetadataListTest, addPluginShouldThrowIfAMatchingPluginAlreadyExists) { @@ -266,7 +266,7 @@ TEST_P(MetadataListTest, addPluginShouldThrowIfAMatchingPluginAlreadyExists) { ASSERT_ANY_THROW(metadataList.AddPlugin(PluginMetadata(blankEsm))); } -TEST_P(MetadataListTest, erasePluginShouldRemoveStoredMetadatForTheGivenPlugin) { +TEST_P(MetadataListTest, erasePluginShouldRemoveStoredMetadataForTheGivenPlugin) { MetadataList metadataList; ASSERT_NO_THROW(metadataList.Load(metadataPath)); diff --git a/src/tests/backend/plugin/plugin_sorter_test.h b/src/tests/backend/plugin/plugin_sorter_test.h index e7aabb9a..53c13f93 100644 --- a/src/tests/backend/plugin/plugin_sorter_test.h +++ b/src/tests/backend/plugin/plugin_sorter_test.h @@ -95,11 +95,10 @@ TEST_P(PluginSorterTest, failedSortShouldNotClearExistingGameMessages) { EXPECT_FALSE(game_.GetMessages().empty()); } -TEST_P(PluginSorterTest, sortingShouldEvaluateRelativePriorities) { +TEST_P(PluginSorterTest, sortingShouldEvaluateRelativeGlobalPriorities) { ASSERT_NO_THROW(game_.LoadPlugins(false)); PluginMetadata plugin(blankDifferentMasterDependentEsp); - plugin.Priority(-100000); - plugin.SetPriorityGlobal(true); + plugin.GlobalPriority(Priority(-100)); game_.GetUserlist().AddPlugin(plugin); PluginSorter ps; @@ -121,19 +120,19 @@ TEST_P(PluginSorterTest, sortingShouldEvaluateRelativePriorities) { EXPECT_TRUE(std::equal(begin(sorted), end(sorted), begin(expectedSortedOrder))); } -TEST_P(PluginSorterTest, sortingWithPrioritiesShouldInheritRecursivelyRegardlessOfEvaluationOrder) { +TEST_P(PluginSorterTest, sortingWithGlobalPrioritiesShouldInheritRecursivelyRegardlessOfEvaluationOrder) { ASSERT_NO_THROW(game_.LoadPlugins(false)); // Set Blank.esp's priority. PluginMetadata plugin(blankEsp); - plugin.Priority(2); + plugin.GlobalPriority(Priority(2)); game_.GetUserlist().AddPlugin(plugin); // Load Blank - Master Dependent.esp after Blank.esp so that it // inherits Blank.esp's priority. plugin = PluginMetadata(blankMasterDependentEsp); plugin.LoadAfter({ - File(blankEsp), + File(blankEsp), }); game_.GetUserlist().AddPlugin(plugin); @@ -141,7 +140,7 @@ TEST_P(PluginSorterTest, sortingWithPrioritiesShouldInheritRecursivelyRegardless // that it inherits its inherited priority. plugin = PluginMetadata(blankDifferentEsp); plugin.LoadAfter({ - File(blankMasterDependentEsp), + File(blankMasterDependentEsp), }); game_.GetUserlist().AddPlugin(plugin); @@ -149,23 +148,22 @@ TEST_P(PluginSorterTest, sortingWithPrioritiesShouldInheritRecursivelyRegardless // than 0 but lower than Blank.esp. Need to also make it a global priority // because it doesn't otherwise conflict with the other plugins. plugin = PluginMetadata(blankDifferentMasterDependentEsp); - plugin.Priority(1); - plugin.SetPriorityGlobal(true); + plugin.GlobalPriority(Priority(1)); game_.GetUserlist().AddPlugin(plugin); PluginSorter ps; std::list expectedSortedOrder({ - masterFile, - blankEsm, - blankDifferentEsm, - blankMasterDependentEsm, - blankDifferentMasterDependentEsm, - blankDifferentMasterDependentEsp, - blankEsp, - blankMasterDependentEsp, - blankDifferentEsp, - blankPluginDependentEsp, - blankDifferentPluginDependentEsp, + masterFile, + blankEsm, + blankDifferentEsm, + blankMasterDependentEsm, + blankDifferentMasterDependentEsm, + blankDifferentMasterDependentEsp, + blankEsp, + blankMasterDependentEsp, + blankDifferentEsp, + blankPluginDependentEsp, + blankDifferentPluginDependentEsp, }); std::list sorted = ps.Sort(game_, Language::Code::english); diff --git a/src/tests/gui/html/js/test_game.js b/src/tests/gui/html/js/test_game.js index 27f9ff7a..76272324 100644 --- a/src/tests/gui/html/js/test_game.js +++ b/src/tests/gui/html/js/test_game.js @@ -393,8 +393,8 @@ describe('Game', () => { masterlist: {}, userlist: {}, - priority: 500, - isPriorityGlobal: true, + priority: -100, + globalPriority: 100, messages: [{ type: 'warn', condition: 'file("bar.esp")', @@ -425,7 +425,7 @@ describe('Game', () => { loadsArchive: game._plugins[0].loadsArchive, priority: game._plugins[0].priority, - isPriorityGlobal: game._plugins[0].isPriorityGlobal, + globalPriority: game._plugins[0].globalPriority, messages: game._plugins[0].messages, tags: game._plugins[0].tags, isDirty: game._plugins[0].isDirty, diff --git a/src/tests/gui/html/js/test_plugin.js b/src/tests/gui/html/js/test_plugin.js index 9e3f4863..170ee1d0 100644 --- a/src/tests/gui/html/js/test_plugin.js +++ b/src/tests/gui/html/js/test_plugin.js @@ -183,19 +183,19 @@ describe('Plugin', () => { plugin.priority.should.equal(5); }); - it('should set isPriorityGlobal value to false if no key was passed', () => { + it('should set global priority to 0 if no key was passed', () => { const plugin = new loot.Plugin({ name: 'test' }); - plugin.isPriorityGlobal.should.be.false(); + plugin.globalPriority.should.equal(0); }); - it('should set isPriorityGlobal to passed key\'s value', () => { + it('should set global priority to passed key\'s value', () => { const plugin = new loot.Plugin({ name: 'test', - isPriorityGlobal: true, + globalPriority: 5, }); - plugin.isPriorityGlobal.should.be.true(); + plugin.globalPriority.should.equal(5); }); it('should set messages value to an empty array if no key was passed', () => { @@ -812,7 +812,7 @@ describe('Plugin', () => { handleEvent = (evt) => { evt.detail.pluginId.should.equal(plugin.id); evt.detail.priority.should.equal(plugin.priority); - evt.detail.isPriorityGlobal.should.equal(plugin.isPriorityGlobal); + evt.detail.globalPriority.should.equal(plugin.globalPriority); evt.detail.isEditorOpen.should.equal(plugin.isEditorOpen); evt.detail.hasUserEdits.should.equal(plugin.hasUserEdits); done(); @@ -874,7 +874,7 @@ describe('Plugin', () => { handleEvent = (evt) => { evt.detail.pluginId.should.equal(plugin.id); evt.detail.priority.should.equal(plugin.priority); - evt.detail.isPriorityGlobal.should.equal(plugin.isPriorityGlobal); + evt.detail.globalPriority.should.equal(plugin.globalPriority); evt.detail.isEditorOpen.should.equal(plugin.isEditorOpen); evt.detail.hasUserEdits.should.equal(plugin.hasUserEdits); done(); @@ -886,34 +886,34 @@ describe('Plugin', () => { }); }); - describe('#isPriorityGlobal', () => { + describe('#globalPriority', () => { let handleEvent; afterEach(() => { document.removeEventListener('loot-plugin-item-content-change', handleEvent); }); - it('getting value should return false if it has not been set in the constructor', () => { + it('getting value should return 0 if it has not been set in the constructor', () => { const plugin = new loot.Plugin({ name: 'test' }); - plugin.isPriorityGlobal.should.be.false(); + plugin.globalPriority.should.equal(0); }); it('getting value should return the value that was set', () => { const plugin = new loot.Plugin({ name: 'test', - isPriorityGlobal: true, + globalPriority: 5, }); - plugin.isPriorityGlobal.should.be.true(); + plugin.globalPriority.should.equal(5); }); it('setting value should store set value', () => { const plugin = new loot.Plugin({ name: 'test' }); - plugin.isPriorityGlobal = true; + plugin.globalPriority = 5; - plugin.isPriorityGlobal.should.be.true(); + plugin.globalPriority.should.equal(5); }); it('setting value to the current value should not fire an event', (done) => { @@ -925,7 +925,7 @@ describe('Plugin', () => { document.addEventListener('loot-plugin-item-content-change', handleEvent); - plugin.isPriorityGlobal = plugin.isPriorityGlobal; + plugin.globalPriority = plugin.globalPriority; setTimeout(done, 100); }); @@ -936,7 +936,7 @@ describe('Plugin', () => { handleEvent = (evt) => { evt.detail.pluginId.should.equal(plugin.id); evt.detail.priority.should.equal(plugin.priority); - evt.detail.isPriorityGlobal.should.equal(plugin.isPriorityGlobal); + evt.detail.globalPriority.should.equal(plugin.globalPriority); evt.detail.isEditorOpen.should.equal(plugin.isEditorOpen); evt.detail.hasUserEdits.should.equal(plugin.hasUserEdits); done(); @@ -944,7 +944,7 @@ describe('Plugin', () => { document.addEventListener('loot-plugin-item-content-change', handleEvent); - plugin.isPriorityGlobal = true; + plugin.globalPriority = 5; }); }); @@ -989,7 +989,7 @@ describe('Plugin', () => { handleEvent = (evt) => { evt.detail.pluginId.should.equal(plugin.id); evt.detail.priority.should.equal(plugin.priority); - evt.detail.isPriorityGlobal.should.equal(plugin.isPriorityGlobal); + evt.detail.globalPriority.should.equal(plugin.globalPriority); evt.detail.isEditorOpen.should.equal(plugin.isEditorOpen); evt.detail.hasUserEdits.should.equal(plugin.hasUserEdits); done();