From 6ca40a57d9dfe803f55e26517e98528821620026 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Mon, 1 Aug 2016 16:22:24 +0100 Subject: [PATCH] Fix incorrect "not sorted" message display The "not sorted" message shouldn't be displayed if a sort is cancelled and the load order has been sorted at least once before in the same session. Fixes #621. --- src/backend/game/game_cache.cpp | 21 ++++++++++----- src/backend/game/game_cache.h | 5 ++-- src/backend/plugin/plugin_sorter.cpp | 2 +- src/gui/query_handler.cpp | 2 +- src/tests/backend/game/game_cache_test.h | 34 +++++++++++++++++------- 5 files changed, 45 insertions(+), 19 deletions(-) diff --git a/src/backend/game/game_cache.cpp b/src/backend/game/game_cache.cpp index 921f4c2d..e4ad9be9 100644 --- a/src/backend/game/game_cache.cpp +++ b/src/backend/game/game_cache.cpp @@ -40,7 +40,7 @@ using std::pair; using std::string; namespace loot { -GameCache::GameCache() : isLoadOrderSorted_(false) {} +GameCache::GameCache() : loadOrderSortCount_(0) {} GameCache::GameCache(const GameCache& cache) : masterlist_(cache.masterlist_), @@ -48,7 +48,7 @@ GameCache::GameCache(const GameCache& cache) : conditions_(cache.conditions_), plugins_(cache.plugins_), messages_(cache.messages_), - isLoadOrderSorted_(cache.isLoadOrderSorted_) {} + loadOrderSortCount_(cache.loadOrderSortCount_) {} GameCache& GameCache::operator=(const GameCache& cache) { if (&cache != this) { @@ -57,7 +57,7 @@ GameCache& GameCache::operator=(const GameCache& cache) { conditions_ = cache.conditions_; plugins_ = cache.plugins_; messages_ = cache.messages_; - isLoadOrderSorted_ = cache.isLoadOrderSorted_; + loadOrderSortCount_ = cache.loadOrderSortCount_; } return *this; @@ -116,7 +116,7 @@ void GameCache::AddPlugin(const Plugin&& plugin) { std::vector GameCache::GetMessages() const { std::vector output(messages_); - if (!isLoadOrderSorted_) + if (loadOrderSortCount_ == 0) output.push_back(Message(Message::Type::warn, "You have not sorted your load order this session.")); return output; @@ -128,8 +128,17 @@ void GameCache::AppendMessage(const Message& message) { messages_.push_back(message); } -void GameCache::SetLoadOrderSorted(bool isLoadOrderSorted) { - this->isLoadOrderSorted_ = isLoadOrderSorted; +void GameCache::IncrementLoadOrderSortCount() { + lock_guard guard(mutex_); + + ++loadOrderSortCount_; +} + +void GameCache::DecrementLoadOrderSortCount() { + lock_guard guard(mutex_); + + if (loadOrderSortCount_ > 0) + --loadOrderSortCount_; } void GameCache::ClearCachedConditions() { diff --git a/src/backend/game/game_cache.h b/src/backend/game/game_cache.h index e729a7de..87845649 100644 --- a/src/backend/game/game_cache.h +++ b/src/backend/game/game_cache.h @@ -55,7 +55,8 @@ public: std::vector GetMessages() const; void AppendMessage(const Message& message); - void SetLoadOrderSorted(bool isLoadOrderSorted); + void IncrementLoadOrderSortCount(); + void DecrementLoadOrderSortCount(); void ClearCachedConditions(); void ClearCachedPlugins(); @@ -66,7 +67,7 @@ private: std::unordered_map conditions_; std::unordered_map plugins_; std::vector messages_; - bool isLoadOrderSorted_; + unsigned short loadOrderSortCount_; mutable std::mutex mutex_; }; diff --git a/src/backend/plugin/plugin_sorter.cpp b/src/backend/plugin/plugin_sorter.cpp index dfc2b970..285c1e36 100644 --- a/src/backend/plugin/plugin_sorter.cpp +++ b/src/backend/plugin/plugin_sorter.cpp @@ -161,7 +161,7 @@ std::list PluginSorter::Sort(Game& game, const Language::Code language) plugins.push_back(graph_[vertex]); } - game.SetLoadOrderSorted(true); + game.IncrementLoadOrderSortCount(); return plugins; } diff --git a/src/gui/query_handler.cpp b/src/gui/query_handler.cpp index f97f4ecf..01244193 100644 --- a/src/gui/query_handler.cpp +++ b/src/gui/query_handler.cpp @@ -144,7 +144,7 @@ bool QueryHandler::OnQuery(CefRefPtr browser, return true; } else if (request == "cancelSort") { lootState_.decrementUnappliedChangeCounter(); - lootState_.getCurrentGame().SetLoadOrderSorted(false); + lootState_.getCurrentGame().DecrementLoadOrderSortCount(); YAML::Node node(GetGeneralMessages()); callback->Success(JSON::stringify(node)); diff --git a/src/tests/backend/game/game_cache_test.h b/src/tests/backend/game/game_cache_test.h index b9a22f6e..3aed5774 100644 --- a/src/tests/backend/game/game_cache_test.h +++ b/src/tests/backend/game/game_cache_test.h @@ -66,7 +66,7 @@ TEST_P(GameCacheTest, copyConstructorShouldCopyCachedData) { cache_.AddPlugin(Plugin(game_, blankEsm, true)); Message expectedMessage(Message::Type::say, "1"); cache_.AppendMessage(expectedMessage); - cache_.SetLoadOrderSorted(true); + cache_.IncrementLoadOrderSortCount(); GameCache otherCache(cache_); EXPECT_EQ(std::make_pair(true, true), otherCache.GetCachedCondition(conditionLowercase)); @@ -82,7 +82,7 @@ TEST_P(GameCacheTest, assignmentOperatorShouldCopyCachedData) { cache_.AddPlugin(Plugin(game_, blankEsm, true)); Message expectedMessage(Message::Type::say, "1"); cache_.AppendMessage(expectedMessage); - cache_.SetLoadOrderSorted(true); + cache_.IncrementLoadOrderSortCount(); GameCache otherCache = cache_; EXPECT_EQ(std::make_pair(true, true), otherCache.GetCachedCondition(conditionLowercase)); @@ -180,18 +180,34 @@ TEST_P(GameCacheTest, aMessageShouldBeCachedByDefault) { ASSERT_EQ(1, cache_.GetMessages().size()); } -TEST_P(GameCacheTest, settingLoadOrderSortedToTrueShouldSupressDefaultCachedMessage) { - cache_.SetLoadOrderSorted(true); +TEST_P(GameCacheTest, incrementLoadOrderSortCountShouldSupressTheDefaultCachedMessage) { + cache_.IncrementLoadOrderSortCount(); - ASSERT_TRUE(cache_.GetMessages().empty()); + EXPECT_TRUE(cache_.GetMessages().empty()); } -TEST_P(GameCacheTest, settingLoadOrderSortedToFalseShouldReverseTheDefaultCachedMessageSuppression) { +TEST_P(GameCacheTest, decrementingLoadOrderSortCountToZeroShouldShowTheDefaultCachedMessage) { auto expectedMessages = cache_.GetMessages(); - cache_.SetLoadOrderSorted(true); - cache_.SetLoadOrderSorted(false); + cache_.IncrementLoadOrderSortCount(); + cache_.DecrementLoadOrderSortCount(); - ASSERT_EQ(expectedMessages, cache_.GetMessages()); + EXPECT_EQ(expectedMessages, cache_.GetMessages()); +} + +TEST_P(GameCacheTest, decrementingLoadOrderSortCountThatIsAlreadyZeroShouldShowTheDefaultCachedMessage) { + auto expectedMessages = cache_.GetMessages(); + cache_.DecrementLoadOrderSortCount(); + + EXPECT_EQ(expectedMessages, cache_.GetMessages()); +} + +TEST_P(GameCacheTest, decrementingLoadOrderSortCountToANonZeroValueShouldSupressTheDefaultCachedMessage) { + auto expectedMessages = cache_.GetMessages(); + cache_.IncrementLoadOrderSortCount(); + cache_.IncrementLoadOrderSortCount(); + cache_.DecrementLoadOrderSortCount(); + + EXPECT_TRUE(cache_.GetMessages().empty()); } TEST_P(GameCacheTest, appendingMessagesShouldStoreThemInTheGivenOrder) {