From 89988581f33f5e517f477182935cd1770c258cd6 Mon Sep 17 00:00:00 2001 From: Marc Van de Wiele Date: Mon, 22 Apr 2024 03:43:56 +0200 Subject: [PATCH 1/4] Add shift-click feature on memory view to put focus on pointed address Enhanced user experience by providing a convenient way to navigate to specific pointed memory addresses --- src/ui/viewmodels/MemoryViewerViewModel.cpp | 10 ++++++++++ src/ui/viewmodels/MemoryViewerViewModel.hh | 1 + .../win32/bindings/MemoryViewerControlBinding.cpp | 15 ++++++++++++++- .../win32/bindings/MemoryViewerControlBinding.hh | 1 + 4 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/ui/viewmodels/MemoryViewerViewModel.cpp b/src/ui/viewmodels/MemoryViewerViewModel.cpp index b44ca9e8e..17e0f0c75 100644 --- a/src/ui/viewmodels/MemoryViewerViewModel.cpp +++ b/src/ui/viewmodels/MemoryViewerViewModel.cpp @@ -4,6 +4,7 @@ #include "data\context\EmulatorContext.hh" #include "data\context\GameContext.hh" +#include "data\context\ConsoleContext.hh" #include "ui\EditorTheme.hh" #include "ui\viewmodels\WindowManager.hh" @@ -895,6 +896,15 @@ void MemoryViewerViewModel::OnClick(int nX, int nY) } } +void MemoryViewerViewModel::OnShiftClick(ra::ByteAddress nAddress) +{ + const auto& pConsoleContext = ra::services::ServiceLocator::Get(); + const auto nConvertedAddress = pConsoleContext.ByteAddressFromRealAddress(nAddress); + if (nConvertedAddress != 0xFFFFFFFF) + nAddress = nConvertedAddress; + SetAddress(nAddress); +} + void MemoryViewerViewModel::OnResized(int nWidth, int nHeight) { if (s_pFontSurface == nullptr) diff --git a/src/ui/viewmodels/MemoryViewerViewModel.hh b/src/ui/viewmodels/MemoryViewerViewModel.hh index f295118cc..bdc6c4342 100644 --- a/src/ui/viewmodels/MemoryViewerViewModel.hh +++ b/src/ui/viewmodels/MemoryViewerViewModel.hh @@ -161,6 +161,7 @@ public: void SetSize(MemSize value) { SetValue(SizeProperty, ra::etoi(value)); } void OnClick(int nX, int nY); + void OnShiftClick(ByteAddress address); void OnResized(int nWidth, int nHeight); bool OnChar(char c); void OnGotFocus(); diff --git a/src/ui/win32/bindings/MemoryViewerControlBinding.cpp b/src/ui/win32/bindings/MemoryViewerControlBinding.cpp index 9cf8128d0..9e47cb8ef 100644 --- a/src/ui/win32/bindings/MemoryViewerControlBinding.cpp +++ b/src/ui/win32/bindings/MemoryViewerControlBinding.cpp @@ -16,6 +16,7 @@ constexpr UINT WM_USER_INVALIDATE = WM_USER + 1; INT_PTR CALLBACK MemoryViewerControlBinding::WndProc(HWND hControl, UINT uMsg, WPARAM wParam, LPARAM lParam) { + const bool bShiftHeld = (GetKeyState(VK_SHIFT) < 0); switch (uMsg) { case WM_PAINT: @@ -34,7 +35,10 @@ INT_PTR CALLBACK MemoryViewerControlBinding::WndProc(HWND hControl, UINT uMsg, W return FALSE; case WM_LBUTTONUP: - OnClick({ GET_X_LPARAM(lParam) - MEMVIEW_MARGIN, GET_Y_LPARAM(lParam) - MEMVIEW_MARGIN }); + if (bShiftHeld) + OnShiftClick({GET_X_LPARAM(lParam) - MEMVIEW_MARGIN, GET_Y_LPARAM(lParam) - MEMVIEW_MARGIN}); + else + OnClick({ GET_X_LPARAM(lParam) - MEMVIEW_MARGIN, GET_Y_LPARAM(lParam) - MEMVIEW_MARGIN }); return FALSE; case WM_KEYDOWN: @@ -264,6 +268,15 @@ void MemoryViewerControlBinding::OnClick(POINT point) Invalidate(); } +void MemoryViewerControlBinding::OnShiftClick(POINT point) +{ + OnClick(point); + const auto& pEmulatorContext = ra::services::ServiceLocator::Get(); + ra::ByteAddress nAddress = + pEmulatorContext.ReadMemory(m_pViewModel.GetAddress(), m_pViewModel.GetSize()) & 0xFFFFFF; + m_pViewModel.OnShiftClick(nAddress); +} + void MemoryViewerControlBinding::OnGotFocus() { m_pViewModel.OnGotFocus(); diff --git a/src/ui/win32/bindings/MemoryViewerControlBinding.hh b/src/ui/win32/bindings/MemoryViewerControlBinding.hh index 627a0ba5c..f3347c4ff 100644 --- a/src/ui/win32/bindings/MemoryViewerControlBinding.hh +++ b/src/ui/win32/bindings/MemoryViewerControlBinding.hh @@ -40,6 +40,7 @@ public: void ScrollDown(); void OnClick(POINT point); + void OnShiftClick(POINT point); bool OnKeyDown(UINT nChar); bool OnEditInput(UINT c); From 2fc324b649b704c08f64ed93a5db63785756ffa1 Mon Sep 17 00:00:00 2001 From: Marc Van de Wiele Date: Sat, 27 Apr 2024 00:55:54 +0200 Subject: [PATCH 2/4] Refactor pointer follower feature following review - Moved logic to ViewModel - Removed 'OnShiftClick' function in ControlBinding and simply add a keystate check on the 'OnClick' one - Removed unecessary mask - Unit Test added --- src/ui/viewmodels/MemoryViewerViewModel.cpp | 8 +- src/ui/viewmodels/MemoryViewerViewModel.hh | 2 +- .../bindings/MemoryViewerControlBinding.cpp | 20 ++--- .../bindings/MemoryViewerControlBinding.hh | 1 - .../MemoryViewerViewModel_Tests.cpp | 74 +++++++++++++++++++ 5 files changed, 87 insertions(+), 18 deletions(-) diff --git a/src/ui/viewmodels/MemoryViewerViewModel.cpp b/src/ui/viewmodels/MemoryViewerViewModel.cpp index 17e0f0c75..4b2d7c14e 100644 --- a/src/ui/viewmodels/MemoryViewerViewModel.cpp +++ b/src/ui/viewmodels/MemoryViewerViewModel.cpp @@ -896,12 +896,18 @@ void MemoryViewerViewModel::OnClick(int nX, int nY) } } -void MemoryViewerViewModel::OnShiftClick(ra::ByteAddress nAddress) +void MemoryViewerViewModel::OnShiftClick(int nX, int nY) { + const auto& pEmulatorContext = ra::services::ServiceLocator::Get(); const auto& pConsoleContext = ra::services::ServiceLocator::Get(); + + OnClick(nX, nY); + + ra::ByteAddress nAddress = pEmulatorContext.ReadMemory(GetAddress(), GetSize()); const auto nConvertedAddress = pConsoleContext.ByteAddressFromRealAddress(nAddress); if (nConvertedAddress != 0xFFFFFFFF) nAddress = nConvertedAddress; + SetAddress(nAddress); } diff --git a/src/ui/viewmodels/MemoryViewerViewModel.hh b/src/ui/viewmodels/MemoryViewerViewModel.hh index bdc6c4342..642f81bab 100644 --- a/src/ui/viewmodels/MemoryViewerViewModel.hh +++ b/src/ui/viewmodels/MemoryViewerViewModel.hh @@ -161,7 +161,7 @@ public: void SetSize(MemSize value) { SetValue(SizeProperty, ra::etoi(value)); } void OnClick(int nX, int nY); - void OnShiftClick(ByteAddress address); + void OnShiftClick(int nX, int nY); void OnResized(int nWidth, int nHeight); bool OnChar(char c); void OnGotFocus(); diff --git a/src/ui/win32/bindings/MemoryViewerControlBinding.cpp b/src/ui/win32/bindings/MemoryViewerControlBinding.cpp index 9e47cb8ef..fe69e73e2 100644 --- a/src/ui/win32/bindings/MemoryViewerControlBinding.cpp +++ b/src/ui/win32/bindings/MemoryViewerControlBinding.cpp @@ -16,7 +16,6 @@ constexpr UINT WM_USER_INVALIDATE = WM_USER + 1; INT_PTR CALLBACK MemoryViewerControlBinding::WndProc(HWND hControl, UINT uMsg, WPARAM wParam, LPARAM lParam) { - const bool bShiftHeld = (GetKeyState(VK_SHIFT) < 0); switch (uMsg) { case WM_PAINT: @@ -35,10 +34,7 @@ INT_PTR CALLBACK MemoryViewerControlBinding::WndProc(HWND hControl, UINT uMsg, W return FALSE; case WM_LBUTTONUP: - if (bShiftHeld) - OnShiftClick({GET_X_LPARAM(lParam) - MEMVIEW_MARGIN, GET_Y_LPARAM(lParam) - MEMVIEW_MARGIN}); - else - OnClick({ GET_X_LPARAM(lParam) - MEMVIEW_MARGIN, GET_Y_LPARAM(lParam) - MEMVIEW_MARGIN }); + OnClick({ GET_X_LPARAM(lParam) - MEMVIEW_MARGIN, GET_Y_LPARAM(lParam) - MEMVIEW_MARGIN }); return FALSE; case WM_KEYDOWN: @@ -260,7 +256,10 @@ void MemoryViewerControlBinding::OnClick(POINT point) // multiple properties may change while typing, we'll do a single Invalidate after we're done m_bSuppressMemoryViewerInvalidate = true; - m_pViewModel.OnClick(point.x, point.y); + if (GetKeyState(VK_SHIFT) < 0) + m_pViewModel.OnShiftClick(point.x, point.y); + else + m_pViewModel.OnClick(point.x, point.y); m_bSuppressMemoryViewerInvalidate = false; SetFocus(m_hWnd); @@ -268,15 +267,6 @@ void MemoryViewerControlBinding::OnClick(POINT point) Invalidate(); } -void MemoryViewerControlBinding::OnShiftClick(POINT point) -{ - OnClick(point); - const auto& pEmulatorContext = ra::services::ServiceLocator::Get(); - ra::ByteAddress nAddress = - pEmulatorContext.ReadMemory(m_pViewModel.GetAddress(), m_pViewModel.GetSize()) & 0xFFFFFF; - m_pViewModel.OnShiftClick(nAddress); -} - void MemoryViewerControlBinding::OnGotFocus() { m_pViewModel.OnGotFocus(); diff --git a/src/ui/win32/bindings/MemoryViewerControlBinding.hh b/src/ui/win32/bindings/MemoryViewerControlBinding.hh index f3347c4ff..627a0ba5c 100644 --- a/src/ui/win32/bindings/MemoryViewerControlBinding.hh +++ b/src/ui/win32/bindings/MemoryViewerControlBinding.hh @@ -40,7 +40,6 @@ public: void ScrollDown(); void OnClick(POINT point); - void OnShiftClick(POINT point); bool OnKeyDown(UINT nChar); bool OnEditInput(UINT c); diff --git a/tests/ui/viewmodels/MemoryViewerViewModel_Tests.cpp b/tests/ui/viewmodels/MemoryViewerViewModel_Tests.cpp index b900ee33b..7d494aad9 100644 --- a/tests/ui/viewmodels/MemoryViewerViewModel_Tests.cpp +++ b/tests/ui/viewmodels/MemoryViewerViewModel_Tests.cpp @@ -7,6 +7,7 @@ #include "tests\RA_UnitTestHelpers.h" #include "tests\mocks\MockEmulatorContext.hh" #include "tests\mocks\MockGameContext.hh" +#include "tests\mocks\MockConsoleContext.hh" #include "tests\mocks\MockWindowManager.hh" #undef GetMessage @@ -71,6 +72,9 @@ TEST_CLASS(MemoryViewerViewModel_Tests) bool IsReadOnly() const noexcept { return m_bReadOnly; } void SetReadOnly(bool value) noexcept { m_bReadOnly = value; } + bool IsAddressFixed() const noexcept { return m_bAddressFixed; } + void SetAddressFixed(bool value) noexcept { m_bAddressFixed = value; } + void InitializeMemory(size_t nSize) { unsigned char* pBytes = new unsigned char[nSize]; @@ -1918,8 +1922,78 @@ TEST_CLASS(MemoryViewerViewModel_Tests) Assert::IsTrue(viewer.NeedsRedraw()); viewer.MockRender(); } + + TEST_METHOD(TestOnShiftClickEightBit) + { + ra::data::context::mocks::MockConsoleContext mockConsole(PlayStation, L"Playstation"); + + MemoryViewerViewModelHarness viewer; + viewer.InitializeMemory(128); // 8 rows of 16 bytes + viewer.mockEmulatorContext.WriteMemoryByte(0U, 0x20); + viewer.mockEmulatorContext.WriteMemoryByte(1U, 0xff); + viewer.mockEmulatorContext.WriteMemoryByte(2U, 0x0); + + Assert::AreEqual(MemSize::EightBit, viewer.GetSize()); + Assert::AreEqual({ 0U }, viewer.GetAddress()); + Assert::AreEqual({ 0U }, viewer.GetSelectedNibble()); + + // If fixed address, ignore + viewer.SetAddressFixed(true); + viewer.OnShiftClick(10 * CHAR_WIDTH, CHAR_HEIGHT + 4); + Assert::AreEqual({0x0U}, viewer.GetAddress()); + + // If not fixed address and Shift click on the first byte containing 0x20 should lead to address 0x20 + viewer.SetAddressFixed(false); + viewer.OnShiftClick(10 * CHAR_WIDTH, CHAR_HEIGHT + 4); + Assert::AreEqual({ 0x20U }, viewer.GetAddress()); + + // Shift click on the second byte containing 0xFF should lead to address 0x7F as 0xFF is bigger than the last + // address in memory + viewer.OnShiftClick(13 * CHAR_WIDTH, CHAR_HEIGHT + 4); + Assert::AreEqual({0x7FU}, viewer.GetAddress()); + + // Shift click on the third byte containing 0x0 should lead back to address 0x0 + viewer.OnShiftClick(16 * CHAR_WIDTH, CHAR_HEIGHT + 4); + Assert::AreEqual({0x0U}, viewer.GetAddress()); + } + + TEST_METHOD(TestOnShiftClickSixTeenBit) + { + ra::data::context::mocks::MockConsoleContext mockConsole(PlayStation, L"Playstation"); + + MemoryViewerViewModelHarness viewer; + viewer.InitializeMemory(512); // 32 rows of 16 bytes + viewer.SetSize(MemSize::SixteenBit); + viewer.mockEmulatorContext.WriteMemory(0U, MemSize::SixteenBit, 0x20); + viewer.mockEmulatorContext.WriteMemory(2U, MemSize::SixteenBit, 0xff); + viewer.mockEmulatorContext.WriteMemory(4U, MemSize::SixteenBit, 0xffff); + + Assert::AreEqual(MemSize::SixteenBit, viewer.GetSize()); + Assert::AreEqual({0U}, viewer.GetAddress()); + Assert::AreEqual({0U}, viewer.GetSelectedNibble()); + + // If fixed address, ignore + viewer.SetAddressFixed(true); + viewer.OnShiftClick(10 * CHAR_WIDTH, CHAR_HEIGHT + 4); + Assert::AreEqual({0x0U}, viewer.GetAddress()); + + // If not fixed address and Shift click on the first word containing 0x20 should lead to address 0x20 + viewer.SetAddressFixed(false); + viewer.OnShiftClick(10 * CHAR_WIDTH, CHAR_HEIGHT + 4); + Assert::AreEqual({0x20U}, viewer.GetAddress()); + + // Shift click on the second word containing 0x40 should lead to address 0xFF + viewer.OnShiftClick(15 * CHAR_WIDTH, CHAR_HEIGHT + 4); + Assert::AreEqual({0xFFU}, viewer.GetAddress()); + + // Shift click on the third word containing 0xFFFF should lead to address 0x1FF as 0xFFFF is bigger than the + // last address in memory + viewer.OnShiftClick(19 * CHAR_WIDTH, CHAR_HEIGHT + 4); + Assert::AreEqual({0x1FFU}, viewer.GetAddress()); + } }; + } // namespace tests } // namespace viewmodels } // namespace ui From 8312f48bc92b39fa3db7a84df8a2e9b13865cd4f Mon Sep 17 00:00:00 2001 From: Marc Van de Wiele Date: Sun, 28 Apr 2024 17:56:25 +0200 Subject: [PATCH 3/4] Update following review - Improve unit tests - Add a mask to handle complex pointer like PSX --- src/ui/viewmodels/MemoryViewerViewModel.cpp | 18 +++- .../MemoryViewerViewModel_Tests.cpp | 94 ++++++++++++++----- 2 files changed, 82 insertions(+), 30 deletions(-) diff --git a/src/ui/viewmodels/MemoryViewerViewModel.cpp b/src/ui/viewmodels/MemoryViewerViewModel.cpp index 4b2d7c14e..e4d6ab523 100644 --- a/src/ui/viewmodels/MemoryViewerViewModel.cpp +++ b/src/ui/viewmodels/MemoryViewerViewModel.cpp @@ -903,12 +903,22 @@ void MemoryViewerViewModel::OnShiftClick(int nX, int nY) OnClick(nX, nY); - ra::ByteAddress nAddress = pEmulatorContext.ReadMemory(GetAddress(), GetSize()); + // We need to create a mask based on the last address of the current console to be able to point to more complexe pointer format + // like on Playstation on which pointers are represented as a 24-bit pointer prefixed by 80. + std::wstring sMaxAddress = ra::data::MemSizeFormat(pConsoleContext.MaxAddress(), MemSize::ThirtyTwoBit, MemFormat::Hex); + std::wstring sMask; + + for (wchar_t ch : sMaxAddress) + if (ch != L'0') + sMask.push_back(L'F'); + + auto nMask = std::stoi(sMask, 0, 16); + ra::ByteAddress nAddress = (pEmulatorContext.ReadMemory(GetAddress(), GetSize())) & nMask; const auto nConvertedAddress = pConsoleContext.ByteAddressFromRealAddress(nAddress); if (nConvertedAddress != 0xFFFFFFFF) - nAddress = nConvertedAddress; - - SetAddress(nAddress); + { + SetAddress(nConvertedAddress); + } } void MemoryViewerViewModel::OnResized(int nWidth, int nHeight) diff --git a/tests/ui/viewmodels/MemoryViewerViewModel_Tests.cpp b/tests/ui/viewmodels/MemoryViewerViewModel_Tests.cpp index 7d494aad9..c6e370ad0 100644 --- a/tests/ui/viewmodels/MemoryViewerViewModel_Tests.cpp +++ b/tests/ui/viewmodels/MemoryViewerViewModel_Tests.cpp @@ -1925,19 +1925,17 @@ TEST_CLASS(MemoryViewerViewModel_Tests) TEST_METHOD(TestOnShiftClickEightBit) { - ra::data::context::mocks::MockConsoleContext mockConsole(PlayStation, L"Playstation"); + ra::data::context::mocks::MockConsoleContext mockConsole(PlayStation, L"PlayStation"); MemoryViewerViewModelHarness viewer; - viewer.InitializeMemory(128); // 8 rows of 16 bytes - viewer.mockEmulatorContext.WriteMemoryByte(0U, 0x20); - viewer.mockEmulatorContext.WriteMemoryByte(1U, 0xff); - viewer.mockEmulatorContext.WriteMemoryByte(2U, 0x0); + viewer.InitializeMemory(0xFFFF); Assert::AreEqual(MemSize::EightBit, viewer.GetSize()); Assert::AreEqual({ 0U }, viewer.GetAddress()); Assert::AreEqual({ 0U }, viewer.GetSelectedNibble()); // If fixed address, ignore + viewer.mockEmulatorContext.WriteMemoryByte(0U, 0x20); viewer.SetAddressFixed(true); viewer.OnShiftClick(10 * CHAR_WIDTH, CHAR_HEIGHT + 4); Assert::AreEqual({0x0U}, viewer.GetAddress()); @@ -1947,26 +1945,20 @@ TEST_CLASS(MemoryViewerViewModel_Tests) viewer.OnShiftClick(10 * CHAR_WIDTH, CHAR_HEIGHT + 4); Assert::AreEqual({ 0x20U }, viewer.GetAddress()); - // Shift click on the second byte containing 0xFF should lead to address 0x7F as 0xFF is bigger than the last - // address in memory - viewer.OnShiftClick(13 * CHAR_WIDTH, CHAR_HEIGHT + 4); - Assert::AreEqual({0x7FU}, viewer.GetAddress()); - - // Shift click on the third byte containing 0x0 should lead back to address 0x0 - viewer.OnShiftClick(16 * CHAR_WIDTH, CHAR_HEIGHT + 4); - Assert::AreEqual({0x0U}, viewer.GetAddress()); + // Shift click on the second byte containing 0x0 should do nothing + viewer.SetAddress(0); + viewer.mockEmulatorContext.WriteMemoryByte(1U, 0x0); + viewer.OnShiftClick(15 * CHAR_WIDTH, CHAR_HEIGHT + 4); + Assert::AreEqual({0x1U}, viewer.GetAddress()); } - TEST_METHOD(TestOnShiftClickSixTeenBit) + TEST_METHOD(TestOnShiftClickSixteenBit) { - ra::data::context::mocks::MockConsoleContext mockConsole(PlayStation, L"Playstation"); + ra::data::context::mocks::MockConsoleContext mockConsole(PlayStation, L"PlayStation"); MemoryViewerViewModelHarness viewer; - viewer.InitializeMemory(512); // 32 rows of 16 bytes + viewer.InitializeMemory(0xFFFF); viewer.SetSize(MemSize::SixteenBit); - viewer.mockEmulatorContext.WriteMemory(0U, MemSize::SixteenBit, 0x20); - viewer.mockEmulatorContext.WriteMemory(2U, MemSize::SixteenBit, 0xff); - viewer.mockEmulatorContext.WriteMemory(4U, MemSize::SixteenBit, 0xffff); Assert::AreEqual(MemSize::SixteenBit, viewer.GetSize()); Assert::AreEqual({0U}, viewer.GetAddress()); @@ -1978,18 +1970,68 @@ TEST_CLASS(MemoryViewerViewModel_Tests) Assert::AreEqual({0x0U}, viewer.GetAddress()); // If not fixed address and Shift click on the first word containing 0x20 should lead to address 0x20 + viewer.SetAddress(0); viewer.SetAddressFixed(false); + viewer.mockEmulatorContext.WriteMemory(0U, MemSize::SixteenBit, 0x0020); viewer.OnShiftClick(10 * CHAR_WIDTH, CHAR_HEIGHT + 4); Assert::AreEqual({0x20U}, viewer.GetAddress()); - // Shift click on the second word containing 0x40 should lead to address 0xFF - viewer.OnShiftClick(15 * CHAR_WIDTH, CHAR_HEIGHT + 4); - Assert::AreEqual({0xFFU}, viewer.GetAddress()); + // Shift click on the first word containing 0x0140 should lead to address 0x0140 + viewer.SetAddress(0); + viewer.mockEmulatorContext.WriteMemory(0U, MemSize::SixteenBit, 0x0140); + viewer.OnShiftClick(10 * CHAR_WIDTH, CHAR_HEIGHT + 4); + Assert::AreEqual({0x0140U}, viewer.GetAddress()); + + // Shift click on the second word containing 0x0 should do nothing + viewer.SetAddress(0x4); + viewer.mockEmulatorContext.WriteMemory(4U, MemSize::SixteenBit, 0x0); + viewer.OnShiftClick(22 * CHAR_WIDTH, CHAR_HEIGHT + 4); + Assert::AreEqual({0x4U}, viewer.GetAddress()); + } + + TEST_METHOD(TestOnShiftClickThirtyTwoBit) + { + ra::data::context::mocks::MockConsoleContext mockConsole(PlayStation, L"Playstation"); + + MemoryViewerViewModelHarness viewer; + viewer.InitializeMemory(0x1FFFFF); // PSX full memory + viewer.SetSize(MemSize::ThirtyTwoBit); + + Assert::AreEqual(MemSize::ThirtyTwoBit, viewer.GetSize()); + Assert::AreEqual({0U}, viewer.GetAddress()); + Assert::AreEqual({0U}, viewer.GetSelectedNibble()); - // Shift click on the third word containing 0xFFFF should lead to address 0x1FF as 0xFFFF is bigger than the - // last address in memory - viewer.OnShiftClick(19 * CHAR_WIDTH, CHAR_HEIGHT + 4); - Assert::AreEqual({0x1FFU}, viewer.GetAddress()); + // If fixed address, ignore + viewer.SetAddressFixed(true); + viewer.OnShiftClick(10 * CHAR_WIDTH, CHAR_HEIGHT + 4); + Assert::AreEqual({0x0U}, viewer.GetAddress()); + + // If not fixed address and Shift click on the first dword containing 0x1234 should lead to address 0x1234 + viewer.SetAddress(0); + viewer.SetAddressFixed(false); + viewer.mockEmulatorContext.WriteMemory(0U, MemSize::ThirtyTwoBit, 0x1234); + viewer.OnShiftClick(10 * CHAR_WIDTH, CHAR_HEIGHT + 4); + Assert::AreEqual({0x1234U}, viewer.GetAddress()); + + // Shift click on the first dword containing 0x80012345 should lead to address 0x00012345 as the conversion keeps + // only the same number of nibbles as the Max address from console context (PSX : 1FFFFF) + viewer.SetAddress(0); + viewer.mockEmulatorContext.WriteMemory(0U, MemSize::ThirtyTwoBit, 0x00012345); + viewer.OnShiftClick(10 * CHAR_WIDTH, CHAR_HEIGHT + 4); + Assert::AreEqual({0x00012345}, viewer.GetAddress()); + + // Shift click on the second dword containing 0x0 should do nothing + viewer.SetAddress(0x8); + viewer.mockEmulatorContext.WriteMemory(8U, MemSize::ThirtyTwoBit, 0x0); + viewer.OnShiftClick(28 * CHAR_WIDTH, CHAR_HEIGHT + 4); + Assert::AreEqual({0x8U}, viewer.GetAddress()); + + // Shift click on the first dword containing 0x005FFFFF should do nothing as 0x005FFFFF exceed the + // last address in the current console context (PSX : 0x1FFFFF) and can't be converted to real address + viewer.SetAddress(0); + viewer.mockEmulatorContext.WriteMemory(0U, MemSize::ThirtyTwoBit, 0x005FFFFF); + viewer.OnShiftClick(10 * CHAR_WIDTH, CHAR_HEIGHT + 4); + Assert::AreEqual({0x0}, viewer.GetAddress()); } }; From d2e5216a9daf3f415ce6b4a3d22fb9c314563d55 Mon Sep 17 00:00:00 2001 From: Marc Van de Wiele Date: Fri, 3 May 2024 03:09:39 +0200 Subject: [PATCH 4/4] Removed pointless Mask and adjusted tests --- src/ui/viewmodels/MemoryViewerViewModel.cpp | 12 +----------- tests/ui/viewmodels/MemoryViewerViewModel_Tests.cpp | 12 ++++++------ 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/src/ui/viewmodels/MemoryViewerViewModel.cpp b/src/ui/viewmodels/MemoryViewerViewModel.cpp index e4d6ab523..61e010211 100644 --- a/src/ui/viewmodels/MemoryViewerViewModel.cpp +++ b/src/ui/viewmodels/MemoryViewerViewModel.cpp @@ -903,17 +903,7 @@ void MemoryViewerViewModel::OnShiftClick(int nX, int nY) OnClick(nX, nY); - // We need to create a mask based on the last address of the current console to be able to point to more complexe pointer format - // like on Playstation on which pointers are represented as a 24-bit pointer prefixed by 80. - std::wstring sMaxAddress = ra::data::MemSizeFormat(pConsoleContext.MaxAddress(), MemSize::ThirtyTwoBit, MemFormat::Hex); - std::wstring sMask; - - for (wchar_t ch : sMaxAddress) - if (ch != L'0') - sMask.push_back(L'F'); - - auto nMask = std::stoi(sMask, 0, 16); - ra::ByteAddress nAddress = (pEmulatorContext.ReadMemory(GetAddress(), GetSize())) & nMask; + ra::ByteAddress nAddress = (pEmulatorContext.ReadMemory(GetAddress(), GetSize())); const auto nConvertedAddress = pConsoleContext.ByteAddressFromRealAddress(nAddress); if (nConvertedAddress != 0xFFFFFFFF) { diff --git a/tests/ui/viewmodels/MemoryViewerViewModel_Tests.cpp b/tests/ui/viewmodels/MemoryViewerViewModel_Tests.cpp index c6e370ad0..26aa60e4c 100644 --- a/tests/ui/viewmodels/MemoryViewerViewModel_Tests.cpp +++ b/tests/ui/viewmodels/MemoryViewerViewModel_Tests.cpp @@ -1957,7 +1957,7 @@ TEST_CLASS(MemoryViewerViewModel_Tests) ra::data::context::mocks::MockConsoleContext mockConsole(PlayStation, L"PlayStation"); MemoryViewerViewModelHarness viewer; - viewer.InitializeMemory(0xFFFF); + viewer.InitializeMemory(0x1FFF); viewer.SetSize(MemSize::SixteenBit); Assert::AreEqual(MemSize::SixteenBit, viewer.GetSize()); @@ -1994,7 +1994,7 @@ TEST_CLASS(MemoryViewerViewModel_Tests) ra::data::context::mocks::MockConsoleContext mockConsole(PlayStation, L"Playstation"); MemoryViewerViewModelHarness viewer; - viewer.InitializeMemory(0x1FFFFF); // PSX full memory + viewer.InitializeMemory(0x1FFF); // PSX full memory viewer.SetSize(MemSize::ThirtyTwoBit); Assert::AreEqual(MemSize::ThirtyTwoBit, viewer.GetSize()); @@ -2013,12 +2013,12 @@ TEST_CLASS(MemoryViewerViewModel_Tests) viewer.OnShiftClick(10 * CHAR_WIDTH, CHAR_HEIGHT + 4); Assert::AreEqual({0x1234U}, viewer.GetAddress()); - // Shift click on the first dword containing 0x80012345 should lead to address 0x00012345 as the conversion keeps - // only the same number of nibbles as the Max address from console context (PSX : 1FFFFF) + // Shift click on the first dword containing 0x80000123 should lead to address 0x00000123 as PSX has mirrored RAM + // on 0x80000000-0x801FFFFF mapped to 0x00000000-0x001FFFFF viewer.SetAddress(0); - viewer.mockEmulatorContext.WriteMemory(0U, MemSize::ThirtyTwoBit, 0x00012345); + viewer.mockEmulatorContext.WriteMemory(0U, MemSize::ThirtyTwoBit, 0x80000123); viewer.OnShiftClick(10 * CHAR_WIDTH, CHAR_HEIGHT + 4); - Assert::AreEqual({0x00012345}, viewer.GetAddress()); + Assert::AreEqual({0x00000123}, viewer.GetAddress()); // Shift click on the second dword containing 0x0 should do nothing viewer.SetAddress(0x8);