From 41b32b2011d7f8b041ad0054e6046443a28eeacf Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Thu, 5 Nov 2015 20:55:20 +0000 Subject: [PATCH] Use PluginMetadata when not dealing with files Plugin objects should represent actual files, and PluginMetadata should be used for metadata that hasn't been linked to an installed plugin. As part of this, remove the unnecessary comparison operators on Plugin and add string comparison operators for PluginMetadata. --- src/api/api.cpp | 12 +- src/backend/metadata/plugin_metadata.cpp | 11 ++ src/backend/metadata/plugin_metadata.h | 4 + src/backend/plugin/plugin.cpp | 8 -- src/backend/plugin/plugin.h | 4 - src/gui/handler.cpp | 2 +- src/tests/backend/test_plugin_sorter.h | 154 +++++++++++------------ 7 files changed, 94 insertions(+), 101 deletions(-) diff --git a/src/api/api.cpp b/src/api/api.cpp index 7b6b5793..fd29de1b 100644 --- a/src/api/api.cpp +++ b/src/api/api.cpp @@ -767,8 +767,8 @@ LOOT_API unsigned int loot_get_dirty_info(loot_db db, const char * const plugin, *needsCleaning = loot_needs_cleaning_unknown; // Is there any dirty info? Testing for applicability happens in loot_eval_lists(). - if (!db->masterlist.FindPlugin(loot::Plugin(plugin)).DirtyInfo().empty() - || !db->userlist.FindPlugin(loot::Plugin(plugin)).DirtyInfo().empty()) { + if (!db->masterlist.FindPlugin(loot::PluginMetadata(plugin)).DirtyInfo().empty() + || !db->userlist.FindPlugin(loot::PluginMetadata(plugin)).DirtyInfo().empty()) { *needsCleaning = loot_needs_cleaning_yes; } @@ -776,9 +776,9 @@ LOOT_API unsigned int loot_get_dirty_info(loot_db db, const char * const plugin, // This isn't a very reliable system, because if the lists have been evaluated in some language // other than English, the strings will be in different languages (and the API can't tell what they'd be) // and the strings may be non-standard and begin with something other than "Do not clean." anyway. - std::list messages(db->masterlist.FindPlugin(loot::Plugin(plugin)).Messages()); + std::list messages(db->masterlist.FindPlugin(loot::PluginMetadata(plugin)).Messages()); - std::list temp(db->userlist.FindPlugin(loot::Plugin(plugin)).Messages()); + std::list temp(db->userlist.FindPlugin(loot::PluginMetadata(plugin)).Messages()); messages.insert(messages.end(), temp.begin(), temp.end()); for (const auto& message : messages) { @@ -806,9 +806,9 @@ LOOT_API unsigned int loot_write_minimal_list(loot_db db, const char * const out return c_error(loot_error_file_write_fail, "Output file exists but overwrite is not set to true."); loot::Masterlist temp = db->masterlist; - std::unordered_set minimalPlugins; + std::unordered_set minimalPlugins; for (const auto &plugin : temp.Plugins()) { - loot::Plugin p(plugin.Name()); + loot::PluginMetadata p(plugin.Name()); p.Tags(plugin.Tags()); p.DirtyInfo(plugin.DirtyInfo()); minimalPlugins.insert(p); diff --git a/src/backend/metadata/plugin_metadata.cpp b/src/backend/metadata/plugin_metadata.cpp index 36d5702e..e5a6ca0a 100644 --- a/src/backend/metadata/plugin_metadata.cpp +++ b/src/backend/metadata/plugin_metadata.cpp @@ -360,6 +360,17 @@ namespace loot { bool PluginMetadata::operator != (const PluginMetadata& rhs) const { return !(*this == rhs); } + + bool PluginMetadata::operator == (const std::string& rhs) const { + if (IsRegexPlugin()) + return regex_match(PluginMetadata(rhs).Name(), regex(name, regex::ECMAScript | regex::icase)); + else + return boost::iequals(name, PluginMetadata(rhs).Name()); + } + + bool PluginMetadata::operator != (const std::string& rhs) const { + return !(*this == rhs); + } } namespace YAML { diff --git a/src/backend/metadata/plugin_metadata.h b/src/backend/metadata/plugin_metadata.h index ff74571b..9212bd0e 100644 --- a/src/backend/metadata/plugin_metadata.h +++ b/src/backend/metadata/plugin_metadata.h @@ -96,6 +96,10 @@ namespace loot { //Compare name strings. bool operator == (const PluginMetadata& rhs) const; bool operator != (const PluginMetadata& rhs) const; + + //Compare name string. + bool operator == (const std::string& rhs) const; + bool operator != (const std::string& rhs) const; protected: std::string name; bool enabled; //Default to true. diff --git a/src/backend/plugin/plugin.cpp b/src/backend/plugin/plugin.cpp index 55aa6380..b09c0e57 100644 --- a/src/backend/plugin/plugin.cpp +++ b/src/backend/plugin/plugin.cpp @@ -166,14 +166,6 @@ namespace loot { BOOST_LOG_TRIVIAL(trace) << name << ": " << "Plugin loading complete."; } - bool Plugin::operator == (const Plugin& rhs) const { - return PluginMetadata(name) == PluginMetadata(rhs.Name()); - } - - bool Plugin::operator != (const Plugin& rhs) const { - return !(*this == rhs); - } - const std::set& Plugin::FormIDs() const { return formIDs; } diff --git a/src/backend/plugin/plugin.h b/src/backend/plugin/plugin.h index 642f0af8..a1c2ed80 100644 --- a/src/backend/plugin/plugin.h +++ b/src/backend/plugin/plugin.h @@ -56,10 +56,6 @@ namespace loot { bool LoadsBSA(const Game& game) const; bool IsActive(const Game& game) const; - //Compare name strings. - bool operator == (const Plugin& rhs) const; - bool operator != (const Plugin& rhs) const; - //Load ordering functions. bool DoFormIDsOverlap(const Plugin& plugin) const; std::set OverlapFormIDs(const Plugin& plugin) const; diff --git a/src/gui/handler.cpp b/src/gui/handler.cpp index 6e136107..14c5219d 100644 --- a/src/gui/handler.cpp +++ b/src/gui/handler.cpp @@ -464,7 +464,7 @@ namespace loot { std::string Handler::ClearPluginMetadata(const std::string& pluginName) { BOOST_LOG_TRIVIAL(debug) << "Clearing user metadata for plugin " << pluginName; - _lootState.CurrentGame().userlist.ErasePlugin(Plugin(pluginName)); + _lootState.CurrentGame().userlist.ErasePlugin(PluginMetadata(pluginName)); // Save userlist edits. _lootState.CurrentGame().userlist.Save(_lootState.CurrentGame().UserlistPath()); diff --git a/src/tests/backend/test_plugin_sorter.h b/src/tests/backend/test_plugin_sorter.h index 4acb739d..8cb04b69 100644 --- a/src/tests/backend/test_plugin_sorter.h +++ b/src/tests/backend/test_plugin_sorter.h @@ -40,6 +40,22 @@ protected: callback = [](const std::string&) {}; } + inline std::list GetExpectedSortedOrder() const { + return std::list({ + "Skyrim.esm", + "Blank.esm", + "Blank - Different.esm", + "Blank - Master Dependent.esm", + "Blank - Different Master Dependent.esm", + "Blank.esp", + "Blank - Different.esp", + "Blank - Master Dependent.esp", + "Blank - Different Master Dependent.esp", + "Blank - Plugin Dependent.esp", + "Blank - Different Plugin Dependent.esp", + }); + } + loot::Game game; std::function callback; }; @@ -54,84 +70,54 @@ TEST_F(PluginSorter, Sort) { ASSERT_NO_THROW(game.LoadPlugins(false)); loot::PluginSorter ps; + std::list expectedSortedOrder = GetExpectedSortedOrder(); + std::list sorted = ps.Sort(game, loot::Language::english, callback); - EXPECT_EQ(std::list({ - loot::Plugin("Skyrim.esm"), - loot::Plugin("Blank.esm"), - loot::Plugin("Blank - Different.esm"), - loot::Plugin("Blank - Master Dependent.esm"), - loot::Plugin("Blank - Different Master Dependent.esm"), - loot::Plugin("Blank.esp"), - loot::Plugin("Blank - Different.esp"), - loot::Plugin("Blank - Master Dependent.esp"), - loot::Plugin("Blank - Different Master Dependent.esp"), - loot::Plugin("Blank - Plugin Dependent.esp"), - loot::Plugin("Blank - Different Plugin Dependent.esp"), - }), sorted); + EXPECT_TRUE(std::equal(begin(sorted), end(sorted), begin(expectedSortedOrder))); // Check stability. sorted = ps.Sort(game, loot::Language::english, callback); - EXPECT_EQ(std::list({ - loot::Plugin("Skyrim.esm"), - loot::Plugin("Blank.esm"), - loot::Plugin("Blank - Different.esm"), - loot::Plugin("Blank - Master Dependent.esm"), - loot::Plugin("Blank - Different Master Dependent.esm"), - loot::Plugin("Blank.esp"), - loot::Plugin("Blank - Different.esp"), - loot::Plugin("Blank - Master Dependent.esp"), - loot::Plugin("Blank - Different Master Dependent.esp"), - loot::Plugin("Blank - Plugin Dependent.esp"), - loot::Plugin("Blank - Different Plugin Dependent.esp"), - }), sorted); + EXPECT_TRUE(std::equal(begin(sorted), end(sorted), begin(expectedSortedOrder))); } TEST_F(PluginSorter, Sort_HeadersOnly) { ASSERT_NO_THROW(game.LoadPlugins(true)); loot::PluginSorter ps; + std::list expectedSortedOrder = GetExpectedSortedOrder(); + std::list sorted = ps.Sort(game, loot::Language::english, callback); - EXPECT_EQ(std::list({ - loot::Plugin("Skyrim.esm"), - loot::Plugin("Blank.esm"), - loot::Plugin("Blank - Different.esm"), - loot::Plugin("Blank - Master Dependent.esm"), - loot::Plugin("Blank - Different Master Dependent.esm"), - loot::Plugin("Blank.esp"), - loot::Plugin("Blank - Different.esp"), - loot::Plugin("Blank - Master Dependent.esp"), - loot::Plugin("Blank - Different Master Dependent.esp"), - loot::Plugin("Blank - Plugin Dependent.esp"), - loot::Plugin("Blank - Different Plugin Dependent.esp"), - }), sorted); + EXPECT_TRUE(std::equal(begin(sorted), end(sorted), begin(expectedSortedOrder))); } TEST_F(PluginSorter, Sort_WithPriority) { ASSERT_NO_THROW(game.LoadPlugins(false)); - loot::Plugin plugin("Blank - Different Master Dependent.esp"); + loot::PluginMetadata plugin("Blank - Different Master Dependent.esp"); plugin.Priority(-1100000); game.userlist.AddPlugin(plugin); loot::PluginSorter ps; + std::list expectedSortedOrder({ + "Skyrim.esm", + "Blank.esm", + "Blank - Different.esm", + "Blank - Master Dependent.esm", + "Blank - Different Master Dependent.esm", + "Blank - Different Master Dependent.esp", + "Blank.esp", + "Blank - Different.esp", + "Blank - Master Dependent.esp", + "Blank - Plugin Dependent.esp", + "Blank - Different Plugin Dependent.esp", + }); + std::list sorted = ps.Sort(game, loot::Language::english, callback); - EXPECT_EQ(std::list({ - loot::Plugin("Skyrim.esm"), - loot::Plugin("Blank.esm"), - loot::Plugin("Blank - Different.esm"), - loot::Plugin("Blank - Master Dependent.esm"), - loot::Plugin("Blank - Different Master Dependent.esm"), - loot::Plugin("Blank - Different Master Dependent.esp"), - loot::Plugin("Blank.esp"), - loot::Plugin("Blank - Different.esp"), - loot::Plugin("Blank - Master Dependent.esp"), - loot::Plugin("Blank - Plugin Dependent.esp"), - loot::Plugin("Blank - Different Plugin Dependent.esp"), - }), sorted); + EXPECT_TRUE(std::equal(begin(sorted), end(sorted), begin(expectedSortedOrder))); } TEST_F(PluginSorter, Sort_WithLoadAfter) { ASSERT_NO_THROW(game.LoadPlugins(false)); - loot::Plugin plugin("Blank.esp"); + loot::PluginMetadata plugin("Blank.esp"); plugin.LoadAfter({ loot::File("Blank - Different.esp"), loot::File("Blank - Different Plugin Dependent.esp"), @@ -139,25 +125,27 @@ TEST_F(PluginSorter, Sort_WithLoadAfter) { game.userlist.AddPlugin(plugin); loot::PluginSorter ps; + std::list expectedSortedOrder({ + "Skyrim.esm", + "Blank.esm", + "Blank - Different.esm", + "Blank - Master Dependent.esm", + "Blank - Different Master Dependent.esm", + "Blank - Different.esp", + "Blank - Master Dependent.esp", + "Blank - Different Master Dependent.esp", + "Blank - Different Plugin Dependent.esp", + "Blank.esp", + "Blank - Plugin Dependent.esp", + }); + std::list sorted = ps.Sort(game, loot::Language::english, callback); - EXPECT_EQ(std::list({ - loot::Plugin("Skyrim.esm"), - loot::Plugin("Blank.esm"), - loot::Plugin("Blank - Different.esm"), - loot::Plugin("Blank - Master Dependent.esm"), - loot::Plugin("Blank - Different Master Dependent.esm"), - loot::Plugin("Blank - Different.esp"), - loot::Plugin("Blank - Master Dependent.esp"), - loot::Plugin("Blank - Different Master Dependent.esp"), - loot::Plugin("Blank - Different Plugin Dependent.esp"), - loot::Plugin("Blank.esp"), - loot::Plugin("Blank - Plugin Dependent.esp"), - }), sorted); + EXPECT_TRUE(std::equal(begin(sorted), end(sorted), begin(expectedSortedOrder))); } TEST_F(PluginSorter, Sort_WithRequirements) { ASSERT_NO_THROW(game.LoadPlugins(false)); - loot::Plugin plugin("Blank.esp"); + loot::PluginMetadata plugin("Blank.esp"); plugin.Reqs({ loot::File("Blank - Different.esp"), loot::File("Blank - Different Plugin Dependent.esp"), @@ -165,25 +153,27 @@ TEST_F(PluginSorter, Sort_WithRequirements) { game.userlist.AddPlugin(plugin); loot::PluginSorter ps; + std::list expectedSortedOrder({ + "Skyrim.esm", + "Blank.esm", + "Blank - Different.esm", + "Blank - Master Dependent.esm", + "Blank - Different Master Dependent.esm", + "Blank - Different.esp", + "Blank - Master Dependent.esp", + "Blank - Different Master Dependent.esp", + "Blank - Different Plugin Dependent.esp", + "Blank.esp", + "Blank - Plugin Dependent.esp", + }); + std::list sorted = ps.Sort(game, loot::Language::english, callback); - EXPECT_EQ(std::list({ - loot::Plugin("Skyrim.esm"), - loot::Plugin("Blank.esm"), - loot::Plugin("Blank - Different.esm"), - loot::Plugin("Blank - Master Dependent.esm"), - loot::Plugin("Blank - Different Master Dependent.esm"), - loot::Plugin("Blank - Different.esp"), - loot::Plugin("Blank - Master Dependent.esp"), - loot::Plugin("Blank - Different Master Dependent.esp"), - loot::Plugin("Blank - Different Plugin Dependent.esp"), - loot::Plugin("Blank.esp"), - loot::Plugin("Blank - Plugin Dependent.esp"), - }), sorted); + EXPECT_TRUE(std::equal(begin(sorted), end(sorted), begin(expectedSortedOrder))); } TEST_F(PluginSorter, Sort_HasCycle) { ASSERT_NO_THROW(game.LoadPlugins(false)); - loot::Plugin plugin("Blank.esm"); + loot::PluginMetadata plugin("Blank.esm"); plugin.LoadAfter({loot::File("Blank - Master Dependent.esm")}); game.userlist.AddPlugin(plugin);