Repository: RetroAchievements/RAIntegration
Pull request #1103 (2024-06-06 23:21:07)

Handle Indirect Address Navigation for Right-Click and Tooltip with Remember and Recall
Updates logic to handle remember and recall potentially being involved in resolving the indirect address for purposes of right-click navigation and displaying the resolved address in the tooltip. Processes the pause logic in a first pass if there is pause logic, so that remembered values that may be in later pause logic are attributed properly should they be later used in non-pause logic.

Design decision is to navigate as if no remember conditions were affect by a pause.

Some possible improvement could be made were the rcheevos logic to flag that remember/recall are used in the conditionset as then additional processing of the pause logic as well as add source, sub source, and remember could be skipped.


Patch:
diff --git a/src/ui/viewmodels/TriggerConditionViewModel.cpp b/src/ui/viewmodels/TriggerConditionViewModel.cpp
index 0c2d03c1..52a329ad 100644
--- a/src/ui/viewmodels/TriggerConditionViewModel.cpp
+++ b/src/ui/viewmodels/TriggerConditionViewModel.cpp
@@ -682,7 +682,8 @@ static bool IsIndirectMemref(const rc_operand_t& operand) noexcept
     return rc_operand_is_memref(&operand) && operand.value.memref->value.is_indirect;
 }
 
-ra::ByteAddress TriggerConditionViewModel::GetIndirectAddress(ra::ByteAddress nAddress, ra::ByteAddress* pPointerAddress) const
+ra::ByteAddress TriggerConditionViewModel::GetIndirectAddress(ra::ByteAddress nAddress,
+    ra::ByteAddress* pPointerAddress) const
 {
     if (pPointerAddress != nullptr)
         *pPointerAddress = UNKNOWN_ADDRESS;
@@ -696,9 +697,10 @@ ra::ByteAddress TriggerConditionViewModel::GetIndirectAddress(ra::ByteAddress nA
     if (pGroup == nullptr)
         return nAddress;
 
-    rc_condition_t* pCondition = nullptr;
+    rc_condition_t* pFirstCondition = nullptr;
 
     const auto* pTrigger = pTriggerViewModel->GetTriggerFromString();
+    bool bProcessPause = false;
     if (pTrigger != nullptr)
     {
         // if the trigger is managed by the viewmodel (not the runtime) then we need to update the memrefs
@@ -707,7 +709,8 @@ ra::ByteAddress TriggerConditionViewModel::GetIndirectAddress(ra::ByteAddress nA
         // find the condset associated to the selected group
         if (nIndex == 0)
         {
-            pCondition = pTrigger->requirement->conditions;
+            pFirstCondition = pTrigger->requirement->conditions;
+            bProcessPause = pTrigger->requirement->has_pause;
         }
         else
         {
@@ -718,40 +721,70 @@ ra::ByteAddress TriggerConditionViewModel::GetIndirectAddress(ra::ByteAddress nA
             if (!pAlt)
                 return nAddress;
 
-            pCondition = pAlt->conditions;
+            pFirstCondition = pAlt->conditions;
+            bProcessPause = pAlt->has_pause;
         }
     }
     else
     {
         Expects(pGroup->m_pConditionSet != nullptr);
-        pCondition = pGroup->m_pConditionSet->conditions;
+        pFirstCondition = pGroup->m_pConditionSet->conditions;
+        bProcessPause = pGroup->m_pConditionSet->has_pause;
     }
 
     bool bIsIndirect = false;
     bool bIsMultiLevelIndirect = false;
+    rc_typed_value_t value = {};
     rc_eval_state_t oEvalState;
     memset(&oEvalState, 0, sizeof(oEvalState));
-    rc_typed_value_t value = {};
     oEvalState.peek = rc_peek_callback;
+    oEvalState.recall_value.type = RC_VALUE_TYPE_UNSIGNED;
+    oEvalState.recall_value.value.u32 = 0;
+
+    int nPassesLeft = (bProcessPause ? 2 : 1); //If there are pauses, they may have remembered values that need to be processed first, so we do [up to] two passes.
+    while (nPassesLeft > 0) {
+        rc_condition_t* pCondition = pFirstCondition;
+        gsl::index nConditionIndex = 0;
+        for (; pCondition != nullptr; pCondition = pCondition->next)
+        {
+            auto* vmCondition = pTriggerViewModel->Conditions().GetItemAt(nConditionIndex++);
+            if (!vmCondition)
+                break;
 
-    gsl::index nConditionIndex = 0;
-    for (; pCondition != nullptr; pCondition = pCondition->next)
-    {
-        auto* vmCondition = pTriggerViewModel->Conditions().GetItemAt(nConditionIndex++);
-        if (!vmCondition)
-            break;
+            if ((pCondition->pause > 0) != bProcessPause) continue;
 
-        if (vmCondition == this)
-        {
-            if (bIsMultiLevelIndirect && pPointerAddress != nullptr)
-                *pPointerAddress = NESTED_POINTER_ADDRESS;
+            if (vmCondition == this)
+            {
+                // found the condition we're looking for
+                if (bIsMultiLevelIndirect && pPointerAddress != nullptr)
+                    *pPointerAddress = NESTED_POINTER_ADDRESS;
 
-            return nAddress + oEvalState.add_address;
-        }
+                return nAddress + oEvalState.add_address;
+            }
 
-        if (pCondition->type == RC_CONDITION_ADD_ADDRESS)
-        {
-            bIsMultiLevelIndirect = bIsIndirect;
+            switch (pCondition->type)
+            {
+            case RC_CONDITION_ADD_ADDRESS:
+                // AddAddress after an AddAddress is a multi-level indirect
+                bIsMultiLevelIndirect = bIsIndirect;
+                break;
+
+            case RC_CONDITION_ADD_SOURCE:
+            case RC_CONDITION_SUB_SOURCE:
+                // these can modify the Remembered value
+                break;
+
+            case RC_CONDITION_REMEMBER:
+                // Remember/Recall can be used in AddAddress chains
+                break;
+
+            default:
+                // other flags aren't part of the AddAddress chain
+                bIsIndirect = bIsMultiLevelIndirect = false;
+                oEvalState.add_address = 0;
+                oEvalState.add_value.type = RC_VALUE_TYPE_NONE;
+                continue;
+            }
 
             if (bIsIndirect && pTrigger == nullptr)
             {
@@ -774,26 +807,49 @@ ra::ByteAddress TriggerConditionViewModel::GetIndirectAddress(ra::ByteAddress nA
                 }
 
                 rc_evaluate_condition_value(&value, &oCondition, &oEvalState);
-                rc_typed_value_convert(&value, RC_VALUE_TYPE_UNSIGNED);
-                oEvalState.add_address = value.value.u32;
             }
             else
             {
                 rc_evaluate_condition_value(&value, pCondition, &oEvalState);
+
+                if (pCondition->type == RC_CONDITION_ADD_ADDRESS)
+                {
+                    // live AddAddress chain. capture the pointer value
+                    if (pPointerAddress != nullptr && rc_operand_is_memref(&pCondition->operand1))
+                        *pPointerAddress = (bIsIndirect) ? NESTED_POINTER_ADDRESS : pCondition->operand1.value.memref->address;
+                }
+            }
+
+            switch (pCondition->type)
+            {
+            case RC_CONDITION_ADD_ADDRESS:
                 rc_typed_value_convert(&value, RC_VALUE_TYPE_UNSIGNED);
                 oEvalState.add_address = value.value.u32;
+                bIsIndirect = true;
+                break;
 
-                if (pPointerAddress != nullptr && rc_operand_is_memref(&pCondition->operand1))
-                    *pPointerAddress = (bIsIndirect) ? NESTED_POINTER_ADDRESS : pCondition->operand1.value.memref->address;
+            case RC_CONDITION_SUB_SOURCE:
+                rc_typed_value_negate(&value);
+                _FALLTHROUGH; // to RC_CONDITION_ADD_SOURCE
 
-                bIsIndirect = true;
+            case RC_CONDITION_ADD_SOURCE:
+                rc_typed_value_add(&oEvalState.add_value, &value);
+                oEvalState.add_address = 0;
+                bIsIndirect = bIsMultiLevelIndirect = false;
+                break;
+
+            case RC_CONDITION_REMEMBER:
+                rc_typed_value_add(&value, &oEvalState.add_value);
+                oEvalState.recall_value.type = value.type;
+                oEvalState.recall_value.value = value.value;
+                oEvalState.add_value.type = RC_VALUE_TYPE_NONE;
+                oEvalState.add_address = 0;
+                bIsIndirect = bIsMultiLevelIndirect = false;
+                break;
             }
         }
-        else
-        {
-            bIsIndirect = bIsMultiLevelIndirect = false;
-            oEvalState.add_address = 0;
-        }
+        nPassesLeft--;
+        bProcessPause = false; //pause pass is always first, so we are no longer to process pause logic
     }
 
     return nAddress;


Test patch:
diff --git a/tests/ui/viewmodels/TriggerConditionViewModel_Tests.cpp b/tests/ui/viewmodels/TriggerConditionViewModel_Tests.cpp
index cee4568c..cfd2a49f 100644
--- a/tests/ui/viewmodels/TriggerConditionViewModel_Tests.cpp
+++ b/tests/ui/viewmodels/TriggerConditionViewModel_Tests.cpp
@@ -1066,6 +1066,179 @@ public:
         Assert::AreEqual(std::wstring(L"0x0008 (indirect)\r\n[No code note]"), pCondition3->GetTooltip(TriggerConditionViewModel::SourceValueProperty));
     }
 
+    TEST_METHOD(TestTooltipRecallBasic)
+    {
+        IndirectAddressTriggerViewModelHarness vmTrigger;
+        vmTrigger.mockGameContext.InitializeCodeNotes();
+        vmTrigger.Parse("K:0xH0001_I:{recall}_I:0xH0002_0xH0003=4");
+        vmTrigger.mockConfiguration.SetFeatureEnabled(ra::services::Feature::PreferDecimal, true);
+        vmTrigger.mockGameContext.SetCodeNote({ 1U }, L"[8-bit pointer]\n+2=First Level A\n  +3=Second Level.");
+
+        const auto* pCondition1 = vmTrigger.Conditions().GetItemAt(0);
+        Expects(pCondition1 != nullptr);
+        const auto* pCondition2 = vmTrigger.Conditions().GetItemAt(1);
+        Expects(pCondition2 != nullptr);
+        const auto* pCondition3 = vmTrigger.Conditions().GetItemAt(2);
+        Expects(pCondition3 != nullptr);
+        const auto* pCondition4 = vmTrigger.Conditions().GetItemAt(3);
+        Expects(pCondition4 != nullptr);
+
+        Assert::IsFalse(pCondition1->IsIndirect());
+        Assert::IsFalse(pCondition2->IsIndirect());
+        Assert::IsTrue(pCondition3->IsIndirect());
+        Assert::IsTrue(pCondition4->IsIndirect());
+
+        // $0001 = 1, 1+2 = $0003, $0003 = 3, 3+3 = $0006
+        Assert::AreEqual(std::wstring(L""), pCondition2->GetTooltip(TriggerConditionViewModel::SourceValueProperty));
+        Assert::AreEqual(std::wstring(L"0x0003 (indirect)\r\nFirst Level A\n  +3=Second Level."), pCondition3->GetTooltip(TriggerConditionViewModel::SourceValueProperty)); //Kind of a quirk because recall value is a code-noted address.
+        Assert::AreEqual(std::wstring(L"0x0006 (indirect)\r\n[Nested pointer code note not supported]"), pCondition4->GetTooltip(TriggerConditionViewModel::SourceValueProperty));
+
+        // $0001 = 3, 3+2 = $0005, $0005 = 5, 5+3 = $0008
+        vmTrigger.SetMemory({ 1 }, 3);
+        Assert::AreEqual(std::wstring(L"0x0005 (indirect)\r\n[No code note]"), pCondition3->GetTooltip(TriggerConditionViewModel::SourceValueProperty));
+        Assert::AreEqual(std::wstring(L"0x0008 (indirect)\r\n[Nested pointer code note not supported]"), pCondition4->GetTooltip(TriggerConditionViewModel::SourceValueProperty));
+    }
+
+    TEST_METHOD(TestTooltipRecallAddSource)
+    {
+        IndirectAddressTriggerViewModelHarness vmTrigger;
+        vmTrigger.mockGameContext.InitializeCodeNotes();
+        vmTrigger.Parse("A:1_K:0xH0001_I:{recall}_I:0xH0002_0xH0003=4");
+        vmTrigger.mockConfiguration.SetFeatureEnabled(ra::services::Feature::PreferDecimal, true);
+
+        const auto* pCondition1 = vmTrigger.Conditions().GetItemAt(0);
+        Expects(pCondition1 != nullptr);
+        const auto* pCondition2 = vmTrigger.Conditions().GetItemAt(1);
+        Expects(pCondition2 != nullptr);
+        const auto* pCondition3 = vmTrigger.Conditions().GetItemAt(2);
+        Expects(pCondition3 != nullptr);
+        const auto* pCondition4 = vmTrigger.Conditions().GetItemAt(3);
+        Expects(pCondition4 != nullptr);
+        const auto* pCondition5 = vmTrigger.Conditions().GetItemAt(4);
+        Expects(pCondition5 != nullptr);
+
+        Assert::IsFalse(pCondition1->IsIndirect());
+        Assert::IsFalse(pCondition2->IsIndirect());
+        Assert::IsFalse(pCondition3->IsIndirect());
+        Assert::IsTrue(pCondition4->IsIndirect());
+        Assert::IsTrue(pCondition5->IsIndirect());
+
+        // $0001 = 1, 1+1+2 = $0004, $0004 = 4, 4+3 = $0007
+        Assert::AreEqual(std::wstring(L"0x0004 (indirect)\r\n[No code note]"), pCondition4->GetTooltip(TriggerConditionViewModel::SourceValueProperty));
+        Assert::AreEqual(std::wstring(L"0x0007 (indirect)\r\n[Nested pointer code note not supported]"), pCondition5->GetTooltip(TriggerConditionViewModel::SourceValueProperty));
+
+        // $0001 = 3, 1+3+2 = $0006, $0006 = 6, 6+3 = $0009
+        vmTrigger.SetMemory({ 1 }, 3);
+        Assert::AreEqual(std::wstring(L"0x0006 (indirect)\r\n[No code note]"), pCondition4->GetTooltip(TriggerConditionViewModel::SourceValueProperty));
+        Assert::AreEqual(std::wstring(L"0x0009 (indirect)\r\n[Nested pointer code note not supported]"), pCondition5->GetTooltip(TriggerConditionViewModel::SourceValueProperty));
+    }
+
+    TEST_METHOD(TestTooltipRecallSubSource)
+    {
+        IndirectAddressTriggerViewModelHarness vmTrigger;
+        vmTrigger.mockGameContext.InitializeCodeNotes();
+        vmTrigger.Parse("B:2_K:0xH0003_I:{recall}_I:0xH0002_0xH0003=4");
+        vmTrigger.mockConfiguration.SetFeatureEnabled(ra::services::Feature::PreferDecimal, true);
+
+        const auto* pCondition1 = vmTrigger.Conditions().GetItemAt(0);
+        Expects(pCondition1 != nullptr);
+        const auto* pCondition2 = vmTrigger.Conditions().GetItemAt(1);
+        Expects(pCondition2 != nullptr);
+        const auto* pCondition3 = vmTrigger.Conditions().GetItemAt(2);
+        Expects(pCondition3 != nullptr);
+        const auto* pCondition4 = vmTrigger.Conditions().GetItemAt(3);
+        Expects(pCondition4 != nullptr);
+        const auto* pCondition5 = vmTrigger.Conditions().GetItemAt(4);
+        Expects(pCondition5 != nullptr);
+
+        Assert::IsFalse(pCondition1->IsIndirect());
+        Assert::IsFalse(pCondition2->IsIndirect());
+        Assert::IsFalse(pCondition3->IsIndirect());
+        Assert::IsTrue(pCondition4->IsIndirect());
+        Assert::IsTrue(pCondition5->IsIndirect());
+
+        // $0003 = 3, 3-2+2 = $0003, $0003 = 3, 3+3 = $0006
+        Assert::AreEqual(std::wstring(L"0x0003 (indirect)\r\n[No code note]"), pCondition4->GetTooltip(TriggerConditionViewModel::SourceValueProperty));
+        Assert::AreEqual(std::wstring(L"0x0006 (indirect)\r\n[Nested pointer code note not supported]"), pCondition5->GetTooltip(TriggerConditionViewModel::SourceValueProperty));
+
+        // $0003 = 5, 5-2 +2 = $0005, 5+4 = $0008
+        vmTrigger.SetMemory({ 3 }, 5);
+        Assert::AreEqual(std::wstring(L"0x0005 (indirect)\r\n[No code note]"), pCondition4->GetTooltip(TriggerConditionViewModel::SourceValueProperty));
+        Assert::AreEqual(std::wstring(L"0x0008 (indirect)\r\n[Nested pointer code note not supported]"), pCondition5->GetTooltip(TriggerConditionViewModel::SourceValueProperty));
+    }
+
+    TEST_METHOD(TestTooltipRecallPauseIfEarly)
+    {
+        IndirectAddressTriggerViewModelHarness vmTrigger;
+        vmTrigger.mockGameContext.InitializeCodeNotes();
+        vmTrigger.Parse("K:0xH0001_P:0=1_I:{recall}_I:0xH0002_0xH0003=4");
+        vmTrigger.mockConfiguration.SetFeatureEnabled(ra::services::Feature::PreferDecimal, true);
+        vmTrigger.mockGameContext.SetCodeNote({ 1U }, L"[8-bit pointer]\n+2=First Level A\n  +3=Second Level.");
+
+        const auto* pCondition1 = vmTrigger.Conditions().GetItemAt(0);
+        Expects(pCondition1 != nullptr);
+        const auto* pCondition2 = vmTrigger.Conditions().GetItemAt(1);
+        Expects(pCondition2 != nullptr);
+        const auto* pCondition3 = vmTrigger.Conditions().GetItemAt(2);
+        Expects(pCondition3 != nullptr);
+        const auto* pCondition4 = vmTrigger.Conditions().GetItemAt(3);
+        Expects(pCondition4 != nullptr);
+        const auto* pCondition5 = vmTrigger.Conditions().GetItemAt(4);
+        Expects(pCondition5 != nullptr);
+
+        Assert::IsFalse(pCondition1->IsIndirect());
+        Assert::IsFalse(pCondition2->IsIndirect());
+        Assert::IsFalse(pCondition3->IsIndirect());
+        Assert::IsTrue(pCondition4->IsIndirect());
+        Assert::IsTrue(pCondition5->IsIndirect());
+
+        // $0001 = 1, 1+2 = $0003, $0003 = 3, 3+3 = $0006
+        Assert::AreEqual(std::wstring(L""), pCondition3->GetTooltip(TriggerConditionViewModel::SourceValueProperty));
+        Assert::AreEqual(std::wstring(L"0x0003 (indirect)\r\nFirst Level A\n  +3=Second Level."), pCondition4->GetTooltip(TriggerConditionViewModel::SourceValueProperty));
+        Assert::AreEqual(std::wstring(L"0x0006 (indirect)\r\n[Nested pointer code note not supported]"), pCondition5->GetTooltip(TriggerConditionViewModel::SourceValueProperty));
+
+        // $0001 = 3, 3+2 = $0005, $0005 = 5, 5+3 = $0008
+        vmTrigger.SetMemory({ 1 }, 3);
+        Assert::AreEqual(std::wstring(L"0x0005 (indirect)\r\n[No code note]"), pCondition4->GetTooltip(TriggerConditionViewModel::SourceValueProperty));
+        Assert::AreEqual(std::wstring(L"0x0008 (indirect)\r\n[Nested pointer code note not supported]"), pCondition5->GetTooltip(TriggerConditionViewModel::SourceValueProperty));
+    }
+
+    TEST_METHOD(TestTooltipRecallPauseIfLate)
+    {
+        IndirectAddressTriggerViewModelHarness vmTrigger;
+        vmTrigger.mockGameContext.InitializeCodeNotes();
+        vmTrigger.Parse("I:{recall}_I:0xH0002_0xH0003=4_K:0xH0001_P:0=1");
+        vmTrigger.mockConfiguration.SetFeatureEnabled(ra::services::Feature::PreferDecimal, true);
+        vmTrigger.mockGameContext.SetCodeNote({ 1U }, L"[8-bit pointer]\n+2=First Level A\n  +3=Second Level.");
+
+        const auto* pCondition1 = vmTrigger.Conditions().GetItemAt(0);
+        Expects(pCondition1 != nullptr);
+        const auto* pCondition2 = vmTrigger.Conditions().GetItemAt(1);
+        Expects(pCondition2 != nullptr);
+        const auto* pCondition3 = vmTrigger.Conditions().GetItemAt(2);
+        Expects(pCondition3 != nullptr);
+        const auto* pCondition4 = vmTrigger.Conditions().GetItemAt(3);
+        Expects(pCondition4 != nullptr);
+        const auto* pCondition5 = vmTrigger.Conditions().GetItemAt(4);
+        Expects(pCondition5 != nullptr);
+
+        Assert::IsFalse(pCondition1->IsIndirect());
+        Assert::IsTrue(pCondition2->IsIndirect());
+        Assert::IsTrue(pCondition3->IsIndirect());
+        Assert::IsFalse(pCondition4->IsIndirect());
+        Assert::IsFalse(pCondition5->IsIndirect());
+
+        // $0001 = 1, 1+2 = $0003, $0003 = 3, 3+3 = $0006
+        Assert::AreEqual(std::wstring(L""), pCondition1->GetTooltip(TriggerConditionViewModel::SourceValueProperty));
+        Assert::AreEqual(std::wstring(L"0x0003 (indirect)\r\nFirst Level A\n  +3=Second Level."), pCondition2->GetTooltip(TriggerConditionViewModel::SourceValueProperty));
+        Assert::AreEqual(std::wstring(L"0x0006 (indirect)\r\n[Nested pointer code note not supported]"), pCondition3->GetTooltip(TriggerConditionViewModel::SourceValueProperty));
+
+        // $0001 = 3, 3+2 = $0005, $0005 = 5, 5+3 = $0008
+        vmTrigger.SetMemory({ 1 }, 3);
+        Assert::AreEqual(std::wstring(L"0x0005 (indirect)\r\n[No code note]"), pCondition2->GetTooltip(TriggerConditionViewModel::SourceValueProperty));
+        Assert::AreEqual(std::wstring(L"0x0008 (indirect)\r\n[Nested pointer code note not supported]"), pCondition3->GetTooltip(TriggerConditionViewModel::SourceValueProperty));
+    }
+
     TEST_METHOD(TestIsModifying)
     {
         TriggerConditionViewModelHarness condition;