Repository: RetroAchievements/RAIntegration
Pull request #718 (2021-05-02 01:55:38)

don't throw validation error on lost focus for local badge
Prevents the "Only values that can be represented as decimal are allowed" error when the Badge field loses focus and a local badge is selected.


Patch:
diff --git a/RAInterface b/RAInterface
index d6855020..45e820e1 160000
--- a/RAInterface
+++ b/RAInterface
@@ -1,1 +1,1 @@
-Subproject commit d6855020408bb96503726b1d9b155a273cc99508
+Subproject commit 45e820e1ae3189dde0a5b2338aac38f7a19b05d5
diff --git a/src/api/impl/ConnectedServer.cpp b/src/api/impl/ConnectedServer.cpp
index 4d5de45a..aaa2bd57 100644
--- a/src/api/impl/ConnectedServer.cpp
+++ b/src/api/impl/ConnectedServer.cpp
@@ -25,6 +25,12 @@ namespace ra {
 namespace api {
 namespace impl {
 
+ConnectedServer::ConnectedServer(const std::string& sHost)
+    : m_sHost(sHost)
+{
+    rc_api_set_host(sHost.c_str());
+}
+
 _NODISCARD static bool HandleHttpError(_In_ const ra::services::Http::StatusCode nStatusCode,
                                        _Inout_ ApiResponseBase& pResponse)
 {
diff --git a/src/api/impl/ConnectedServer.hh b/src/api/impl/ConnectedServer.hh
index fb7c5e04..c5ee41e5 100644
--- a/src/api/impl/ConnectedServer.hh
+++ b/src/api/impl/ConnectedServer.hh
@@ -11,7 +11,7 @@ namespace impl {
 class ConnectedServer : public ServerBase
 {
 public:
-    explicit ConnectedServer(const std::string& sHost) : m_sHost(sHost) {}
+    explicit ConnectedServer(const std::string& sHost);
 
     const char* Name() const noexcept override { return m_sHost.c_str(); }
 
diff --git a/src/ui/viewmodels/AssetEditorViewModel.cpp b/src/ui/viewmodels/AssetEditorViewModel.cpp
index fc72d9c5..07f107af 100644
--- a/src/ui/viewmodels/AssetEditorViewModel.cpp
+++ b/src/ui/viewmodels/AssetEditorViewModel.cpp
@@ -101,6 +101,13 @@ void AssetEditorViewModel::LoadAsset(ra::data::models::AssetModelBase* pAsset)
         SetState(pAsset->GetState());
         SetCategory(pAsset->GetCategory());
 
+        // normally this happens in OnValueChanged, but only if m_pAsset is set, which doesn't happen until later
+        // and we don't want the other stuff in OnValueChanged to run right now
+        if (pAsset->GetState() == ra::data::models::AssetState::Waiting)
+            SetValue(WaitingLabelProperty, L"Waiting");
+        else
+            SetValue(WaitingLabelProperty, WaitingLabelProperty.GetDefaultValue());
+
         if (pAsset->GetCategory() == ra::data::models::AssetCategory::Local)
             SetValue(IDProperty, 0);
         else
@@ -155,6 +162,7 @@ void AssetEditorViewModel::LoadAsset(ra::data::models::AssetModelBase* pAsset)
         SetPauseOnReset(PauseOnResetProperty.GetDefaultValue());
         SetPauseOnTrigger(PauseOnTriggerProperty.GetDefaultValue());
         SetWindowTitle(L"Achievement Editor");
+        SetValue(WaitingLabelProperty, WaitingLabelProperty.GetDefaultValue());
 
         ra::data::models::CapturedTriggerHits pCapturedHits;
         Trigger().InitializeFrom("", pCapturedHits);
diff --git a/src/ui/viewmodels/AssetListViewModel.cpp b/src/ui/viewmodels/AssetListViewModel.cpp
index 0d49e203..7c819813 100644
--- a/src/ui/viewmodels/AssetListViewModel.cpp
+++ b/src/ui/viewmodels/AssetListViewModel.cpp
@@ -954,7 +954,7 @@ static std::wstring ValidateCondSet(const rc_condset_t* pCondSet)
         if (pCondition->type == RC_CONDITION_TRIGGER)
             return L"Trigger is pre-release functionality";
         if (pCondition->type == RC_CONDITION_SUB_HITS)
-            return L"Trigger is pre-release functionality";
+            return L"SubHits is pre-release functionality";
         if (pCondition->oper == RC_OPERATOR_MULT)
             return L"Multiplication is pre-release functionality";
         if (pCondition->oper == RC_OPERATOR_DIV)
@@ -1078,6 +1078,8 @@ void AssetListViewModel::ResetSelected()
     if (vmMessageBox.ShowModal() == DialogResult::No)
         return;
 
+    std::vector<ra::AchievementID> vActiveAchievementsBefore;
+
     auto& pGameContext = ra::services::ServiceLocator::GetMutable<ra::data::context::GameContext>();
     pGameContext.Assets().BeginUpdate();
 
@@ -1088,8 +1090,14 @@ void AssetListViewModel::ResetSelected()
         for (gsl::index nIndex = gsl::narrow_cast<gsl::index>(pGameContext.Assets().Count()) - 1; nIndex >= 0; --nIndex)
         {
             auto* pAsset = pGameContext.Assets().GetItemAt(nIndex);
-            if (pAsset != nullptr && pAsset->GetChanges() == ra::data::models::AssetChanges::New)
-                pGameContext.Assets().RemoveAt(nIndex);
+            if (pAsset != nullptr)
+            {
+                if (pAsset->GetType() == ra::data::models::AssetType::Achievement && pAsset->IsActive())
+                    vActiveAchievementsBefore.push_back(pAsset->GetID());
+
+                if (pAsset->GetChanges() == ra::data::models::AssetChanges::New)
+                    pGameContext.Assets().RemoveAt(nIndex);
+            }
         }
 
         // when resetting all, always read the file to pick up new items
@@ -1107,6 +1115,9 @@ void AssetListViewModel::ResetSelected()
                 auto* pAsset = pGameContext.Assets().GetItemAt(nIndex);
                 if (pAsset->GetID() == nId && pAsset->GetType() == nType)
                 {
+                    if (nType == ra::data::models::AssetType::Achievement && pAsset->IsActive())
+                        vActiveAchievementsBefore.push_back(nId);
+
                     if (pAsset->GetChanges() == ra::data::models::AssetChanges::New)
                         pGameContext.Assets().RemoveAt(nIndex);
                     else
@@ -1121,7 +1132,25 @@ void AssetListViewModel::ResetSelected()
             pGameContext.Assets().ReloadAssets(vAssetsToReset);
     }
 
+    // if the asset that is loaded in the editor no longer exists, clear out the editor
+    // do this before EndUpdate so the asset won't have been destroyed yet
+    auto& pWindowManager = ra::services::ServiceLocator::GetMutable<ra::ui::viewmodels::WindowManager>();
+    const auto* pEditorAsset = pWindowManager.AssetEditor.GetAsset();
+    if (pEditorAsset != nullptr && !pGameContext.Assets().FindAsset(pEditorAsset->GetType(), pEditorAsset->GetID()))
+        pWindowManager.AssetEditor.LoadAsset(nullptr);
+
     pGameContext.Assets().EndUpdate();
+
+    // if any achievements were deleted, disable their challenge indicators
+    for (auto nId : vActiveAchievementsBefore)
+    {
+        if (!pGameContext.Assets().FindAchievement(nId))
+        {
+            // when an active achievement is deleted, the challenge indicator needs to be hidden as no event will be raised
+            auto& pOverlayManager = ra::services::ServiceLocator::GetMutable<ra::ui::viewmodels::OverlayManager>();
+            pOverlayManager.RemoveChallengeIndicator(nId);
+        }
+    }
 }
 
 void AssetListViewModel::RevertSelected()
@@ -1201,6 +1230,18 @@ void AssetListViewModel::RevertSelected()
         {
             // reverting a local item deletes it
             pAsset->SetDeleted();
+
+            // when an active achievement is deleted, the challenge indicator needs to be hidden as no event will be raised
+            if (pAsset->IsActive() && pAsset->GetType() == ra::data::models::AssetType::Achievement)
+            {
+                auto& pOverlayManager = ra::services::ServiceLocator::GetMutable<ra::ui::viewmodels::OverlayManager>();
+                pOverlayManager.RemoveChallengeIndicator(pAsset->GetID());
+            }
+
+            // if the asset is open in the editor, select no asset
+            auto& pWindowManager = ra::services::ServiceLocator::GetMutable<ra::ui::viewmodels::WindowManager>();
+            if (pWindowManager.AssetEditor.GetAsset() == pAsset)
+                pWindowManager.AssetEditor.LoadAsset(nullptr);
         }
         else
         {
diff --git a/src/ui/win32/AssetEditorDialog.cpp b/src/ui/win32/AssetEditorDialog.cpp
index b7db339b..e50f4815 100644
--- a/src/ui/win32/AssetEditorDialog.cpp
+++ b/src/ui/win32/AssetEditorDialog.cpp
@@ -332,6 +332,10 @@ void AssetEditorDialog::BadgeNameBinding::UpdateSourceFromText(const std::wstrin
     std::wstring sError;
     if (!sValue.empty())
     {
+        // ignore virtual tag that's hiding a more complex path
+        if (sValue == L"[local]")
+            return;
+
         // special case - don't validate if the string is entirely made of 0s.
         // the default value is "00000", which is less than the minimum.
         const wchar_t* pChar = &sValue.at(0);


Test patch:
diff --git a/tests/ui/viewmodels/AssetEditorViewModel_Tests.cpp b/tests/ui/viewmodels/AssetEditorViewModel_Tests.cpp
index b93fbaa8..6a86092b 100644
--- a/tests/ui/viewmodels/AssetEditorViewModel_Tests.cpp
+++ b/tests/ui/viewmodels/AssetEditorViewModel_Tests.cpp
@@ -254,12 +254,61 @@ public:
         Assert::AreEqual(std::wstring(L"83295"), editor.GetBadge());
         Assert::IsTrue(editor.IsAssetLoaded());
         Assert::IsFalse(editor.HasAssetValidationError());
+        Assert::AreEqual(std::wstring(L"Waiting"), editor.GetWaitingLabel());
 
         // changes should have been discarded
         Assert::AreEqual(AssetChanges::None, achievement.GetChanges());
         Assert::AreEqual(std::wstring(L"Do something cool"), achievement.GetDescription());
     }
 
+    TEST_METHOD(TestLoadAchievementWaiting)
+    {
+        AssetEditorViewModelHarness editor;
+        Assert::AreEqual(std::wstring(L"Active"), editor.GetWaitingLabel());
+
+        AchievementModel achievement;
+        achievement.SetName(L"Test Achievement");
+        achievement.SetID(1234U);
+        achievement.SetState(AssetState::Active);
+        achievement.SetDescription(L"Do something cool");
+        achievement.SetTrigger("M:0xH1234=6.11.");
+        achievement.SetCategory(AssetCategory::Unofficial);
+        achievement.SetPoints(10);
+        achievement.SetBadge(L"58329");
+        achievement.CreateServerCheckpoint();
+        achievement.CreateLocalCheckpoint();
+
+        AchievementModel achievement2;
+        achievement2.SetName(L"Test Achievement");
+        achievement2.SetID(1234U);
+        achievement2.SetState(AssetState::Waiting);
+        achievement2.SetDescription(L"Do something cool");
+        achievement2.SetTrigger("M:0xH1234=6.11.");
+        achievement2.SetCategory(AssetCategory::Unofficial);
+        achievement2.SetPoints(10);
+        achievement2.SetBadge(L"58329");
+        achievement2.CreateServerCheckpoint();
+        achievement2.CreateLocalCheckpoint();
+
+        editor.LoadAsset(&achievement);
+        Assert::AreEqual(std::wstring(L"Active"), editor.GetWaitingLabel());
+
+        editor.LoadAsset(nullptr);
+        Assert::AreEqual(std::wstring(L"Active"), editor.GetWaitingLabel());
+
+        editor.LoadAsset(&achievement2);
+        Assert::AreEqual(std::wstring(L"Waiting"), editor.GetWaitingLabel());
+
+        editor.LoadAsset(nullptr);
+        Assert::AreEqual(std::wstring(L"Active"), editor.GetWaitingLabel());
+
+        editor.LoadAsset(&achievement2);
+        Assert::AreEqual(std::wstring(L"Waiting"), editor.GetWaitingLabel());
+
+        editor.LoadAsset(&achievement);
+        Assert::AreEqual(std::wstring(L"Active"), editor.GetWaitingLabel());
+    }
+
     TEST_METHOD(TestLoadAchievementMeasured)
     {
         AssetEditorViewModelHarness editor;
diff --git a/tests/ui/viewmodels/AssetListViewModel_Tests.cpp b/tests/ui/viewmodels/AssetListViewModel_Tests.cpp
index 64eaae81..d69af5f0 100644
--- a/tests/ui/viewmodels/AssetListViewModel_Tests.cpp
+++ b/tests/ui/viewmodels/AssetListViewModel_Tests.cpp
@@ -2914,6 +2914,81 @@ public:
         Assert::AreEqual(AssetChanges::Unpublished, pAsset->GetChanges());
     }
 
+    TEST_METHOD(TestResetSelectedNoLongerExistsInFileOpenInEditor)
+    {
+        AssetListViewModelHarness vmAssetList;
+        vmAssetList.MockGameId(22U);
+        vmAssetList.AddAchievement(AssetCategory::Local, 5, L"Test1", L"Desc1", L"12345", "0xH1234=1");
+        vmAssetList.SetFilterCategory(AssetListViewModel::FilterCategory::Local);
+        vmAssetList.FilteredAssets().GetItemAt(0)->SetSelected(true);
+        vmAssetList.ForceUpdateButtons();
+        vmAssetList.AssertButtonState(ResetButtonState::Reset);
+
+        auto* pAsset = vmAssetList.mockGameContext.Assets().FindAchievement({ 1U });
+        Assert::IsNotNull(pAsset);
+        Ensures(pAsset != nullptr);
+        vmAssetList.mockWindowManager.AssetEditor.LoadAsset(pAsset);
+        Assert::IsTrue(vmAssetList.mockWindowManager.AssetEditor.IsAssetLoaded());
+        Assert::IsTrue(vmAssetList.mockWindowManager.AssetEditor.GetAsset() == pAsset);
+
+        vmAssetList.MockUserFileContents("\n");
+
+        bool bDialogShown = false;
+        vmAssetList.mockDesktop.ExpectWindow<MessageBoxViewModel>([&bDialogShown](MessageBoxViewModel&)
+        {
+            bDialogShown = true;
+            return DialogResult::Yes;
+        });
+
+        vmAssetList.ResetSelected();
+
+        Assert::IsTrue(bDialogShown);
+
+        // achievement should be removed from the editor
+        Assert::IsNull(vmAssetList.mockGameContext.Assets().FindAchievement({ 1U }));
+        Assert::IsFalse(vmAssetList.mockWindowManager.AssetEditor.IsAssetLoaded());
+        Assert::IsNull(vmAssetList.mockWindowManager.AssetEditor.GetAsset());
+    }
+
+    TEST_METHOD(TestResetSelectedNoLongerExistsInFileActiveChallenge)
+    {
+        AssetListViewModelHarness vmAssetList;
+        vmAssetList.MockGameId(22U);
+        vmAssetList.AddAchievement(AssetCategory::Local, 5, L"Test1", L"Desc1", L"12345", "0xH1234=1");
+        vmAssetList.SetFilterCategory(AssetListViewModel::FilterCategory::Local);
+        vmAssetList.FilteredAssets().GetItemAt(0)->SetSelected(true);
+        vmAssetList.ForceUpdateButtons();
+        vmAssetList.AssertButtonState(ResetButtonState::Reset);
+
+        auto* vmAsset = vmAssetList.mockGameContext.Assets().GetItemAt(0);
+        Assert::IsNotNull(vmAsset);
+        Ensures(vmAsset != nullptr);
+        vmAsset->SetState(ra::data::models::AssetState::Primed);
+
+        vmAssetList.mockOverlayManager.AddChallengeIndicator(vmAsset->GetID(), ra::ui::ImageType::Badge, "12345");
+        const auto* pIndicator = vmAssetList.mockOverlayManager.GetChallengeIndicator(vmAsset->GetID());
+        Assert::IsNotNull(pIndicator);
+        Ensures(pIndicator != nullptr);
+        Assert::IsFalse(pIndicator->IsDestroyPending());
+
+        vmAssetList.MockUserFileContents("\n");
+
+        bool bDialogShown = false;
+        vmAssetList.mockDesktop.ExpectWindow<MessageBoxViewModel>([&bDialogShown](MessageBoxViewModel&)
+        {
+            bDialogShown = true;
+            return DialogResult::Yes;
+        });
+
+        vmAssetList.ResetSelected();
+
+        Assert::IsTrue(bDialogShown);
+
+        // RemoveChallengeIndicator only marks the item as to be destroyed.
+        // it still exists until the next Render()
+        Assert::IsTrue(pIndicator->IsDestroyPending());
+    }
+
     TEST_METHOD(TestResetSelectedNew)
     {
         AssetListViewModelHarness vmAssetList;
@@ -3161,6 +3236,78 @@ public:
         Assert::IsNull(pAsset);
     }
 
+    TEST_METHOD(TestRevertSelectedLocalOpenInEditor)
+    {
+        AssetListViewModelHarness vmAssetList;
+        vmAssetList.MockGameId(22U);
+        vmAssetList.AddAchievement(AssetCategory::Local, 5, L"Test1", L"Desc1", L"12345", "0xH1234=1");
+        vmAssetList.SetFilterCategory(AssetListViewModel::FilterCategory::Local);
+        vmAssetList.FilteredAssets().GetItemAt(0)->SetSelected(true);
+        vmAssetList.ForceUpdateButtons();
+        vmAssetList.AssertButtonState(RevertButtonState::Delete);
+
+        bool bDialogShown = false;
+        vmAssetList.mockDesktop.ExpectWindow<MessageBoxViewModel>([&bDialogShown](MessageBoxViewModel& vmMessageBox)
+        {
+            Assert::AreEqual(std::wstring(L"Permanently delete?"), vmMessageBox.GetHeader());
+            Assert::AreEqual(std::wstring(L"Assets not available on the server will be permanently deleted."), vmMessageBox.GetMessage());
+
+            bDialogShown = true;
+            return DialogResult::Yes;
+        });
+
+        auto* pAsset = vmAssetList.mockGameContext.Assets().FindAchievement({ 1U });
+        vmAssetList.mockWindowManager.AssetEditor.LoadAsset(pAsset);
+        Assert::IsTrue(vmAssetList.mockWindowManager.AssetEditor.IsAssetLoaded());
+        Assert::IsTrue(vmAssetList.mockWindowManager.AssetEditor.GetAsset() == pAsset);
+
+        vmAssetList.RevertSelected();
+        Assert::IsTrue(bDialogShown);
+
+        Assert::IsFalse(vmAssetList.mockWindowManager.AssetEditor.IsAssetLoaded());
+        Assert::IsNull(vmAssetList.mockWindowManager.AssetEditor.GetAsset());
+    }
+
+    TEST_METHOD(TestRevertSelectedLocalActiveChallenge)
+    {
+        AssetListViewModelHarness vmAssetList;
+        vmAssetList.MockGameId(22U);
+        vmAssetList.AddAchievement(AssetCategory::Local, 5, L"Test1", L"Desc1", L"12345", "0xH1234=1");
+        vmAssetList.SetFilterCategory(AssetListViewModel::FilterCategory::Local);
+        vmAssetList.FilteredAssets().GetItemAt(0)->SetSelected(true);
+
+        auto* vmAsset = vmAssetList.mockGameContext.Assets().GetItemAt(0);
+        Assert::IsNotNull(vmAsset);
+        Ensures(vmAsset != nullptr);
+        vmAsset->SetState(ra::data::models::AssetState::Primed);
+
+        vmAssetList.mockOverlayManager.AddChallengeIndicator(vmAsset->GetID(), ra::ui::ImageType::Badge, "12345");
+        const auto* pIndicator = vmAssetList.mockOverlayManager.GetChallengeIndicator(vmAsset->GetID());
+        Assert::IsNotNull(pIndicator);
+        Ensures(pIndicator != nullptr);
+        Assert::IsFalse(pIndicator->IsDestroyPending());
+
+        vmAssetList.ForceUpdateButtons();
+        vmAssetList.AssertButtonState(RevertButtonState::Delete);
+
+        bool bDialogShown = false;
+        vmAssetList.mockDesktop.ExpectWindow<MessageBoxViewModel>([&bDialogShown](MessageBoxViewModel& vmMessageBox)
+        {
+            Assert::AreEqual(std::wstring(L"Permanently delete?"), vmMessageBox.GetHeader());
+            Assert::AreEqual(std::wstring(L"Assets not available on the server will be permanently deleted."), vmMessageBox.GetMessage());
+
+            bDialogShown = true;
+            return DialogResult::Yes;
+        });
+
+        vmAssetList.RevertSelected();
+        Assert::IsTrue(bDialogShown);
+
+        // RemoveChallengeIndicator only marks the item as to be destroyed.
+        // it still exists until the next Render()
+        Assert::IsTrue(pIndicator->IsDestroyPending());
+    }
+
     TEST_METHOD(TestRevertSelectedCoreHardcore)
     {
         AssetListViewModelHarness vmAssetList;