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 d89d888
Showing 1 changed file with 75 additions and 78 deletions.
153 changes: 75 additions & 78 deletions osu.Game/Screens/SelectV2/Carousel.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ await Task.Run(async () =>
}

log("Updating Y positions");
updateYPositions(items, VisibleHalfHeight, SpacingBetweenPanels);
updateYPositions(items, visibleHalfHeight, SpacingBetweenPanels);
}
catch (OperationCanceledException)
{
Expand All @@ -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,82 +437,68 @@ 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;
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()
{
if (currentKeyboardSelection.CarouselItem != null)
scroll.ScrollTo(currentKeyboardSelection.CarouselItem.CarouselYPosition - VisibleHalfHeight);
scroll.ScrollTo(currentKeyboardSelection.CarouselItem.CarouselYPosition - visibleHalfHeight);
}

#endregion
Expand All @@ -531,7 +522,7 @@ private void scrollToSelection()
/// <summary>
/// Half the height of the visible content.
/// </summary>
protected float VisibleHalfHeight => (DrawHeight + BleedBottom + BleedTop) / 2;
private float visibleHalfHeight => (DrawHeight + BleedBottom + BleedTop) / 2;

protected override void Update()
{
Expand Down Expand Up @@ -563,9 +554,9 @@ protected override void Update()
c.DrawYPosition = Interpolation.DampContinuously(c.DrawYPosition, c.Item.CarouselYPosition, 50, Time.Elapsed);

Vector2 posInScroll = scroll.ToLocalSpace(panel.ScreenSpaceDrawQuad.Centre);
float dist = Math.Abs(1f - posInScroll.Y / VisibleHalfHeight);
float dist = Math.Abs(1f - posInScroll.Y / visibleHalfHeight);

panel.X = offsetX(dist, VisibleHalfHeight);
panel.X = offsetX(dist, visibleHalfHeight);

c.Selected.Value = c.Item == currentSelection?.CarouselItem;
c.KeyboardSelected.Value = c.Item == currentKeyboardSelection?.CarouselItem;
Expand Down Expand Up @@ -615,7 +606,6 @@ private void updateDisplayedRange(DisplayRange range)
? new List<CarouselItem>()
: displayedCarouselItems.GetRange(range.First, range.Last - range.First + 1);

// TODO: make more efficient
toDisplay.RemoveAll(i => !i.IsVisible);

// Iterate over all panels which are already displayed and figure which need to be displayed / removed.
Expand Down Expand Up @@ -655,7 +645,7 @@ private void updateDisplayedRange(DisplayRange range)
if (displayedCarouselItems.Count > 0)
{
var lastItem = displayedCarouselItems[^1];
scroll.SetLayoutHeight((float)(lastItem.CarouselYPosition + lastItem.DrawHeight + VisibleHalfHeight));
scroll.SetLayoutHeight((float)(lastItem.CarouselYPosition + lastItem.DrawHeight + visibleHalfHeight));
}
else
scroll.SetLayoutHeight(0);
Expand All @@ -671,6 +661,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 d89d888

Please sign in to comment.