find correct index when updating collection (#571)

RetroAchievements · May 18, 2020 · 0253cf2 · 0253cf2
1 parent 06e6753
commit 0253cf2
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 1 deletion.
diff --git a/src/ui/ViewModelCollection.hh b/src/ui/ViewModelCollection.hh
@@ -217,10 +217,13 @@ public:
     /// <returns>Index of the first matching item, <c>-1</c> if not found.</returns>
     gsl::index FindItemIndex(const IntModelProperty& pProperty, int nValue) const
     {
+        gsl::index nIndex = 0;
         for (const auto& pItem : m_vItems)
         {
             if (pItem.HasViewModel() && pItem.ViewModel().GetValue(pProperty) == nValue)
-                return pItem.Index();
+                return nIndex;
+
+            ++nIndex;
         }
 
         return -1;

diff --git a/src/ui/win32/bindings/ComboBoxBinding.hh b/src/ui/win32/bindings/ComboBoxBinding.hh
@@ -126,6 +126,11 @@ protected:
         ComboBox_InsertString(m_hWnd, nIndex, NativeStr(pLabel).c_str());
     }
 
+    void OnViewModelRemoved(gsl::index nIndex) noexcept override
+    {
+        ComboBox_DeleteString(m_hWnd, nIndex);
+    }
+
 private:
     void PopulateComboBox()
     {

diff --git a/tests/ui/ViewModelCollection_Tests.cpp b/tests/ui/ViewModelCollection_Tests.cpp
@@ -451,6 +451,36 @@ TEST_CLASS(ViewModelCollection_Tests)
         oNotify.AssertItemReplaced(0);
     }
 
+    TEST_METHOD(TestFindItemIndexAfterRemovedWhileUpdateSuspended)
+    {
+        ViewModelCollection<TestViewModel> vmCollection;
+        vmCollection.Add(1, L"Test1");
+        vmCollection.Add(2, L"Test2");
+        vmCollection.Add(3, L"Test3");
+        vmCollection.Add(4, L"Test4");
+
+        vmCollection.BeginUpdate();
+
+        Assert::AreEqual({ 0 }, vmCollection.FindItemIndex(TestViewModel::IntProperty, 1));
+        Assert::AreEqual({ 1 }, vmCollection.FindItemIndex(TestViewModel::IntProperty, 2));
+        Assert::AreEqual({ 2 }, vmCollection.FindItemIndex(TestViewModel::IntProperty, 3));
+        Assert::AreEqual({ 3 }, vmCollection.FindItemIndex(TestViewModel::IntProperty, 4));
+
+        vmCollection.RemoveAt(1);
+
+        Assert::AreEqual({ 0 }, vmCollection.FindItemIndex(TestViewModel::IntProperty, 1));
+        Assert::AreEqual({ -1 }, vmCollection.FindItemIndex(TestViewModel::IntProperty, 2));
+        Assert::AreEqual({ 1 }, vmCollection.FindItemIndex(TestViewModel::IntProperty, 3));
+        Assert::AreEqual({ 2 }, vmCollection.FindItemIndex(TestViewModel::IntProperty, 4));
+
+        vmCollection.EndUpdate();
+
+        Assert::AreEqual({ 0 }, vmCollection.FindItemIndex(TestViewModel::IntProperty, 1));
+        Assert::AreEqual({ -1 }, vmCollection.FindItemIndex(TestViewModel::IntProperty, 2));
+        Assert::AreEqual({ 1 }, vmCollection.FindItemIndex(TestViewModel::IntProperty, 3));
+        Assert::AreEqual({ 2 }, vmCollection.FindItemIndex(TestViewModel::IntProperty, 4));
+    }
+
     TEST_METHOD(TestMovePropertyNotification)
     {
         ViewModelCollection<TestViewModel> vmCollection;