Merge pull request #8973 from smoogipoo/mania-holdnote-selection-fix

Fix hold note selection pieces disappearing on movement
This commit is contained in:
Dean Herbert 2020-05-10 15:30:58 +09:00 committed by GitHub
commit 36ee54f876
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 128 additions and 28 deletions

View File

@ -10,10 +10,13 @@ using osu.Framework.Bindables;
using osu.Framework.Graphics;
using osu.Framework.Graphics.Containers;
using osu.Framework.Testing;
using osu.Framework.Utils;
using osu.Game.Rulesets.Edit;
using osu.Game.Rulesets.Mania.Beatmaps;
using osu.Game.Rulesets.Mania.Edit;
using osu.Game.Rulesets.Mania.Edit.Blueprints;
using osu.Game.Rulesets.Mania.Objects;
using osu.Game.Rulesets.Mania.Objects.Drawables;
using osu.Game.Rulesets.Mania.Objects.Drawables.Pieces;
using osu.Game.Rulesets.Objects.Drawables;
using osu.Game.Rulesets.UI.Scrolling;
@ -48,6 +51,8 @@ namespace osu.Game.Rulesets.Mania.Tests
DrawableHitObject lastObject = null;
Vector2 originalPosition = Vector2.Zero;
setScrollStep(ScrollingDirection.Up);
AddStep("seek to last object", () =>
{
lastObject = this.ChildrenOfType<DrawableHitObject>().Single(d => d.HitObject == composer.EditorBeatmap.HitObjects.Last());
@ -81,7 +86,7 @@ namespace osu.Game.Rulesets.Mania.Tests
DrawableHitObject lastObject = null;
Vector2 originalPosition = Vector2.Zero;
AddStep("set down scroll", () => ((Bindable<ScrollingDirection>)composer.Composer.ScrollingInfo.Direction).Value = ScrollingDirection.Down);
setScrollStep(ScrollingDirection.Down);
AddStep("seek to last object", () =>
{
@ -116,6 +121,8 @@ namespace osu.Game.Rulesets.Mania.Tests
DrawableHitObject lastObject = null;
Vector2 originalPosition = Vector2.Zero;
setScrollStep(ScrollingDirection.Down);
AddStep("seek to last object", () =>
{
lastObject = this.ChildrenOfType<DrawableHitObject>().Single(d => d.HitObject == composer.EditorBeatmap.HitObjects.Last());
@ -147,6 +154,46 @@ namespace osu.Game.Rulesets.Mania.Tests
AddAssert("hitobjects not moved vertically", () => lastObject.DrawPosition.Y - originalPosition.Y <= DefaultNotePiece.NOTE_HEIGHT);
}
[Test]
public void TestDragHoldNoteSelectionVertically()
{
setScrollStep(ScrollingDirection.Down);
AddStep("setup beatmap", () =>
{
composer.EditorBeatmap.Clear();
composer.EditorBeatmap.Add(new HoldNote
{
Column = 1,
EndTime = 200
});
});
DrawableHoldNote holdNote = null;
AddStep("grab hold note", () =>
{
holdNote = this.ChildrenOfType<DrawableHoldNote>().Single();
InputManager.MoveMouseTo(holdNote);
InputManager.PressButton(MouseButton.Left);
});
AddStep("move drag upwards", () =>
{
InputManager.MoveMouseTo(holdNote, new Vector2(0, -100));
InputManager.ReleaseButton(MouseButton.Left);
});
AddAssert("head note positioned correctly", () => Precision.AlmostEquals(holdNote.ScreenSpaceDrawQuad.BottomLeft, holdNote.Head.ScreenSpaceDrawQuad.BottomLeft));
AddAssert("tail note positioned correctly", () => Precision.AlmostEquals(holdNote.ScreenSpaceDrawQuad.TopLeft, holdNote.Tail.ScreenSpaceDrawQuad.BottomLeft));
AddAssert("head blueprint positioned correctly", () => this.ChildrenOfType<HoldNoteNoteSelectionBlueprint>().ElementAt(0).DrawPosition == holdNote.Head.DrawPosition);
AddAssert("tail blueprint positioned correctly", () => this.ChildrenOfType<HoldNoteNoteSelectionBlueprint>().ElementAt(1).DrawPosition == holdNote.Tail.DrawPosition);
}
private void setScrollStep(ScrollingDirection direction)
=> AddStep($"set scroll direction = {direction}", () => ((Bindable<ScrollingDirection>)composer.Composer.ScrollingInfo.Direction).Value = direction);
private class TestComposer : CompositeDrawable
{
[Cached(typeof(EditorBeatmap))]

View File

@ -399,7 +399,7 @@ namespace osu.Game.Rulesets.Osu.Tests
{
public TestSlider()
{
DefaultsApplied += () =>
DefaultsApplied += _ =>
{
HeadCircle.HitWindows = new TestHitWindows();
TailCircle.HitWindows = new TestHitWindows();

View File

@ -78,7 +78,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables.Connections
private void bindEvents(DrawableOsuHitObject drawableObject)
{
drawableObject.HitObject.PositionBindable.BindValueChanged(_ => scheduleRefresh());
drawableObject.HitObject.DefaultsApplied += scheduleRefresh;
drawableObject.HitObject.DefaultsApplied += _ => scheduleRefresh();
}
private void scheduleRefresh()

View File

@ -25,6 +25,8 @@ namespace osu.Game.Rulesets.Objects.Drawables
[Cached(typeof(DrawableHitObject))]
public abstract class DrawableHitObject : SkinReloadableDrawable
{
public event Action<DrawableHitObject> DefaultsApplied;
public readonly HitObject HitObject;
/// <summary>
@ -148,7 +150,7 @@ namespace osu.Game.Rulesets.Objects.Drawables
samplesBindable.CollectionChanged += (_, __) => loadSamples();
updateState(ArmedState.Idle, true);
onDefaultsApplied();
apply(HitObject);
}
private void loadSamples()
@ -175,7 +177,11 @@ namespace osu.Game.Rulesets.Objects.Drawables
AddInternal(Samples);
}
private void onDefaultsApplied() => apply(HitObject);
private void onDefaultsApplied(HitObject hitObject)
{
apply(hitObject);
DefaultsApplied?.Invoke(this);
}
private void apply(HitObject hitObject)
{

View File

@ -32,7 +32,7 @@ namespace osu.Game.Rulesets.Objects
/// <summary>
/// Invoked after <see cref="ApplyDefaults"/> has completed on this <see cref="HitObject"/>.
/// </summary>
public event Action DefaultsApplied;
public event Action<HitObject> DefaultsApplied;
public readonly Bindable<double> StartTimeBindable = new BindableDouble();
@ -124,7 +124,7 @@ namespace osu.Game.Rulesets.Objects
foreach (var h in nestedHitObjects)
h.ApplyDefaults(controlPointInfo, difficulty);
DefaultsApplied?.Invoke();
DefaultsApplied?.Invoke(this);
}
protected virtual void ApplyDefaultsToSelf(ControlPointInfo controlPointInfo, BeatmapDifficulty difficulty)

View File

@ -16,17 +16,23 @@ namespace osu.Game.Rulesets.UI.Scrolling
{
private readonly IBindable<double> timeRange = new BindableDouble();
private readonly IBindable<ScrollingDirection> direction = new Bindable<ScrollingDirection>();
private readonly Dictionary<DrawableHitObject, Cached> hitObjectInitialStateCache = new Dictionary<DrawableHitObject, Cached>();
[Resolved]
private IScrollingInfo scrollingInfo { get; set; }
private readonly LayoutValue initialStateCache = new LayoutValue(Invalidation.RequiredParentSizeToFit | Invalidation.DrawInfo);
// Responds to changes in the layout. When the layout changes, all hit object states must be recomputed.
private readonly LayoutValue layoutCache = new LayoutValue(Invalidation.RequiredParentSizeToFit | Invalidation.DrawInfo);
// A combined cache across all hit object states to reduce per-update iterations.
// When invalidated, one or more (but not necessarily all) hitobject states must be re-validated.
private readonly Cached combinedObjCache = new Cached();
public ScrollingHitObjectContainer()
{
RelativeSizeAxes = Axes.Both;
AddLayout(initialStateCache);
AddLayout(layoutCache);
}
[BackgroundDependencyLoader]
@ -35,13 +41,14 @@ namespace osu.Game.Rulesets.UI.Scrolling
direction.BindTo(scrollingInfo.Direction);
timeRange.BindTo(scrollingInfo.TimeRange);
direction.ValueChanged += _ => initialStateCache.Invalidate();
timeRange.ValueChanged += _ => initialStateCache.Invalidate();
direction.ValueChanged += _ => layoutCache.Invalidate();
timeRange.ValueChanged += _ => layoutCache.Invalidate();
}
public override void Add(DrawableHitObject hitObject)
{
initialStateCache.Invalidate();
combinedObjCache.Invalidate();
hitObject.DefaultsApplied += onDefaultsApplied;
base.Add(hitObject);
}
@ -51,8 +58,10 @@ namespace osu.Game.Rulesets.UI.Scrolling
if (result)
{
initialStateCache.Invalidate();
combinedObjCache.Invalidate();
hitObjectInitialStateCache.Remove(hitObject);
hitObject.DefaultsApplied -= onDefaultsApplied;
}
return result;
@ -60,23 +69,45 @@ namespace osu.Game.Rulesets.UI.Scrolling
public override void Clear(bool disposeChildren = true)
{
foreach (var h in Objects)
h.DefaultsApplied -= onDefaultsApplied;
base.Clear(disposeChildren);
initialStateCache.Invalidate();
combinedObjCache.Invalidate();
hitObjectInitialStateCache.Clear();
}
private void onDefaultsApplied(DrawableHitObject drawableObject)
{
// The cache may not exist if the hitobject state hasn't been computed yet (e.g. if the hitobject was added + defaults applied in the same frame).
// In such a case, combinedObjCache will take care of updating the hitobject.
if (hitObjectInitialStateCache.TryGetValue(drawableObject, out var objCache))
{
combinedObjCache.Invalidate();
objCache.Invalidate();
}
}
private float scrollLength;
protected override void Update()
{
base.Update();
if (!initialStateCache.IsValid)
if (!layoutCache.IsValid)
{
foreach (var cached in hitObjectInitialStateCache.Values)
cached.Invalidate();
combinedObjCache.Invalidate();
scrollingInfo.Algorithm.Reset();
layoutCache.Validate();
}
if (!combinedObjCache.IsValid)
{
switch (direction.Value)
{
case ScrollingDirection.Up:
@ -89,15 +120,21 @@ namespace osu.Game.Rulesets.UI.Scrolling
break;
}
scrollingInfo.Algorithm.Reset();
foreach (var obj in Objects)
{
if (!hitObjectInitialStateCache.TryGetValue(obj, out var objCache))
objCache = hitObjectInitialStateCache[obj] = new Cached();
if (objCache.IsValid)
continue;
computeLifetimeStartRecursive(obj);
computeInitialStateRecursive(obj);
objCache.Validate();
}
initialStateCache.Validate();
combinedObjCache.Validate();
}
}
@ -109,8 +146,6 @@ namespace osu.Game.Rulesets.UI.Scrolling
computeLifetimeStartRecursive(obj);
}
private readonly Dictionary<DrawableHitObject, Cached> hitObjectInitialStateCache = new Dictionary<DrawableHitObject, Cached>();
private double computeOriginAdjustedLifetimeStart(DrawableHitObject hitObject)
{
float originAdjustment = 0.0f;
@ -142,12 +177,6 @@ namespace osu.Game.Rulesets.UI.Scrolling
// Cant use AddOnce() since the delegate is re-constructed every invocation
private void computeInitialStateRecursive(DrawableHitObject hitObject) => hitObject.Schedule(() =>
{
if (!hitObjectInitialStateCache.TryGetValue(hitObject, out var cached))
cached = hitObjectInitialStateCache[hitObject] = new Cached();
if (cached.IsValid)
return;
if (hitObject.HitObject is IHasEndTime e)
{
switch (direction.Value)
@ -171,8 +200,6 @@ namespace osu.Game.Rulesets.UI.Scrolling
// Nested hitobjects don't need to scroll, but they do need accurate positions
updatePosition(obj, hitObject.HitObject.StartTime);
}
cached.Validate();
});
protected override void UpdateAfterChildrenLife()

View File

@ -4,6 +4,7 @@
using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using JetBrains.Annotations;
using osu.Framework.Allocation;
using osu.Framework.Bindables;
@ -201,6 +202,25 @@ namespace osu.Game.Screens.Edit
updateHitObject(null, true);
}
/// <summary>
/// Clears all <see cref="HitObjects"/> from this <see cref="EditorBeatmap"/>.
/// </summary>
public void Clear()
{
var removed = HitObjects.ToList();
mutableHitObjects.Clear();
foreach (var b in startTimeBindables)
b.Value.UnbindAll();
startTimeBindables.Clear();
foreach (var h in removed)
HitObjectRemoved?.Invoke(h);
updateHitObject(null, true);
}
private void trackStartTime(HitObject hitObject)
{
startTimeBindables[hitObject] = hitObject.StartTimeBindable.GetBoundCopy();