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

Author: Vancleeff

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<ra::data::context::EmulatorContext>();
     const auto& pConsoleContext = ra::services::ServiceLocator::Get<ra::data::context::ConsoleContext>();
+
+    OnClick(nX, nY);
+
+    ra::ByteAddress nAddress = pEmulatorContext.ReadMemory(GetAddress(), GetSize());
     const auto nConvertedAddress = pConsoleContext.ByteAddressFromRealAddress(nAddress);
     if (nConvertedAddress != 0xFFFFFFFF)
         nAddress = nConvertedAddress;
+
     SetAddress(nAddress);
 }
 

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();

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,23 +256,17 @@ 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);
 
     Invalidate();
 }
 
-void MemoryViewerControlBinding::OnShiftClick(POINT point)
-{
-    OnClick(point);
-    const auto& pEmulatorContext = ra::services::ServiceLocator::Get<ra::data::context::EmulatorContext>();
-    ra::ByteAddress nAddress =
-        pEmulatorContext.ReadMemory(m_pViewModel.GetAddress(), m_pViewModel.GetSize()) & 0xFFFFFF;
-    m_pViewModel.OnShiftClick(nAddress);
-}
-
 void MemoryViewerControlBinding::OnGotFocus()
 {
     m_pViewModel.OnGotFocus();

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);

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