Simplify

osu.Game/Screens/SelectV2/Carousel.cs

@@ -269,9 +269,10 @@ await Task.Run(async () =>
             displayedCarouselItems = items.ToList();
             displayedRange = null;
 
+            // Need to call this to ensure correct post-selection logic is handled on the new items list.
             HandleItemSelected(currentSelection.Model);
 
-            updateSelection();
+            refreshAfterSelection();
 
             void log(string text) => Logger.Log($"Carousel[op {cts.GetHashCode().ToString()}] {stopwatch.ElapsedMilliseconds} ms: {text}");
         }
@@ -432,22 +433,22 @@ private void setSelection(object? model)
             if (currentSelection.Model != model)
                 return;
 
-            updateSelection();
+            refreshAfterSelection();
             scrollToSelection();
         }
 
         private void setKeyboardSelection(object? model)
         {
             currentKeyboardSelection = new Selection(model);
 
-            updateSelection();
+            refreshAfterSelection();
             scrollToSelection();
         }
 
         /// <summary>
         /// Call after a selection of items change to re-attach <see cref="CarouselItem"/>s to current <see cref="Selection"/>s.
         /// </summary>
-        private void updateSelection()
+        private void refreshAfterSelection()
         {
             float yPos = VisibleHalfHeight;
 
@@ -462,46 +463,26 @@ private void updateSelection()
                 return;
             }
 
-            // let's make some early assertions in order to try and optimise this down
-            //
-            // when do we arrive here?
-            // - this method is called after a potential selection change
-            //   (in this case, the selection will not have anything except a model)
-            // - this method is called after a potential CarouselItem refresh
-            //   (in this case, the selection may have a CarouselItem but it may not be in the current list)
-            //
-            // what do we do here?
-            // - check whether the current selection has changed position (and adjust scroll)
-            // - update y positions if they have changed (but that can be some separately)
-
             float spacing = SpacingBetweenPanels;
+            int count = displayedCarouselItems.Count;
+
+            Selection prevKeyboard = currentKeyboardSelection;
 
-            // TODO: we need to be able to avoid this iteration.
-            for (int i = 0; i < displayedCarouselItems.Count; i++)
+            for (int i = 0; i < count; i++)
             {
                 var item = displayedCarouselItems[i];
 
                 updateItemYPosition(item, ref yPos, spacing);
 
-                bool isSelected = item.Model == currentSelection?.Model;
-                bool isKeyboardSelected = item.Model == currentKeyboardSelection?.Model;
-
-                if (isKeyboardSelected)
-                {
-                    double? previousYPosition = currentKeyboardSelection?.YPosition;
-
+                if (ReferenceEquals(item.Model, currentKeyboardSelection.Model))
                     currentKeyboardSelection = new Selection(item.Model, item, item.CarouselYPosition, i);
 
-                    if (previousYPosition != null && previousYPosition != item.CarouselYPosition)
-                    {
-                        float adjustment = (float)(item.CarouselYPosition - previousYPosition.Value);
-                        scroll.OffsetScrollPosition(adjustment);
-                    }
-                }
-
-                if (isSelected)
+                if (ReferenceEquals(item.Model, currentSelection.Model))
                     currentSelection = new Selection(item.Model, item, item.CarouselYPosition, i);
             }
+
+            if (prevKeyboard.YPosition != null && currentKeyboardSelection.YPosition != prevKeyboard.YPosition)
+                scroll.OffsetScrollPosition((float)(currentKeyboardSelection.YPosition!.Value - prevKeyboard.YPosition.Value));
         }
 
         private void scrollToSelection()
@@ -675,9 +656,9 @@ private static void expirePanelImmediately(Drawable panel)
         /// Bookkeeping for a current selection.
         /// </summary>
         /// <param name="Model">The selected model. If <c>null</c>, there's no selection.</param>
-        /// <param name="CarouselItem">A related carousel item representation for the model. May be null if selection is not present as an item, or if <see cref="Carousel{T}.updateSelection"/> has not been run yet.</param>
-        /// <param name="YPosition">The Y position of the selection as of the last run of <see cref="Carousel{T}.updateSelection"/>. May be null if selection is not present as an item, or if <see cref="Carousel{T}.updateSelection"/> has not been run yet.</param>
-        /// <param name="Index">The index of the selection as of the last run of <see cref="Carousel{T}.updateSelection"/>. May be null if selection is not present as an item, or if <see cref="Carousel{T}.updateSelection"/> has not been run yet.</param>
+        /// <param name="CarouselItem">A related carousel item representation for the model. May be null if selection is not present as an item, or if <see cref="Carousel{T}.refreshAfterSelection"/> has not been run yet.</param>
+        /// <param name="YPosition">The Y position of the selection as of the last run of <see cref="Carousel{T}.refreshAfterSelection"/>. May be null if selection is not present as an item, or if <see cref="Carousel{T}.refreshAfterSelection"/> has not been run yet.</param>
+        /// <param name="Index">The index of the selection as of the last run of <see cref="Carousel{T}.refreshAfterSelection"/>. May be null if selection is not present as an item, or if <see cref="Carousel{T}.refreshAfterSelection"/> has not been run yet.</param>
         private record Selection(object? Model = null, CarouselItem? CarouselItem = null, double? YPosition = null, int? Index = null);
 
         private record DisplayRange(int First, int Last);