Repository: RetroAchievements/RAIntegration
Pull request #743 (2021-06-04 17:21:06)

don't update filters when creating new assets unless necessary
Prevents changing filter from "all" to "local" when cloning/creating new achievements as they would already be visible.
Also changes filter from "active" (or similar) to "all" when cloning/creating new achievements so they will be visible.


Patch:
diff --git a/src/ui/viewmodels/AssetListViewModel.cpp b/src/ui/viewmodels/AssetListViewModel.cpp
index 36d6db08..b1cf2b66 100644
--- a/src/ui/viewmodels/AssetListViewModel.cpp
+++ b/src/ui/viewmodels/AssetListViewModel.cpp
@@ -354,6 +354,22 @@ void AssetListViewModel::ApplyFilter()
     UpdateButtons();
 }
 
+void AssetListViewModel::EnsureAppearsInFilteredList(const ra::data::models::AssetModelBase& pAsset)
+{
+    // if the filter category is not all, ensure it matches the asset
+    const auto nFilterCategory = GetFilterCategory();
+    if (nFilterCategory != FilterCategory::All)
+    {
+        const auto nAssetCategory = ra::itoe<ra::data::models::AssetCategory>(ra::etoi(nFilterCategory));
+        if (pAsset.GetCategory() != nAssetCategory)
+            SetValue(FilterCategoryProperty, ra::etoi(pAsset.GetCategory()));
+    }
+
+    // if there's a special filter and it doesn't match, clear the special filter
+    if (GetSpecialFilter() != SpecialFilter::All && !MatchesFilter(pAsset))
+        SetSpecialFilter(ra::ui::viewmodels::AssetListViewModel::SpecialFilter::All);
+}
+
 bool AssetListViewModel::MatchesFilter(const ra::data::models::AssetModelBase& pAsset)
 {
     const auto nFilterCategory = GetFilterCategory();
@@ -1343,8 +1359,6 @@ void AssetListViewModel::CreateNew()
 
     FilteredAssets().BeginUpdate();
 
-    SetFilterCategory(FilterCategory::Local);
-
     auto& vmAchievement = pGameContext.Assets().NewAchievement();
     vmAchievement.SetCategory(ra::data::models::AssetCategory::Local);
     vmAchievement.SetPoints(0);
@@ -1352,6 +1366,8 @@ void AssetListViewModel::CreateNew()
     vmAchievement.UpdateServerCheckpoint();
     vmAchievement.SetNew();
 
+    EnsureAppearsInFilteredList(vmAchievement);
+
     const auto nId = ra::to_signed(vmAchievement.GetID());
 
     // select the new viewmodel, and deselect everything else
@@ -1416,8 +1432,6 @@ void AssetListViewModel::CloneSelected()
 
     FilteredAssets().BeginUpdate();
 
-    SetFilterCategory(FilterCategory::Local);
-
     // add the cloned items
     std::vector<int> vNewIDs;
     for (const auto* pAsset : vSelectedAssets)
@@ -1436,6 +1450,8 @@ void AssetListViewModel::CloneSelected()
             vmAchievement.SetTrigger(pSourceAchievement->GetTrigger());
             vmAchievement.SetNew();
 
+            EnsureAppearsInFilteredList(vmAchievement);
+
             vNewIDs.push_back(vmAchievement.GetID());
         }
     }
diff --git a/src/ui/viewmodels/AssetListViewModel.hh b/src/ui/viewmodels/AssetListViewModel.hh
index b0945424..786a7df8 100644
--- a/src/ui/viewmodels/AssetListViewModel.hh
+++ b/src/ui/viewmodels/AssetListViewModel.hh
@@ -281,6 +281,7 @@ private:
     void DoUpdateButtons();
     std::atomic_bool m_bNeedToUpdateButtons = false;
 
+    void EnsureAppearsInFilteredList(const ra::data::models::AssetModelBase& pAsset);
     bool MatchesFilter(const ra::data::models::AssetModelBase& pAsset);
     void AddOrRemoveFilteredItem(gsl::index nAssetIndex);
     bool AddOrRemoveFilteredItem(const ra::data::models::AssetModelBase& pAsset);


Test patch:
diff --git a/tests/ui/viewmodels/AssetListViewModel_Tests.cpp b/tests/ui/viewmodels/AssetListViewModel_Tests.cpp
index 53baab04..42587a55 100644
--- a/tests/ui/viewmodels/AssetListViewModel_Tests.cpp
+++ b/tests/ui/viewmodels/AssetListViewModel_Tests.cpp
@@ -55,6 +55,29 @@ std::wstring ToString<ra::ui::viewmodels::AssetListViewModel::FilterCategory>(
     }
 }
 
+template<>
+std::wstring ToString<ra::ui::viewmodels::AssetListViewModel::SpecialFilter>(
+    const ra::ui::viewmodels::AssetListViewModel::SpecialFilter& filter)
+{
+    switch (filter)
+    {
+        case ra::ui::viewmodels::AssetListViewModel::SpecialFilter::Active:
+            return L"Active";
+        case ra::ui::viewmodels::AssetListViewModel::SpecialFilter::Inactive:
+            return L"Inactive";
+        case ra::ui::viewmodels::AssetListViewModel::SpecialFilter::Modified:
+            return L"Modified";
+        case ra::ui::viewmodels::AssetListViewModel::SpecialFilter::Unpublished:
+            return L"Unpublished";
+        case ra::ui::viewmodels::AssetListViewModel::SpecialFilter::Authored:
+            return L"Authored";
+        case ra::ui::viewmodels::AssetListViewModel::SpecialFilter::All:
+            return L"All";
+        default:
+            return std::to_wstring(ra::etoi(filter));
+    }
+}
+
 } // namespace CppUnitTestFramework
 } // namespace VisualStudio
 } // namespace Microsoft
@@ -2510,6 +2533,72 @@ public:
         Assert::IsTrue(pAsset->IsSelected());
     }
 
+    TEST_METHOD(TestCreateNewFilterCategoryAll)
+    {
+        AssetListViewModelHarness vmAssetList;
+        vmAssetList.mockUserContext.Initialize("User1", "FOO");
+        vmAssetList.MockGameId(22U);
+        vmAssetList.AddAchievement(AssetCategory::Core, 5, L"Test1", L"Desc1", L"12345", "0xH1234=1");
+        vmAssetList.AddAchievement(AssetCategory::Core, 7, L"Test2", L"Desc2", L"11111", "0xH1111=1");
+        vmAssetList.SetFilterCategory(AssetListViewModel::FilterCategory::All);
+        vmAssetList.ForceUpdateButtons();
+
+        Assert::AreEqual({ 2U }, vmAssetList.mockGameContext.Assets().Count());
+        Assert::AreEqual({ 2U }, vmAssetList.FilteredAssets().Count());
+        Assert::AreEqual(AssetListViewModel::FilterCategory::All, vmAssetList.GetFilterCategory());
+
+        vmAssetList.CreateNew();
+
+        // new Local achievement should be created and focused
+        Assert::AreEqual({ 3U }, vmAssetList.mockGameContext.Assets().Count());
+        Assert::AreEqual({ 3U }, vmAssetList.FilteredAssets().Count());
+        Assert::AreEqual(AssetListViewModel::FilterCategory::All, vmAssetList.GetFilterCategory());
+
+        const auto* pAsset = vmAssetList.FilteredAssets().GetItemAt(2);
+        Expects(pAsset != nullptr);
+        Assert::IsTrue(pAsset->IsSelected());
+        Assert::AreEqual(std::wstring(), pAsset->GetLabel());
+        Assert::AreEqual(AssetCategory::Local, pAsset->GetCategory());
+        Assert::AreEqual(AssetState::Inactive, pAsset->GetState());
+        Assert::AreEqual(AssetChanges::New, pAsset->GetChanges());
+        Assert::AreEqual({ 111000001U }, pAsset->GetId());
+        Assert::AreEqual(0, pAsset->GetPoints());
+    }
+
+    TEST_METHOD(TestCreateNewSpecialFilterActive)
+    {
+        AssetListViewModelHarness vmAssetList;
+        vmAssetList.mockUserContext.Initialize("User1", "FOO");
+        vmAssetList.MockGameId(22U);
+        vmAssetList.AddAchievement(AssetCategory::Core, 5, L"Test1", L"Desc1", L"12345", "0xH1234=1");
+        vmAssetList.mockGameContext.Assets().FindAchievement(1)->SetState(AssetState::Active);
+        vmAssetList.AddAchievement(AssetCategory::Core, 7, L"Test2", L"Desc2", L"11111", "0xH1111=1");
+        vmAssetList.mockGameContext.Assets().FindAchievement(2)->SetState(AssetState::Active);
+        vmAssetList.SetSpecialFilter(AssetListViewModel::SpecialFilter::Active);
+        vmAssetList.ForceUpdateButtons();
+
+        Assert::AreEqual({ 2U }, vmAssetList.mockGameContext.Assets().Count());
+        Assert::AreEqual({ 2U }, vmAssetList.FilteredAssets().Count());
+        Assert::AreEqual(AssetListViewModel::SpecialFilter::Active, vmAssetList.GetSpecialFilter());
+
+        vmAssetList.CreateNew();
+
+        // new Local achievement should be created and focused
+        Assert::AreEqual({ 3U }, vmAssetList.mockGameContext.Assets().Count());
+        Assert::AreEqual({ 1U }, vmAssetList.FilteredAssets().Count());
+        Assert::AreEqual(AssetListViewModel::SpecialFilter::All, vmAssetList.GetSpecialFilter());
+
+        const auto* pAsset = vmAssetList.FilteredAssets().GetItemAt(0);
+        Expects(pAsset != nullptr);
+        Assert::IsTrue(pAsset->IsSelected());
+        Assert::AreEqual(std::wstring(), pAsset->GetLabel());
+        Assert::AreEqual(AssetCategory::Local, pAsset->GetCategory());
+        Assert::AreEqual(AssetState::Inactive, pAsset->GetState());
+        Assert::AreEqual(AssetChanges::New, pAsset->GetChanges());
+        Assert::AreEqual({ 111000001U }, pAsset->GetId());
+        Assert::AreEqual(0, pAsset->GetPoints());
+    }
+
     TEST_METHOD(TestCloneSingle)
     {
         AssetListViewModelHarness vmAssetList;
@@ -2633,6 +2722,81 @@ public:
         Assert::AreEqual(2, pLocalBadges->GetReferenceCount(sLocalBadgeName, false));
     }
 
+    TEST_METHOD(TestCloneSingleFilterCategoryAll)
+    {
+        AssetListViewModelHarness vmAssetList;
+        vmAssetList.SetGameId(22U);
+        vmAssetList.AddAchievement(AssetCategory::Core, 5, L"Test1", L"Desc1", L"12345", "0xH1234=1");
+        vmAssetList.AddAchievement(AssetCategory::Core, 7, L"Test2", L"Desc2", L"11111", "0xH1111=1");
+        vmAssetList.SetFilterCategory(AssetListViewModel::FilterCategory::All);
+
+        Assert::AreEqual({ 2U }, vmAssetList.mockGameContext.Assets().Count());
+        Assert::AreEqual({ 2U }, vmAssetList.FilteredAssets().Count());
+        Assert::AreEqual(AssetListViewModel::FilterCategory::All, vmAssetList.GetFilterCategory());
+
+        vmAssetList.FilteredAssets().GetItemAt(1)->SetSelected(true);
+        vmAssetList.ForceUpdateButtons();
+
+        vmAssetList.CloneSelected();
+
+        // new Local achievement should be created and focused without changing the filter category
+        Assert::AreEqual({ 3U }, vmAssetList.mockGameContext.Assets().Count());
+        Assert::AreEqual({ 3U }, vmAssetList.FilteredAssets().Count());
+        Assert::AreEqual(AssetListViewModel::FilterCategory::All, vmAssetList.GetFilterCategory());
+
+        const auto* pAsset = vmAssetList.FilteredAssets().GetItemAt(0);
+        Expects(pAsset != nullptr);
+        Assert::AreEqual({ 1U }, pAsset->GetId());
+        Assert::IsFalse(pAsset->IsSelected());
+
+        pAsset = vmAssetList.FilteredAssets().GetItemAt(2);
+        Expects(pAsset != nullptr);
+        Assert::AreEqual({ 111000001U }, pAsset->GetId());
+        Assert::AreEqual(AssetCategory::Local, pAsset->GetCategory());
+        Assert::IsTrue(pAsset->IsSelected());
+    }
+
+    TEST_METHOD(TestCloneSingleSpecialFilterActive)
+    {
+        AssetListViewModelHarness vmAssetList;
+
+        vmAssetList.SetGameId(22U);
+        vmAssetList.SetFilterCategory(AssetListViewModel::FilterCategory::Local);
+        vmAssetList.AddAchievement(AssetCategory::Local, 5, L"Test1", L"Desc1", L"12345", "0xH1234=1");
+        auto* pSourceAchievement = vmAssetList.mockGameContext.Assets().FindAchievement(1);
+        Expects(pSourceAchievement != nullptr);
+        pSourceAchievement->SetState(ra::data::models::AssetState::Active);
+        vmAssetList.SetSpecialFilter(AssetListViewModel::SpecialFilter::Active);
+        Assert::AreEqual(AssetListViewModel::SpecialFilter::Active, vmAssetList.GetSpecialFilter());
+
+        Assert::AreEqual({ 1U }, vmAssetList.mockGameContext.Assets().Count());
+        Assert::AreEqual({ 1U }, vmAssetList.FilteredAssets().Count());
+
+        vmAssetList.FilteredAssets().GetItemAt(0)->SetSelected(true);
+        vmAssetList.ForceUpdateButtons();
+
+        vmAssetList.CloneSelected();
+
+        // new Local achievement should be created and focused
+        Assert::AreEqual({ 2U }, vmAssetList.mockGameContext.Assets().Count());
+        Assert::AreEqual({ 2U }, vmAssetList.FilteredAssets().Count());
+        Assert::AreEqual(AssetListViewModel::FilterCategory::Local, vmAssetList.GetFilterCategory());
+        Assert::AreEqual(AssetListViewModel::SpecialFilter::All, vmAssetList.GetSpecialFilter());
+
+        const auto* pAsset = vmAssetList.FilteredAssets().GetItemAt(0);
+        Expects(pAsset != nullptr);
+        Assert::AreEqual({ 1U }, pAsset->GetId());
+        Assert::AreEqual(AssetState::Active, pAsset->GetState());
+        Assert::IsFalse(pAsset->IsSelected());
+
+        pAsset = vmAssetList.FilteredAssets().GetItemAt(1);
+        Expects(pAsset != nullptr);
+        Assert::AreEqual({ 111000001U }, pAsset->GetId());
+        Assert::AreEqual(AssetCategory::Local, pAsset->GetCategory());
+        Assert::AreEqual(AssetState::Inactive, pAsset->GetState());
+        Assert::IsTrue(pAsset->IsSelected());
+    }
+
     TEST_METHOD(TestCloneMultiple)
     {
         AssetListViewModelHarness vmAssetList;