Merge pull request #12627 from ekrctb/always-use-lifetime-entry

Always use lifetime entry to manage hit objects in HitObjectContainer
This commit is contained in:
Dan Balasescu 2021-05-19 13:12:01 +09:00 committed by GitHub
commit 6717355fbe
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 202 additions and 107 deletions

View File

@ -15,7 +15,6 @@ using osu.Game.Rulesets.Objects.Drawables;
using osu.Game.Rulesets.UI;
using osu.Game.Rulesets.UI.Scrolling;
using osu.Game.Tests.Visual;
using osuTK;
using osuTK.Input;
namespace osu.Game.Rulesets.Mania.Tests.Editor
@ -35,7 +34,11 @@ namespace osu.Game.Rulesets.Mania.Tests.Editor
[Test]
public void TestPlaceBeforeCurrentTimeDownwards()
{
AddStep("move mouse before current time", () => InputManager.MoveMouseTo(this.ChildrenOfType<Column>().Single().ScreenSpaceDrawQuad.BottomLeft - new Vector2(0, 10)));
AddStep("move mouse before current time", () =>
{
var column = this.ChildrenOfType<Column>().Single();
InputManager.MoveMouseTo(column.ScreenSpacePositionAtTime(-100));
});
AddStep("click", () => InputManager.Click(MouseButton.Left));
@ -45,7 +48,11 @@ namespace osu.Game.Rulesets.Mania.Tests.Editor
[Test]
public void TestPlaceAfterCurrentTimeDownwards()
{
AddStep("move mouse after current time", () => InputManager.MoveMouseTo(this.ChildrenOfType<Column>().Single()));
AddStep("move mouse after current time", () =>
{
var column = this.ChildrenOfType<Column>().Single();
InputManager.MoveMouseTo(column.ScreenSpacePositionAtTime(100));
});
AddStep("click", () => InputManager.Click(MouseButton.Left));

View File

@ -0,0 +1,67 @@
// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.
using NUnit.Framework;
using osu.Framework.Graphics;
using osu.Framework.Timing;
using osu.Game.Beatmaps;
using osu.Game.Beatmaps.ControlPoints;
using osu.Game.Rulesets.Mania.Objects;
using osu.Game.Rulesets.Mania.Objects.Drawables;
using osu.Game.Rulesets.Mania.UI;
using osu.Game.Rulesets.UI.Scrolling;
using osu.Game.Tests.Visual;
using osuTK.Graphics;
namespace osu.Game.Rulesets.Mania.Tests
{
public class TestSceneDrawableManiaHitObject : OsuTestScene
{
private readonly ManualClock clock = new ManualClock();
private Column column;
[SetUp]
public void SetUp() => Schedule(() =>
{
Child = new ScrollingTestContainer(ScrollingDirection.Down)
{
Anchor = Anchor.Centre,
Origin = Anchor.Centre,
AutoSizeAxes = Axes.X,
RelativeSizeAxes = Axes.Y,
TimeRange = 2000,
Clock = new FramedClock(clock),
Child = column = new Column(0)
{
Action = { Value = ManiaAction.Key1 },
Height = 0.85f,
AccentColour = Color4.Gray
},
};
});
[Test]
public void TestHoldNoteHeadVisibility()
{
DrawableHoldNote note = null;
AddStep("Add hold note", () =>
{
var h = new HoldNote
{
StartTime = 0,
Duration = 1000
};
h.ApplyDefaults(new ControlPointInfo(), new BeatmapDifficulty());
column.Add(note = new DrawableHoldNote(h));
});
AddStep("Hold key", () =>
{
clock.CurrentTime = 0;
note.OnPressed(ManiaAction.Key1);
});
AddStep("progress time", () => clock.CurrentTime = 500);
AddAssert("head is visible", () => note.Head.Alpha == 1);
}
}
}

View File

@ -4,14 +4,11 @@
using System.Collections.Generic;
using System.Linq;
using NUnit.Framework;
using osu.Framework.Screens;
using osu.Framework.Testing;
using osu.Game.Beatmaps;
using osu.Game.Beatmaps.ControlPoints;
using osu.Game.Replays;
using osu.Game.Rulesets.Judgements;
using osu.Game.Rulesets.Mania.Objects;
using osu.Game.Rulesets.Mania.Objects.Drawables;
using osu.Game.Rulesets.Mania.Replays;
using osu.Game.Rulesets.Mania.Scoring;
using osu.Game.Rulesets.Objects;
@ -411,17 +408,7 @@ namespace osu.Game.Rulesets.Mania.Tests
judgementResults = new List<JudgementResult>();
});
AddUntilStep("Beatmap at 0", () => Beatmap.Value.Track.CurrentTime == 0);
AddUntilStep("Wait until player is loaded", () => currentPlayer.IsCurrentScreen());
AddUntilStep("wait for head", () => currentPlayer.GameplayClockContainer.GameplayClock.CurrentTime >= time_head);
AddAssert("head is visible",
() => currentPlayer.ChildrenOfType<DrawableHoldNote>()
.Single(note => note.HitObject == beatmap.HitObjects[0])
.Head
.Alpha == 1);
AddUntilStep("Wait for completion", () => currentPlayer.ScoreProcessor.HasCompleted.Value);
AddUntilStep("Wait for completion", () => currentPlayer.ScoreProcessor?.HasCompleted.Value == true);
}
private class ScoreAccessibleReplayPlayer : ReplayPlayer

View File

@ -101,7 +101,7 @@ namespace osu.Game.Rulesets.Mania.Edit
foreach (var line in grid.Objects.OfType<DrawableGridLine>())
availableLines.Push(line);
grid.Clear(false);
grid.Clear();
}
if (selectionTimeRange == null)

View File

@ -40,7 +40,7 @@ namespace osu.Game.Rulesets.Taiko.Tests
[SetUpSteps]
public void SetUp()
=> AddStep("clear SHOC", () => hitObjectContainer.Clear(false));
=> AddStep("clear SHOC", () => hitObjectContainer.Clear());
protected void AddHitObject(DrawableHitObject hitObject)
=> AddStep("add to SHOC", () => hitObjectContainer.Add(hitObject));

View File

@ -45,15 +45,16 @@ namespace osu.Game.Tests.Gameplay
AddStep("Create DHO", () =>
{
dho = new TestDrawableHitObject(null);
dho.Apply(entry = new TestLifetimeEntry(new HitObject())
{
LifetimeStart = 0,
LifetimeEnd = 1000,
});
dho.Apply(entry = new TestLifetimeEntry(new HitObject()));
Child = dho;
});
AddStep("KeepAlive = true", () => entry.KeepAlive = true);
AddStep("KeepAlive = true", () =>
{
entry.LifetimeStart = 0;
entry.LifetimeEnd = 1000;
entry.KeepAlive = true;
});
AddAssert("Lifetime is overriden", () => entry.LifetimeStart == double.MinValue && entry.LifetimeEnd == double.MaxValue);
AddStep("Set LifetimeStart", () => dho.LifetimeStart = 500);

View File

@ -3,6 +3,7 @@
#nullable enable
using System;
using System.Diagnostics;
using osu.Framework.Graphics.Performance;
using osu.Framework.Graphics.Pooling;
@ -18,7 +19,7 @@ namespace osu.Game.Rulesets.Objects.Pooling
/// <summary>
/// The entry holding essential state of this <see cref="PoolableDrawableWithLifetime{TEntry}"/>.
/// </summary>
protected TEntry? Entry { get; private set; }
public TEntry? Entry { get; private set; }
/// <summary>
/// Whether <see cref="Entry"/> is applied to this <see cref="PoolableDrawableWithLifetime{TEntry}"/>.
@ -28,14 +29,28 @@ namespace osu.Game.Rulesets.Objects.Pooling
public override double LifetimeStart
{
get => base.LifetimeStart;
set => setLifetime(value, LifetimeEnd);
get => Entry?.LifetimeStart ?? double.MinValue;
set
{
if (Entry == null && LifetimeStart != value)
throw new InvalidOperationException($"Cannot modify lifetime of {nameof(PoolableDrawableWithLifetime<TEntry>)} when entry is not set");
if (Entry != null)
Entry.LifetimeStart = value;
}
}
public override double LifetimeEnd
{
get => base.LifetimeEnd;
set => setLifetime(LifetimeStart, value);
get => Entry?.LifetimeEnd ?? double.MaxValue;
set
{
if (Entry == null && LifetimeEnd != value)
throw new InvalidOperationException($"Cannot modify lifetime of {nameof(PoolableDrawableWithLifetime<TEntry>)} when entry is not set");
if (Entry != null)
Entry.LifetimeEnd = value;
}
}
public override bool RemoveWhenNotAlive => false;
@ -64,11 +79,8 @@ namespace osu.Game.Rulesets.Objects.Pooling
if (HasEntryApplied)
free();
setLifetime(entry.LifetimeStart, entry.LifetimeEnd);
Entry = entry;
OnApply(entry);
HasEntryApplied = true;
}
@ -95,27 +107,12 @@ namespace osu.Game.Rulesets.Objects.Pooling
{
}
private void setLifetime(double start, double end)
{
base.LifetimeStart = start;
base.LifetimeEnd = end;
if (Entry != null)
{
Entry.LifetimeStart = start;
Entry.LifetimeEnd = end;
}
}
private void free()
{
Debug.Assert(Entry != null && HasEntryApplied);
OnFree(Entry);
Entry = null;
setLifetime(double.MaxValue, double.MaxValue);
HasEntryApplied = false;
}
}

View File

@ -17,8 +17,18 @@ using osu.Game.Rulesets.Objects.Drawables;
namespace osu.Game.Rulesets.UI
{
public class HitObjectContainer : LifetimeManagementContainer, IHitObjectContainer
public class HitObjectContainer : CompositeDrawable, IHitObjectContainer
{
/// <summary>
/// All entries in this <see cref="HitObjectContainer"/> including dead entries.
/// </summary>
public IEnumerable<HitObjectLifetimeEntry> Entries => allEntries;
/// <summary>
/// All alive entries and <see cref="DrawableHitObject"/>s used by the entries.
/// </summary>
public IEnumerable<(HitObjectLifetimeEntry Entry, DrawableHitObject Drawable)> AliveEntries => aliveDrawableMap.Select(x => (x.Key, x.Value));
public IEnumerable<DrawableHitObject> Objects => InternalChildren.Cast<DrawableHitObject>().OrderBy(h => h.HitObject.StartTime);
public IEnumerable<DrawableHitObject> AliveObjects => AliveInternalChildren.Cast<DrawableHitObject>().OrderBy(h => h.HitObject.StartTime);
@ -60,8 +70,12 @@ namespace osu.Game.Rulesets.UI
internal double FutureLifetimeExtension { get; set; }
private readonly Dictionary<DrawableHitObject, IBindable> startTimeMap = new Dictionary<DrawableHitObject, IBindable>();
private readonly Dictionary<HitObjectLifetimeEntry, DrawableHitObject> drawableMap = new Dictionary<HitObjectLifetimeEntry, DrawableHitObject>();
private readonly Dictionary<HitObjectLifetimeEntry, DrawableHitObject> aliveDrawableMap = new Dictionary<HitObjectLifetimeEntry, DrawableHitObject>();
private readonly Dictionary<HitObjectLifetimeEntry, DrawableHitObject> nonPooledDrawableMap = new Dictionary<HitObjectLifetimeEntry, DrawableHitObject>();
private readonly LifetimeEntryManager lifetimeManager = new LifetimeEntryManager();
private readonly HashSet<HitObjectLifetimeEntry> allEntries = new HashSet<HitObjectLifetimeEntry>();
[Resolved(CanBeNull = true)]
private IPooledHitObjectProvider pooledObjectProvider { get; set; }
@ -72,6 +86,7 @@ namespace osu.Game.Rulesets.UI
lifetimeManager.EntryBecameAlive += entryBecameAlive;
lifetimeManager.EntryBecameDead += entryBecameDead;
lifetimeManager.EntryCrossedBoundary += entryCrossedBoundary;
}
protected override void LoadAsyncComplete()
@ -84,93 +99,113 @@ namespace osu.Game.Rulesets.UI
#region Pooling support
public void Add(HitObjectLifetimeEntry entry) => lifetimeManager.AddEntry(entry);
public bool Remove(HitObjectLifetimeEntry entry) => lifetimeManager.RemoveEntry(entry);
private void entryBecameAlive(LifetimeEntry entry) => addDrawable((HitObjectLifetimeEntry)entry);
private void entryBecameDead(LifetimeEntry entry) => removeDrawable((HitObjectLifetimeEntry)entry);
private void addDrawable(HitObjectLifetimeEntry entry)
public void Add(HitObjectLifetimeEntry entry)
{
Debug.Assert(!drawableMap.ContainsKey(entry));
allEntries.Add(entry);
lifetimeManager.AddEntry(entry);
}
var drawable = pooledObjectProvider?.GetPooledDrawableRepresentation(entry.HitObject, null);
public bool Remove(HitObjectLifetimeEntry entry)
{
if (!lifetimeManager.RemoveEntry(entry)) return false;
// This logic is not in `Remove(DrawableHitObject)` because a non-pooled drawable may be removed by specifying its entry.
if (nonPooledDrawableMap.Remove(entry, out var drawable))
removeDrawable(drawable);
allEntries.Remove(entry);
return true;
}
private void entryBecameAlive(LifetimeEntry lifetimeEntry)
{
var entry = (HitObjectLifetimeEntry)lifetimeEntry;
Debug.Assert(!aliveDrawableMap.ContainsKey(entry));
bool isNonPooled = nonPooledDrawableMap.TryGetValue(entry, out var drawable);
drawable ??= pooledObjectProvider?.GetPooledDrawableRepresentation(entry.HitObject, null);
if (drawable == null)
throw new InvalidOperationException($"A drawable representation could not be retrieved for hitobject type: {entry.HitObject.GetType().ReadableName()}.");
aliveDrawableMap[entry] = drawable;
OnAdd(drawable);
if (isNonPooled) return;
addDrawable(drawable);
HitObjectUsageBegan?.Invoke(entry.HitObject);
}
private void entryBecameDead(LifetimeEntry lifetimeEntry)
{
var entry = (HitObjectLifetimeEntry)lifetimeEntry;
Debug.Assert(aliveDrawableMap.ContainsKey(entry));
var drawable = aliveDrawableMap[entry];
bool isNonPooled = nonPooledDrawableMap.ContainsKey(entry);
drawable.OnKilled();
aliveDrawableMap.Remove(entry);
OnRemove(drawable);
if (isNonPooled) return;
removeDrawable(drawable);
// The hit object is not freed when the DHO was not pooled.
HitObjectUsageFinished?.Invoke(entry.HitObject);
}
private void addDrawable(DrawableHitObject drawable)
{
drawable.OnNewResult += onNewResult;
drawable.OnRevertResult += onRevertResult;
bindStartTime(drawable);
AddInternal(drawableMap[entry] = drawable, false);
OnAdd(drawable);
HitObjectUsageBegan?.Invoke(entry.HitObject);
AddInternal(drawable);
}
private void removeDrawable(HitObjectLifetimeEntry entry)
private void removeDrawable(DrawableHitObject drawable)
{
Debug.Assert(drawableMap.ContainsKey(entry));
var drawable = drawableMap[entry];
// OnKilled can potentially change the hitobject's result, so it needs to run first before unbinding.
drawable.OnKilled();
drawable.OnNewResult -= onNewResult;
drawable.OnRevertResult -= onRevertResult;
drawableMap.Remove(entry);
OnRemove(drawable);
unbindStartTime(drawable);
RemoveInternal(drawable);
HitObjectUsageFinished?.Invoke(entry.HitObject);
RemoveInternal(drawable);
}
#endregion
#region Non-pooling support
public virtual void Add(DrawableHitObject hitObject)
public virtual void Add(DrawableHitObject drawable)
{
bindStartTime(hitObject);
if (drawable.Entry == null)
throw new InvalidOperationException($"May not add a {nameof(DrawableHitObject)} without {nameof(HitObject)} associated");
hitObject.OnNewResult += onNewResult;
hitObject.OnRevertResult += onRevertResult;
AddInternal(hitObject);
OnAdd(hitObject);
nonPooledDrawableMap.Add(drawable.Entry, drawable);
addDrawable(drawable);
Add(drawable.Entry);
}
public virtual bool Remove(DrawableHitObject hitObject)
public virtual bool Remove(DrawableHitObject drawable)
{
OnRemove(hitObject);
if (!RemoveInternal(hitObject))
if (drawable.Entry == null)
return false;
hitObject.OnNewResult -= onNewResult;
hitObject.OnRevertResult -= onRevertResult;
unbindStartTime(hitObject);
return true;
return Remove(drawable.Entry);
}
public int IndexOf(DrawableHitObject hitObject) => IndexOfInternal(hitObject);
protected override void OnChildLifetimeBoundaryCrossed(LifetimeBoundaryCrossedEvent e)
private void entryCrossedBoundary(LifetimeEntry entry, LifetimeBoundaryKind kind, LifetimeBoundaryCrossingDirection direction)
{
if (!(e.Child is DrawableHitObject hitObject))
return;
if (nonPooledDrawableMap.TryGetValue((HitObjectLifetimeEntry)entry, out var drawable))
OnChildLifetimeBoundaryCrossed(new LifetimeBoundaryCrossedEvent(drawable, kind, direction));
}
if ((e.Kind == LifetimeBoundaryKind.End && e.Direction == LifetimeBoundaryCrossingDirection.Forward)
|| (e.Kind == LifetimeBoundaryKind.Start && e.Direction == LifetimeBoundaryCrossingDirection.Backward))
{
hitObject.OnKilled();
}
protected virtual void OnChildLifetimeBoundaryCrossed(LifetimeBoundaryCrossedEvent e)
{
}
#endregion
@ -195,12 +230,13 @@ namespace osu.Game.Rulesets.UI
{
}
public virtual void Clear(bool disposeChildren = true)
public virtual void Clear()
{
lifetimeManager.ClearEntries();
ClearInternal(disposeChildren);
unbindAllStartTimes();
foreach (var drawable in nonPooledDrawableMap.Values)
removeDrawable(drawable);
nonPooledDrawableMap.Clear();
Debug.Assert(InternalChildren.Count == 0 && startTimeMap.Count == 0 && aliveDrawableMap.Count == 0, "All hit objects should have been removed");
}
protected override bool CheckChildrenLife()

View File

@ -50,9 +50,9 @@ namespace osu.Game.Rulesets.UI.Scrolling
timeRange.ValueChanged += _ => layoutCache.Invalidate();
}
public override void Clear(bool disposeChildren = true)
public override void Clear()
{
base.Clear(disposeChildren);
base.Clear();
toComputeLifetime.Clear();
layoutComputed.Clear();