From e1c4c8f3d5401ce298c4f3392a1464103a931385 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 5 Oct 2020 13:16:45 +0900 Subject: [PATCH 1/6] Add failing test coverage of gameplay sample pausing (during seek) --- .../TestSceneGameplaySamplePlayback.cs | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 osu.Game.Tests/Visual/Gameplay/TestSceneGameplaySamplePlayback.cs diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneGameplaySamplePlayback.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneGameplaySamplePlayback.cs new file mode 100644 index 0000000000..3ab4df20df --- /dev/null +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneGameplaySamplePlayback.cs @@ -0,0 +1,61 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Linq; +using NUnit.Framework; +using osu.Framework.Graphics.Audio; +using osu.Framework.Testing; +using osu.Game.Rulesets; +using osu.Game.Rulesets.Osu; +using osu.Game.Rulesets.Osu.Objects.Drawables; +using osu.Game.Rulesets.UI; +using osu.Game.Screens.Play; +using osu.Game.Skinning; + +namespace osu.Game.Tests.Visual.Gameplay +{ + public class TestSceneGameplaySamplePlayback : PlayerTestScene + { + [Test] + public void TestAllSamplesStopDuringSeek() + { + DrawableSlider slider = null; + DrawableSample[] samples = null; + ISamplePlaybackDisabler gameplayClock = null; + + AddStep("get variables", () => + { + gameplayClock = Player.ChildrenOfType().First().GameplayClock; + slider = Player.ChildrenOfType().OrderBy(s => s.HitObject.StartTime).First(); + samples = slider.ChildrenOfType().ToArray(); + }); + + AddUntilStep("wait for slider sliding then seek", () => + { + if (!slider.Tracking.Value) + return false; + + if (!samples.Any(s => s.Playing)) + return false; + + Player.ChildrenOfType().First().Seek(40000); + return true; + }); + + AddAssert("sample playback disabled", () => gameplayClock.SamplePlaybackDisabled.Value); + + // because we are in frame stable context, it's quite likely that not all samples are "played" at this point. + // the important thing is that at least one started, and that sample has since stopped. + AddAssert("no samples are playing", () => Player.ChildrenOfType().All(s => !s.IsPlaying)); + + AddAssert("sample playback still disabled", () => gameplayClock.SamplePlaybackDisabled.Value); + + AddUntilStep("seek finished, sample playback enabled", () => !gameplayClock.SamplePlaybackDisabled.Value); + AddUntilStep("any sample is playing", () => Player.ChildrenOfType().Any(s => s.IsPlaying)); + } + + protected override bool Autoplay => true; + + protected override Ruleset CreatePlayerRuleset() => new OsuRuleset(); + } +} From 2a46f905ff130c465676019d2e9daed638543870 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 5 Oct 2020 12:45:37 +0900 Subject: [PATCH 2/6] Remove unnecessary IsSeeking checks from taiko drum implementation --- osu.Game.Rulesets.Taiko/UI/InputDrum.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/osu.Game.Rulesets.Taiko/UI/InputDrum.cs b/osu.Game.Rulesets.Taiko/UI/InputDrum.cs index 5966b24b34..1ca1be1bdf 100644 --- a/osu.Game.Rulesets.Taiko/UI/InputDrum.cs +++ b/osu.Game.Rulesets.Taiko/UI/InputDrum.cs @@ -163,16 +163,14 @@ namespace osu.Game.Rulesets.Taiko.UI target = centreHit; back = centre; - if (gameplayClock?.IsSeeking != true) - drumSample.Centre?.Play(); + drumSample.Centre?.Play(); } else if (action == RimAction) { target = rimHit; back = rim; - if (gameplayClock?.IsSeeking != true) - drumSample.Rim?.Play(); + drumSample.Rim?.Play(); } if (target != null) From af7d10afe0f532e7d339a0c28675817cd0b11226 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 5 Oct 2020 12:45:57 +0900 Subject: [PATCH 3/6] Fix FrameStabilityContainer not re-caching its GameplayClock correctly --- osu.Game/Rulesets/UI/FrameStabilityContainer.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs index 55c4edfbd1..668cbbdc35 100644 --- a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs +++ b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs @@ -35,6 +35,7 @@ namespace osu.Game.Rulesets.UI public GameplayClock GameplayClock => stabilityGameplayClock; [Cached(typeof(GameplayClock))] + [Cached(typeof(ISamplePlaybackDisabler))] private readonly StabilityGameplayClock stabilityGameplayClock; public FrameStabilityContainer(double gameplayStartTime = double.MinValue) From e4710f82ec5a06258970ec01d9073838b8d7e581 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 5 Oct 2020 12:46:15 +0900 Subject: [PATCH 4/6] Fix sample disabled status not being updated correctly from seek state --- osu.Game/Rulesets/UI/FrameStabilityContainer.cs | 4 +++- osu.Game/Screens/Play/GameplayClock.cs | 12 ++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs index 668cbbdc35..6956d3c31a 100644 --- a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs +++ b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs @@ -228,7 +228,9 @@ namespace osu.Game.Rulesets.UI { } - public override bool IsSeeking => ParentGameplayClock != null && Math.Abs(CurrentTime - ParentGameplayClock.CurrentTime) > 200; + protected override bool ShouldDisableSamplePlayback => + // handle the case where playback is catching up to real-time. + base.ShouldDisableSamplePlayback || (ParentGameplayClock != null && Math.Abs(CurrentTime - ParentGameplayClock.CurrentTime) > 200); } } } diff --git a/osu.Game/Screens/Play/GameplayClock.cs b/osu.Game/Screens/Play/GameplayClock.cs index 9f2868573e..eeea6777c6 100644 --- a/osu.Game/Screens/Play/GameplayClock.cs +++ b/osu.Game/Screens/Play/GameplayClock.cs @@ -28,6 +28,8 @@ namespace osu.Game.Screens.Play /// public virtual IEnumerable> NonGameplayAdjustments => Enumerable.Empty>(); + private readonly Bindable samplePlaybackDisabled = new Bindable(); + public GameplayClock(IFrameBasedClock underlyingClock) { this.underlyingClock = underlyingClock; @@ -62,13 +64,15 @@ namespace osu.Game.Screens.Play public bool IsRunning => underlyingClock.IsRunning; /// - /// Whether an ongoing seek operation is active. + /// Whether nested samples supporting the interface should be paused. /// - public virtual bool IsSeeking => false; + protected virtual bool ShouldDisableSamplePlayback => IsPaused.Value; public void ProcessFrame() { - // we do not want to process the underlying clock. + // intentionally not updating the underlying clock (handled externally). + + samplePlaybackDisabled.Value = ShouldDisableSamplePlayback; } public double ElapsedFrameTime => underlyingClock.ElapsedFrameTime; @@ -79,6 +83,6 @@ namespace osu.Game.Screens.Play public IClock Source => underlyingClock; - public IBindable SamplePlaybackDisabled => IsPaused; + IBindable ISamplePlaybackDisabler.SamplePlaybackDisabled => samplePlaybackDisabled; } } From ae8bf8cdd4ca70eb5455b4389f4be459783b8c4a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 5 Oct 2020 12:47:00 +0900 Subject: [PATCH 5/6] Fix StabilityGameClock not being updated --- osu.Game/Rulesets/UI/FrameStabilityContainer.cs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs index 6956d3c31a..f32f8d177b 100644 --- a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs +++ b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs @@ -208,11 +208,15 @@ namespace osu.Game.Rulesets.UI private void setClock() { - // in case a parent gameplay clock isn't available, just use the parent clock. - parentGameplayClock ??= Clock; - - Clock = GameplayClock; - ProcessCustomClock = false; + if (parentGameplayClock == null) + { + // in case a parent gameplay clock isn't available, just use the parent clock. + parentGameplayClock ??= Clock; + } + else + { + Clock = GameplayClock; + } } public ReplayInputHandler ReplayInputHandler { get; set; } From 0605bb9b8d565b843dda3fdbf9702474fc8a5592 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 5 Oct 2020 16:20:29 +0900 Subject: [PATCH 6/6] Fix incorrect parent state transfer --- osu.Game/Rulesets/UI/FrameStabilityContainer.cs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs index f32f8d177b..70b3d0c7d4 100644 --- a/osu.Game/Rulesets/UI/FrameStabilityContainer.cs +++ b/osu.Game/Rulesets/UI/FrameStabilityContainer.cs @@ -59,13 +59,16 @@ namespace osu.Game.Rulesets.UI private int direction; [BackgroundDependencyLoader(true)] - private void load(GameplayClock clock) + private void load(GameplayClock clock, ISamplePlaybackDisabler sampleDisabler) { if (clock != null) { parentGameplayClock = stabilityGameplayClock.ParentGameplayClock = clock; GameplayClock.IsPaused.BindTo(clock.IsPaused); } + + // this is a bit temporary. should really be done inside of GameplayClock (but requires large structural changes). + stabilityGameplayClock.ParentSampleDisabler = sampleDisabler; } protected override void LoadComplete() @@ -225,6 +228,8 @@ namespace osu.Game.Rulesets.UI { public GameplayClock ParentGameplayClock; + public ISamplePlaybackDisabler ParentSampleDisabler; + public override IEnumerable> NonGameplayAdjustments => ParentGameplayClock?.NonGameplayAdjustments ?? Enumerable.Empty>(); public StabilityGameplayClock(FramedClock underlyingClock) @@ -234,7 +239,9 @@ namespace osu.Game.Rulesets.UI protected override bool ShouldDisableSamplePlayback => // handle the case where playback is catching up to real-time. - base.ShouldDisableSamplePlayback || (ParentGameplayClock != null && Math.Abs(CurrentTime - ParentGameplayClock.CurrentTime) > 200); + base.ShouldDisableSamplePlayback + || ParentSampleDisabler?.SamplePlaybackDisabled.Value == true + || (ParentGameplayClock != null && Math.Abs(CurrentTime - ParentGameplayClock.CurrentTime) > 200); } } }