Skip to content

Commit

Permalink
Fix timeline blueprints sometimes causing crashes due to current plac…
Browse files Browse the repository at this point in the history
…ement blueprint becoming unsorted

Closes #30324.
  • Loading branch information
bdach committed Oct 21, 2024
1 parent d649764 commit dbc2e78
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,12 @@ public abstract partial class BlueprintContainer<T> : CompositeDrawable, IKeyBin
{
protected DragBox DragBox { get; private set; }

public Container<SelectionBlueprint<T>> SelectionBlueprints { get; private set; }
public SelectionBlueprintContainer SelectionBlueprints { get; private set; }

public partial class SelectionBlueprintContainer : Container<SelectionBlueprint<T>>
{
public new virtual void ChangeChildDepth(SelectionBlueprint<T> child, float newDepth) => base.ChangeChildDepth(child, newDepth);
}

public SelectionHandler<T> SelectionHandler { get; private set; }

Expand Down Expand Up @@ -95,7 +100,7 @@ private void load()
});
}

protected virtual Container<SelectionBlueprint<T>> CreateSelectionBlueprintContainer() => new Container<SelectionBlueprint<T>> { RelativeSizeAxes = Axes.Both };
protected virtual SelectionBlueprintContainer CreateSelectionBlueprintContainer() => new SelectionBlueprintContainer { RelativeSizeAxes = Axes.Both };

/// <summary>
/// Creates a <see cref="Components.SelectionHandler{T}"/> which outlines items and handles movement of selections.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
using System.Linq;
using osu.Framework.Allocation;
using osu.Framework.Graphics;
using osu.Framework.Graphics.Containers;
using osu.Framework.Input;
using osu.Framework.Input.Events;
using osu.Game.Rulesets.Edit;
Expand Down Expand Up @@ -136,7 +135,7 @@ protected override IEnumerable<SelectionBlueprint<HitObject>> ApplySelectionOrde
base.ApplySelectionOrder(blueprints)
.OrderBy(b => Math.Min(Math.Abs(EditorClock.CurrentTime - b.Item.GetEndTime()), Math.Abs(EditorClock.CurrentTime - b.Item.StartTime)));

protected override Container<SelectionBlueprint<HitObject>> CreateSelectionBlueprintContainer() => new HitObjectOrderedSelectionContainer { RelativeSizeAxes = Axes.Both };
protected override SelectionBlueprintContainer CreateSelectionBlueprintContainer() => new HitObjectOrderedSelectionContainer { RelativeSizeAxes = Axes.Both };

protected override SelectionHandler<HitObject> CreateSelectionHandler() => new EditorSelectionHandler();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using osu.Framework.Allocation;
using osu.Framework.Extensions.ObjectExtensions;
using osu.Framework.Graphics;
using osu.Framework.Graphics.Containers;
using osu.Game.Rulesets.Edit;
using osu.Game.Rulesets.Objects;
using osu.Game.Rulesets.Objects.Types;
Expand All @@ -14,7 +13,7 @@ namespace osu.Game.Screens.Edit.Compose.Components
/// <summary>
/// A container for <see cref="SelectionBlueprint{HitObject}"/> ordered by their <see cref="HitObject"/> start times.
/// </summary>
public sealed partial class HitObjectOrderedSelectionContainer : Container<SelectionBlueprint<HitObject>>
public sealed partial class HitObjectOrderedSelectionContainer : BlueprintContainer<HitObject>.SelectionBlueprintContainer
{
[Resolved]
private EditorBeatmap editorBeatmap { get; set; } = null!;
Expand All @@ -28,16 +27,18 @@ protected override void LoadComplete()

public override void Add(SelectionBlueprint<HitObject> drawable)
{
SortInternal();
Sort();
base.Add(drawable);
}

public override bool Remove(SelectionBlueprint<HitObject> drawable, bool disposeImmediately)
{
SortInternal();
Sort();
return base.Remove(drawable, disposeImmediately);
}

internal void Sort() => SortInternal();

protected override int Compare(Drawable x, Drawable y)
{
var xObj = ((SelectionBlueprint<HitObject>)x).Item;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ private void placementChanged(ValueChangedEvent<HitObject> obj)
}
}

protected override Container<SelectionBlueprint<HitObject>> CreateSelectionBlueprintContainer() => new TimelineSelectionBlueprintContainer { RelativeSizeAxes = Axes.Both };
protected override SelectionBlueprintContainer CreateSelectionBlueprintContainer() => new TimelineSelectionBlueprintContainer { RelativeSizeAxes = Axes.Both };

protected override bool OnDragStart(DragStartEvent e)
{
Expand Down Expand Up @@ -287,14 +287,27 @@ protected override void OnHoverLost(HoverLostEvent e)
}
}

protected partial class TimelineSelectionBlueprintContainer : Container<SelectionBlueprint<HitObject>>
protected partial class TimelineSelectionBlueprintContainer : SelectionBlueprintContainer
{
protected override Container<SelectionBlueprint<HitObject>> Content { get; }
protected override HitObjectOrderedSelectionContainer Content { get; }

public TimelineSelectionBlueprintContainer()
{
AddInternal(new TimelinePart<SelectionBlueprint<HitObject>>(Content = new HitObjectOrderedSelectionContainer { RelativeSizeAxes = Axes.Both }) { RelativeSizeAxes = Axes.Both });
}

public override void ChangeChildDepth(SelectionBlueprint<HitObject> child, float newDepth)
{
// timeline blueprint container also contains a blueprint for current placement, if present
// (see `placementChanged()` callback above).
// because the current placement hitobject is generally going to be mutated during the placement,
// it is possible for `Content`'s children to become unsorted when the user moves the placement around,
// which can culminate in a critical failure when attempting to binary-search children here
// using `HitObjectOrderedSelectionContainer`'s custom comparer.
// thus, always force a re-sort of objects before attempting to change child depth to avoid this scenario.
Content.Sort();
base.ChangeChildDepth(child, newDepth);
}
}
}
}

0 comments on commit dbc2e78

Please sign in to comment.