From 555ce7684b9d57183a603f7469dc23bd36354d83 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 20 Jun 2023 20:04:02 +0900 Subject: [PATCH 01/13] Adjust `GameplaySampleTriggerSource` to only switch samples when close enough to the next hit object Closes #23963. To simplify things, I've removed the optimisation of using `AliveObject`s because it would break the way this whole lookup works. --- .../UI/GameplaySampleTriggerSource.cs | 56 +++++++------------ 1 file changed, 19 insertions(+), 37 deletions(-) diff --git a/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs b/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs index fbb7a20a5d..909b633a72 100644 --- a/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs +++ b/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs @@ -7,7 +7,7 @@ using System.Linq; using osu.Framework.Graphics.Containers; using osu.Game.Audio; using osu.Game.Rulesets.Objects; -using osu.Game.Rulesets.Objects.Drawables; +using osu.Game.Rulesets.Scoring; using osu.Game.Skinning; namespace osu.Game.Rulesets.UI @@ -39,7 +39,7 @@ namespace osu.Game.Rulesets.UI }; } - private HitObjectLifetimeEntry fallbackObject; + private HitObjectLifetimeEntry mostValidObject; /// /// Play the most appropriate hit sound for the current point in time. @@ -67,56 +67,38 @@ namespace osu.Game.Rulesets.UI protected HitObject GetMostValidObject() { - // The most optimal lookup case we have is when an object is alive. There are usually very few alive objects so there's no drawbacks in attempting this lookup each time. - var drawableHitObject = hitObjectContainer.AliveObjects.FirstOrDefault(h => h.Result?.HasResult != true); - - if (drawableHitObject != null) - { - // A hit object may have a more valid nested object. - drawableHitObject = getMostValidNestedDrawable(drawableHitObject); - - return drawableHitObject.HitObject; - } - - // In the case a next object isn't available in drawable form, we need to do a somewhat expensive traversal to get a valid sound to play. - // This lookup can be skipped if the last entry is still valid (in the future and not yet hit). - if (fallbackObject == null || fallbackObject.Result?.HasResult == true) + if (mostValidObject == null || isAlreadyHit(mostValidObject)) { // We need to use lifetime entries to find the next object (we can't just use `hitObjectContainer.Objects` due to pooling - it may even be empty). // If required, we can make this lookup more efficient by adding support to get next-future-entry in LifetimeEntryManager. - fallbackObject = hitObjectContainer.Entries - .Where(e => e.Result?.HasResult != true).MinBy(e => e.HitObject.StartTime); - - if (fallbackObject != null) - return getEarliestNestedObject(fallbackObject.HitObject); + var candidate = hitObjectContainer.Entries.Where(e => !isAlreadyHit(e)).MinBy(e => e.HitObject.StartTime); // In the case there are no non-judged objects, the last hit object should be used instead. - fallbackObject ??= hitObjectContainer.Entries.LastOrDefault(); + if (candidate == null) + mostValidObject = hitObjectContainer.Entries.LastOrDefault(); + else + { + if (isCloseEnoughToCurrentTime(candidate)) + mostValidObject = candidate; + else + mostValidObject ??= hitObjectContainer.Entries.FirstOrDefault(); + } } - if (fallbackObject == null) + if (mostValidObject == null) return null; - bool fallbackHasResult = fallbackObject.Result?.HasResult == true; - // If the fallback has been judged then we want the sample from the object itself. - if (fallbackHasResult) - return fallbackObject.HitObject; + if (isAlreadyHit(mostValidObject)) + return mostValidObject.HitObject; // Else we want the earliest (including nested). // In cases of nested objects, they will always have earlier sample data than their parent object. - return getEarliestNestedObject(fallbackObject.HitObject); + return getEarliestNestedObject(mostValidObject.HitObject); } - private DrawableHitObject getMostValidNestedDrawable(DrawableHitObject o) - { - var nestedWithoutResult = o.NestedHitObjects.FirstOrDefault(n => n.Result?.HasResult != true); - - if (nestedWithoutResult == null) - return o; - - return getMostValidNestedDrawable(nestedWithoutResult); - } + private bool isAlreadyHit(HitObjectLifetimeEntry h) => h.Result?.HasResult == true; + private bool isCloseEnoughToCurrentTime(HitObjectLifetimeEntry h) => Time.Current > h.HitObject.StartTime - h.HitObject.HitWindows.WindowFor(HitResult.Miss) * 1.5; private HitObject getEarliestNestedObject(HitObject hitObject) { From 786d5a394b526db81e5e5435edc486d777a9787a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 20 Jun 2023 20:30:07 +0900 Subject: [PATCH 02/13] Add back optimisation and increase time allowance slightly --- osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs b/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs index 909b633a72..bc410acd9a 100644 --- a/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs +++ b/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs @@ -71,17 +71,26 @@ namespace osu.Game.Rulesets.UI { // We need to use lifetime entries to find the next object (we can't just use `hitObjectContainer.Objects` due to pooling - it may even be empty). // If required, we can make this lookup more efficient by adding support to get next-future-entry in LifetimeEntryManager. - var candidate = hitObjectContainer.Entries.Where(e => !isAlreadyHit(e)).MinBy(e => e.HitObject.StartTime); + var candidate = + // Use alive entries first as an optimisation. + hitObjectContainer.AliveEntries.Select(tuple => tuple.Entry).Where(e => !isAlreadyHit(e)).MinBy(e => e.HitObject.StartTime) + ?? hitObjectContainer.Entries.Where(e => !isAlreadyHit(e)).MinBy(e => e.HitObject.StartTime); // In the case there are no non-judged objects, the last hit object should be used instead. if (candidate == null) + { mostValidObject = hitObjectContainer.Entries.LastOrDefault(); + } else { if (isCloseEnoughToCurrentTime(candidate)) + { mostValidObject = candidate; + } else + { mostValidObject ??= hitObjectContainer.Entries.FirstOrDefault(); + } } } @@ -98,7 +107,7 @@ namespace osu.Game.Rulesets.UI } private bool isAlreadyHit(HitObjectLifetimeEntry h) => h.Result?.HasResult == true; - private bool isCloseEnoughToCurrentTime(HitObjectLifetimeEntry h) => Time.Current > h.HitObject.StartTime - h.HitObject.HitWindows.WindowFor(HitResult.Miss) * 1.5; + private bool isCloseEnoughToCurrentTime(HitObjectLifetimeEntry h) => Time.Current >= h.HitObject.StartTime - h.HitObject.HitWindows.WindowFor(HitResult.Miss) * 2; private HitObject getEarliestNestedObject(HitObject hitObject) { From 0e861026814443b4fff965674e2805b77e2f63d5 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 20 Jun 2023 20:45:02 +0900 Subject: [PATCH 03/13] Fix nested lookups --- .../Rulesets/UI/GameplaySampleTriggerSource.cs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs b/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs index bc410acd9a..8de686a0d6 100644 --- a/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs +++ b/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs @@ -3,6 +3,7 @@ #nullable disable +using System.Collections.Generic; using System.Linq; using osu.Framework.Graphics.Containers; using osu.Game.Audio; @@ -101,19 +102,23 @@ namespace osu.Game.Rulesets.UI if (isAlreadyHit(mostValidObject)) return mostValidObject.HitObject; - // Else we want the earliest (including nested). + // Else we want the earliest valid nested. // In cases of nested objects, they will always have earlier sample data than their parent object. - return getEarliestNestedObject(mostValidObject.HitObject); + return getAllNested(mostValidObject.HitObject).OrderBy(h => h.StartTime).FirstOrDefault(h => h.StartTime > Time.Current) ?? mostValidObject.HitObject; } private bool isAlreadyHit(HitObjectLifetimeEntry h) => h.Result?.HasResult == true; private bool isCloseEnoughToCurrentTime(HitObjectLifetimeEntry h) => Time.Current >= h.HitObject.StartTime - h.HitObject.HitWindows.WindowFor(HitResult.Miss) * 2; - private HitObject getEarliestNestedObject(HitObject hitObject) + private IEnumerable getAllNested(HitObject hitObject) { - var nested = hitObject.NestedHitObjects.FirstOrDefault(); + foreach (var h in hitObject.NestedHitObjects) + { + yield return h; - return nested != null ? getEarliestNestedObject(nested) : hitObject; + foreach (var n in getAllNested(h)) + yield return n; + } } private SkinnableSound getNextSample() From 04dad6c6e8a60dad3e7bcceeb5a13e75c0ac0d61 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 20 Jun 2023 20:47:56 +0900 Subject: [PATCH 04/13] Use `IGameplayClock` to ensure our clock source is correct --- .../Gameplay/TestSceneGameplaySampleTriggerSource.cs | 2 +- osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneGameplaySampleTriggerSource.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneGameplaySampleTriggerSource.cs index e52ec6f8cc..ca4a608114 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneGameplaySampleTriggerSource.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneGameplaySampleTriggerSource.cs @@ -101,7 +101,7 @@ namespace osu.Game.Tests.Visual.Gameplay { base.SetUpSteps(); - AddStep("Add trigger source", () => Player.HUDOverlay.Add(sampleTriggerSource = new TestGameplaySampleTriggerSource(Player.DrawableRuleset.Playfield.HitObjectContainer))); + AddStep("Add trigger source", () => Player.GameplayClockContainer.Add(sampleTriggerSource = new TestGameplaySampleTriggerSource(Player.DrawableRuleset.Playfield.HitObjectContainer))); } [Test] diff --git a/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs b/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs index 8de686a0d6..029cab65ab 100644 --- a/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs +++ b/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs @@ -5,10 +5,12 @@ using System.Collections.Generic; using System.Linq; +using osu.Framework.Allocation; using osu.Framework.Graphics.Containers; using osu.Game.Audio; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Scoring; +using osu.Game.Screens.Play; using osu.Game.Skinning; namespace osu.Game.Rulesets.UI @@ -29,6 +31,9 @@ namespace osu.Game.Rulesets.UI private readonly Container hitSounds; + [Resolved] + private IGameplayClock gameplayClock { get; set; } + public GameplaySampleTriggerSource(HitObjectContainer hitObjectContainer) { this.hitObjectContainer = hitObjectContainer; @@ -104,11 +109,11 @@ namespace osu.Game.Rulesets.UI // Else we want the earliest valid nested. // In cases of nested objects, they will always have earlier sample data than their parent object. - return getAllNested(mostValidObject.HitObject).OrderBy(h => h.StartTime).FirstOrDefault(h => h.StartTime > Time.Current) ?? mostValidObject.HitObject; + return getAllNested(mostValidObject.HitObject).OrderBy(h => h.StartTime).FirstOrDefault(h => h.StartTime > gameplayClock.CurrentTime) ?? mostValidObject.HitObject; } private bool isAlreadyHit(HitObjectLifetimeEntry h) => h.Result?.HasResult == true; - private bool isCloseEnoughToCurrentTime(HitObjectLifetimeEntry h) => Time.Current >= h.HitObject.StartTime - h.HitObject.HitWindows.WindowFor(HitResult.Miss) * 2; + private bool isCloseEnoughToCurrentTime(HitObjectLifetimeEntry h) => gameplayClock.CurrentTime >= h.HitObject.StartTime - h.HitObject.HitWindows.WindowFor(HitResult.Miss) * 2; private IEnumerable getAllNested(HitObject hitObject) { From 92e89c7df7f3f5894bb0e22de59dec1d5ea1d2ad Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 20 Jun 2023 21:02:34 +0900 Subject: [PATCH 05/13] Update test expectations --- .../TestSceneGameplaySampleTriggerSource.cs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneGameplaySampleTriggerSource.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneGameplaySampleTriggerSource.cs index ca4a608114..ca76337568 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneGameplaySampleTriggerSource.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneGameplaySampleTriggerSource.cs @@ -18,6 +18,7 @@ using osu.Game.Rulesets.Objects.Drawables; using osu.Game.Rulesets.Objects.Types; using osu.Game.Rulesets.Osu; using osu.Game.Rulesets.Osu.Objects; +using osu.Game.Rulesets.Scoring; using osu.Game.Rulesets.UI; using osu.Game.Storyboards; using osuTK; @@ -62,25 +63,30 @@ namespace osu.Game.Tests.Visual.Gameplay { new HitCircle { + HitWindows = new HitWindows(), StartTime = t += spacing, Samples = new[] { new HitSampleInfo(HitSampleInfo.HIT_NORMAL) } }, new HitCircle { + HitWindows = new HitWindows(), StartTime = t += spacing, Samples = new[] { new HitSampleInfo(HitSampleInfo.HIT_WHISTLE) } }, new HitCircle { + HitWindows = new HitWindows(), StartTime = t += spacing, Samples = new[] { new HitSampleInfo(HitSampleInfo.HIT_NORMAL, HitSampleInfo.BANK_SOFT) }, }, new HitCircle { + HitWindows = new HitWindows(), StartTime = t += spacing, }, new Slider { + HitWindows = new HitWindows(), StartTime = t += spacing, Path = new SliderPath(PathType.Linear, new[] { Vector2.Zero, Vector2.UnitY * 200 }), Samples = new[] { new HitSampleInfo(HitSampleInfo.HIT_WHISTLE, HitSampleInfo.BANK_SOFT) }, @@ -131,7 +137,12 @@ namespace osu.Game.Tests.Visual.Gameplay AddAssert("first object hit", () => getNextAliveObject()?.Entry?.Result?.HasResult == true); - checkValidObjectIndex(1); + // next object is too far away, so we still use the already hit object. + checkValidObjectIndex(0); + + // still too far away. + seekBeforeIndex(1, 400); + checkValidObjectIndex(0); // Still object 1 as it's not hit yet. seekBeforeIndex(1); @@ -168,9 +179,9 @@ namespace osu.Game.Tests.Visual.Gameplay checkValidObjectIndex(4); } - private void seekBeforeIndex(int index) + private void seekBeforeIndex(int index, double amount = 100) { - AddStep($"seek to just before object {index}", () => Player.GameplayClockContainer.Seek(beatmap.HitObjects[index].StartTime - 100)); + AddStep($"seek to {amount} ms before object {index}", () => Player.GameplayClockContainer.Seek(beatmap.HitObjects[index].StartTime - amount)); waitForCatchUp(); } From cb07f2399fc584069578e2791fb8ecf7f5fd5674 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 20 Jun 2023 21:03:55 +0900 Subject: [PATCH 06/13] Apply NRT to `GameplaySampleTriggerSource` --- osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs b/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs index 029cab65ab..472f91ac27 100644 --- a/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs +++ b/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs @@ -1,8 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -#nullable disable - using System.Collections.Generic; using System.Linq; using osu.Framework.Allocation; @@ -31,8 +29,10 @@ namespace osu.Game.Rulesets.UI private readonly Container hitSounds; + private HitObjectLifetimeEntry? mostValidObject; + [Resolved] - private IGameplayClock gameplayClock { get; set; } + private IGameplayClock gameplayClock { get; set; } = null!; public GameplaySampleTriggerSource(HitObjectContainer hitObjectContainer) { @@ -45,14 +45,12 @@ namespace osu.Game.Rulesets.UI }; } - private HitObjectLifetimeEntry mostValidObject; - /// /// Play the most appropriate hit sound for the current point in time. /// public virtual void Play() { - var nextObject = GetMostValidObject(); + HitObject? nextObject = GetMostValidObject(); if (nextObject == null) return; @@ -71,7 +69,7 @@ namespace osu.Game.Rulesets.UI hitSound.Play(); }); - protected HitObject GetMostValidObject() + protected HitObject? GetMostValidObject() { if (mostValidObject == null || isAlreadyHit(mostValidObject)) { From 2f77675fe7f133bb60c26f9e8826f0ea534e2887 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 20 Jun 2023 21:57:32 +0200 Subject: [PATCH 07/13] Fix errors in tests due to mismatching NRT annotations --- .../TestSceneDrumSampleTriggerSource.cs | 2 +- .../Visual/Gameplay/TestSceneGameplaySampleTriggerSource.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Taiko.Tests/TestSceneDrumSampleTriggerSource.cs b/osu.Game.Rulesets.Taiko.Tests/TestSceneDrumSampleTriggerSource.cs index 287d90b406..23e85d1ae0 100644 --- a/osu.Game.Rulesets.Taiko.Tests/TestSceneDrumSampleTriggerSource.cs +++ b/osu.Game.Rulesets.Taiko.Tests/TestSceneDrumSampleTriggerSource.cs @@ -331,7 +331,7 @@ namespace osu.Game.Rulesets.Taiko.Tests LastPlayedSamples = samples; } - public new HitObject GetMostValidObject() => base.GetMostValidObject(); + public new HitObject? GetMostValidObject() => base.GetMostValidObject(); } } } diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneGameplaySampleTriggerSource.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneGameplaySampleTriggerSource.cs index ca76337568..6701871e8d 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneGameplaySampleTriggerSource.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneGameplaySampleTriggerSource.cs @@ -215,7 +215,7 @@ namespace osu.Game.Tests.Visual.Gameplay { } - public new HitObject GetMostValidObject() => base.GetMostValidObject(); + public new HitObject? GetMostValidObject() => base.GetMostValidObject(); } } } From 29697d4999ab144c08a37bc26443f916fd66fd1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 20 Jun 2023 22:00:02 +0200 Subject: [PATCH 08/13] Fix taiko test scene failing due to missing gameplay clock dependency `GameplayClock` is inscrutable. `TestManualClock` is lifted from another test scene because of `FramedBeatmapClock`'s intensely confusing tendency to not work if it is given a non-adjustable `ManuelClock` instead. --- .../TestSceneDrumSampleTriggerSource.cs | 141 ++++++++++++++---- 1 file changed, 112 insertions(+), 29 deletions(-) diff --git a/osu.Game.Rulesets.Taiko.Tests/TestSceneDrumSampleTriggerSource.cs b/osu.Game.Rulesets.Taiko.Tests/TestSceneDrumSampleTriggerSource.cs index 23e85d1ae0..f45b4e23e4 100644 --- a/osu.Game.Rulesets.Taiko.Tests/TestSceneDrumSampleTriggerSource.cs +++ b/osu.Game.Rulesets.Taiko.Tests/TestSceneDrumSampleTriggerSource.cs @@ -6,7 +6,6 @@ using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Graphics; -using osu.Framework.Graphics.Containers; using osu.Framework.Timing; using osu.Game.Audio; using osu.Game.Beatmaps; @@ -17,14 +16,13 @@ using osu.Game.Rulesets.Taiko.Objects.Drawables; using osu.Game.Rulesets.Taiko.UI; using osu.Game.Rulesets.UI; using osu.Game.Rulesets.UI.Scrolling; +using osu.Game.Screens.Play; using osu.Game.Tests.Visual; namespace osu.Game.Rulesets.Taiko.Tests { public partial class TestSceneDrumSampleTriggerSource : OsuTestScene { - private readonly ManualClock manualClock = new ManualClock(); - [Cached(typeof(IScrollingInfo))] private ScrollingTestContainer.TestScrollingInfo info = new ScrollingTestContainer.TestScrollingInfo { @@ -34,23 +32,25 @@ namespace osu.Game.Rulesets.Taiko.Tests private ScrollingHitObjectContainer hitObjectContainer = null!; private TestDrumSampleTriggerSource triggerSource = null!; + private readonly ManualClock manualClock = new TestManualClock(); + private GameplayClockContainer gameplayClock = null!; [SetUp] public void SetUp() => Schedule(() => { - hitObjectContainer = new ScrollingHitObjectContainer(); - manualClock.CurrentTime = 0; - - Child = new Container + gameplayClock = new GameplayClockContainer(manualClock) { - Clock = new FramedClock(manualClock), RelativeSizeAxes = Axes.Both, Children = new Drawable[] { - hitObjectContainer, + hitObjectContainer = new ScrollingHitObjectContainer(), triggerSource = new TestDrumSampleTriggerSource(hitObjectContainer) } }; + gameplayClock.Reset(0); + + hitObjectContainer.Clock = gameplayClock; + Child = gameplayClock; }); [Test] @@ -75,7 +75,7 @@ namespace osu.Game.Rulesets.Taiko.Tests checkSound(HitType.Centre, HitSampleInfo.HIT_NORMAL, SampleControlPoint.DEFAULT_BANK); checkSound(HitType.Rim, HitSampleInfo.HIT_CLAP, SampleControlPoint.DEFAULT_BANK); - AddStep("seek past hit", () => manualClock.CurrentTime = 200); + AddStep("seek past hit", () => gameplayClock.Seek(200)); AddAssert("most valid object is hit", () => triggerSource.GetMostValidObject(), Is.InstanceOf); checkSound(HitType.Centre, HitSampleInfo.HIT_NORMAL, SampleControlPoint.DEFAULT_BANK); checkSound(HitType.Rim, HitSampleInfo.HIT_CLAP, SampleControlPoint.DEFAULT_BANK); @@ -103,12 +103,67 @@ namespace osu.Game.Rulesets.Taiko.Tests checkSound(HitType.Centre, HitSampleInfo.HIT_NORMAL, HitSampleInfo.BANK_SOFT); checkSound(HitType.Rim, HitSampleInfo.HIT_CLAP, HitSampleInfo.BANK_SOFT); - AddStep("seek past hit", () => manualClock.CurrentTime = 200); + seekTo(200); AddAssert("most valid object is hit", () => triggerSource.GetMostValidObject(), Is.InstanceOf); checkSound(HitType.Centre, HitSampleInfo.HIT_NORMAL, HitSampleInfo.BANK_SOFT); checkSound(HitType.Rim, HitSampleInfo.HIT_CLAP, HitSampleInfo.BANK_SOFT); } + [Test] + public void TestBetweenHits() + { + Hit first = null!, second = null!; + + AddStep("add hit with normal samples", () => + { + first = new Hit + { + StartTime = 100, + Samples = new List + { + new HitSampleInfo(HitSampleInfo.HIT_NORMAL) + } + }; + first.ApplyDefaults(new ControlPointInfo(), new BeatmapDifficulty()); + var drawableHit = new DrawableHit(first); + hitObjectContainer.Add(drawableHit); + }); + AddStep("add hit with soft samples", () => + { + second = new Hit + { + StartTime = 500, + Samples = new List + { + new HitSampleInfo(HitSampleInfo.HIT_NORMAL, HitSampleInfo.BANK_SOFT), + new HitSampleInfo(HitSampleInfo.HIT_CLAP, HitSampleInfo.BANK_SOFT) + } + }; + second.ApplyDefaults(new ControlPointInfo(), new BeatmapDifficulty()); + var drawableHit = new DrawableHit(second); + hitObjectContainer.Add(drawableHit); + }); + + AddAssert("most valid object is first hit", () => triggerSource.GetMostValidObject(), () => Is.EqualTo(first)); + checkSound(HitType.Centre, HitSampleInfo.HIT_NORMAL, HitSampleInfo.BANK_NORMAL); + checkSound(HitType.Rim, HitSampleInfo.HIT_CLAP, HitSampleInfo.BANK_NORMAL); + + seekTo(120); + AddAssert("most valid object is first hit", () => triggerSource.GetMostValidObject(), () => Is.EqualTo(first)); + checkSound(HitType.Centre, HitSampleInfo.HIT_NORMAL, HitSampleInfo.BANK_NORMAL); + checkSound(HitType.Rim, HitSampleInfo.HIT_CLAP, HitSampleInfo.BANK_NORMAL); + + seekTo(480); + AddAssert("most valid object is second hit", () => triggerSource.GetMostValidObject(), () => Is.EqualTo(second)); + checkSound(HitType.Centre, HitSampleInfo.HIT_NORMAL, HitSampleInfo.BANK_SOFT); + checkSound(HitType.Rim, HitSampleInfo.HIT_CLAP, HitSampleInfo.BANK_SOFT); + + seekTo(700); + AddAssert("most valid object is second hit", () => triggerSource.GetMostValidObject(), () => Is.EqualTo(second)); + checkSound(HitType.Centre, HitSampleInfo.HIT_NORMAL, HitSampleInfo.BANK_SOFT); + checkSound(HitType.Rim, HitSampleInfo.HIT_CLAP, HitSampleInfo.BANK_SOFT); + } + [Test] public void TestDrumStrongHit() { @@ -128,11 +183,11 @@ namespace osu.Game.Rulesets.Taiko.Tests hitObjectContainer.Add(drawableHit); }); - AddAssert("most valid object is strong nested hit", () => triggerSource.GetMostValidObject(), Is.InstanceOf); + AddAssert("most valid object is nested strong hit", () => triggerSource.GetMostValidObject(), Is.InstanceOf); checkSound(HitType.Centre, HitSampleInfo.HIT_NORMAL, "drum"); checkSound(HitType.Rim, HitSampleInfo.HIT_CLAP, "drum"); - AddStep("seek past hit", () => manualClock.CurrentTime = 200); + seekTo(200); AddAssert("most valid object is hit", () => triggerSource.GetMostValidObject(), Is.InstanceOf); checkSound(HitType.Centre, HitSampleInfo.HIT_NORMAL, "drum"); checkSound(HitType.Rim, HitSampleInfo.HIT_CLAP, "drum"); @@ -161,12 +216,12 @@ namespace osu.Game.Rulesets.Taiko.Tests checkSound(HitType.Centre, HitSampleInfo.HIT_NORMAL, SampleControlPoint.DEFAULT_BANK); checkSound(HitType.Rim, HitSampleInfo.HIT_CLAP, SampleControlPoint.DEFAULT_BANK); - AddStep("seek to middle of drum roll", () => manualClock.CurrentTime = 600); + seekTo(600); AddAssert("most valid object is drum roll tick", () => triggerSource.GetMostValidObject(), Is.InstanceOf); checkSound(HitType.Centre, HitSampleInfo.HIT_NORMAL, SampleControlPoint.DEFAULT_BANK); checkSound(HitType.Rim, HitSampleInfo.HIT_CLAP, SampleControlPoint.DEFAULT_BANK); - AddStep("seek past drum roll", () => manualClock.CurrentTime = 1200); + seekTo(1200); AddAssert("most valid object is drum roll", () => triggerSource.GetMostValidObject(), Is.InstanceOf); checkSound(HitType.Centre, HitSampleInfo.HIT_NORMAL, SampleControlPoint.DEFAULT_BANK); checkSound(HitType.Rim, HitSampleInfo.HIT_CLAP, SampleControlPoint.DEFAULT_BANK); @@ -195,12 +250,12 @@ namespace osu.Game.Rulesets.Taiko.Tests checkSound(HitType.Centre, HitSampleInfo.HIT_NORMAL, HitSampleInfo.BANK_SOFT); checkSound(HitType.Rim, HitSampleInfo.HIT_CLAP, HitSampleInfo.BANK_SOFT); - AddStep("seek to middle of drum roll", () => manualClock.CurrentTime = 600); + seekTo(600); AddAssert("most valid object is drum roll tick", () => triggerSource.GetMostValidObject(), Is.InstanceOf); checkSound(HitType.Centre, HitSampleInfo.HIT_NORMAL, HitSampleInfo.BANK_SOFT); checkSound(HitType.Rim, HitSampleInfo.HIT_CLAP, HitSampleInfo.BANK_SOFT); - AddStep("seek past drum roll", () => manualClock.CurrentTime = 1200); + seekTo(1200); AddAssert("most valid object is drum roll", () => triggerSource.GetMostValidObject(), Is.InstanceOf); checkSound(HitType.Centre, HitSampleInfo.HIT_NORMAL, HitSampleInfo.BANK_SOFT); checkSound(HitType.Rim, HitSampleInfo.HIT_CLAP, HitSampleInfo.BANK_SOFT); @@ -226,16 +281,16 @@ namespace osu.Game.Rulesets.Taiko.Tests hitObjectContainer.Add(drawableDrumRoll); }); - AddAssert("most valid object is drum roll tick's nested strong hit", () => triggerSource.GetMostValidObject(), Is.InstanceOf); + AddAssert("most valid object is drum roll tick", () => triggerSource.GetMostValidObject(), Is.InstanceOf); checkSound(HitType.Centre, HitSampleInfo.HIT_NORMAL, "drum"); checkSound(HitType.Rim, HitSampleInfo.HIT_CLAP, "drum"); - AddStep("seek to middle of drum roll", () => manualClock.CurrentTime = 600); - AddAssert("most valid object is drum roll tick's nested strong hit", () => triggerSource.GetMostValidObject(), Is.InstanceOf); + seekTo(600); + AddAssert("most valid object is drum roll tick", () => triggerSource.GetMostValidObject(), Is.InstanceOf); checkSound(HitType.Centre, HitSampleInfo.HIT_NORMAL, "drum"); checkSound(HitType.Rim, HitSampleInfo.HIT_CLAP, "drum"); - AddStep("seek past drum roll", () => manualClock.CurrentTime = 1200); + seekTo(1200); AddAssert("most valid object is drum roll", () => triggerSource.GetMostValidObject(), Is.InstanceOf); checkSound(HitType.Centre, HitSampleInfo.HIT_NORMAL, "drum"); checkSound(HitType.Rim, HitSampleInfo.HIT_CLAP, "drum"); @@ -260,16 +315,16 @@ namespace osu.Game.Rulesets.Taiko.Tests hitObjectContainer.Add(drawableSwell); }); - AddAssert("most valid object is swell tick", () => triggerSource.GetMostValidObject(), Is.InstanceOf); + AddAssert("most valid object is swell", () => triggerSource.GetMostValidObject(), Is.InstanceOf); checkSound(HitType.Centre, HitSampleInfo.HIT_NORMAL, SampleControlPoint.DEFAULT_BANK); checkSound(HitType.Rim, HitSampleInfo.HIT_CLAP, SampleControlPoint.DEFAULT_BANK); - AddStep("seek to middle of swell", () => manualClock.CurrentTime = 600); - AddAssert("most valid object is swell tick", () => triggerSource.GetMostValidObject(), Is.InstanceOf); + seekTo(600); + AddAssert("most valid object is swell", () => triggerSource.GetMostValidObject(), Is.InstanceOf); checkSound(HitType.Centre, HitSampleInfo.HIT_NORMAL, SampleControlPoint.DEFAULT_BANK); checkSound(HitType.Rim, HitSampleInfo.HIT_CLAP, SampleControlPoint.DEFAULT_BANK); - AddStep("seek past swell", () => manualClock.CurrentTime = 1200); + seekTo(1200); AddAssert("most valid object is swell", () => triggerSource.GetMostValidObject(), Is.InstanceOf); checkSound(HitType.Centre, HitSampleInfo.HIT_NORMAL, SampleControlPoint.DEFAULT_BANK); checkSound(HitType.Rim, HitSampleInfo.HIT_CLAP, SampleControlPoint.DEFAULT_BANK); @@ -294,16 +349,16 @@ namespace osu.Game.Rulesets.Taiko.Tests hitObjectContainer.Add(drawableSwell); }); - AddAssert("most valid object is swell tick", () => triggerSource.GetMostValidObject(), Is.InstanceOf); + AddAssert("most valid object is swell", () => triggerSource.GetMostValidObject(), Is.InstanceOf); checkSound(HitType.Centre, HitSampleInfo.HIT_NORMAL, "drum"); checkSound(HitType.Rim, HitSampleInfo.HIT_CLAP, "drum"); - AddStep("seek to middle of swell", () => manualClock.CurrentTime = 600); - AddAssert("most valid object is swell tick", () => triggerSource.GetMostValidObject(), Is.InstanceOf); + seekTo(600); + AddAssert("most valid object is swell", () => triggerSource.GetMostValidObject(), Is.InstanceOf); checkSound(HitType.Centre, HitSampleInfo.HIT_NORMAL, "drum"); checkSound(HitType.Rim, HitSampleInfo.HIT_CLAP, "drum"); - AddStep("seek past swell", () => manualClock.CurrentTime = 1200); + seekTo(1200); AddAssert("most valid object is swell", () => triggerSource.GetMostValidObject(), Is.InstanceOf); checkSound(HitType.Centre, HitSampleInfo.HIT_NORMAL, "drum"); checkSound(HitType.Rim, HitSampleInfo.HIT_CLAP, "drum"); @@ -316,6 +371,8 @@ namespace osu.Game.Rulesets.Taiko.Tests AddAssert($"last played sample has {expectedBank} bank", () => triggerSource.LastPlayedSamples!.OfType().Single().Bank, () => Is.EqualTo(expectedBank)); } + private void seekTo(double time) => AddStep($"seek to {time}", () => gameplayClock.Seek(time)); + private partial class TestDrumSampleTriggerSource : DrumSampleTriggerSource { public ISampleInfo[]? LastPlayedSamples { get; private set; } @@ -333,5 +390,31 @@ namespace osu.Game.Rulesets.Taiko.Tests public new HitObject? GetMostValidObject() => base.GetMostValidObject(); } + + private class TestManualClock : ManualClock, IAdjustableClock + { + public TestManualClock() + { + IsRunning = true; + } + + public void Start() => IsRunning = true; + + public void Stop() => IsRunning = false; + + public bool Seek(double position) + { + CurrentTime = position; + return true; + } + + public void Reset() + { + } + + public void ResetSpeedAdjustments() + { + } + } } } From a7172e74699265b34898083317c38632e94599b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 20 Jun 2023 23:32:38 +0200 Subject: [PATCH 09/13] Fix one remaining seek --- .../TestSceneDrumSampleTriggerSource.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Taiko.Tests/TestSceneDrumSampleTriggerSource.cs b/osu.Game.Rulesets.Taiko.Tests/TestSceneDrumSampleTriggerSource.cs index f45b4e23e4..e8da7f6937 100644 --- a/osu.Game.Rulesets.Taiko.Tests/TestSceneDrumSampleTriggerSource.cs +++ b/osu.Game.Rulesets.Taiko.Tests/TestSceneDrumSampleTriggerSource.cs @@ -75,7 +75,7 @@ namespace osu.Game.Rulesets.Taiko.Tests checkSound(HitType.Centre, HitSampleInfo.HIT_NORMAL, SampleControlPoint.DEFAULT_BANK); checkSound(HitType.Rim, HitSampleInfo.HIT_CLAP, SampleControlPoint.DEFAULT_BANK); - AddStep("seek past hit", () => gameplayClock.Seek(200)); + seekTo(200); AddAssert("most valid object is hit", () => triggerSource.GetMostValidObject(), Is.InstanceOf); checkSound(HitType.Centre, HitSampleInfo.HIT_NORMAL, SampleControlPoint.DEFAULT_BANK); checkSound(HitType.Rim, HitSampleInfo.HIT_CLAP, SampleControlPoint.DEFAULT_BANK); From 4ff52752082f93906df68ef0d8cb1a1048d44f92 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 21 Jun 2023 17:33:42 +0900 Subject: [PATCH 10/13] Make `IGameplayClock` optional in `GameplaySampleTriggerSource` to ease testing --- osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs b/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs index 472f91ac27..b67e977822 100644 --- a/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs +++ b/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs @@ -32,7 +32,7 @@ namespace osu.Game.Rulesets.UI private HitObjectLifetimeEntry? mostValidObject; [Resolved] - private IGameplayClock gameplayClock { get; set; } = null!; + private IGameplayClock? gameplayClock { get; set; } public GameplaySampleTriggerSource(HitObjectContainer hitObjectContainer) { @@ -87,7 +87,7 @@ namespace osu.Game.Rulesets.UI } else { - if (isCloseEnoughToCurrentTime(candidate)) + if (isCloseEnoughToCurrentTime(candidate.HitObject)) { mostValidObject = candidate; } @@ -107,11 +107,13 @@ namespace osu.Game.Rulesets.UI // Else we want the earliest valid nested. // In cases of nested objects, they will always have earlier sample data than their parent object. - return getAllNested(mostValidObject.HitObject).OrderBy(h => h.StartTime).FirstOrDefault(h => h.StartTime > gameplayClock.CurrentTime) ?? mostValidObject.HitObject; + return getAllNested(mostValidObject.HitObject).OrderBy(h => h.StartTime).FirstOrDefault(h => h.StartTime > getReferenceTime()) ?? mostValidObject.HitObject; } private bool isAlreadyHit(HitObjectLifetimeEntry h) => h.Result?.HasResult == true; - private bool isCloseEnoughToCurrentTime(HitObjectLifetimeEntry h) => gameplayClock.CurrentTime >= h.HitObject.StartTime - h.HitObject.HitWindows.WindowFor(HitResult.Miss) * 2; + private bool isCloseEnoughToCurrentTime(HitObject h) => getReferenceTime() >= h.StartTime - h.HitWindows.WindowFor(HitResult.Miss) * 2; + + private double getReferenceTime() => (gameplayClock?.CurrentTime ?? Clock.CurrentTime); private IEnumerable getAllNested(HitObject hitObject) { From 79606317ab671c93bc6c8d650f25240d1ed52cfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 22 Jun 2023 20:02:10 +0200 Subject: [PATCH 11/13] Remove redundant parentheses --- osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs b/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs index b67e977822..d8f421e54f 100644 --- a/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs +++ b/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs @@ -113,7 +113,7 @@ namespace osu.Game.Rulesets.UI private bool isAlreadyHit(HitObjectLifetimeEntry h) => h.Result?.HasResult == true; private bool isCloseEnoughToCurrentTime(HitObject h) => getReferenceTime() >= h.StartTime - h.HitWindows.WindowFor(HitResult.Miss) * 2; - private double getReferenceTime() => (gameplayClock?.CurrentTime ?? Clock.CurrentTime); + private double getReferenceTime() => gameplayClock?.CurrentTime ?? Clock.CurrentTime; private IEnumerable getAllNested(HitObject hitObject) { From 64b726d5ecaaf1ef6c3b71bed4523990117300e9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 23 Jun 2023 13:48:13 +0900 Subject: [PATCH 12/13] Fix nested logic not being completely correct (favouring already-passed rather than near-future) --- osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs b/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs index d8f421e54f..c554318357 100644 --- a/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs +++ b/osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs @@ -107,7 +107,7 @@ namespace osu.Game.Rulesets.UI // Else we want the earliest valid nested. // In cases of nested objects, they will always have earlier sample data than their parent object. - return getAllNested(mostValidObject.HitObject).OrderBy(h => h.StartTime).FirstOrDefault(h => h.StartTime > getReferenceTime()) ?? mostValidObject.HitObject; + return getAllNested(mostValidObject.HitObject).OrderBy(h => h.GetEndTime()).SkipWhile(h => h.GetEndTime() <= getReferenceTime()).FirstOrDefault() ?? mostValidObject.HitObject; } private bool isAlreadyHit(HitObjectLifetimeEntry h) => h.Result?.HasResult == true; From d53996336cb81a217e06f998ebdc440c17cf1145 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 23 Jun 2023 14:01:10 +0900 Subject: [PATCH 13/13] Add note about swells and their ticks --- .../TestSceneDrumSampleTriggerSource.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/osu.Game.Rulesets.Taiko.Tests/TestSceneDrumSampleTriggerSource.cs b/osu.Game.Rulesets.Taiko.Tests/TestSceneDrumSampleTriggerSource.cs index e8da7f6937..bce855ae45 100644 --- a/osu.Game.Rulesets.Taiko.Tests/TestSceneDrumSampleTriggerSource.cs +++ b/osu.Game.Rulesets.Taiko.Tests/TestSceneDrumSampleTriggerSource.cs @@ -315,6 +315,9 @@ namespace osu.Game.Rulesets.Taiko.Tests hitObjectContainer.Add(drawableSwell); }); + // You might think that this should be a SwellTick since we're before the swell, but SwellTicks get no StartTime (ie. they are zero). + // This works fine in gameplay because they are judged whenever the user pressed, rather than being timed hits. + // But for sample playback purposes they can be ignored as noise. AddAssert("most valid object is swell", () => triggerSource.GetMostValidObject(), Is.InstanceOf); checkSound(HitType.Centre, HitSampleInfo.HIT_NORMAL, SampleControlPoint.DEFAULT_BANK); checkSound(HitType.Rim, HitSampleInfo.HIT_CLAP, SampleControlPoint.DEFAULT_BANK); @@ -349,6 +352,9 @@ namespace osu.Game.Rulesets.Taiko.Tests hitObjectContainer.Add(drawableSwell); }); + // You might think that this should be a SwellTick since we're before the swell, but SwellTicks get no StartTime (ie. they are zero). + // This works fine in gameplay because they are judged whenever the user pressed, rather than being timed hits. + // But for sample playback purposes they can be ignored as noise. AddAssert("most valid object is swell", () => triggerSource.GetMostValidObject(), Is.InstanceOf); checkSound(HitType.Centre, HitSampleInfo.HIT_NORMAL, "drum"); checkSound(HitType.Rim, HitSampleInfo.HIT_CLAP, "drum");