Repository: RetroAchievements/RAIntegration
Pull request #748 (2021-07-14 04:30:06)

highlight ranged notes in memory inspector
Provides a secondary color for memory associated to a ranged code note in the memory viewer

![image](https://user-images.githubusercontent.com/32680403/125560780-a14b371d-c829-486d-b98a-68d7c546df77.png)

Here, you can see several `[32-bit]` values (the first few are pointers) and one `[16-bit]` value. Also works with large byte ranges.

Includes support for arbitrarily sized bit ranges (i.e. `[24-bit]`, `[128-bit]`, `[999-bit]`). Previously only 16-bit and 32-bit were detected. Arbitrarily sized byte ranges were already supported.


Patch:
diff --git a/src/data/context/GameContext.cpp b/src/data/context/GameContext.cpp
index e6886234..cb3d75d7 100644
--- a/src/data/context/GameContext.cpp
+++ b/src/data/context/GameContext.cpp
@@ -843,53 +843,51 @@ void GameContext::RefreshCodeNotes()
 void GameContext::AddCodeNote(ra::ByteAddress nAddress, const std::string& sAuthor, const std::wstring& sNote)
 {
     unsigned int nBytes = 1;
+    bool bIsBytes = false;
 
     // attempt to match "X byte", "X Byte", "XX bytes", or "XX Bytes"
     auto nIndex = sNote.find(L"yte");
-    if (nIndex != std::string::npos && nIndex >= 3)
+    if (nIndex != std::string::npos && nIndex >= 2) // have to have room for the 'b' of bytes plus at least one digit
     {
-        wchar_t c = sNote.at(--nIndex);
+        const wchar_t c = sNote.at(--nIndex);
         if (c == L'b' || c == L'B')
-        {
-            c = sNote.at(--nIndex);
-            if (c == L'-' || c == L' ')
-                c = sNote.at(--nIndex);
-
-            if (c >= L'0' && c <= L'9')
-            {
-                int nMultiplier = 1;
-                nBytes = 0;
-                do
-                {
-                    nBytes += (c - L'0') * nMultiplier;
-                    if (nIndex == 0)
-                        break;
-
-                    nMultiplier *= 10;
-                    c = sNote.at(--nIndex);
-                } while (c >= L'0' && c <= L'9');
-            }
-        }
+            bIsBytes = true;
     }
 
-    if (nBytes == 1)
+    if (!bIsBytes)
     {
-        // attempt to match "16-bit" or "32-bit"
+        // attempt to match "N bit" or "N bits"
         nIndex = sNote.find(L"Bit");
         if (nIndex == std::string::npos)
             nIndex = sNote.find(L"bit");
+    }
 
-        if (nIndex != std::string::npos && nIndex >= 3)
+    if (nIndex != std::string::npos && nIndex >= 2)
+    {
+        // allow "N-bits", "N bits" or "Nbits". ignore if any other character appears between "N" and "bits"
+        wchar_t c = sNote.at(--nIndex);
+        if (c == L'-' || c == L' ')
+            c = sNote.at(--nIndex);
+
+        if (c >= L'0' && c <= L'9')
         {
-            wchar_t c = sNote.at(--nIndex);
-            if (c == L'-' || c == L' ')
-                c = sNote.at(--nIndex);
+            int nMultiplier = 1;
+            nBytes = 0;
+            do
+            {
+                nBytes += (c - L'0') * nMultiplier;
+                if (nIndex == 0)
+                    break;
 
-            if (c == L'6' && sNote.at(nIndex - 1) == L'1')
-                nBytes = 2;
-            else if (c == L'2' && sNote.at(nIndex - 1) == L'3')
-                nBytes = 4;
+                nMultiplier *= 10;
+                c = sNote.at(--nIndex);
+            } while (c >= L'0' && c <= L'9');
         }
+
+        if (nBytes == 0) // sanity check
+            nBytes = 1;
+        else if (!bIsBytes) // convert bits to bytes, rounding up
+            nBytes = (nBytes + 7) / 8;
     }
 
     m_mCodeNotes.insert_or_assign(nAddress, CodeNote{ sAuthor, sNote, nBytes });
@@ -918,14 +916,19 @@ ra::ByteAddress GameContext::FindCodeNoteStart(ra::ByteAddress nAddress) const
     if (pIter != m_mCodeNotes.end() && pIter->first == nAddress)
         return nAddress;
 
-    // lower_bound returns the first item _after_ the search value. we want to look at the item _before_.
+    // lower_bound returns the first item _after_ the search value. scan all items before
+    // the found item to see if any of them contain the target address. have to scan
+    // all items because a singular note may exist within a range.
     if (pIter != m_mCodeNotes.begin())
     {
-        --pIter;
+        do
+        {
+            --pIter;
+
+            if (pIter->second.Bytes > 1 && pIter->second.Bytes + pIter->first > nAddress)
+                return pIter->first;
 
-        // check to see if the item before the search value contains the search value
-        if (pIter->first + pIter->second.Bytes > nAddress)
-            return pIter->first;
+        } while (pIter != m_mCodeNotes.begin());
     }
 
     return 0xFFFFFFFF;
diff --git a/src/ui/EditorTheme.hh b/src/ui/EditorTheme.hh
index 5c42f881..f84b39b9 100644
--- a/src/ui/EditorTheme.hh
+++ b/src/ui/EditorTheme.hh
@@ -30,6 +30,7 @@ public:
     Color ColorNormal() const noexcept { return m_colorNormal; }
     Color ColorSelected() const noexcept { return m_colorSelected; }
     Color ColorHasNote() const noexcept { return m_colorHasNote; }
+    Color ColorHasSurrogateNote() const noexcept { return m_colorHasSurrogateNote; }
     Color ColorHasBookmark() const noexcept { return m_colorHasBookmark; }
     Color ColorFrozen() const noexcept { return m_colorFrozen; }
     Color ColorHeader() const noexcept { return m_colorHeader; }
@@ -57,6 +58,7 @@ private:
     Color m_colorNormal{ 255, 0, 0, 0 };
     Color m_colorSelected{ 255, 255, 0, 0 };
     Color m_colorHasNote{ 255, 0, 0, 255 };
+    Color m_colorHasSurrogateNote{ 255, 32, 64, 160 };
     Color m_colorHasBookmark{ 255, 0, 160, 0 };
     Color m_colorFrozen{ 255, 255, 200, 0 };
     Color m_colorHeader{ 255, 128, 128, 128 };
diff --git a/src/ui/Theme.cpp b/src/ui/Theme.cpp
index d99e9f17..574bdf14 100644
--- a/src/ui/Theme.cpp
+++ b/src/ui/Theme.cpp
@@ -183,6 +183,7 @@ void EditorTheme::LoadFromFile()
             ReadColor(m_colorNormal, colors, "Normal");
             ReadColor(m_colorSelected, colors, "Selected");
             ReadColor(m_colorHasNote, colors, "HasNote");
+            ReadColor(m_colorHasSurrogateNote, colors, "HasSurrogateNote");
             ReadColor(m_colorHasBookmark, colors, "HasBookmark");
             ReadColor(m_colorFrozen, colors, "Frozen");
             ReadColor(m_colorHeader, colors, "Header");
diff --git a/src/ui/viewmodels/MemoryViewerViewModel.cpp b/src/ui/viewmodels/MemoryViewerViewModel.cpp
index ee5d7e14..908a9d7f 100644
--- a/src/ui/viewmodels/MemoryViewerViewModel.cpp
+++ b/src/ui/viewmodels/MemoryViewerViewModel.cpp
@@ -144,7 +144,7 @@ int MemoryViewerViewModel::NibblesPerWord() const
 
 static MemoryViewerViewModel::TextColor GetColor(ra::ByteAddress nAddress,
     const ra::ui::viewmodels::MemoryBookmarksViewModel& pBookmarksViewModel,
-    const ra::data::context::GameContext& pGameContext)
+    const ra::data::context::GameContext& pGameContext, bool bCheckBookmarks = true)
 {
     if (pBookmarksViewModel.HasBookmark(nAddress))
     {
@@ -154,23 +154,75 @@ static MemoryViewerViewModel::TextColor GetColor(ra::ByteAddress nAddress,
         return MemoryViewerViewModel::TextColor::HasBookmark;
     }
 
-    if (pGameContext.FindCodeNote(nAddress) != nullptr)
-        return MemoryViewerViewModel::TextColor::HasNote;
+    if (bCheckBookmarks)
+    {
+        const auto nNoteStart = pGameContext.FindCodeNoteStart(nAddress);
+        if (nNoteStart != 0xFFFFFFFF)
+        {
+            if (nNoteStart == nAddress)
+                return MemoryViewerViewModel::TextColor::HasNote;
+            else
+                return MemoryViewerViewModel::TextColor::HasSurrogateNote;
+        }
+    }
 
     return MemoryViewerViewModel::TextColor::Default;
 }
 
 void MemoryViewerViewModel::UpdateColors()
 {
-    const auto& pBookmarksViewModel = ra::services::ServiceLocator::Get<ra::ui::viewmodels::WindowManager>().MemoryBookmarks;
     const auto& pGameContext = ra::services::ServiceLocator::Get<ra::data::context::GameContext>();
 
     const auto nVisibleLines = GetNumVisibleLines();
     const auto nFirstAddress = GetFirstAddress();
 
+    // identify bookmarks
+    const auto& pBookmarksViewModel = ra::services::ServiceLocator::Get<ra::ui::viewmodels::WindowManager>().MemoryBookmarks;
     for (int i = 0; i < nVisibleLines * 16; ++i)
-        m_pColor[i] = STALE_COLOR | gsl::narrow_cast<uint8_t>(ra::etoi(GetColor(nFirstAddress + i, pBookmarksViewModel, pGameContext)));
+        m_pColor[i] = STALE_COLOR | gsl::narrow_cast<uint8_t>(ra::etoi(GetColor(nFirstAddress + i, pBookmarksViewModel, pGameContext, false)));
+
+    // apply code notes
+    const auto nStopAddress = nFirstAddress + nVisibleLines * 16;
+    pGameContext.EnumerateCodeNotes([nFirstAddress, nStopAddress, this](ra::ByteAddress nAddress, unsigned nBytes, const std::wstring&) {
+        if (nAddress + nBytes <= nFirstAddress)
+            return true;
+        if (nAddress >= nStopAddress)
+            return false;
+
+        uint8_t* pOffset = m_pColor;
+        Expects(pOffset != nullptr);
+        if (nAddress < nFirstAddress)
+        {
+            nBytes -= (nFirstAddress - nAddress);
+            nAddress = nFirstAddress;
+
+            // so first processed surrogate will be nFirstAddress
+            --pOffset;
+            ++nBytes;
+        }
+        else
+        {
+            pOffset += (nAddress - nFirstAddress);
+            if ((*pOffset & 0x0F) == ra::etoi(TextColor::Default))
+                *pOffset |= ra::etoi(TextColor::HasNote);
+        }
+
+        if (nBytes > 1)
+        {
+            nBytes = std::min(nBytes, nStopAddress - nAddress + 1);
+            for (unsigned i = 1; i < nBytes; ++i)
+            {
+                ++pOffset;
+
+                if ((*pOffset & 0x0F) == ra::etoi(TextColor::Default))
+                    *pOffset |= ra::etoi(TextColor::HasSurrogateNote);
+            }
+        }
+
+        return true;
+    });
 
+    // flag invalid regions
     const auto& pEmulatorContext = ra::services::ServiceLocator::Get<ra::data::context::EmulatorContext>();
     if (pEmulatorContext.HasInvalidRegions())
     {
@@ -178,6 +230,7 @@ void MemoryViewerViewModel::UpdateColors()
             m_pInvalid[i] = pEmulatorContext.IsValidAddress(nFirstAddress + i) ? 0 : 1;
     }
 
+    // update cursor
     UpdateHighlight(GetAddress(), NibblesPerWord() / 2, 0);
 
     m_nNeedsRedraw |= REDRAW_MEMORY;
@@ -524,20 +577,35 @@ void MemoryViewerViewModel::OnCodeNoteChanged(ra::ByteAddress nAddress, const st
     if (nAddress < nFirstAddress)
         return;
 
-    const auto nOffset = nAddress - nFirstAddress;
+    auto nOffset = nAddress - nFirstAddress;
     const auto nVisibleLines = GetNumVisibleLines();
     if (nOffset >= ra::to_unsigned(nVisibleLines * 16))
         return;
 
     const auto& pBookmarksViewModel = ra::services::ServiceLocator::Get<ra::ui::viewmodels::WindowManager>().MemoryBookmarks;
     const auto& pGameContext = ra::services::ServiceLocator::Get<ra::data::context::GameContext>();
-    const auto nNewColor = (nAddress == GetAddress()) ? ra::itoe<TextColor>(HIGHLIGHTED_COLOR & 0x0F) :
-        GetColor(nAddress, pBookmarksViewModel, pGameContext);
 
-    if ((m_pColor[nOffset] & 0x0F) != ra::etoi(nNewColor))
+    const auto nSelectedAddress = GetAddress();
+
+    const auto nMax = nFirstAddress + nVisibleLines * 16 - nAddress;
+    const auto nSize = std::min(pGameContext.FindCodeNoteSize(nAddress), nMax);
+    for (unsigned i = 0; i < nSize; ++i)
     {
-        m_pColor[nOffset] = STALE_COLOR | gsl::narrow_cast<uint8_t>(ra::etoi(nNewColor));
-        m_nNeedsRedraw |= REDRAW_MEMORY;
+        auto nNewColor = (nAddress == nSelectedAddress) ?
+            ra::itoe<TextColor>(HIGHLIGHTED_COLOR & 0x0F):
+            GetColor(nAddress, pBookmarksViewModel, pGameContext, false);
+
+        if (nNewColor == TextColor::Default)
+            nNewColor = (i == 0) ? TextColor::HasNote : TextColor::HasSurrogateNote;
+
+        if ((m_pColor[nOffset] & 0x0F) != ra::etoi(nNewColor))
+        {
+            m_pColor[nOffset] = STALE_COLOR | gsl::narrow_cast<uint8_t>(ra::etoi(nNewColor));
+            m_nNeedsRedraw |= REDRAW_MEMORY;
+        }
+
+        ++nAddress;
+        ++nOffset;
     }
 }
 
@@ -849,12 +917,13 @@ void MemoryViewerViewModel::BuildFontSurface()
     {
         switch (ra::itoe<TextColor>(i))
         {
-            case TextColor::Default:      nColor = pEditorTheme.ColorNormal(); break;
-            case TextColor::HasNote:      nColor = pEditorTheme.ColorHasNote(); break;
-            case TextColor::HasBookmark:  nColor = pEditorTheme.ColorHasBookmark(); break;
+            case TextColor::Default:          nColor = pEditorTheme.ColorNormal(); break;
+            case TextColor::HasNote:          nColor = pEditorTheme.ColorHasNote(); break;
+            case TextColor::HasSurrogateNote: nColor = pEditorTheme.ColorHasSurrogateNote(); break;
+            case TextColor::HasBookmark:      nColor = pEditorTheme.ColorHasBookmark(); break;
             case TextColor::Cursor:
-            case TextColor::Selected:     nColor = pEditorTheme.ColorSelected(); break;
-            case TextColor::Frozen:       nColor = pEditorTheme.ColorFrozen(); break;
+            case TextColor::Selected:         nColor = pEditorTheme.ColorSelected(); break;
+            case TextColor::Frozen:           nColor = pEditorTheme.ColorFrozen(); break;
         }
 
         for (int j = 0; j < gsl::narrow_cast<int>(g_sHexChars.size()); ++j)
diff --git a/src/ui/viewmodels/MemoryViewerViewModel.hh b/src/ui/viewmodels/MemoryViewerViewModel.hh
index 05ee01eb..7bb82f8e 100644
--- a/src/ui/viewmodels/MemoryViewerViewModel.hh
+++ b/src/ui/viewmodels/MemoryViewerViewModel.hh
@@ -73,6 +73,7 @@ public:
         Selected,        // selected
         Cursor,          // selected and highlighted
         HasNote,         // has notes
+        HasSurrogateNote,// part of a multibyte note
         HasBookmark,     // has bookmark
         Frozen,          // has frozen bookmark
 


Test patch:
diff --git a/tests/data/context/GameContext_Tests.cpp b/tests/data/context/GameContext_Tests.cpp
index a9fa35f6..800c7823 100644
--- a/tests/data/context/GameContext_Tests.cpp
+++ b/tests/data/context/GameContext_Tests.cpp
@@ -2170,20 +2170,29 @@ public:
         game.mockThreadPool.ExecuteNextTask(); // FetchUserUnlocks and FetchCodeNotes are async
         game.mockThreadPool.ExecuteNextTask();
 
-        Assert::AreEqual(nExpectedSize, game.GetCodeNoteSize(1234U));
+        Assert::AreEqual(nExpectedSize, game.GetCodeNoteSize(1234U), sNote.c_str());
     }
 
     TEST_METHOD(TestLoadCodeNotesSized)
     {
         TestCodeNoteSize(L"Test", 1U);
+        TestCodeNoteSize(L"16-bit Test", 2U);
         TestCodeNoteSize(L"[16-bit] Test", 2U);
         TestCodeNoteSize(L"[16 bit] Test", 2U);
         TestCodeNoteSize(L"[16 Bit] Test", 2U);
+        TestCodeNoteSize(L"[24-bit] Test", 3U);
         TestCodeNoteSize(L"[32-bit] Test", 4U);
         TestCodeNoteSize(L"[32 bit] Test", 4U);
         TestCodeNoteSize(L"[32bit] Test", 4U);
         TestCodeNoteSize(L"Test [16-bit]", 2U);
         TestCodeNoteSize(L"Test (16-bit)", 2U);
+        TestCodeNoteSize(L"[64-bit] Test", 8U);
+        TestCodeNoteSize(L"[128-bit] Test", 16U);
+        TestCodeNoteSize(L"[17-bit] Test", 3U);
+        TestCodeNoteSize(L"[100-bit] Test", 13U);
+        TestCodeNoteSize(L"[4-bit] Test", 1U);
+        TestCodeNoteSize(L"[0-bit] Test", 1U);
+
         TestCodeNoteSize(L"[2 Byte] Test", 2U);
         TestCodeNoteSize(L"[4 Byte] Test", 4U);
         TestCodeNoteSize(L"[4 Byte - Float] Test", 4U);
@@ -2193,8 +2202,10 @@ public:
         TestCodeNoteSize(L"[2-byte] Test", 2U);
         TestCodeNoteSize(L"Test (6 bytes)", 6U);
         TestCodeNoteSize(L"[2byte] Test", 2U);
+
         TestCodeNoteSize(L"4=bitten", 1U);
         TestCodeNoteSize(L"bit by bit", 1U);
+        TestCodeNoteSize(L"bit1=chest", 1U);
     }
 
     TEST_METHOD(TestFindCodeNoteSized)
@@ -2338,6 +2349,60 @@ public:
         Assert::AreEqual(0xFFFFFFFF, game.FindCodeNoteStart(1220));
     }
 
+    TEST_METHOD(TestFindCodeNoteStartOverlap)
+    {
+        GameContextHarness game;
+        game.mockServer.HandleRequest<ra::api::FetchGameData>([](const ra::api::FetchGameData::Request&, ra::api::FetchGameData::Response&)
+        {
+            return true;
+        });
+
+        game.mockServer.HandleRequest<ra::api::FetchUserUnlocks>([](const ra::api::FetchUserUnlocks::Request&, ra::api::FetchUserUnlocks::Response&)
+        {
+            return true;
+        });
+
+        game.mockServer.HandleRequest<ra::api::FetchCodeNotes>([](const ra::api::FetchCodeNotes::Request& request, ra::api::FetchCodeNotes::Response& response)
+        {
+            Assert::AreEqual(1U, request.GameId);
+
+            response.Notes.emplace_back(ra::api::FetchCodeNotes::Response::CodeNote{ 1000, L"[100 bytes] Outer", "Author" });
+            response.Notes.emplace_back(ra::api::FetchCodeNotes::Response::CodeNote{ 1010, L"[10 bytes] Inner", "Author" });
+            response.Notes.emplace_back(ra::api::FetchCodeNotes::Response::CodeNote{ 1015, L"Individual", "Author" });
+            response.Notes.emplace_back(ra::api::FetchCodeNotes::Response::CodeNote{ 1120, L"[10 bytes] Secondary", "Author" });
+            response.Notes.emplace_back(ra::api::FetchCodeNotes::Response::CodeNote{ 1125, L"[10 bytes] Overlap", "Author" });
+            response.Notes.emplace_back(ra::api::FetchCodeNotes::Response::CodeNote{ 1200, L"Extra", "Author" });
+            return true;
+        });
+
+        game.LoadGame(1U);
+        game.mockThreadPool.ExecuteNextTask(); // FetchUserUnlocks and FetchCodeNotes are async
+        game.mockThreadPool.ExecuteNextTask();
+
+        Assert::AreEqual(0xFFFFFFFF, game.FindCodeNoteStart(999));
+        Assert::AreEqual(1000U, game.FindCodeNoteStart(1000));
+        Assert::AreEqual(1000U, game.FindCodeNoteStart(1009));
+        Assert::AreEqual(1010U, game.FindCodeNoteStart(1010));
+        Assert::AreEqual(1010U, game.FindCodeNoteStart(1014));
+        Assert::AreEqual(1015U, game.FindCodeNoteStart(1015));
+        Assert::AreEqual(1010U, game.FindCodeNoteStart(1016));
+        Assert::AreEqual(1010U, game.FindCodeNoteStart(1019));
+        Assert::AreEqual(1000U, game.FindCodeNoteStart(1020));
+        Assert::AreEqual(1000U, game.FindCodeNoteStart(1099));
+        Assert::AreEqual(0xFFFFFFFF, game.FindCodeNoteStart(1100));
+        Assert::AreEqual(0xFFFFFFFF, game.FindCodeNoteStart(1119));
+        Assert::AreEqual(1120U, game.FindCodeNoteStart(1120));
+        Assert::AreEqual(1120U, game.FindCodeNoteStart(1124));
+        Assert::AreEqual(1125U, game.FindCodeNoteStart(1125));
+        Assert::AreEqual(1125U, game.FindCodeNoteStart(1126));
+        Assert::AreEqual(1125U, game.FindCodeNoteStart(1130));
+        Assert::AreEqual(1125U, game.FindCodeNoteStart(1134));
+        Assert::AreEqual(0xFFFFFFFF, game.FindCodeNoteStart(1135));
+        Assert::AreEqual(0xFFFFFFFF, game.FindCodeNoteStart(1199));
+        Assert::AreEqual(1200U, game.FindCodeNoteStart(1200));
+        Assert::AreEqual(0xFFFFFFFF, game.FindCodeNoteStart(1201));
+    }
+
     TEST_METHOD(TestSetCodeNote)
     {
         GameContextNotifyTarget notifyTarget;