Skip to content

Commit

Permalink
Large refactoring and clean-up pass
Browse files Browse the repository at this point in the history
  • Loading branch information
@peppy
peppy committed Jan 22, 2025
1 parent 410859f commit 8f025d4
Showing 1 changed file with 68 additions and 70 deletions.
138 changes: 68 additions & 70 deletions osu.Game/Screens/SelectV2/Carousel.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}");
}
Expand Down Expand Up @@ -325,87 +326,91 @@ public void OnReleased(KeyBindingReleaseEvent<GlobalAction> e)
{
}

/// <summary>
/// Select the next valid selection relative to a current selection.
/// This is generally for keyboard based traversal.
/// </summary>
/// <param name="direction">Positive for downwards, negative for upwards.</param>
/// <param name="isGroupSelection">Whether the selection should traverse groups. Group selection updates the actual selection immediately, while non-group selection will only prepare a future keyboard selection.</param>
/// <returns>Whether selection was possible.</returns>
private bool selectNext(int direction, bool isGroupSelection)
{
// Ensure sanity
Debug.Assert(direction != 0);
direction = direction > 0 ? 1 : -1;

if (displayedCarouselItems == null || displayedCarouselItems.Count == 0)
return false;

CarouselItem? originalSelection = currentKeyboardSelection.CarouselItem;
int currentSelectionIndex = currentKeyboardSelection.Index ?? -1;
CarouselItem? selectionItem = currentKeyboardSelection.CarouselItem;
int selectionIndex = currentKeyboardSelection.Index ?? -1;

// To keep things simple, let's first handle the cases where there's no selection yet.
// Once we've confirmed that the first or last isn't valid, we can handle everything
// using the same process.
if (originalSelection == null || currentSelectionIndex < 0)
if (selectionItem == null || selectionIndex < 0)
{
if (direction > 0)
{
originalSelection = displayedCarouselItems.First();
currentSelectionIndex = 0;
selectionItem = displayedCarouselItems.First();
selectionIndex = 0;
}
else
{
originalSelection = displayedCarouselItems.Last();
currentSelectionIndex = displayedCarouselItems.Count - 1;
selectionItem = displayedCarouselItems.Last();
selectionIndex = displayedCarouselItems.Count - 1;
}

if (isValidItem(originalSelection))
{
performSelection(originalSelection);
if (attemptSelection(selectionItem))
return true;
}
}

Debug.Assert(originalSelection != null);
Debug.Assert(currentSelectionIndex >= 0);
Debug.Assert(selectionItem != null);
Debug.Assert(selectionIndex >= 0);

// As a special case, if the user has a different keyboard selection and requests
// group selection, first transfer the keyboard selection.
if (isGroupSelection && currentSelection.CarouselItem != originalSelection)
// group selection, first transfer the keyboard selection to actual selection.
if (isGroupSelection && currentSelection.CarouselItem != selectionItem)
{
setSelection(originalSelection.Model);
setSelection(selectionItem.Model);
return true;
}

int newSelectionIndex = currentSelectionIndex;

// As a second special case, if we're group selecting backwards and the current selection isn't
// a group, base this selection operation from the closest previous group.
if (isGroupSelection && direction < 0)
{
while (!displayedCarouselItems[newSelectionIndex].IsGroupSelectionTarget)
newSelectionIndex--;
while (!displayedCarouselItems[selectionIndex].IsGroupSelectionTarget)
selectionIndex--;
}

CarouselItem newItem;
CarouselItem? newItem;

while ((newItem = selectNextPanel()) != originalSelection)
// Iterate over every item back to the current selection, finding the first valid item.
// The fail condition is when we reach the selection after a cyclic loop over every item.
do
{
if (isValidItem(newItem))
break;
}
selectionIndex += direction;
newItem = displayedCarouselItems[(selectionIndex + displayedCarouselItems.Count) % displayedCarouselItems.Count];

if (newItem == originalSelection)
return false;
if (attemptSelection(newItem))
return true;
} while (newItem != selectionItem);

performSelection(newItem);
return true;
return false;

CarouselItem selectNextPanel()
bool attemptSelection(CarouselItem item)
{
newSelectionIndex += direction;
return newItem = displayedCarouselItems[(newSelectionIndex + displayedCarouselItems.Count) % displayedCarouselItems.Count];
}
if (!item.IsVisible || (isGroupSelection && !item.IsGroupSelectionTarget))
return false;

void performSelection(CarouselItem item)
{
if (isGroupSelection)
setSelection(item.Model);
else
setKeyboardSelection(item.Model);
}

bool isValidItem(CarouselItem item) => item.IsVisible && (!isGroupSelection || item.IsGroupSelectionTarget);
return true;
}
}

#endregion
Expand All @@ -432,76 +437,62 @@ 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;

// Invalidate display range as panel positions and visible status may have changed.
// Position transfer won't happen unless we invalidate this.
displayedRange = null;

// The case where no items are available for display yet.
if (displayedCarouselItems == null)
{
currentKeyboardSelection = new Selection();
currentSelection = new Selection();
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;

// TODO: we need to be able to avoid this iteration.
for (int i = 0; i < displayedCarouselItems.Count; i++)
Selection prevKeyboard = currentKeyboardSelection;

// We are performing two important operations here:
// - Update all Y positions. After a selection occurs, panels may have changed visibility state and therefore Y positions.
// - Link selected models to CarouselItems. If a selection changed, this is where we find the relevant CarouselItems for further use.
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 a keyboard selection is currently made, we want to keep the view stable around the selection.
// That means that we should offset the immediate scroll position by any change in Y position for the selection.
if (prevKeyboard.YPosition != null && currentKeyboardSelection.YPosition != prevKeyboard.YPosition)
scroll.OffsetScrollPosition((float)(currentKeyboardSelection.YPosition!.Value - prevKeyboard.YPosition.Value));
}

private void scrollToSelection()
Expand Down Expand Up @@ -671,6 +662,13 @@ private static void expirePanelImmediately(Drawable panel)

#region Internal helper classes

/// <summary>
/// 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}.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);
Expand Down

0 comments on commit 8f025d4

Please sign in to comment.