From 7b28a66fc074a869d3af3f86ca2cf5c1d5e463d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 29 Feb 2024 11:11:30 +0100 Subject: [PATCH 1/4] Add failing test case --- .../TestSceneSliderInput.cs | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs index 12be74c4cc..286e4bd775 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs @@ -457,6 +457,33 @@ namespace osu.Game.Rulesets.Osu.Tests assertMidSliderJudgementFail(); } + [Test] + public void TestRewindHandling() + { + performTest(new List + { + new OsuReplayFrame { Position = new Vector2(0), Actions = { OsuAction.LeftButton }, Time = time_slider_start }, + new OsuReplayFrame { Position = new Vector2(175, 0), Actions = { OsuAction.LeftButton }, Time = 3250 }, + new OsuReplayFrame { Position = new Vector2(175, 0), Actions = { OsuAction.LeftButton }, Time = time_slider_end }, + }, new Slider + { + StartTime = time_slider_start, + Position = new Vector2(0, 0), + Path = new SliderPath(PathType.PERFECT_CURVE, new[] + { + Vector2.Zero, + new Vector2(250, 0), + }, 250), + }); + + AddUntilStep("wait for completion", () => currentPlayer.ScoreProcessor.HasCompleted.Value); + AddAssert("no miss judgements recorded", () => judgementResults.All(r => r.Type.IsHit())); + + AddStep("rewind to middle of slider", () => currentPlayer.Seek(time_during_slide_4)); + AddUntilStep("wait for completion", () => currentPlayer.ScoreProcessor.HasCompleted.Value); + AddAssert("no miss judgements recorded", () => judgementResults.All(r => r.Type.IsHit())); + } + private void assertAllMaxJudgements() { AddAssert("All judgements max", () => From d05b31933fabb30da23e90cad95f3ea91ac40e48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 29 Feb 2024 11:44:14 +0100 Subject: [PATCH 2/4] Fix slider tracking state not restoring correctly in all cases on rewind --- .../Objects/Drawables/SliderInputManager.cs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/SliderInputManager.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/SliderInputManager.cs index 148cf79337..58fa04bb4c 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/SliderInputManager.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/SliderInputManager.cs @@ -5,11 +5,13 @@ using System; using System.Collections.Generic; using System.Diagnostics; using System.Linq; +using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Input; using osu.Framework.Input.Events; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Objects.Types; +using osu.Game.Screens.Play; using osuTK; namespace osu.Game.Rulesets.Osu.Objects.Drawables @@ -21,6 +23,11 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables /// public bool Tracking { get; private set; } + [Resolved] + private IGameplayClock? gameplayClock { get; set; } + + private readonly Stack<(double time, bool tracking)> trackingHistory = new Stack<(double, bool)>(); + /// /// The point in time after which we can accept any key for tracking. Before this time, we may need to restrict tracking to the key used to hit the head circle. /// @@ -208,6 +215,19 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables /// Whether the current mouse position is valid to begin tracking. private void updateTracking(bool isValidTrackingPosition) { + if (gameplayClock?.IsRewinding == true) + { + while (trackingHistory.TryPeek(out var historyEntry) && Time.Current < historyEntry.time) + trackingHistory.Pop(); + + Debug.Assert(trackingHistory.Count > 0); + + Tracking = trackingHistory.Peek().tracking; + return; + } + + bool wasTracking = Tracking; + // from the point at which the head circle is hit, this will be non-null. // it may be null if the head circle was missed. OsuAction? headCircleHitAction = getInitialHitAction(); @@ -247,6 +267,9 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables && isValidTrackingPosition // valid action && validTrackingAction; + + if (wasTracking != Tracking) + trackingHistory.Push((Time.Current, Tracking)); } private OsuAction? getInitialHitAction() => slider.HeadCircle?.HitAction; From 1d1db951f08731604676bcca3c470a8416e23dce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 29 Feb 2024 11:54:42 +0100 Subject: [PATCH 3/4] Reset slider input manager state completely on new object application Kind of scary this wasn't happening already. Mirrors `SpinnerRotationTracker`. --- .../Objects/Drawables/SliderInputManager.cs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/SliderInputManager.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/SliderInputManager.cs index 58fa04bb4c..5daf8ed972 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/SliderInputManager.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/SliderInputManager.cs @@ -10,6 +10,7 @@ using osu.Framework.Graphics; using osu.Framework.Input; using osu.Framework.Input.Events; using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Objects.Drawables; using osu.Game.Rulesets.Objects.Types; using osu.Game.Screens.Play; using osuTK; @@ -56,6 +57,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables public SliderInputManager(DrawableSlider slider) { this.slider = slider; + this.slider.HitObjectApplied += resetState; } /// @@ -287,5 +289,22 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables return action == OsuAction.LeftButton || action == OsuAction.RightButton; } + + private void resetState(DrawableHitObject obj) + { + Tracking = false; + trackingHistory.Clear(); + trackingHistory.Push((double.NegativeInfinity, false)); + timeToAcceptAnyKeyAfter = null; + lastPressedActions.Clear(); + screenSpaceMousePosition = null; + } + + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + + slider.HitObjectApplied -= resetState; + } } } From 876b80642357996f90e8469cc25eb1e490f1f224 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 29 Feb 2024 12:11:50 +0100 Subject: [PATCH 4/4] Store tracking history to slider judgement result instead --- .../Judgements/OsuSliderJudgementResult.cs | 20 +++++++++++++++++++ .../Objects/Drawables/DrawableSlider.cs | 6 ++++++ .../Objects/Drawables/SliderInputManager.cs | 7 ++----- 3 files changed, 28 insertions(+), 5 deletions(-) create mode 100644 osu.Game.Rulesets.Osu/Judgements/OsuSliderJudgementResult.cs diff --git a/osu.Game.Rulesets.Osu/Judgements/OsuSliderJudgementResult.cs b/osu.Game.Rulesets.Osu/Judgements/OsuSliderJudgementResult.cs new file mode 100644 index 0000000000..f52294cdd7 --- /dev/null +++ b/osu.Game.Rulesets.Osu/Judgements/OsuSliderJudgementResult.cs @@ -0,0 +1,20 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Collections.Generic; +using osu.Game.Rulesets.Judgements; +using osu.Game.Rulesets.Objects; + +namespace osu.Game.Rulesets.Osu.Judgements +{ + public class OsuSliderJudgementResult : OsuJudgementResult + { + public readonly Stack<(double time, bool tracking)> TrackingHistory = new Stack<(double, bool)>(); + + public OsuSliderJudgementResult(HitObject hitObject, Judgement judgement) + : base(hitObject, judgement) + { + TrackingHistory.Push((double.NegativeInfinity, false)); + } + } +} diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs index b7ce712e2c..e519e51562 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs @@ -14,8 +14,10 @@ using osu.Framework.Graphics.Containers; using osu.Framework.Layout; using osu.Game.Audio; using osu.Game.Graphics.Containers; +using osu.Game.Rulesets.Judgements; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Objects.Drawables; +using osu.Game.Rulesets.Osu.Judgements; using osu.Game.Rulesets.Osu.Skinning.Default; using osu.Game.Rulesets.Scoring; using osu.Game.Skinning; @@ -27,6 +29,8 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables { public new Slider HitObject => (Slider)base.HitObject; + public new OsuSliderJudgementResult Result => (OsuSliderJudgementResult)base.Result; + public DrawableSliderHead HeadCircle => headContainer.Child; public DrawableSliderTail TailCircle => tailContainer.Child; @@ -134,6 +138,8 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables }, true); } + protected override JudgementResult CreateResult(Judgement judgement) => new OsuSliderJudgementResult(HitObject, judgement); + protected override void OnApply() { base.OnApply(); diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/SliderInputManager.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/SliderInputManager.cs index 5daf8ed972..c75ae35a1a 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/SliderInputManager.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/SliderInputManager.cs @@ -27,8 +27,6 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables [Resolved] private IGameplayClock? gameplayClock { get; set; } - private readonly Stack<(double time, bool tracking)> trackingHistory = new Stack<(double, bool)>(); - /// /// The point in time after which we can accept any key for tracking. Before this time, we may need to restrict tracking to the key used to hit the head circle. /// @@ -219,6 +217,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables { if (gameplayClock?.IsRewinding == true) { + var trackingHistory = slider.Result.TrackingHistory; while (trackingHistory.TryPeek(out var historyEntry) && Time.Current < historyEntry.time) trackingHistory.Pop(); @@ -271,7 +270,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables && validTrackingAction; if (wasTracking != Tracking) - trackingHistory.Push((Time.Current, Tracking)); + slider.Result.TrackingHistory.Push((Time.Current, Tracking)); } private OsuAction? getInitialHitAction() => slider.HeadCircle?.HitAction; @@ -293,8 +292,6 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables private void resetState(DrawableHitObject obj) { Tracking = false; - trackingHistory.Clear(); - trackingHistory.Push((double.NegativeInfinity, false)); timeToAcceptAnyKeyAfter = null; lastPressedActions.Clear(); screenSpaceMousePosition = null;