Repository: RetroAchievements/RAIntegration
Pull request #891 (2022-05-08 16:19:05)

fix client ignoring server-modified rich presence script
If the server version of a rich presence script changed, the client would see the local copy as a user-defined modification and the user would have to manually revert to the server state.

This change detects that the local copy is the previous server version and overwrites it with the new server version.


Patch:
diff --git a/src/data/context/GameContext.cpp b/src/data/context/GameContext.cpp
index be878f47..b3008e5c 100644
--- a/src/data/context/GameContext.cpp
+++ b/src/data/context/GameContext.cpp
@@ -88,6 +88,19 @@ void GameContext::LoadGame(unsigned int nGameId, Mode nMode)
     pLocalBadges->CreateLocalCheckpoint();
     m_vAssets.Append(std::move(pLocalBadges));
 
+    // capture the old rich presence data so we can tell if it changed on the server
+    std::string sOldRichPresence = "[NONE]";
+    {
+        auto& pLocalStorage = ra::services::ServiceLocator::GetMutable<ra::services::ILocalStorage>();
+        auto pData = pLocalStorage.ReadText(ra::services::StorageItemType::GameData, std::to_wstring(nGameId));
+        if (pData != nullptr)
+        {
+            rapidjson::Document pDocument;
+            if (LoadDocument(pDocument, *pData) && pDocument.HasMember("RichPresencePatch"))
+                sOldRichPresence = pDocument["RichPresencePatch"].GetString();
+        }
+    }
+
     // download the game data
     ra::api::FetchGameData::Request request;
     request.GameId = nGameId;
@@ -225,7 +238,20 @@ void GameContext::LoadGame(unsigned int nGameId, Mode nMode)
     auto* pRichPresence = m_vAssets.FindRichPresence();
     if (pRichPresence)
     {
-        if (pRichPresence->GetScript().empty() && !pRichPresence->IsModified())
+        // if the server value differs from the local value, the model will appear as Unpublished
+        if (pRichPresence->GetChanges() != ra::data::models::AssetChanges::None)
+        {
+            // populate another model with the old script so the string gets normalized correctly
+            ra::data::models::RichPresenceModel pOldRichPresenceModel;
+            pOldRichPresenceModel.SetScript(sOldRichPresence);
+
+            // if the old value matches the current value, then the value on the server changed and
+            // there are no local modifications. revert to the server state.
+            if (pRichPresence->GetScript() == pOldRichPresenceModel.GetScript())
+                pRichPresence->RestoreServerCheckpoint();
+        }
+
+        if (pRichPresence->GetScript().empty() && pRichPresence->GetChanges() == ra::data::models::AssetChanges::None)
         {
             const auto nIndex = m_vAssets.FindItemIndex(ra::data::models::AssetModelBase::TypeProperty,
                                                         ra::etoi(ra::data::models::AssetType::RichPresence));
diff --git a/src/ui/viewmodels/AssetListViewModel.cpp b/src/ui/viewmodels/AssetListViewModel.cpp
index d83f7101..e4ce62c0 100644
--- a/src/ui/viewmodels/AssetListViewModel.cpp
+++ b/src/ui/viewmodels/AssetListViewModel.cpp
@@ -1617,6 +1617,7 @@ void AssetListViewModel::CreateNew()
     }
 
     FilteredAssets().BeginUpdate();
+    bool bUpdateAsset = true;
 
     auto& pGameContext = ra::services::ServiceLocator::GetMutable<ra::data::context::GameContext>();
     ra::data::models::AssetModelBase* pNewAsset = nullptr;
@@ -1632,17 +1633,36 @@ void AssetListViewModel::CreateNew()
             pNewAsset = &pGameContext.Assets().NewLeaderboard();
             break;
 
+        case ra::data::models::AssetType::RichPresence:
+            pNewAsset = pGameContext.Assets().FindRichPresence();
+            if (pNewAsset != nullptr)
+            {
+                bUpdateAsset = false;
+            }
+            else
+            {
+                RA_LOG_INFO("Creating rich presence");
+                auto pRichPresence = std::make_unique<ra::data::models::RichPresenceModel>();
+                pRichPresence->CreateServerCheckpoint();
+                pRichPresence->CreateLocalCheckpoint();
+                pNewAsset = &pGameContext.Assets().Append(std::move(pRichPresence));
+            }
+            break;
+
         default:
             break;
     }
 
     Expects(pNewAsset != nullptr);
 
-    const auto& pUserContext = ra::services::ServiceLocator::GetMutable<ra::data::context::UserContext>();
-    pNewAsset->SetAuthor(ra::Widen(pUserContext.GetDisplayName()));
-    pNewAsset->SetCategory(ra::data::models::AssetCategory::Local);
-    pNewAsset->UpdateServerCheckpoint();
-    pNewAsset->SetNew();
+    if (bUpdateAsset)
+    {
+        const auto& pUserContext = ra::services::ServiceLocator::GetMutable<ra::data::context::UserContext>();
+        pNewAsset->SetAuthor(ra::Widen(pUserContext.GetDisplayName()));
+        pNewAsset->SetCategory(ra::data::models::AssetCategory::Local);
+        pNewAsset->UpdateServerCheckpoint();
+        pNewAsset->SetNew();
+    }
 
     EnsureAppearsInFilteredList(*pNewAsset);
 
diff --git a/src/ui/viewmodels/RichPresenceMonitorViewModel.cpp b/src/ui/viewmodels/RichPresenceMonitorViewModel.cpp
index d836f49d..b1d0631d 100644
--- a/src/ui/viewmodels/RichPresenceMonitorViewModel.cpp
+++ b/src/ui/viewmodels/RichPresenceMonitorViewModel.cpp
@@ -44,8 +44,18 @@ void RichPresenceMonitorViewModel::StartMonitoring()
         case MonitorState::None:
             // not monitoring - start doing so.
             m_nState = MonitorState::Active;
-            UpdateDisplayString();
             m_tRichPresenceFileTime = GetRichPresenceModified();
+            if (m_tRichPresenceFileTime)
+            {
+                auto& pGameContext = ra::services::ServiceLocator::GetMutable<ra::data::context::GameContext>();
+                auto* pRichPresence = pGameContext.Assets().FindRichPresence();
+                if (pRichPresence != nullptr)
+                {
+                    pRichPresence->ReloadRichPresenceScript();
+                    pRichPresence->Activate();
+                }
+            }
+            UpdateDisplayString();
             ScheduleUpdateDisplayString();
             break;
 


Test patch:
diff --git a/tests/data/context/GameContext_Tests.cpp b/tests/data/context/GameContext_Tests.cpp
index d9eafc51..90ee9a02 100644
--- a/tests/data/context/GameContext_Tests.cpp
+++ b/tests/data/context/GameContext_Tests.cpp
@@ -438,6 +438,56 @@ public:
         Assert::AreEqual(ra::data::models::AssetChanges::Unpublished, pRichPresence->GetChanges());
     }
 
+    TEST_METHOD(TestLoadGameRichPresenceUpdatedFromServer)
+    {
+        GameContextHarness game;
+        game.mockStorage.MockStoredData(ra::services::StorageItemType::GameData, L"1", "{\"RichPresencePatch\": \"Display:\\nOld\\n\"}");
+        game.mockStorage.MockStoredData(ra::services::StorageItemType::RichPresence, L"1", "Display:\nOld\n");
+        game.mockServer.HandleRequest<ra::api::FetchGameData>([](const ra::api::FetchGameData::Request&, ra::api::FetchGameData::Response& response)
+        {
+            response.RichPresence = "Display:\nHello, World";
+            return true;
+        });
+
+        game.LoadGame(1U);
+
+        Assert::IsTrue(game.HasRichPresence());
+        Assert::AreEqual(std::string("Display:\nHello, World\n"),
+            game.mockStorage.GetStoredData(ra::services::StorageItemType::RichPresence, L"1"));
+        Assert::IsFalse(game.IsRichPresenceFromFile());
+
+        const auto* pRichPresence = game.Assets().FindRichPresence();
+        Assert::IsNotNull(pRichPresence);
+        Ensures(pRichPresence != nullptr);
+        Assert::AreEqual(ra::data::models::AssetCategory::Core, pRichPresence->GetCategory());
+        Assert::AreEqual(ra::data::models::AssetChanges::None, pRichPresence->GetChanges());
+    }
+
+    TEST_METHOD(TestLoadGameRichPresenceUpdatedFromServerLocalModifications)
+    {
+        GameContextHarness game;
+        game.mockStorage.MockStoredData(ra::services::StorageItemType::GameData, L"1", "{\"RichPresencePatch\": \"Display:\\nOld\\n\"}");
+        game.mockStorage.MockStoredData(ra::services::StorageItemType::RichPresence, L"1", "Display:\nLocal\n");
+        game.mockServer.HandleRequest<ra::api::FetchGameData>([](const ra::api::FetchGameData::Request&, ra::api::FetchGameData::Response& response)
+        {
+            response.RichPresence = "Display:\nHello, World";
+            return true;
+        });
+
+        game.LoadGame(1U);
+
+        Assert::IsTrue(game.HasRichPresence());
+        Assert::AreEqual(std::string("Display:\nLocal\n"),
+            game.mockStorage.GetStoredData(ra::services::StorageItemType::RichPresence, L"1"));
+        Assert::IsTrue(game.IsRichPresenceFromFile());
+
+        const auto* pRichPresence = game.Assets().FindRichPresence();
+        Assert::IsNotNull(pRichPresence);
+        Ensures(pRichPresence != nullptr);
+        Assert::AreEqual(ra::data::models::AssetCategory::Core, pRichPresence->GetCategory());
+        Assert::AreEqual(ra::data::models::AssetChanges::Unpublished, pRichPresence->GetChanges());
+    }
+
     TEST_METHOD(TestLoadGameNoRichPresence)
     {
         GameContextHarness game;
diff --git a/tests/ui/viewmodels/AssetListViewModel_Tests.cpp b/tests/ui/viewmodels/AssetListViewModel_Tests.cpp
index 8df44448..e6c55749 100644
--- a/tests/ui/viewmodels/AssetListViewModel_Tests.cpp
+++ b/tests/ui/viewmodels/AssetListViewModel_Tests.cpp
@@ -3290,6 +3290,89 @@ public:
         Assert::IsFalse(bEditorShown);
     }
 
+    TEST_METHOD(TestCreateNewRichPresenceFilterTypeRichPresence)
+    {
+        AssetListViewModelHarness vmAssetList;
+        ra::services::mocks::MockFileSystem mockFileSystem;
+        ra::services::impl::FileLocalStorage storage(mockFileSystem);
+        ra::services::ServiceLocator::ServiceOverride<ra::services::ILocalStorage> storageOverride(&storage);
+
+        vmAssetList.mockUserContext.Initialize("User1", "FOO");
+        vmAssetList.MockGameId(22U);
+        vmAssetList.AddAchievement(AssetCategory::Core, 5, L"Test1", L"Desc1", L"12345", "0xH1234=1");
+        vmAssetList.AddLeaderboard(ra::data::models::AssetCategory::Core, L"Leaderboard1");
+        vmAssetList.SetAssetTypeFilter(ra::data::models::AssetType::RichPresence);
+        vmAssetList.ForceUpdateButtons();
+
+        Assert::AreEqual({ 2U }, vmAssetList.mockGameContext.Assets().Count());
+        Assert::AreEqual({ 0U }, vmAssetList.FilteredAssets().Count());
+
+        Assert::IsTrue(vmAssetList.CanCreate());
+        vmAssetList.CreateNew();
+
+        // new Local rich presence should be created and focused
+        Assert::AreEqual({ 3U }, vmAssetList.mockGameContext.Assets().Count());
+        Assert::AreEqual({ 1U }, vmAssetList.FilteredAssets().Count());
+        Assert::AreEqual(AssetListViewModel::FilterCategory::Local, vmAssetList.GetFilterCategory());
+        Assert::AreEqual(ra::data::models::AssetType::RichPresence, vmAssetList.GetAssetTypeFilter());
+
+        const auto* pAsset = vmAssetList.FilteredAssets().GetItemAt(0);
+        Expects(pAsset != nullptr);
+        Assert::IsTrue(pAsset->IsSelected());
+        Assert::AreEqual(std::wstring(L"Rich Presence"), pAsset->GetLabel());
+        Assert::AreEqual(AssetCategory::Local, pAsset->GetCategory());
+        Assert::AreEqual(AssetState::Inactive, pAsset->GetState());
+        Assert::AreEqual(AssetChanges::New, pAsset->GetChanges());
+        Assert::AreEqual({ 0 }, pAsset->GetId());
+        Assert::AreEqual(0, pAsset->GetPoints());
+
+        // and loaded externally
+        Assert::IsFalse(vmAssetList.mockWindowManager.AssetEditor.IsVisible());
+        Assert::AreEqual(std::string("file://localhost/./RACache/Data/22-Rich.txt"), vmAssetList.mockDesktop.LastOpenedUrl());
+    }
+
+    TEST_METHOD(TestCreateNewRichPresenceFilterTypeRichPresenceExisting)
+    {
+        AssetListViewModelHarness vmAssetList;
+        ra::services::mocks::MockFileSystem mockFileSystem;
+        ra::services::impl::FileLocalStorage storage(mockFileSystem);
+        ra::services::ServiceLocator::ServiceOverride<ra::services::ILocalStorage> storageOverride(&storage);
+
+        vmAssetList.mockUserContext.Initialize("User1", "FOO");
+        vmAssetList.MockGameId(22U);
+        vmAssetList.AddAchievement(AssetCategory::Core, 5, L"Test1", L"Desc1", L"12345", "0xH1234=1");
+        vmAssetList.AddLeaderboard(ra::data::models::AssetCategory::Core, L"Leaderboard1");
+        vmAssetList.AddRichPresence("Display\nTest\n");
+        vmAssetList.SetAssetTypeFilter(ra::data::models::AssetType::RichPresence);
+        vmAssetList.ForceUpdateButtons();
+
+        Assert::AreEqual({ 3U }, vmAssetList.mockGameContext.Assets().Count());
+        Assert::AreEqual({ 1U }, vmAssetList.FilteredAssets().Count());
+
+        Assert::IsTrue(vmAssetList.CanCreate());
+        vmAssetList.CreateNew();
+
+        // existing Local rich presence should be focused
+        Assert::AreEqual({ 3U }, vmAssetList.mockGameContext.Assets().Count());
+        Assert::AreEqual({ 1U }, vmAssetList.FilteredAssets().Count());
+        Assert::AreEqual(AssetListViewModel::FilterCategory::Core, vmAssetList.GetFilterCategory());
+        Assert::AreEqual(ra::data::models::AssetType::RichPresence, vmAssetList.GetAssetTypeFilter());
+
+        const auto* pAsset = vmAssetList.FilteredAssets().GetItemAt(0);
+        Expects(pAsset != nullptr);
+        Assert::IsTrue(pAsset->IsSelected());
+        Assert::AreEqual(std::wstring(L"Rich Presence"), pAsset->GetLabel());
+        Assert::AreEqual(AssetCategory::Core, pAsset->GetCategory());
+        Assert::AreEqual(AssetState::Inactive, pAsset->GetState());
+        Assert::AreEqual(AssetChanges::None, pAsset->GetChanges());
+        Assert::AreEqual({ 0 }, pAsset->GetId());
+        Assert::AreEqual(0, pAsset->GetPoints());
+
+        // and loaded externally
+        Assert::IsFalse(vmAssetList.mockWindowManager.AssetEditor.IsVisible());
+        Assert::AreEqual(std::string("file://localhost/./RACache/Data/22-Rich.txt"), vmAssetList.mockDesktop.LastOpenedUrl());
+    }
+
     TEST_METHOD(TestCloneSingle)
     {
         AssetListViewModelHarness vmAssetList;