diff --git a/osu.Game.Rulesets.Mania.Tests/Editor/TestSceneNotePlacementBlueprint.cs b/osu.Game.Rulesets.Mania.Tests/Editor/TestSceneNotePlacementBlueprint.cs index 36c34a8fb9..a162c5ec44 100644 --- a/osu.Game.Rulesets.Mania.Tests/Editor/TestSceneNotePlacementBlueprint.cs +++ b/osu.Game.Rulesets.Mania.Tests/Editor/TestSceneNotePlacementBlueprint.cs @@ -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().Single().ScreenSpaceDrawQuad.BottomLeft - new Vector2(0, 10))); + AddStep("move mouse before current time", () => + { + var column = this.ChildrenOfType().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().Single())); + AddStep("move mouse after current time", () => + { + var column = this.ChildrenOfType().Single(); + InputManager.MoveMouseTo(column.ScreenSpacePositionAtTime(100)); + }); AddStep("click", () => InputManager.Click(MouseButton.Left)); diff --git a/osu.Game.Rulesets.Mania.Tests/TestSceneDrawableManiaHitObject.cs b/osu.Game.Rulesets.Mania.Tests/TestSceneDrawableManiaHitObject.cs new file mode 100644 index 0000000000..4a6c59e297 --- /dev/null +++ b/osu.Game.Rulesets.Mania.Tests/TestSceneDrawableManiaHitObject.cs @@ -0,0 +1,67 @@ +// Copyright (c) ppy Pty Ltd . 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); + } + } +} diff --git a/osu.Game.Rulesets.Mania.Tests/TestSceneHoldNoteInput.cs b/osu.Game.Rulesets.Mania.Tests/TestSceneHoldNoteInput.cs index 668487f673..387c5f4195 100644 --- a/osu.Game.Rulesets.Mania.Tests/TestSceneHoldNoteInput.cs +++ b/osu.Game.Rulesets.Mania.Tests/TestSceneHoldNoteInput.cs @@ -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(); }); - 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() - .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 diff --git a/osu.Game.Rulesets.Mania/Edit/ManiaBeatSnapGrid.cs b/osu.Game.Rulesets.Mania/Edit/ManiaBeatSnapGrid.cs index afc08dcc96..9d1f5429a1 100644 --- a/osu.Game.Rulesets.Mania/Edit/ManiaBeatSnapGrid.cs +++ b/osu.Game.Rulesets.Mania/Edit/ManiaBeatSnapGrid.cs @@ -101,7 +101,7 @@ namespace osu.Game.Rulesets.Mania.Edit foreach (var line in grid.Objects.OfType()) availableLines.Push(line); - grid.Clear(false); + grid.Clear(); } if (selectionTimeRange == null) diff --git a/osu.Game.Rulesets.Taiko.Tests/HitObjectApplicationTestScene.cs b/osu.Game.Rulesets.Taiko.Tests/HitObjectApplicationTestScene.cs index a1d000386f..ac01508081 100644 --- a/osu.Game.Rulesets.Taiko.Tests/HitObjectApplicationTestScene.cs +++ b/osu.Game.Rulesets.Taiko.Tests/HitObjectApplicationTestScene.cs @@ -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)); diff --git a/osu.Game.Tests/Gameplay/TestSceneDrawableHitObject.cs b/osu.Game.Tests/Gameplay/TestSceneDrawableHitObject.cs index 2e3f192f1b..0bec02c488 100644 --- a/osu.Game.Tests/Gameplay/TestSceneDrawableHitObject.cs +++ b/osu.Game.Tests/Gameplay/TestSceneDrawableHitObject.cs @@ -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); diff --git a/osu.Game/Rulesets/Objects/Pooling/PoolableDrawableWithLifetime.cs b/osu.Game/Rulesets/Objects/Pooling/PoolableDrawableWithLifetime.cs index 93e476be76..ed0430012a 100644 --- a/osu.Game/Rulesets/Objects/Pooling/PoolableDrawableWithLifetime.cs +++ b/osu.Game/Rulesets/Objects/Pooling/PoolableDrawableWithLifetime.cs @@ -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 /// /// The entry holding essential state of this . /// - protected TEntry? Entry { get; private set; } + public TEntry? Entry { get; private set; } /// /// Whether is applied to this . @@ -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)} 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)} 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; } } diff --git a/osu.Game/Rulesets/UI/HitObjectContainer.cs b/osu.Game/Rulesets/UI/HitObjectContainer.cs index 11312a46df..dcf350cbd4 100644 --- a/osu.Game/Rulesets/UI/HitObjectContainer.cs +++ b/osu.Game/Rulesets/UI/HitObjectContainer.cs @@ -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 { + /// + /// All entries in this including dead entries. + /// + public IEnumerable Entries => allEntries; + + /// + /// All alive entries and s used by the entries. + /// + public IEnumerable<(HitObjectLifetimeEntry Entry, DrawableHitObject Drawable)> AliveEntries => aliveDrawableMap.Select(x => (x.Key, x.Value)); + public IEnumerable Objects => InternalChildren.Cast().OrderBy(h => h.HitObject.StartTime); public IEnumerable AliveObjects => AliveInternalChildren.Cast().OrderBy(h => h.HitObject.StartTime); @@ -60,8 +70,12 @@ namespace osu.Game.Rulesets.UI internal double FutureLifetimeExtension { get; set; } private readonly Dictionary startTimeMap = new Dictionary(); - private readonly Dictionary drawableMap = new Dictionary(); + + private readonly Dictionary aliveDrawableMap = new Dictionary(); + private readonly Dictionary nonPooledDrawableMap = new Dictionary(); + private readonly LifetimeEntryManager lifetimeManager = new LifetimeEntryManager(); + private readonly HashSet allEntries = new HashSet(); [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() diff --git a/osu.Game/Rulesets/UI/Scrolling/ScrollingHitObjectContainer.cs b/osu.Game/Rulesets/UI/Scrolling/ScrollingHitObjectContainer.cs index 289578f3d8..a9eaf3da68 100644 --- a/osu.Game/Rulesets/UI/Scrolling/ScrollingHitObjectContainer.cs +++ b/osu.Game/Rulesets/UI/Scrolling/ScrollingHitObjectContainer.cs @@ -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();