diff --git a/osu.Game.Rulesets.Osu.Tests/Mods/TestSceneOsuModAutoplay.cs b/osu.Game.Rulesets.Osu.Tests/Mods/TestSceneOsuModAutoplay.cs index 6980438b59..ace7f23989 100644 --- a/osu.Game.Rulesets.Osu.Tests/Mods/TestSceneOsuModAutoplay.cs +++ b/osu.Game.Rulesets.Osu.Tests/Mods/TestSceneOsuModAutoplay.cs @@ -51,8 +51,9 @@ namespace osu.Game.Rulesets.Osu.Tests.Mods FinalRate = { Value = 1.3 } }); - [Test] - public void TestPerfectScoreOnShortSliderWithRepeat() + [TestCase(6.25f)] + [TestCase(20)] + public void TestPerfectScoreOnShortSliderWithRepeat(float pathLength) { AddStep("set score to standardised", () => LocalConfig.SetValue(OsuSetting.ScoreDisplayMode, ScoringMode.Standardised)); @@ -70,7 +71,7 @@ namespace osu.Game.Rulesets.Osu.Tests.Mods Path = new SliderPath(new[] { new PathControlPoint(), - new PathControlPoint(new Vector2(0, 6.25f)) + new PathControlPoint(new Vector2(0, pathLength)) }), RepeatCount = 1, SliderVelocityMultiplier = 10 diff --git a/osu.Game.Rulesets.Osu.Tests/OsuBeatmapConversionTest.cs b/osu.Game.Rulesets.Osu.Tests/OsuBeatmapConversionTest.cs index 11e69dc133..3e0a86d39c 100644 --- a/osu.Game.Rulesets.Osu.Tests/OsuBeatmapConversionTest.cs +++ b/osu.Game.Rulesets.Osu.Tests/OsuBeatmapConversionTest.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Linq; using NUnit.Framework; using osu.Framework.Utils; using osu.Game.Rulesets.Objects; @@ -33,8 +34,21 @@ namespace osu.Game.Rulesets.Osu.Tests switch (hitObject) { case Slider slider: + var objects = new List(); + foreach (var nested in slider.NestedHitObjects) - yield return createConvertValue((OsuHitObject)nested); + objects.Add(createConvertValue((OsuHitObject)nested, slider)); + + // stable does slider tail leniency by offsetting the last tick 36ms back. + // based on player feedback, we're doing this a little different in lazer, + // and the lazer method does not require offsetting the last tick + // (see `DrawableSliderTail.CheckForResult()`). + // however, in conversion tests, just so the output matches, we're bringing + // the 36ms offset back locally. + // in particular, on some sliders, this may rearrange nested objects, + // so we sort them again by start time to prevent test failures. + foreach (var obj in objects.OrderBy(cv => cv.StartTime)) + yield return obj; break; @@ -44,13 +58,29 @@ namespace osu.Game.Rulesets.Osu.Tests break; } - static ConvertValue createConvertValue(OsuHitObject obj) => new ConvertValue + static ConvertValue createConvertValue(OsuHitObject obj, OsuHitObject? parent = null) { - StartTime = obj.StartTime, - EndTime = obj.GetEndTime(), - X = obj.StackedPosition.X, - Y = obj.StackedPosition.Y - }; + double startTime = obj.StartTime; + double endTime = obj.GetEndTime(); + + // as stated in the inline comment above, this is locally bringing back + // the stable treatment of the "legacy last tick" just to make sure + // that the conversion output matches. + // compare: `SliderEventGenerator.Generate()`, and the calculation of `legacyLastTickTime`. + if (obj is SliderTailCircle && parent is Slider slider) + { + startTime = Math.Max(startTime + SliderEventGenerator.TAIL_LENIENCY, slider.StartTime + slider.Duration / 2); + endTime = Math.Max(endTime + SliderEventGenerator.TAIL_LENIENCY, slider.StartTime + slider.Duration / 2); + } + + return new ConvertValue + { + StartTime = startTime, + EndTime = endTime, + X = obj.StackedPosition.X, + Y = obj.StackedPosition.Y + }; + } } protected override Ruleset CreateRuleset() => new OsuRuleset(); diff --git a/osu.Game.Rulesets.Osu.Tests/OsuDifficultyCalculatorTest.cs b/osu.Game.Rulesets.Osu.Tests/OsuDifficultyCalculatorTest.cs index 9c6449cfa9..f1c1c4734d 100644 --- a/osu.Game.Rulesets.Osu.Tests/OsuDifficultyCalculatorTest.cs +++ b/osu.Game.Rulesets.Osu.Tests/OsuDifficultyCalculatorTest.cs @@ -17,16 +17,19 @@ namespace osu.Game.Rulesets.Osu.Tests [TestCase(6.7115569159190587d, 206, "diffcalc-test")] [TestCase(1.4391311903612753d, 45, "zero-length-sliders")] + [TestCase(0.42506480230838789d, 2, "very-fast-slider")] [TestCase(0.14102693012101306d, 1, "nan-slider")] public void Test(double expectedStarRating, int expectedMaxCombo, string name) => base.Test(expectedStarRating, expectedMaxCombo, name); [TestCase(8.9757300665532966d, 206, "diffcalc-test")] + [TestCase(0.55071082800473514d, 2, "very-fast-slider")] [TestCase(1.7437232654020756d, 45, "zero-length-sliders")] public void TestClockRateAdjusted(double expectedStarRating, int expectedMaxCombo, string name) => Test(expectedStarRating, expectedMaxCombo, name, new OsuModDoubleTime()); [TestCase(6.7115569159190587d, 239, "diffcalc-test")] + [TestCase(0.42506480230838789d, 4, "very-fast-slider")] [TestCase(1.4391311903612753d, 54, "zero-length-sliders")] public void TestClassicMod(double expectedStarRating, int expectedMaxCombo, string name) => Test(expectedStarRating, expectedMaxCombo, name, new OsuModClassic()); diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs index 644524f532..247aa4f445 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs @@ -1,13 +1,12 @@ // 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 NUnit.Framework; using osu.Framework.Screens; using osu.Game.Beatmaps; +using osu.Game.Beatmaps.ControlPoints; using osu.Game.Replays; using osu.Game.Rulesets.Judgements; using osu.Game.Rulesets.Objects; @@ -33,7 +32,100 @@ namespace osu.Game.Rulesets.Osu.Tests private const double time_during_slide_4 = 3800; private const double time_slider_end = 4000; - private List judgementResults; + private ScoreAccessibleReplayPlayer currentPlayer = null!; + + private const float slider_path_length = 25; + + private readonly List judgementResults = new List(); + + // Making these too short causes breakage from frames not being processed fast enough. + // To keep things simple, these tests are crafted to always be >16ms length. + // If sliders shorter than this are ever used in gameplay it will probably break things and we can revisit. + [TestCase(30, 0)] + [TestCase(30, 1)] + [TestCase(40, 0)] + [TestCase(40, 1)] + [TestCase(50, 1)] + [TestCase(60, 1)] + [TestCase(70, 1)] + [TestCase(80, 1)] + [TestCase(80, 0)] + [TestCase(80, 10)] + [TestCase(90, 1)] + [Ignore("headless test doesn't run at high enough precision for this to always enter a tracking state in time.")] + public void TestVeryShortSlider(float sliderLength, int repeatCount) + { + Slider slider; + + performTest(new List + { + new OsuReplayFrame { Position = new Vector2(10, 0), Actions = { OsuAction.LeftButton, OsuAction.RightButton }, Time = time_slider_start - 10 }, + new OsuReplayFrame { Position = new Vector2(10, 0), Actions = { OsuAction.LeftButton, OsuAction.RightButton }, Time = time_slider_start + 2000 }, + }, slider = new Slider + { + StartTime = time_slider_start, + Position = new Vector2(0, 0), + SliderVelocityMultiplier = 10f, + RepeatCount = repeatCount, + Path = new SliderPath(PathType.Linear, new[] + { + Vector2.Zero, + new Vector2(sliderLength, 0), + }), + }, 240, 1); + + assertAllMaxJudgements(); + + // Even if the last tick is hit early, the slider should always execute its final judgement at its endtime. + // If not, hitsounds will not play on time. + AddAssert("Judgement offset is zero", () => judgementResults.Last().TimeOffset == 0); + AddAssert("Slider judged at end time", () => judgementResults.Last().TimeAbsolute, () => Is.EqualTo(slider.EndTime)); + + AddAssert("Slider is last judgement", () => judgementResults[^1].HitObject, Is.TypeOf); + AddAssert("Tail is second last judgement", () => judgementResults[^2].HitObject, Is.TypeOf); + } + + [TestCase(300, false)] + [TestCase(200, true)] + [TestCase(150, true)] + [TestCase(120, true)] + [TestCase(60, true)] + [TestCase(10, true)] + [TestCase(0, true)] + [TestCase(-30, false)] + [Ignore("headless test doesn't run at high enough precision for this to always enter a tracking state in time.")] + public void TestTailLeniency(float finalPosition, bool hit) + { + Slider slider; + + performTest(new List + { + new OsuReplayFrame { Position = Vector2.Zero, Actions = { OsuAction.LeftButton, OsuAction.RightButton }, Time = time_slider_start }, + new OsuReplayFrame { Position = new Vector2(finalPosition, slider_path_length * 3), Actions = { OsuAction.LeftButton, OsuAction.RightButton }, Time = time_slider_start + 20 }, + }, slider = new Slider + { + StartTime = time_slider_start, + Position = new Vector2(0, 0), + SliderVelocityMultiplier = 10f, + Path = new SliderPath(PathType.Linear, new[] + { + Vector2.Zero, + new Vector2(slider_path_length * 10, 0), + new Vector2(slider_path_length * 10, slider_path_length * 3), + new Vector2(0, slider_path_length * 3), + }), + }, 240, 1); + + if (hit) + assertAllMaxJudgements(); + else + AddAssert("Tracking dropped", assertMidSliderJudgementFail); + + // Even if the last tick is hit early, the slider should always execute its final judgement at its endtime. + // If not, hitsounds will not play on time. + AddAssert("Judgement offset is zero", () => judgementResults.Last().TimeOffset == 0); + AddAssert("Slider judged at end time", () => judgementResults.Last().TimeAbsolute, () => Is.EqualTo(slider.EndTime)); + } [Test] public void TestPressBothKeysSimultaneouslyAndReleaseOne() @@ -44,7 +136,7 @@ namespace osu.Game.Rulesets.Osu.Tests new OsuReplayFrame { Position = Vector2.Zero, Actions = { OsuAction.RightButton }, Time = time_during_slide_1 }, }); - AddAssert("Tracking retained", assertMaxJudge); + assertAllMaxJudgements(); } /// @@ -86,7 +178,7 @@ namespace osu.Game.Rulesets.Osu.Tests new OsuReplayFrame { Position = new Vector2(0, 0), Actions = { OsuAction.RightButton }, Time = time_during_slide_2 }, }); - AddAssert("Tracking retained", assertMaxJudge); + assertAllMaxJudgements(); } /// @@ -107,7 +199,7 @@ namespace osu.Game.Rulesets.Osu.Tests new OsuReplayFrame { Position = new Vector2(0, 0), Actions = { OsuAction.RightButton }, Time = time_during_slide_1 }, }); - AddAssert("Tracking retained", assertMaxJudge); + assertAllMaxJudgements(); } /// @@ -128,7 +220,7 @@ namespace osu.Game.Rulesets.Osu.Tests new OsuReplayFrame { Position = new Vector2(0, 0), Actions = { OsuAction.RightButton }, Time = time_during_slide_1 }, }); - AddAssert("Tracking retained", assertMaxJudge); + assertAllMaxJudgements(); } /// @@ -301,7 +393,7 @@ namespace osu.Game.Rulesets.Osu.Tests new OsuReplayFrame { Position = new Vector2(slider_path_length, OsuHitObject.OBJECT_RADIUS * 1.199f), Actions = { OsuAction.LeftButton }, Time = time_slider_end }, }); - AddAssert("Tracking kept", assertMaxJudge); + assertAllMaxJudgements(); } /// @@ -325,7 +417,13 @@ namespace osu.Game.Rulesets.Osu.Tests AddAssert("Tracking dropped", assertMidSliderJudgementFail); } - private bool assertMaxJudge() => judgementResults.Any() && judgementResults.All(t => t.Type == t.Judgement.MaxResult); + private void assertAllMaxJudgements() + { + AddAssert("All judgements max", () => + { + return judgementResults.Select(j => (j.HitObject, j.Type)); + }, () => Is.EqualTo(judgementResults.Select(j => (j.HitObject, j.Judgement.MaxResult)))); + } private bool assertHeadMissTailTracked() => judgementResults[^2].Type == HitResult.SmallTickHit && !judgementResults.First().IsHit; @@ -333,35 +431,36 @@ namespace osu.Game.Rulesets.Osu.Tests private bool assertMidSliderJudgementFail() => judgementResults[^2].Type == HitResult.SmallTickMiss; - private ScoreAccessibleReplayPlayer currentPlayer; - - private const float slider_path_length = 25; - - private void performTest(List frames) + private void performTest(List frames, Slider? slider = null, double? bpm = null, int? tickRate = null) { + slider ??= new Slider + { + StartTime = time_slider_start, + Position = new Vector2(0, 0), + SliderVelocityMultiplier = 0.1f, + Path = new SliderPath(PathType.PerfectCurve, new[] + { + Vector2.Zero, + new Vector2(slider_path_length, 0), + }, slider_path_length), + }; + AddStep("load player", () => { + var cpi = new ControlPointInfo(); + + if (bpm != null) + cpi.Add(0, new TimingControlPoint { BeatLength = 60000 / bpm.Value }); + Beatmap.Value = CreateWorkingBeatmap(new Beatmap { - HitObjects = - { - new Slider - { - StartTime = time_slider_start, - Position = new Vector2(0, 0), - SliderVelocityMultiplier = 0.1f, - Path = new SliderPath(PathType.PerfectCurve, new[] - { - Vector2.Zero, - new Vector2(slider_path_length, 0), - }, slider_path_length), - } - }, + HitObjects = { slider }, BeatmapInfo = { - Difficulty = new BeatmapDifficulty { SliderTickRate = 3 }, - Ruleset = new OsuRuleset().RulesetInfo + Difficulty = new BeatmapDifficulty { SliderTickRate = tickRate ?? 3 }, + Ruleset = new OsuRuleset().RulesetInfo, }, + ControlPointInfo = cpi, }); var p = new ScoreAccessibleReplayPlayer(new Score { Replay = new Replay { Frames = frames } }); @@ -375,7 +474,7 @@ namespace osu.Game.Rulesets.Osu.Tests }; LoadScreen(currentPlayer = p); - judgementResults = new List(); + judgementResults.Clear(); }); AddUntilStep("Beatmap at 0", () => Beatmap.Value.Track.CurrentTime == 0); diff --git a/osu.Game.Rulesets.Osu/Difficulty/Evaluators/AimEvaluator.cs b/osu.Game.Rulesets.Osu/Difficulty/Evaluators/AimEvaluator.cs index cf35b08822..3d1939acac 100644 --- a/osu.Game.Rulesets.Osu/Difficulty/Evaluators/AimEvaluator.cs +++ b/osu.Game.Rulesets.Osu/Difficulty/Evaluators/AimEvaluator.cs @@ -24,7 +24,7 @@ namespace osu.Game.Rulesets.Osu.Difficulty.Evaluators /// and slider difficulty. /// /// - public static double EvaluateDifficultyOf(DifficultyHitObject current, bool withSliders) + public static double EvaluateDifficultyOf(DifficultyHitObject current, bool withSliderTravelDistance) { if (current.BaseObject is Spinner || current.Index <= 1 || current.Previous(0).BaseObject is Spinner) return 0; @@ -37,7 +37,7 @@ namespace osu.Game.Rulesets.Osu.Difficulty.Evaluators double currVelocity = osuCurrObj.LazyJumpDistance / osuCurrObj.StrainTime; // But if the last object is a slider, then we extend the travel velocity through the slider into the current object. - if (osuLastObj.BaseObject is Slider && withSliders) + if (osuLastObj.BaseObject is Slider && withSliderTravelDistance) { double travelVelocity = osuLastObj.TravelDistance / osuLastObj.TravelTime; // calculate the slider velocity from slider head to slider end. double movementVelocity = osuCurrObj.MinimumJumpDistance / osuCurrObj.MinimumJumpTime; // calculate the movement velocity from slider end to current object @@ -48,7 +48,7 @@ namespace osu.Game.Rulesets.Osu.Difficulty.Evaluators // As above, do the same for the previous hitobject. double prevVelocity = osuLastObj.LazyJumpDistance / osuLastObj.StrainTime; - if (osuLastLastObj.BaseObject is Slider && withSliders) + if (osuLastLastObj.BaseObject is Slider && withSliderTravelDistance) { double travelVelocity = osuLastLastObj.TravelDistance / osuLastLastObj.TravelTime; double movementVelocity = osuLastObj.MinimumJumpDistance / osuLastObj.MinimumJumpTime; @@ -122,7 +122,7 @@ namespace osu.Game.Rulesets.Osu.Difficulty.Evaluators aimStrain += Math.Max(acuteAngleBonus * acute_angle_multiplier, wideAngleBonus * wide_angle_multiplier + velocityChangeBonus * velocity_change_multiplier); // Add in additional slider velocity bonus. - if (withSliders) + if (withSliderTravelDistance) aimStrain += sliderBonus * slider_multiplier; return aimStrain; diff --git a/osu.Game.Rulesets.Osu/Difficulty/Preprocessing/OsuDifficultyHitObject.cs b/osu.Game.Rulesets.Osu/Difficulty/Preprocessing/OsuDifficultyHitObject.cs index e627c9ad67..488d1e2e9f 100644 --- a/osu.Game.Rulesets.Osu/Difficulty/Preprocessing/OsuDifficultyHitObject.cs +++ b/osu.Game.Rulesets.Osu/Difficulty/Preprocessing/OsuDifficultyHitObject.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Linq; using osu.Game.Rulesets.Difficulty.Preprocessing; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Osu.Mods; @@ -214,7 +215,45 @@ namespace osu.Game.Rulesets.Osu.Difficulty.Preprocessing if (slider.LazyEndPosition != null) return; - slider.LazyTravelTime = slider.NestedHitObjects[^1].StartTime - slider.StartTime; + // TODO: This commented version is actually correct by the new lazer implementation, but intentionally held back from + // difficulty calculator to preserve known behaviour. + // double trackingEndTime = Math.Max( + // // SliderTailCircle always occurs at the final end time of the slider, but the player only needs to hold until within a lenience before it. + // slider.Duration + SliderEventGenerator.TAIL_LENIENCY, + // // There's an edge case where one or more ticks/repeats fall within that leniency range. + // // In such a case, the player needs to track until the final tick or repeat. + // slider.NestedHitObjects.LastOrDefault(n => n is not SliderTailCircle)?.StartTime ?? double.MinValue + // ); + + double trackingEndTime = Math.Max( + slider.StartTime + slider.Duration + SliderEventGenerator.TAIL_LENIENCY, + slider.StartTime + slider.Duration / 2 + ); + + IList nestedObjects = slider.NestedHitObjects; + + SliderTick? lastRealTick = slider.NestedHitObjects.OfType().LastOrDefault(); + + if (lastRealTick?.StartTime > trackingEndTime) + { + trackingEndTime = lastRealTick.StartTime; + + // When the last tick falls after the tracking end time, we need to re-sort the nested objects + // based on time. This creates a somewhat weird ordering which is counter to how a user would + // understand the slider, but allows a zero-diff with known diffcalc output. + // + // To reiterate, this is definitely not correct from a difficulty calculation perspective + // and should be revisited at a later date (likely by replacing this whole code with the commented + // version above). + List reordered = nestedObjects.ToList(); + + reordered.Remove(lastRealTick); + reordered.Add(lastRealTick); + + nestedObjects = reordered; + } + + slider.LazyTravelTime = trackingEndTime - slider.StartTime; double endTimeMin = slider.LazyTravelTime / slider.SpanDuration; if (endTimeMin % 2 >= 1) @@ -223,12 +262,14 @@ namespace osu.Game.Rulesets.Osu.Difficulty.Preprocessing endTimeMin %= 1; slider.LazyEndPosition = slider.StackedPosition + slider.Path.PositionAt(endTimeMin); // temporary lazy end position until a real result can be derived. - var currCursorPosition = slider.StackedPosition; + + Vector2 currCursorPosition = slider.StackedPosition; + double scalingFactor = NORMALISED_RADIUS / slider.Radius; // lazySliderDistance is coded to be sensitive to scaling, this makes the maths easier with the thresholds being used. - for (int i = 1; i < slider.NestedHitObjects.Count; i++) + for (int i = 1; i < nestedObjects.Count; i++) { - var currMovementObj = (OsuHitObject)slider.NestedHitObjects[i]; + var currMovementObj = (OsuHitObject)nestedObjects[i]; Vector2 currMovement = Vector2.Subtract(currMovementObj.StackedPosition, currCursorPosition); double currMovementLength = scalingFactor * currMovement.Length; @@ -236,7 +277,7 @@ namespace osu.Game.Rulesets.Osu.Difficulty.Preprocessing // Amount of movement required so that the cursor position needs to be updated. double requiredMovement = assumed_slider_radius; - if (i == slider.NestedHitObjects.Count - 1) + if (i == nestedObjects.Count - 1) { // The end of a slider has special aim rules due to the relaxed time constraint on position. // There is both a lazy end position as well as the actual end slider position. We assume the player takes the simpler movement. @@ -263,7 +304,7 @@ namespace osu.Game.Rulesets.Osu.Difficulty.Preprocessing slider.LazyTravelDistance += (float)currMovementLength; } - if (i == slider.NestedHitObjects.Count - 1) + if (i == nestedObjects.Count - 1) slider.LazyEndPosition = currCursorPosition; } } diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModStrictTracking.cs b/osu.Game.Rulesets.Osu/Mods/OsuModStrictTracking.cs index 78062a0632..c465ab8732 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModStrictTracking.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModStrictTracking.cs @@ -129,7 +129,7 @@ namespace osu.Game.Rulesets.Osu.Mods }); break; - case SliderEventType.LastTick: + case SliderEventType.Tail: AddNested(TailCircle = new StrictTrackingSliderTailCircle(this) { RepeatIndex = e.SpanIndex, diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs index 789755652f..09973ce5fd 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs @@ -264,7 +264,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables protected override void CheckForResult(bool userTriggered, double timeOffset) { - if (userTriggered || Time.Current < HitObject.EndTime) + if (userTriggered || !TailCircle.Judged || Time.Current < HitObject.EndTime) return; // If only the nested hitobjects are judged, then the slider's own judgement is ignored for scoring purposes. diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderBall.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderBall.cs index 47214f1e53..292f2ffd7d 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderBall.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderBall.cs @@ -11,6 +11,7 @@ using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; 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.Rulesets.Osu.Skinning.Default; @@ -153,9 +154,12 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables Tracking = // in valid time range - Time.Current >= drawableSlider.HitObject.StartTime && Time.Current < drawableSlider.HitObject.EndTime && + Time.Current >= drawableSlider.HitObject.StartTime + // even in an edge case where current time has exceeded the slider's time, we may not have finished judging. + // we don't want to potentially update from Tracking=true to Tracking=false at this point. + && (!drawableSlider.AllJudged || Time.Current <= drawableSlider.HitObject.GetEndTime()) // in valid position range - lastScreenSpaceMousePosition.HasValue && followCircleReceptor.ReceivePositionalInputAt(lastScreenSpaceMousePosition.Value) && + && lastScreenSpaceMousePosition.HasValue && followCircleReceptor.ReceivePositionalInputAt(lastScreenSpaceMousePosition.Value) && // valid action (actions?.Any(isValidTrackingAction) ?? false); diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderTail.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderTail.cs index 9fbc97c484..df55f3e73f 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderTail.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderTail.cs @@ -8,6 +8,7 @@ using JetBrains.Annotations; using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; +using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Objects.Drawables; using osu.Game.Rulesets.Objects.Types; using osu.Game.Skinning; @@ -125,8 +126,22 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables protected override void CheckForResult(bool userTriggered, double timeOffset) { - if (!userTriggered && timeOffset >= 0) - ApplyResult(r => r.Type = Tracking ? r.Judgement.MaxResult : r.Judgement.MinResult); + if (userTriggered) + return; + + // Ensure the tail can only activate after all previous ticks already have. + // + // This covers the edge case where the lenience may allow the tail to activate before + // the last tick, changing ordering of score/combo awarding. + if (DrawableSlider.NestedHitObjects.Count > 1 && !DrawableSlider.NestedHitObjects[^2].Judged) + return; + + // The player needs to have engaged in tracking at any point after the tail leniency cutoff. + // An actual tick miss should only occur if reaching the tick itself. + if (timeOffset >= SliderEventGenerator.TAIL_LENIENCY && Tracking) + ApplyResult(r => r.Type = r.Judgement.MaxResult); + else if (timeOffset > 0) + ApplyResult(r => r.Type = r.Judgement.MinResult); } protected override void OnApply() diff --git a/osu.Game.Rulesets.Osu/Objects/Slider.cs b/osu.Game.Rulesets.Osu/Objects/Slider.cs index 443e4229d2..c62659d67a 100644 --- a/osu.Game.Rulesets.Osu/Objects/Slider.cs +++ b/osu.Game.Rulesets.Osu/Objects/Slider.cs @@ -204,11 +204,7 @@ namespace osu.Game.Rulesets.Osu.Objects }); break; - case SliderEventType.LastTick: - // Of note, we are directly mapping LastTick (instead of `SliderEventType.Tail`) to SliderTailCircle. - // It is required as difficulty calculation and gameplay relies on reading this value. - // (although it is displayed in classic skins, which may be a concern). - // If this is to change, we should revisit this. + case SliderEventType.Tail: AddNested(TailCircle = new SliderTailCircle(this) { RepeatIndex = e.SpanIndex, diff --git a/osu.Game.Rulesets.Osu/Objects/SliderTailCircle.cs b/osu.Game.Rulesets.Osu/Objects/SliderTailCircle.cs index fb81936837..54d2afb444 100644 --- a/osu.Game.Rulesets.Osu/Objects/SliderTailCircle.cs +++ b/osu.Game.Rulesets.Osu/Objects/SliderTailCircle.cs @@ -2,16 +2,11 @@ // See the LICENCE file in the repository root for full licence text. using osu.Game.Rulesets.Judgements; -using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Osu.Judgements; using osu.Game.Rulesets.Scoring; namespace osu.Game.Rulesets.Osu.Objects { - /// - /// Note that this should not be used for timing correctness. - /// See usage in for more information. - /// public class SliderTailCircle : SliderEndCircle { public SliderTailCircle(Slider slider) diff --git a/osu.Game.Rulesets.Osu/Resources/Testing/Beatmaps/very-fast-slider.osu b/osu.Game.Rulesets.Osu/Resources/Testing/Beatmaps/very-fast-slider.osu new file mode 100644 index 0000000000..58ef36e70c --- /dev/null +++ b/osu.Game.Rulesets.Osu/Resources/Testing/Beatmaps/very-fast-slider.osu @@ -0,0 +1,21 @@ +osu file format v128 + +[Difficulty] +HPDrainRate: 3 +CircleSize: 4 +OverallDifficulty: 9 +ApproachRate: 9.3 +SliderMultiplier: 3.59999990463257 +SliderTickRate: 1 + +[TimingPoints] +812,342.857142857143,4,1,1,70,1,0 +57383,-28.5714285714286,4,1,1,70,0,0 + +[HitObjects] +// Taken from https://osu.ppy.sh/beatmapsets/881996#osu/1844019 +// This slider is 42 ms in length, triggering the LegacyLastTick edge case. +// The tick will be at 21.5 ms (sliderDuration / 2) instead of 6 ms (sliderDuration - LAST_TICK_LENIENCE). +416,41,57383,6,0,L|467:217,1,157.499997329712,2|0,3:3|3:0,3:0:0:0: +// Include the next slider as well to cover the jump back to the start position. +407,73,57469,2,0,L|470:215,1,129.599999730835,2|0,0:0|0:0,0:0:0:0: diff --git a/osu.Game.Tests/Beatmaps/SliderEventGenerationTest.cs b/osu.Game.Tests/Beatmaps/SliderEventGenerationTest.cs index 37a91c8611..c7cf3fe956 100644 --- a/osu.Game.Tests/Beatmaps/SliderEventGenerationTest.cs +++ b/osu.Game.Tests/Beatmaps/SliderEventGenerationTest.cs @@ -87,8 +87,8 @@ namespace osu.Game.Tests.Beatmaps { var events = SliderEventGenerator.Generate(start_time, span_duration, 1, span_duration / 2, span_duration, 1).ToArray(); - Assert.That(events[2].Type, Is.EqualTo(SliderEventType.LastTick)); - Assert.That(events[2].Time, Is.EqualTo(span_duration + SliderEventGenerator.LAST_TICK_OFFSET)); + Assert.That(events[2].Type, Is.EqualTo(SliderEventType.LegacyLastTick)); + Assert.That(events[2].Time, Is.EqualTo(span_duration + SliderEventGenerator.TAIL_LENIENCY)); } [Test] diff --git a/osu.Game/Rulesets/Objects/SliderEventGenerator.cs b/osu.Game/Rulesets/Objects/SliderEventGenerator.cs index b3477a5fde..9b8375f208 100644 --- a/osu.Game/Rulesets/Objects/SliderEventGenerator.cs +++ b/osu.Game/Rulesets/Objects/SliderEventGenerator.cs @@ -16,8 +16,12 @@ namespace osu.Game.Rulesets.Objects /// until the true end of the slider. This very small amount of leniency makes it easier to jump away from fast sliders to the next hit object. /// /// After discussion on how this should be handled going forward, players have unanimously stated that this lenience should remain in some way. + /// These days, this is implemented in the drawable implementation of Slider in the osu! ruleset. + /// + /// We need to keep the *only* for osu!catch conversion, which relies on it to generate tiny ticks + /// correctly. /// - public const double LAST_TICK_OFFSET = -36; + public const double TAIL_LENIENCY = -36; public static IEnumerable Generate(double startTime, double spanDuration, double velocity, double tickDistance, double totalDistance, int spanCount, CancellationToken cancellationToken = default) @@ -84,18 +88,27 @@ namespace osu.Game.Rulesets.Objects int finalSpanIndex = spanCount - 1; double finalSpanStartTime = startTime + finalSpanIndex * spanDuration; - double finalSpanEndTime = Math.Max(startTime + totalDuration / 2, (finalSpanStartTime + spanDuration) + LAST_TICK_OFFSET); - double finalProgress = (finalSpanEndTime - finalSpanStartTime) / spanDuration; - if (spanCount % 2 == 0) finalProgress = 1 - finalProgress; + // Note that `finalSpanStartTime + spanDuration ≈ startTime + totalDuration`, but we write it like this to match floating point precision + // of stable. + // + // So thinking about this in a saner way, the time of the LegacyLastTick is + // + // `slider.StartTime + max(slider.Duration / 2, slider.Duration - 36)` + // + // As a slider gets shorter than 72 ms, the leniency offered falls below the 36 ms `TAIL_LENIENCY` constant. + double legacyLastTickTime = Math.Max(startTime + totalDuration / 2, (finalSpanStartTime + spanDuration) + TAIL_LENIENCY); + double legacyLastTickProgress = (legacyLastTickTime - finalSpanStartTime) / spanDuration; + + if (spanCount % 2 == 0) legacyLastTickProgress = 1 - legacyLastTickProgress; yield return new SliderEventDescriptor { - Type = SliderEventType.LastTick, + Type = SliderEventType.LegacyLastTick, SpanIndex = finalSpanIndex, SpanStartTime = finalSpanStartTime, - Time = finalSpanEndTime, - PathProgress = finalProgress, + Time = legacyLastTickTime, + PathProgress = legacyLastTickProgress, }; yield return new SliderEventDescriptor @@ -183,9 +196,10 @@ namespace osu.Game.Rulesets.Objects Tick, /// - /// Occurs just before the tail. See . + /// Occurs just before the tail. See . + /// Should generally be ignored. /// - LastTick, + LegacyLastTick, Head, Tail, Repeat