Repository: RetroAchievements/RAIntegration (cpp)

Issue:
'Initial' filter removing valid results on aligned searches
Doing an '= Initial' filter with a 16-bit or 32-bit aligned search is removing some values that do match their initial result. Retested with an unaligned search, and the '= Initial' filter correctly kept the values, at least the ones I was watching for.

RA_Integration 1.0.4, initially encountered on pcsx2 test build, reproduced on RALibretro version 1.3.11, with ppsspp core.

![pcsx2 filter bug](https://user-images.githubusercontent.com/112431988/189037355-75705ccd-fb9b-4fcb-887e-4d839cb58908.png)

Here, 0x76ccf0 does not equal its initial value, 0x76cd00 does. The two addresses remaining after the filter both match their initial value.


Pull request:
fix initial search filtering for aligned searches
fixes #909 

When doing a comparison against Initial search values, a slice of the initially captured results has to be made so only the currently active filtered results are compared. For aligned searches, virtual addresses are used to make the data structure more compact. The code that was generating the slice was not converting the virtual address back to a real address, so the slice was being created incorrectly, leading to the filter being applied incorrectly.

Patch:
diff --git a/src/data/context/EmulatorContext.cpp b/src/data/context/EmulatorContext.cpp
index db1367fc..a98afa0b 100644
--- a/src/data/context/EmulatorContext.cpp
+++ b/src/data/context/EmulatorContext.cpp
@@ -39,7 +39,7 @@ static std::string FormatAddressLarge(ra::ByteAddress nAddress)
 static std::string FormatAddressMedium(ra::ByteAddress nAddress)
 {
     if (nAddress & 0xFF000000)
-        return FormatAddressMedium(nAddress);
+        return FormatAddressLarge(nAddress);
 
     std::string sAddress;
     sAddress.resize(8);
diff --git a/src/services/SearchResults.cpp b/src/services/SearchResults.cpp
index 33f7ad63..e84f6840 100644
--- a/src/services/SearchResults.cpp
+++ b/src/services/SearchResults.cpp
@@ -1122,7 +1122,7 @@ public:
 
     std::wstring GetFormattedValue(const SearchResults& pResults, ra::ByteAddress nAddress, MemSize) const override
     {
-        std::array<unsigned char, 16> pBuffer;
+        std::array<unsigned char, 16> pBuffer = {0};
         pResults.GetBytes(nAddress, &pBuffer.at(0), pBuffer.size());
 
         std::wstring sText;
@@ -1524,8 +1524,11 @@ void SearchResults::Initialize(ra::ByteAddress nAddress, size_t nBytes, SearchTy
     m_nType = nType;
 
     const auto& pEmulatorContext = ra::services::ServiceLocator::Get<ra::data::context::EmulatorContext>();
-    if (nBytes + nAddress > pEmulatorContext.TotalMemorySize())
-        nBytes = pEmulatorContext.TotalMemorySize() - nAddress;
+    const auto nTotalMemorySize = pEmulatorContext.TotalMemorySize();
+    if (nAddress > nTotalMemorySize)
+        nAddress = 0;
+    if (nBytes + nAddress > nTotalMemorySize)
+        nBytes = nTotalMemorySize - nAddress;
 
     switch (nType)
     {
@@ -1608,14 +1611,15 @@ void SearchResults::MergeSearchResults(const SearchResults& srMemory, const Sear
         // srAddresses collection.
         auto& pNewBlock = m_vBlocks.emplace_back(pSrcBlock);
         unsigned int nSize = pNewBlock.GetBytesSize();
-        ra::ByteAddress nAddress = pNewBlock.GetFirstAddress();
+        ra::ByteAddress nAddress = m_pImpl->ConvertToRealAddress(pNewBlock.GetFirstAddress());
         unsigned char* pWrite = pNewBlock.GetBytes();
 
         for (const auto& pMemBlock : srMemory.m_vBlocks)
         {
-            if (nAddress >= pMemBlock.GetFirstAddress() && nAddress < pMemBlock.GetFirstAddress() + pMemBlock.GetBytesSize())
+            const auto nBlockFirstAddress = m_pImpl->ConvertToRealAddress(pMemBlock.GetFirstAddress());
+            if (nAddress >= nBlockFirstAddress && nAddress < nBlockFirstAddress + pMemBlock.GetBytesSize())
             {
-                const auto nOffset = nAddress - pMemBlock.GetFirstAddress();
+                const auto nOffset = nAddress - nBlockFirstAddress;
                 const auto nAvailable = pMemBlock.GetBytesSize() - nOffset;
                 if (nAvailable >= nSize)
                 {
diff --git a/src/ui/viewmodels/MemoryBookmarksViewModel.cpp b/src/ui/viewmodels/MemoryBookmarksViewModel.cpp
index b0e14023..8e7dc735 100644
--- a/src/ui/viewmodels/MemoryBookmarksViewModel.cpp
+++ b/src/ui/viewmodels/MemoryBookmarksViewModel.cpp
@@ -11,6 +11,7 @@
 #include "services\IConfiguration.hh"
 #include "services\IFileSystem.hh"
 #include "services\ILocalStorage.hh"
+#include "services\SearchResults.h"
 #include "services\ServiceLocator.hh"
 
 #include "ui\viewmodels\FileDialogViewModel.hh"
@@ -43,6 +44,8 @@ const BoolModelProperty MemoryBookmarksViewModel::MemoryBookmarkViewModel::ReadO
 const BoolModelProperty MemoryBookmarksViewModel::MemoryBookmarkViewModel::IsDirtyProperty("MemoryBookmarkViewModel", "IsDirty", false);
 const StringModelProperty MemoryBookmarksViewModel::FreezeButtonTextProperty("MemoryBookmarksViewModel", "FreezeButtonText", L"Freeze");
 
+constexpr int MaxTextBookmarkLength = 8;
+
 MemoryBookmarksViewModel::MemoryBookmarksViewModel() noexcept
 {
     SetWindowTitle(L"Memory Bookmarks");
@@ -59,6 +62,7 @@ MemoryBookmarksViewModel::MemoryBookmarksViewModel() noexcept
     m_vSizes.Add(ra::etoi(MemSize::Nibble_Upper), L"Upper4");
     m_vSizes.Add(ra::etoi(MemSize::Float), L"Float");
     m_vSizes.Add(ra::etoi(MemSize::MBF32), L"MBF32");
+    m_vSizes.Add(ra::etoi(MemSize::Text), L"ASCII");
 
     m_vFormats.Add(ra::etoi(MemFormat::Hex), L"Hex");
     m_vFormats.Add(ra::etoi(MemFormat::Dec), L"Dec");
@@ -93,7 +97,17 @@ void MemoryBookmarksViewModel::MemoryBookmarkViewModel::OnValueChanged(const Int
     else if (args.Property == MemoryBookmarkViewModel::SizeProperty)
     {
         m_nSize = ra::itoe<MemSize>(args.tNewValue);
-        SetReadOnly(m_nSize == MemSize::BitCount);
+        switch (m_nSize)
+        {
+            case MemSize::BitCount:
+            case MemSize::Text:
+                SetReadOnly(true);
+                break;
+
+            default:
+                SetReadOnly(false);
+                break;
+        }
 
         if (m_bInitialized)
         {
@@ -162,6 +176,20 @@ void MemoryBookmarksViewModel::MemoryBookmarkViewModel::OnValueChanged(const Str
 unsigned MemoryBookmarksViewModel::MemoryBookmarkViewModel::ReadValue() const
 {
     const auto& pEmulatorContext = ra::services::ServiceLocator::Get<ra::data::context::EmulatorContext>();
+    if (m_nSize == MemSize::Text)
+    {
+        // only have 32 bits to store the value in. generate a hash for the string
+        std::array<uint8_t, MaxTextBookmarkLength + 1> pBuffer;
+        pEmulatorContext.ReadMemory(m_nAddress, &pBuffer.at(0), pBuffer.size() - 1);
+        pBuffer.at(pBuffer.size() - 1) = '\0';
+
+        const char* pText;
+        GSL_SUPPRESS_TYPE1 pText = reinterpret_cast<char*>(&pBuffer.at(0));
+        std::string sText(pText);
+
+        return ra::StringHash(sText);
+    }
+
     return pEmulatorContext.ReadMemory(m_nAddress, m_nSize);
 }
 
@@ -265,6 +293,13 @@ void MemoryBookmarksViewModel::MemoryBookmarkViewModel::UpdateCurrentValue()
 
 std::wstring MemoryBookmarksViewModel::MemoryBookmarkViewModel::BuildCurrentValue() const
 {
+    if (m_nSize == MemSize::Text)
+    {
+        ra::services::SearchResults pResults;
+        pResults.Initialize(m_nAddress, MaxTextBookmarkLength, ra::services::SearchType::AsciiText);
+        return pResults.GetFormattedValue(m_nAddress, MemSize::Text);
+    }
+
     return ra::data::MemSizeFormat(m_nValue, m_nSize, GetFormat());
 }
 
@@ -505,7 +540,8 @@ void MemoryBookmarksViewModel::OnByteWritten(ra::ByteAddress nAddress, uint8_t)
         if (nAddress < nBookmarkAddress)
             continue;
 
-        const auto nBytes = ra::data::MemSizeBytes(pBookmark.GetSize());
+        const auto nSize = pBookmark.GetSize();
+        const auto nBytes = (nSize == MemSize::Text) ? MaxTextBookmarkLength : ra::data::MemSizeBytes(nSize);
         if (nAddress >= nBookmarkAddress + nBytes)
             continue;
 
diff --git a/src/ui/viewmodels/MemorySearchViewModel.cpp b/src/ui/viewmodels/MemorySearchViewModel.cpp
index ec972364..bb7c06f9 100644
--- a/src/ui/viewmodels/MemorySearchViewModel.cpp
+++ b/src/ui/viewmodels/MemorySearchViewModel.cpp
@@ -1070,12 +1070,6 @@ void MemorySearchViewModel::BookmarkSelected()
         return;
 
     const MemSize nSize = m_vSearchResults.back().pResults.GetSize();
-    if (nSize == MemSize::Text)
-    {
-        ra::ui::viewmodels::MessageBoxViewModel::ShowInfoMessage(L"Text bookmarks are not supported");
-        return;
-    }
-
     auto& vmBookmarks = ra::services::ServiceLocator::GetMutable<ra::ui::viewmodels::WindowManager>().MemoryBookmarks;
     if (!vmBookmarks.IsVisible())
         vmBookmarks.Show();
diff --git a/src/ui/win32/MemoryBookmarksDialog.cpp b/src/ui/win32/MemoryBookmarksDialog.cpp
index efd2242f..eb5b6fcb 100644
--- a/src/ui/win32/MemoryBookmarksDialog.cpp
+++ b/src/ui/win32/MemoryBookmarksDialog.cpp
@@ -91,7 +91,7 @@ public:
     HWND CreateInPlaceEditor(HWND hParent, InPlaceEditorInfo& pInfo) override
     {
         const auto& vmItems = static_cast<ra::ui::win32::bindings::GridBinding*>(pInfo.pGridBinding)->GetItems();
-        if (vmItems.GetItemValue(pInfo.nColumnIndex, MemoryBookmarksViewModel::MemoryBookmarkViewModel::ReadOnlyProperty))
+        if (vmItems.GetItemValue(pInfo.nItemIndex, MemoryBookmarksViewModel::MemoryBookmarkViewModel::ReadOnlyProperty))
             return nullptr;
 
         return ra::ui::win32::bindings::GridTextColumnBinding::CreateInPlaceEditor(hParent, pInfo);
@@ -159,6 +159,7 @@ private:
         {
             case MemSize::Float:
             case MemSize::MBF32:
+            case MemSize::Text:
                 return true;
 
             default:


Test patch:
diff --git a/tests/data/context/EmulatorContext_Tests.cpp b/tests/data/context/EmulatorContext_Tests.cpp
index 1a9e4369..6116e0fb 100644
--- a/tests/data/context/EmulatorContext_Tests.cpp
+++ b/tests/data/context/EmulatorContext_Tests.cpp
@@ -1714,6 +1714,71 @@ public:
         Assert::AreEqual(ra::data::models::AssetState::Inactive, emulator.mockGameContext.Assets().GetItemAt(2)->GetState());
         Assert::AreEqual(ra::data::models::AssetState::Inactive, emulator.mockGameContext.Assets().GetItemAt(3)->GetState());
     }
+
+    TEST_METHOD(TestFormatAddress)
+    {
+        EmulatorContextHarness emulator;
+        emulator.AddMemoryBlock(0, 0x80, nullptr, nullptr);
+
+        Assert::AreEqual(std::string("0x0000"), emulator.FormatAddress(0x00000000U));
+        Assert::AreEqual(std::string("0x00ff"), emulator.FormatAddress(0x000000FFU));
+        Assert::AreEqual(std::string("0x0100"), emulator.FormatAddress(0x00000100U));
+        Assert::AreEqual(std::string("0xffff"), emulator.FormatAddress(0x0000FFFFU));
+        Assert::AreEqual(std::string("0x010000"), emulator.FormatAddress(0x00010000U));
+        Assert::AreEqual(std::string("0xffffff"), emulator.FormatAddress(0x00FFFFFFU));
+        Assert::AreEqual(std::string("0x01000000"), emulator.FormatAddress(0x01000000U));
+        Assert::AreEqual(std::string("0xffffffff"), emulator.FormatAddress(0xFFFFFFFFU));
+
+        emulator.AddMemoryBlock(1, 0x80, nullptr, nullptr); // total size = 0x100
+        Assert::AreEqual(std::string("0x0000"), emulator.FormatAddress(0x00000000U));
+        Assert::AreEqual(std::string("0x00ff"), emulator.FormatAddress(0x000000FFU));
+        Assert::AreEqual(std::string("0x0100"), emulator.FormatAddress(0x00000100U));
+        Assert::AreEqual(std::string("0xffff"), emulator.FormatAddress(0x0000FFFFU));
+        Assert::AreEqual(std::string("0x010000"), emulator.FormatAddress(0x00010000U));
+        Assert::AreEqual(std::string("0xffffff"), emulator.FormatAddress(0x00FFFFFFU));
+        Assert::AreEqual(std::string("0x01000000"), emulator.FormatAddress(0x01000000U));
+        Assert::AreEqual(std::string("0xffffffff"), emulator.FormatAddress(0xFFFFFFFFU));
+
+        emulator.AddMemoryBlock(2, 0xFF00, nullptr, nullptr); // total size = 0x10000
+        Assert::AreEqual(std::string("0x0000"), emulator.FormatAddress(0x00000000U));
+        Assert::AreEqual(std::string("0x00ff"), emulator.FormatAddress(0x000000FFU));
+        Assert::AreEqual(std::string("0x0100"), emulator.FormatAddress(0x00000100U));
+        Assert::AreEqual(std::string("0xffff"), emulator.FormatAddress(0x0000FFFFU));
+        Assert::AreEqual(std::string("0x010000"), emulator.FormatAddress(0x00010000U));
+        Assert::AreEqual(std::string("0xffffff"), emulator.FormatAddress(0x00FFFFFFU));
+        Assert::AreEqual(std::string("0x01000000"), emulator.FormatAddress(0x01000000U));
+        Assert::AreEqual(std::string("0xffffffff"), emulator.FormatAddress(0xFFFFFFFFU));
+
+        emulator.AddMemoryBlock(3, 1, nullptr, nullptr); // total size = 0x10001
+        Assert::AreEqual(std::string("0x000000"), emulator.FormatAddress(0x00000000U));
+        Assert::AreEqual(std::string("0x0000ff"), emulator.FormatAddress(0x000000FFU));
+        Assert::AreEqual(std::string("0x000100"), emulator.FormatAddress(0x00000100U));
+        Assert::AreEqual(std::string("0x00ffff"), emulator.FormatAddress(0x0000FFFFU));
+        Assert::AreEqual(std::string("0x010000"), emulator.FormatAddress(0x00010000U));
+        Assert::AreEqual(std::string("0xffffff"), emulator.FormatAddress(0x00FFFFFFU));
+        Assert::AreEqual(std::string("0x01000000"), emulator.FormatAddress(0x01000000U));
+        Assert::AreEqual(std::string("0xffffffff"), emulator.FormatAddress(0xFFFFFFFFU));
+
+        emulator.AddMemoryBlock(4, 0xFEFFFF, nullptr, nullptr); // total size = 0x1000000
+        Assert::AreEqual(std::string("0x000000"), emulator.FormatAddress(0x00000000U));
+        Assert::AreEqual(std::string("0x0000ff"), emulator.FormatAddress(0x000000FFU));
+        Assert::AreEqual(std::string("0x000100"), emulator.FormatAddress(0x00000100U));
+        Assert::AreEqual(std::string("0x00ffff"), emulator.FormatAddress(0x0000FFFFU));
+        Assert::AreEqual(std::string("0x010000"), emulator.FormatAddress(0x00010000U));
+        Assert::AreEqual(std::string("0xffffff"), emulator.FormatAddress(0x00FFFFFFU));
+        Assert::AreEqual(std::string("0x01000000"), emulator.FormatAddress(0x01000000U));
+        Assert::AreEqual(std::string("0xffffffff"), emulator.FormatAddress(0xFFFFFFFFU));
+
+        emulator.AddMemoryBlock(5, 1, nullptr, nullptr); // total size = 0x1000001
+        Assert::AreEqual(std::string("0x00000000"), emulator.FormatAddress(0x00000000U));
+        Assert::AreEqual(std::string("0x000000ff"), emulator.FormatAddress(0x000000FFU));
+        Assert::AreEqual(std::string("0x00000100"), emulator.FormatAddress(0x00000100U));
+        Assert::AreEqual(std::string("0x0000ffff"), emulator.FormatAddress(0x0000FFFFU));
+        Assert::AreEqual(std::string("0x00010000"), emulator.FormatAddress(0x00010000U));
+        Assert::AreEqual(std::string("0x00ffffff"), emulator.FormatAddress(0x00FFFFFFU));
+        Assert::AreEqual(std::string("0x01000000"), emulator.FormatAddress(0x01000000U));
+        Assert::AreEqual(std::string("0xffffffff"), emulator.FormatAddress(0xFFFFFFFFU));
+    }
 };
 
 std::array<uint8_t, 64> EmulatorContext_Tests::memory;
diff --git a/tests/services/SearchResults_Tests.cpp b/tests/services/SearchResults_Tests.cpp
index 951303ec..763e7629 100644
--- a/tests/services/SearchResults_Tests.cpp
+++ b/tests/services/SearchResults_Tests.cpp
@@ -1048,6 +1048,40 @@ public:
         Assert::IsTrue(results2.MatchesFilter(results1, result));
     }
 
+    TEST_METHOD(TestInitializeFromResultsThirtyTwoBitAlignedOffset)
+    {
+        std::array<unsigned char, 12> memory{ 0x00, 0x12, 0x34, 0xAB, 0x56, 0xCD, 0x44, 0x20, 0x11, 0x22, 0x33, 0x44 };
+        ra::data::context::mocks::MockEmulatorContext mockEmulatorContext;
+        mockEmulatorContext.MockMemory(memory);
+
+        SearchResults results1;
+        results1.Initialize(4U, 12U, ra::services::SearchType::ThirtyTwoBitAligned);
+        Assert::AreEqual({ 2U }, results1.MatchingAddressCount());
+
+        memory.at(6) = 0x55;
+        SearchResults results2;
+        results2.Initialize(results1, ComparisonType::Equals, ra::services::SearchFilterType::LastKnownValue, L"");
+
+        Assert::AreEqual({ 1U }, results2.MatchingAddressCount());
+        Assert::IsFalse(results2.ContainsAddress(0U));
+        Assert::IsFalse(results2.ContainsAddress(4U));
+        Assert::IsTrue(results2.ContainsAddress(8U));
+
+        SearchResults results;
+        results.Initialize(results1, results2, ComparisonType::Equals, ra::services::SearchFilterType::LastKnownValue, L"");
+
+        Assert::AreEqual({ 1U }, results.MatchingAddressCount());
+        Assert::IsFalse(results.ContainsAddress(0U));
+        Assert::IsFalse(results.ContainsAddress(4U));
+        Assert::IsTrue(results.ContainsAddress(8U));
+
+        SearchResults::Result result;
+        Assert::IsTrue(results.GetMatchingAddress(0U, result));
+        Assert::AreEqual(8U, result.nAddress);
+        Assert::AreEqual(MemSize::ThirtyTwoBit, result.nSize);
+        Assert::AreEqual(0x44332211U, result.nValue);
+    }
+
     TEST_METHOD(TestInitializeFromResultsSixteenBitBigEndianNotEqualPrevious)
     {
         std::array<unsigned char, 5> memory{ 0x00, 0x12, 0x34, 0xAB, 0x56 };
@@ -2046,6 +2080,19 @@ public:
         Assert::IsTrue(results2.MatchesFilter(results1, result2));  // 18 >= 18 (18) ?
         Assert::IsTrue(results2.MatchesFilter(results1, result3));  // 24 >= 18 (24) ?
     }
+
+    TEST_METHOD(TestInitializeFromMemoryAsciiEmpty)
+    {
+        ra::data::context::mocks::MockEmulatorContext mockEmulatorContext;
+        Assert::AreEqual({0U}, mockEmulatorContext.TotalMemorySize());
+
+        SearchResults results;
+        results.Initialize(0U, 16, ra::services::SearchType::AsciiText);
+
+        Assert::AreEqual({0U}, results.MatchingAddressCount());
+
+        Assert::AreEqual(std::wstring(), results.GetFormattedValue(0U, MemSize::Text));
+    }
 };
 
 } // namespace tests
diff --git a/tests/ui/viewmodels/MemoryBookmarksViewModel_Tests.cpp b/tests/ui/viewmodels/MemoryBookmarksViewModel_Tests.cpp
index 03da5454..d3cabc6b 100644
--- a/tests/ui/viewmodels/MemoryBookmarksViewModel_Tests.cpp
+++ b/tests/ui/viewmodels/MemoryBookmarksViewModel_Tests.cpp
@@ -84,7 +84,7 @@ public:
 
         Assert::AreEqual({ 0U }, bookmarks.Bookmarks().Count());
 
-        Assert::AreEqual({ 12U }, bookmarks.Sizes().Count());
+        Assert::AreEqual({ 13U }, bookmarks.Sizes().Count());
         Assert::AreEqual((int)MemSize::EightBit, bookmarks.Sizes().GetItemAt(0)->GetId());
         Assert::AreEqual(std::wstring(L" 8-bit"), bookmarks.Sizes().GetItemAt(0)->GetLabel());
         Assert::AreEqual((int)MemSize::SixteenBit, bookmarks.Sizes().GetItemAt(1)->GetId());
@@ -109,6 +109,8 @@ public:
         Assert::AreEqual(std::wstring(L"Float"), bookmarks.Sizes().GetItemAt(10)->GetLabel());
         Assert::AreEqual((int)MemSize::MBF32, bookmarks.Sizes().GetItemAt(11)->GetId());
         Assert::AreEqual(std::wstring(L"MBF32"), bookmarks.Sizes().GetItemAt(11)->GetLabel());
+        Assert::AreEqual((int)MemSize::Text, bookmarks.Sizes().GetItemAt(12)->GetId());
+        Assert::AreEqual(std::wstring(L"ASCII"), bookmarks.Sizes().GetItemAt(12)->GetLabel());
 
         Assert::AreEqual({ 2U }, bookmarks.Formats().Count());
         Assert::AreEqual((int)MemFormat::Hex, bookmarks.Formats().GetItemAt(0)->GetId());
@@ -1227,10 +1229,17 @@ public:
             const auto nSize = ra::itoe<MemSize>(bookmarks.Sizes().GetItemAt(nIndex)->GetId());
             bookmark.SetSize(nSize);
 
-            if (nSize == MemSize::BitCount)
-                Assert::IsTrue(bookmark.IsReadOnly());
-            else
-                Assert::IsFalse(bookmark.IsReadOnly());
+            switch (nSize)
+            {
+                case MemSize::BitCount:
+                case MemSize::Text:
+                    Assert::IsTrue(bookmark.IsReadOnly());
+                    break;
+
+                default:
+                    Assert::IsFalse(bookmark.IsReadOnly());
+                    break;
+            }
         }
     }
 
@@ -1270,6 +1279,53 @@ public:
         Assert::AreEqual(2U, bookmark.GetChanges());
     }
 
+    TEST_METHOD(TestAddBookmarkASCII)
+    {
+        MemoryBookmarksViewModelHarness bookmarks;
+
+        std::array<uint8_t, 64> memory = {};
+        bookmarks.mockEmulatorContext.MockMemory(memory);
+
+        bookmarks.AddBookmark(1U, MemSize::Text);
+
+        Assert::AreEqual({ 1U }, bookmarks.Bookmarks().Count());
+        const auto& bookmark = *bookmarks.Bookmarks().GetItemAt(0);
+        Assert::AreEqual(std::wstring(L""), bookmark.GetDescription());
+        Assert::AreEqual(1U, bookmark.GetAddress());
+        Assert::AreEqual(MemSize::Text, bookmark.GetSize());
+        Assert::AreEqual(MemoryBookmarksViewModel::BookmarkBehavior::None, bookmark.GetBehavior());
+        Assert::AreEqual(std::wstring(L""), bookmark.GetCurrentValue());
+        Assert::AreEqual(std::wstring(L""), bookmark.GetPreviousValue());
+        Assert::AreEqual(0U, bookmark.GetChanges());
+
+        memcpy(&memory.at(1), "Test", 5);
+        bookmarks.DoFrame();
+        Assert::AreEqual(std::wstring(L"Test"), bookmark.GetCurrentValue());
+        Assert::AreEqual(std::wstring(L""), bookmark.GetPreviousValue());
+        Assert::AreEqual(1U, bookmark.GetChanges());
+
+        bookmarks.mockEmulatorContext.WriteMemoryByte(3U, 0x78);
+        Assert::AreEqual(std::wstring(L"Text"), bookmark.GetCurrentValue());
+        Assert::AreEqual(std::wstring(L"Test"), bookmark.GetPreviousValue());
+        Assert::AreEqual(2U, bookmark.GetChanges());
+
+        memcpy(&memory.at(1), "EightCharacters", 15);
+        bookmarks.DoFrame();
+        Assert::AreEqual(std::wstring(L"EightCha"), bookmark.GetCurrentValue());
+        Assert::AreEqual(std::wstring(L"Text"), bookmark.GetPreviousValue());
+        Assert::AreEqual(3U, bookmark.GetChanges());
+
+        bookmarks.mockEmulatorContext.WriteMemoryByte(9U, 0x73);
+        Assert::AreEqual(std::wstring(L"EightCha"), bookmark.GetCurrentValue());
+        Assert::AreEqual(std::wstring(L"Text"), bookmark.GetPreviousValue());
+        Assert::AreEqual(3U, bookmark.GetChanges());
+
+        bookmarks.mockEmulatorContext.WriteMemoryByte(8U, 0x73);
+        Assert::AreEqual(std::wstring(L"EightChs"), bookmark.GetCurrentValue());
+        Assert::AreEqual(std::wstring(L"EightCha"), bookmark.GetPreviousValue());
+        Assert::AreEqual(4U, bookmark.GetChanges());
+    }
+
     TEST_METHOD(TestSetCurrentValue)
     {
         MemoryBookmarksViewModelHarness bookmarks;
diff --git a/tests/ui/viewmodels/MemorySearchViewModel_Tests.cpp b/tests/ui/viewmodels/MemorySearchViewModel_Tests.cpp
index e65e8fcb..9f0044b7 100644
--- a/tests/ui/viewmodels/MemorySearchViewModel_Tests.cpp
+++ b/tests/ui/viewmodels/MemorySearchViewModel_Tests.cpp
@@ -1276,21 +1276,13 @@ public:
         Assert::AreEqual({ 1U }, search.Results().Count());
         search.Results().GetItemAt(0)->SetSelected(true);
 
-        bool bSawDialog = false;
-        search.mockDesktop.ExpectWindow<ra::ui::viewmodels::MessageBoxViewModel>([&bSawDialog](ra::ui::viewmodels::MessageBoxViewModel& vmMessageBox)
-        {
-            bSawDialog = true;
-            Assert::AreEqual(std::wstring(L"Text bookmarks are not supported"), vmMessageBox.GetMessage());
-            return ra::ui::DialogResult::OK;
-        });
-
         search.BookmarkSelected();
-        Assert::IsTrue(bSawDialog);
         Assert::AreEqual({ 1U }, search.Results().Count());
 
         const auto& pBookmarks = search.mockWindowManager.MemoryBookmarks.Bookmarks();
-        Assert::AreEqual({ 0U }, pBookmarks.Count());
-
+        Assert::AreEqual({ 1U }, pBookmarks.Count());
+        Assert::AreEqual({ 8U }, pBookmarks.GetItemAt(0)->GetAddress());
+        Assert::AreEqual(MemSize::Text, pBookmarks.GetItemAt(0)->GetSize());
         Assert::IsTrue(search.Results().GetItemAt(0)->IsSelected());
     }