Repository: RetroAchievements/RAIntegration
Pull request #884 (2022-04-25 20:11:43)

limit 'does not trigger' submission to 5 achievements at a time
Makes it harder for a user to ticket an entire set because they have the wrong ROM, an unsupported core, or an old version of RetroArch.

Does not affect 'triggered at the wrong time' submissions as it's entirely possible that an entire set needs save protection.


Patch:
diff --git a/RAInterface b/RAInterface
index 7b99be66..a154fa35 160000
--- a/RAInterface
+++ b/RAInterface
@@ -1,1 +1,1 @@
-Subproject commit 7b99be66ed2b555d23c23609ec526528ce349b39
+Subproject commit a154fa35f46abfa420ddd9924fcf028159f1f7c5
diff --git a/src/Exports.cpp b/src/Exports.cpp
index c3d3c2bf..4a5afa99 100644
--- a/src/Exports.cpp
+++ b/src/Exports.cpp
@@ -202,6 +202,37 @@ API HMENU CCONV _RA_CreatePopupMenu()
     return hMenu;
 }
 
+API int CCONV _RA_GetPopupMenuItems(RA_MenuItem *pItems)
+{
+    // have to keep a static variable here to hold label references
+    static ra::ui::viewmodels::LookupItemViewModelCollection vmMenuItems;
+
+    vmMenuItems.Clear();
+    ra::ui::viewmodels::IntegrationMenuViewModel::BuildMenu(vmMenuItems);
+
+    for (gsl::index i = 0; i < gsl::narrow_cast<gsl::index>(vmMenuItems.Count()); ++i)
+    {
+        const auto* pItem = vmMenuItems.GetItemAt(i);
+        Expects(pItem != nullptr);
+
+        const int nId = pItem->GetId();
+        pItems[i].nID = nId;
+
+        if (nId == 0)
+        {
+            pItems[i].sLabel = nullptr;
+            pItems[i].bChecked = 0;
+        }
+        else
+        {
+            pItems[i].sLabel = pItem->GetLabel().c_str();
+            pItems[i].bChecked = pItem->IsSelected() ? 1 : 0;
+        }
+    }
+
+    return gsl::narrow_cast<int>(vmMenuItems.Count());
+}
+
 API void CCONV _RA_InvokeDialog(LPARAM nID)
 {
     ra::ui::viewmodels::IntegrationMenuViewModel::ActivateMenuItem(gsl::narrow_cast<int>(nID));
diff --git a/src/Exports.hh b/src/Exports.hh
index 1c03caad..ad687e19 100644
--- a/src/Exports.hh
+++ b/src/Exports.hh
@@ -45,6 +45,10 @@ extern "C" {
     // Allocates and configures a popup menu, to be called, embedded and managed by the app.
     API HMENU CCONV _RA_CreatePopupMenu();
 
+    // Gets items for building a popup menu to be embedded and managed by the app.
+    struct RA_MenuItem;
+    API int CCONV _RA_GetPopupMenuItems(RA_MenuItem* pItems);
+
     // Check all achievement sets for changes, and displays a dlg box to warn lost changes.
     API int CCONV _RA_ConfirmLoadNewRom(int bQuittingApp);
 
diff --git a/src/data/context/EmulatorContext.cpp b/src/data/context/EmulatorContext.cpp
index e525f1f9..e4a84515 100644
--- a/src/data/context/EmulatorContext.cpp
+++ b/src/data/context/EmulatorContext.cpp
@@ -142,7 +142,16 @@ void EmulatorContext::UpdateUserAgent()
     {
         sUserAgent.append("UnknownClient/");
     }
-    sUserAgent.append(m_sVersion);
+
+    size_t nOffset = 0;
+    if (!m_sVersion.empty() && (m_sVersion.at(0) == 'v' || m_sVersion.at(0) == 'V'))
+    {
+        /* version string starts with a "v". assume it's part of a prefix (i.e. "v1.2", or "version 1.2") and ignore it */
+        while (nOffset < m_sVersion.length() && (isalpha(m_sVersion.at(nOffset)) || isspace(m_sVersion.at(nOffset))))
+            ++nOffset;
+    }
+
+    sUserAgent.append(m_sVersion, nOffset);
     sUserAgent.append(" (");
 
     const auto& pDesktop = ra::services::ServiceLocator::Get<ra::ui::IDesktop>();
@@ -163,6 +172,11 @@ void EmulatorContext::UpdateUserAgent()
 
 static unsigned long long ParseVersion(const char* sVersion)
 {
+    Expects(sVersion != nullptr);
+
+    while (isalpha(*sVersion) || isspace(*sVersion))
+        ++sVersion;
+
     char* pPart{};
     const auto major = strtoull(sVersion, &pPart, 10);
     Expects(pPart != nullptr);
@@ -295,8 +309,10 @@ bool EmulatorContext::ValidateClientVersion(bool& bHardcore)
     if (nLocalVersion >= nServerVersion)
         return true;
 
-    // remove any trailing ".0"s off the client version
+    // remove leading prefix and any trailing ".0"s off the client version
     std::string sClientVersion = m_sVersion;
+    while (!sClientVersion.empty() && (isalpha(sClientVersion.at(0)) || isspace(sClientVersion.at(0))))
+        sClientVersion.erase(0, 1);
     while (ra::StringEndsWith(sClientVersion, ".0"))
         sClientVersion.resize(sClientVersion.length() - 2);
     if (sClientVersion.find('.') == std::string::npos)
diff --git a/src/ui/drawing/gdi/ImageRepository.cpp b/src/ui/drawing/gdi/ImageRepository.cpp
index f6c108b3..2b235eb7 100644
--- a/src/ui/drawing/gdi/ImageRepository.cpp
+++ b/src/ui/drawing/gdi/ImageRepository.cpp
@@ -234,8 +234,11 @@ std::string ImageRepository::StoreImage(ImageType nType, const std::wstring& sPa
     if (!pFileSystem.DirectoryExists(sDirectory))
         pFileSystem.CreateDirectory(sDirectory);
 
+    auto sExtension = pFileSystem.GetExtension(sPath);
+    ra::StringMakeLowercase(sExtension);
+
     const auto& pGameContext = ra::services::ServiceLocator::Get<ra::data::context::GameContext>();
-    sFileMD5 = ra::StringPrintf("local\\%u-%s", pGameContext.GameId(), sFileMD5);
+    sFileMD5 = ra::StringPrintf("local\\%u-%s.%s", pGameContext.GameId(), sFileMD5, sExtension);
 
     std::wstring sFilename = GetFilename(nType, sFileMD5);
 
diff --git a/src/ui/viewmodels/BrokenAchievementsViewModel.cpp b/src/ui/viewmodels/BrokenAchievementsViewModel.cpp
index 990532ee..8e03cdee 100644
--- a/src/ui/viewmodels/BrokenAchievementsViewModel.cpp
+++ b/src/ui/viewmodels/BrokenAchievementsViewModel.cpp
@@ -161,6 +161,13 @@ bool BrokenAchievementsViewModel::Submit()
     switch (request.Problem)
     {
         case ra::api::SubmitTicket::ProblemType::DidNotTrigger:
+            if (request.AchievementIds.size() > 5)
+            {
+                ra::ui::viewmodels::MessageBoxViewModel::ShowErrorMessage(L"Too many achievements selected.",
+                    L"You cannot report more than 5 achievements at a time for not triggering. Please provide separate details for each achievement that did not trigger for you.");
+                return false;
+            }
+
             if (nUnachievedSelected == 0)
             {
                 if (ra::ui::viewmodels::MessageBoxViewModel::ShowWarningMessage(L"Submit anyway?",


Test patch:
diff --git a/tests/Exports_Tests.cpp b/tests/Exports_Tests.cpp
index aa8ac312..440ec142 100644
--- a/tests/Exports_Tests.cpp
+++ b/tests/Exports_Tests.cpp
@@ -3,6 +3,7 @@
 #include "Exports.hh"
 
 #include "RA_BuildVer.h"
+#include "RA_Resource.h"
 
 #include "tests\mocks\MockAudioSystem.hh"
 #include "tests\mocks\MockClock.hh"
@@ -949,6 +950,90 @@ public:
         Ensures(pScore4 != nullptr);
         Assert::AreEqual(std::wstring(L"5678"), pScore4->GetDisplayText());
     }
+
+private:
+    void AssertMenuItem(const RA_MenuItem* pItem, LPARAM nId, const wchar_t* sLabel, bool bChecked = false)
+    {
+        Assert::AreEqual(nId, pItem->nID);
+
+        if (sLabel == nullptr)
+            Assert::IsNull(pItem->sLabel);
+        else
+            Assert::AreEqual(std::wstring(sLabel), std::wstring(pItem->sLabel));
+
+        Assert::AreEqual(bChecked ? 1 : 0, pItem->bChecked);
+    }
+
+    TEST_METHOD(TestGetPopupMenuItemsNotLoggedIn)
+    {
+        ra::services::mocks::MockConfiguration mockConfiguration;
+        ra::data::context::mocks::MockUserContext mockUserContext;
+
+        RA_MenuItem menu[32];
+        Assert::AreEqual(1, _RA_GetPopupMenuItems(menu));
+        AssertMenuItem(&menu[0], IDM_RA_FILES_LOGIN, L"&Login");
+    }
+
+    TEST_METHOD(TestGetPopupMenuItemsLoggedIn)
+    {
+        ra::services::mocks::MockConfiguration mockConfiguration;
+        ra::data::context::mocks::MockUserContext mockUserContext;
+        mockUserContext.Initialize("User", "TOKEN");
+
+        RA_MenuItem menu[32];
+        Assert::AreEqual(19, _RA_GetPopupMenuItems(menu));
+        AssertMenuItem(&menu[0], IDM_RA_FILES_LOGOUT, L"Log&out");
+        AssertMenuItem(&menu[1], 0, nullptr);
+        AssertMenuItem(&menu[2], IDM_RA_OPENUSERPAGE, L"Open my &User Page");
+        AssertMenuItem(&menu[3], IDM_RA_OPENGAMEPAGE, L"Open this &Game's Page");
+        AssertMenuItem(&menu[4], 0, nullptr);
+        AssertMenuItem(&menu[5], IDM_RA_HARDCORE_MODE, L"&Hardcore Mode");
+        AssertMenuItem(&menu[6], IDM_RA_NON_HARDCORE_WARNING, L"Non-Hardcore &Warning");
+        AssertMenuItem(&menu[7], 0, nullptr);
+        AssertMenuItem(&menu[8], IDM_RA_TOGGLELEADERBOARDS, L"Enable &Leaderboards");
+        AssertMenuItem(&menu[9], IDM_RA_OVERLAYSETTINGS, L"O&verlay Settings");
+        AssertMenuItem(&menu[10], 0, nullptr);
+        AssertMenuItem(&menu[11], IDM_RA_FILES_ACHIEVEMENTS, L"Assets Li&st");
+        AssertMenuItem(&menu[12], IDM_RA_FILES_ACHIEVEMENTEDITOR, L"Assets &Editor");
+        AssertMenuItem(&menu[13], IDM_RA_FILES_MEMORYFINDER, L"&Memory Inspector");
+        AssertMenuItem(&menu[14], IDM_RA_FILES_MEMORYBOOKMARKS, L"Memory &Bookmarks");
+        AssertMenuItem(&menu[15], IDM_RA_PARSERICHPRESENCE, L"Rich &Presence Monitor");
+        AssertMenuItem(&menu[16], 0, nullptr);
+        AssertMenuItem(&menu[17], IDM_RA_REPORTBROKENACHIEVEMENTS, L"&Report Achievement Problem");
+        AssertMenuItem(&menu[18], IDM_RA_GETROMCHECKSUM, L"View Game H&ash");
+    }
+
+    TEST_METHOD(TestGetPopupMenuItemsChecked)
+    {
+        ra::services::mocks::MockConfiguration mockConfiguration;
+        ra::data::context::mocks::MockUserContext mockUserContext;
+        mockUserContext.Initialize("User", "TOKEN");
+        mockConfiguration.SetFeatureEnabled(ra::services::Feature::Hardcore, true);
+        mockConfiguration.SetFeatureEnabled(ra::services::Feature::NonHardcoreWarning, true);
+        mockConfiguration.SetFeatureEnabled(ra::services::Feature::Leaderboards, true);
+
+        RA_MenuItem menu[32];
+        Assert::AreEqual(19, _RA_GetPopupMenuItems(menu));
+        AssertMenuItem(&menu[0], IDM_RA_FILES_LOGOUT, L"Log&out");
+        AssertMenuItem(&menu[1], 0, nullptr);
+        AssertMenuItem(&menu[2], IDM_RA_OPENUSERPAGE, L"Open my &User Page");
+        AssertMenuItem(&menu[3], IDM_RA_OPENGAMEPAGE, L"Open this &Game's Page");
+        AssertMenuItem(&menu[4], 0, nullptr);
+        AssertMenuItem(&menu[5], IDM_RA_HARDCORE_MODE, L"&Hardcore Mode", true);
+        AssertMenuItem(&menu[6], IDM_RA_NON_HARDCORE_WARNING, L"Non-Hardcore &Warning", true);
+        AssertMenuItem(&menu[7], 0, nullptr);
+        AssertMenuItem(&menu[8], IDM_RA_TOGGLELEADERBOARDS, L"Enable &Leaderboards", true);
+        AssertMenuItem(&menu[9], IDM_RA_OVERLAYSETTINGS, L"O&verlay Settings");
+        AssertMenuItem(&menu[10], 0, nullptr);
+        AssertMenuItem(&menu[11], IDM_RA_FILES_ACHIEVEMENTS, L"Assets Li&st");
+        AssertMenuItem(&menu[12], IDM_RA_FILES_ACHIEVEMENTEDITOR, L"Assets &Editor");
+        AssertMenuItem(&menu[13], IDM_RA_FILES_MEMORYFINDER, L"&Memory Inspector");
+        AssertMenuItem(&menu[14], IDM_RA_FILES_MEMORYBOOKMARKS, L"Memory &Bookmarks");
+        AssertMenuItem(&menu[15], IDM_RA_PARSERICHPRESENCE, L"Rich &Presence Monitor");
+        AssertMenuItem(&menu[16], 0, nullptr);
+        AssertMenuItem(&menu[17], IDM_RA_REPORTBROKENACHIEVEMENTS, L"&Report Achievement Problem");
+        AssertMenuItem(&menu[18], IDM_RA_GETROMCHECKSUM, L"View Game H&ash");
+    }
 };
 
 } // namespace tests
diff --git a/tests/RA_Interface_Tests.cpp b/tests/RA_Interface_Tests.cpp
index f0b092cb..3e9cbc9d 100644
--- a/tests/RA_Interface_Tests.cpp
+++ b/tests/RA_Interface_Tests.cpp
@@ -76,6 +76,7 @@ public:
         Assert::IsNotNull((const void*)_RA_InstallSharedFunctions);
         Assert::IsNotNull((const void*)_RA_SetForceRepaint);
         Assert::IsNotNull((const void*)_RA_CreatePopupMenu);
+        Assert::IsNotNull((const void*)_RA_GetPopupMenuItems);
         Assert::IsNotNull((const void*)_RA_InvokeDialog);
         Assert::IsNotNull((const void*)_RA_SetUserAgentDetail);
         Assert::IsNotNull((const void*)_RA_AttemptLogin);
@@ -118,6 +119,7 @@ public:
         Assert::IsNull((const void*)_RA_InstallSharedFunctions);
         Assert::IsNull((const void*)_RA_SetForceRepaint);
         Assert::IsNull((const void*)_RA_CreatePopupMenu);
+        Assert::IsNull((const void*)_RA_GetPopupMenuItems);
         Assert::IsNull((const void*)_RA_InvokeDialog);
         Assert::IsNull((const void*)_RA_SetUserAgentDetail);
         Assert::IsNull((const void*)_RA_AttemptLogin);
diff --git a/tests/data/context/EmulatorContext_Tests.cpp b/tests/data/context/EmulatorContext_Tests.cpp
index 6a903f7e..a7744df9 100644
--- a/tests/data/context/EmulatorContext_Tests.cpp
+++ b/tests/data/context/EmulatorContext_Tests.cpp
@@ -513,6 +513,24 @@ public:
         Assert::IsTrue(emulator.mockDesktop.WasDialogShown());
     }
 
+    TEST_METHOD(TestValidateClientVersionWithPrefix)
+    {
+        EmulatorContextHarness emulator;
+        emulator.Initialize(EmulatorID::RA_Snes9x, nullptr);
+        emulator.mockConfiguration.SetHostName("host");
+        emulator.mockConfiguration.SetFeatureEnabled(ra::services::Feature::Hardcore, true);
+        emulator.MockVersions("v1.0", "1.0.1.0");
+        emulator.mockDesktop.ExpectWindow<ra::ui::viewmodels::MessageBoxViewModel>([](ra::ui::viewmodels::MessageBoxViewModel& vmMessageBox)
+        {
+            Assert::AreEqual(std::wstring(L"A newer client is required for hardcore mode."), vmMessageBox.GetHeader());
+            Assert::AreEqual(std::wstring(L"A new version of RASnes9X is available to download at host.\n\n- Current version: 1.0\n- New version: 1.0.1\n\nPress OK to logout and download the new version, or Cancel to disable hardcore mode and proceed."), vmMessageBox.GetMessage());
+            return ra::ui::DialogResult::Cancel;
+        });
+
+        Assert::IsTrue(emulator.ValidateClientVersion());
+        Assert::IsTrue(emulator.mockDesktop.WasDialogShown());
+    }
+
     static void ReplaceIntegrationVersion(std::string& sVersion)
     {
         const auto nIndex = sVersion.find("Integration/") + 12;
@@ -540,8 +558,15 @@ public:
         ReplaceIntegrationVersion(sVersion);
         Assert::AreEqual(std::string("RAppleWin/1.1.1 (UnitTests) Integration/VERSION"), sVersion);
 
+        emulator.Initialize(EmulatorID::RA_Libretro, nullptr);
+        emulator.SetClientVersion("v1.3.8");
+
+        sVersion = emulator.mockHttpRequester.GetUserAgent();
+        ReplaceIntegrationVersion(sVersion);
+        Assert::AreEqual(std::string("RALibRetro/1.3.8 (UnitTests) Integration/VERSION"), sVersion);
+
         emulator.Initialize(EmulatorID::UnknownEmulator, "CustomName");
-        emulator.SetClientVersion("3.1a");
+        emulator.SetClientVersion("ver 3.1a");
 
         sVersion = emulator.mockHttpRequester.GetUserAgent();
         ReplaceIntegrationVersion(sVersion);
@@ -981,6 +1006,9 @@ public:
 
         emulator.SetClientVersion("1.0.12.0");
         Assert::AreEqual(std::wstring(L"RASnes9X - 1.0"), emulator.GetAppTitle(""));
+
+        emulator.SetClientVersion("v2.5.6");
+        Assert::AreEqual(std::wstring(L"RASnes9X - v2.5"), emulator.GetAppTitle(""));
     }
 
     TEST_METHOD(TestGetAppTitleUserName)
diff --git a/tests/ui/viewmodels/BrokenAchievementsViewModel_Tests.cpp b/tests/ui/viewmodels/BrokenAchievementsViewModel_Tests.cpp
index fec400be..70889051 100644
--- a/tests/ui/viewmodels/BrokenAchievementsViewModel_Tests.cpp
+++ b/tests/ui/viewmodels/BrokenAchievementsViewModel_Tests.cpp
@@ -72,6 +72,11 @@ private:
             ach5.SetID(5U);
             ach5.SetName(L"Title5");
             ach5.SetState(ra::data::models::AssetState::Active);
+            auto& ach6 = mockGameContext.Assets().NewAchievement();
+            ach6.SetCategory(ra::data::models::AssetCategory::Core);
+            ach6.SetID(6U);
+            ach6.SetName(L"Title6");
+            ach6.SetState(ra::data::models::AssetState::Active);
 
             Assert::IsTrue(InitializeAchievements());
         }
@@ -460,6 +465,74 @@ public:
         Assert::IsTrue(bDialogSeen);
     }
 
+    TEST_METHOD(TestSubmitMultipleAchievementsDidNotTriggerLimit)
+    {
+        BrokenAchievementsViewModelHarness vmBrokenAchievements;
+        vmBrokenAchievements.MockAchievements();
+        vmBrokenAchievements.SetSelectedProblemId(2);
+        vmBrokenAchievements.Achievements().GetItemAt(0)->SetSelected(true);
+        vmBrokenAchievements.Achievements().GetItemAt(1)->SetSelected(true);
+        vmBrokenAchievements.Achievements().GetItemAt(2)->SetSelected(true);
+        vmBrokenAchievements.Achievements().GetItemAt(3)->SetSelected(true);
+        vmBrokenAchievements.Achievements().GetItemAt(4)->SetSelected(true);
+        vmBrokenAchievements.Achievements().GetItemAt(5)->SetSelected(true);
+        vmBrokenAchievements.SetDefaultComment();
+
+        bool bDialogSeen = false;
+        vmBrokenAchievements.mockDesktop.ExpectWindow<MessageBoxViewModel>(
+            [&bDialogSeen](const MessageBoxViewModel& vmMessageBox) {
+                bDialogSeen = true;
+                Assert::AreEqual(std::wstring(L"Too many achievements selected."), vmMessageBox.GetHeader());
+                Assert::AreEqual(std::wstring(L"You cannot report more than 5 achievements at a time for not triggering. Please provide separate details for each achievement that did not trigger for you."), vmMessageBox.GetMessage());
+                return ra::ui::DialogResult::OK;
+            });
+
+        vmBrokenAchievements.mockServer.ExpectUncalled<ra::api::SubmitTicket>();
+
+        Assert::IsFalse(vmBrokenAchievements.Submit());
+        Assert::IsTrue(bDialogSeen);
+    }
+
+    TEST_METHOD(TestSubmitMultipleAchievementsWrongTimeNoLimit)
+    {
+        BrokenAchievementsViewModelHarness vmBrokenAchievements;
+        vmBrokenAchievements.MockAchievements();
+        vmBrokenAchievements.SetSelectedProblemId(1);
+        vmBrokenAchievements.Achievements().GetItemAt(0)->SetSelected(true);
+        vmBrokenAchievements.Achievements().GetItemAt(1)->SetSelected(true);
+        vmBrokenAchievements.Achievements().GetItemAt(2)->SetSelected(true);
+        vmBrokenAchievements.Achievements().GetItemAt(3)->SetSelected(true);
+        vmBrokenAchievements.Achievements().GetItemAt(4)->SetSelected(true);
+        vmBrokenAchievements.Achievements().GetItemAt(5)->SetSelected(true);
+        vmBrokenAchievements.SetDefaultComment();
+
+        bool bDialogSeen = false;
+        vmBrokenAchievements.mockDesktop.ExpectWindow<MessageBoxViewModel>([&bDialogSeen](const MessageBoxViewModel& vmMessageBox)
+        {
+            if (vmMessageBox.GetButtons() == MessageBoxViewModel::Buttons::OK)
+            {
+                bDialogSeen = true;
+                Assert::AreEqual(std::wstring(L"6 tickets created"), vmMessageBox.GetHeader());
+                return ra::ui::DialogResult::OK;
+            }
+
+            return ra::ui::DialogResult::Yes;
+        });
+
+        vmBrokenAchievements.mockServer.HandleRequest<ra::api::SubmitTicket>([](const ra::api::SubmitTicket::Request& request, ra::api::SubmitTicket::Response& response)
+        {
+            Assert::AreEqual(1, ra::etoi(request.Problem));
+            Assert::AreEqual({ 6U }, request.AchievementIds.size());
+
+            response.Result = ra::api::ApiResult::Success;
+            response.TicketsCreated = 6;
+            return true;
+        });
+
+        Assert::IsTrue(vmBrokenAchievements.Submit());
+        Assert::IsTrue(bDialogSeen);
+    }
+
     TEST_METHOD(TestSubmitOneAchievementFailed)
     {
         BrokenAchievementsViewModelHarness vmBrokenAchievements;