From 49598774e0886279bd17caacdcbe4a57af646481 Mon Sep 17 00:00:00 2001 From: Givikap120 Date: Thu, 24 Aug 2023 15:00:32 +0300 Subject: [PATCH 01/64] Added a "Adjust pitch" switch Adjust pitch switch in DoubleTime and HalfTime mods Also Nightcore and Daycore is now inherit from RateAdjust --- osu.Game/Rulesets/Mods/ModDaycore.cs | 26 +++++++++++++++++++++- osu.Game/Rulesets/Mods/ModDoubleTime.cs | 26 ++++++++++++++++++++++ osu.Game/Rulesets/Mods/ModHalfTime.cs | 26 ++++++++++++++++++++++ osu.Game/Rulesets/Mods/ModNightcore.cs | 29 ++++++++++++++++++++++++- osu.Game/Rulesets/Mods/ModRateAdjust.cs | 5 +---- 5 files changed, 106 insertions(+), 6 deletions(-) diff --git a/osu.Game/Rulesets/Mods/ModDaycore.cs b/osu.Game/Rulesets/Mods/ModDaycore.cs index de1a5ab56c..c5810cc9e5 100644 --- a/osu.Game/Rulesets/Mods/ModDaycore.cs +++ b/osu.Game/Rulesets/Mods/ModDaycore.cs @@ -5,16 +5,26 @@ using osu.Framework.Audio; using osu.Framework.Bindables; using osu.Framework.Graphics.Sprites; using osu.Framework.Localisation; +using osu.Game.Configuration; namespace osu.Game.Rulesets.Mods { - public abstract class ModDaycore : ModHalfTime + public abstract class ModDaycore : ModRateAdjust { public override string Name => "Daycore"; public override string Acronym => "DC"; public override IconUsage? Icon => null; + public override ModType Type => ModType.DifficultyReduction; public override LocalisableString Description => "Whoaaaaa..."; + [SettingSource("Speed decrease", "The actual decrease to apply")] + public override BindableNumber SpeedChange { get; } = new BindableDouble(0.75) + { + MinValue = 0.5, + MaxValue = 0.99, + Precision = 0.01, + }; + private readonly BindableNumber tempoAdjust = new BindableDouble(1); private readonly BindableNumber freqAdjust = new BindableDouble(1); @@ -33,5 +43,19 @@ namespace osu.Game.Rulesets.Mods track.AddAdjustment(AdjustableProperty.Frequency, freqAdjust); track.AddAdjustment(AdjustableProperty.Tempo, tempoAdjust); } + + public override double ScoreMultiplier + { + get + { + // Round to the nearest multiple of 0.1. + double value = (int)(SpeedChange.Value * 10) / 10.0; + + // Offset back to 0. + value -= 1; + + return 1 + value; + } + } } } diff --git a/osu.Game/Rulesets/Mods/ModDoubleTime.cs b/osu.Game/Rulesets/Mods/ModDoubleTime.cs index 733610c040..ad196ec83c 100644 --- a/osu.Game/Rulesets/Mods/ModDoubleTime.cs +++ b/osu.Game/Rulesets/Mods/ModDoubleTime.cs @@ -1,6 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using osu.Framework.Audio; using osu.Framework.Bindables; using osu.Framework.Graphics.Sprites; using osu.Framework.Localisation; @@ -25,6 +26,31 @@ namespace osu.Game.Rulesets.Mods Precision = 0.01, }; + private IAdjustableAudioComponent? track; + + [SettingSource("Adjust pitch", "Should pitch be adjusted with speed")] + public virtual BindableBool AdjustPitch { get; } = new BindableBool(false); + + protected ModDoubleTime() + { + AdjustPitch.BindValueChanged(adjustPitchChanged); + } + + private void adjustPitchChanged(ValueChangedEvent adjustPitchSetting) + { + track?.RemoveAdjustment(adjustmentForPitchSetting(adjustPitchSetting.OldValue), SpeedChange); + track?.AddAdjustment(adjustmentForPitchSetting(adjustPitchSetting.NewValue), SpeedChange); + } + + private AdjustableProperty adjustmentForPitchSetting(bool adjustPitchSettingValue) + => adjustPitchSettingValue ? AdjustableProperty.Frequency : AdjustableProperty.Tempo; + + public override void ApplyToTrack(IAdjustableAudioComponent track) + { + this.track = track; + AdjustPitch.TriggerChange(); + } + public override double ScoreMultiplier { get diff --git a/osu.Game/Rulesets/Mods/ModHalfTime.cs b/osu.Game/Rulesets/Mods/ModHalfTime.cs index 06c7750035..047dd4bd04 100644 --- a/osu.Game/Rulesets/Mods/ModHalfTime.cs +++ b/osu.Game/Rulesets/Mods/ModHalfTime.cs @@ -1,6 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using osu.Framework.Audio; using osu.Framework.Bindables; using osu.Framework.Graphics.Sprites; using osu.Framework.Localisation; @@ -25,6 +26,31 @@ namespace osu.Game.Rulesets.Mods Precision = 0.01, }; + private IAdjustableAudioComponent? track; + + [SettingSource("Adjust pitch", "Should pitch be adjusted with speed")] + public virtual BindableBool AdjustPitch { get; } = new BindableBool(false); + + protected ModHalfTime() + { + AdjustPitch.BindValueChanged(adjustPitchChanged); + } + + private void adjustPitchChanged(ValueChangedEvent adjustPitchSetting) + { + track?.RemoveAdjustment(adjustmentForPitchSetting(adjustPitchSetting.OldValue), SpeedChange); + track?.AddAdjustment(adjustmentForPitchSetting(adjustPitchSetting.NewValue), SpeedChange); + } + + private AdjustableProperty adjustmentForPitchSetting(bool adjustPitchSettingValue) + => adjustPitchSettingValue ? AdjustableProperty.Frequency : AdjustableProperty.Tempo; + + public override void ApplyToTrack(IAdjustableAudioComponent track) + { + this.track = track; + AdjustPitch.TriggerChange(); + } + public override double ScoreMultiplier { get diff --git a/osu.Game/Rulesets/Mods/ModNightcore.cs b/osu.Game/Rulesets/Mods/ModNightcore.cs index 9b1f7d5cf7..80b9314aec 100644 --- a/osu.Game/Rulesets/Mods/ModNightcore.cs +++ b/osu.Game/Rulesets/Mods/ModNightcore.cs @@ -11,6 +11,7 @@ using osu.Framework.Localisation; using osu.Game.Audio; using osu.Game.Beatmaps.ControlPoints; using osu.Game.Beatmaps.Timing; +using osu.Game.Configuration; using osu.Game.Graphics; using osu.Game.Graphics.Containers; using osu.Game.Rulesets.Objects; @@ -19,12 +20,38 @@ using osu.Game.Skinning; namespace osu.Game.Rulesets.Mods { - public abstract class ModNightcore : ModDoubleTime + public abstract class ModNightcore : ModRateAdjust { public override string Name => "Nightcore"; public override string Acronym => "NC"; public override IconUsage? Icon => OsuIcon.ModNightcore; + public override ModType Type => ModType.DifficultyIncrease; public override LocalisableString Description => "Uguuuuuuuu..."; + + [SettingSource("Speed increase", "The actual increase to apply")] + public override BindableNumber SpeedChange { get; } = new BindableDouble(1.5) + { + MinValue = 1.01, + MaxValue = 2, + Precision = 0.01, + }; + + public override double ScoreMultiplier + { + get + { + // Round to the nearest multiple of 0.1. + double value = (int)(SpeedChange.Value * 10) / 10.0; + + // Offset back to 0. + value -= 1; + + // Each 0.1 multiple changes score multiplier by 0.02. + value /= 5; + + return 1 + value; + } + } } public abstract partial class ModNightcore : ModNightcore, IApplicableToDrawableRuleset diff --git a/osu.Game/Rulesets/Mods/ModRateAdjust.cs b/osu.Game/Rulesets/Mods/ModRateAdjust.cs index 7b55ba4ad0..6bf4b65e3b 100644 --- a/osu.Game/Rulesets/Mods/ModRateAdjust.cs +++ b/osu.Game/Rulesets/Mods/ModRateAdjust.cs @@ -13,10 +13,7 @@ namespace osu.Game.Rulesets.Mods public abstract BindableNumber SpeedChange { get; } - public virtual void ApplyToTrack(IAdjustableAudioComponent track) - { - track.AddAdjustment(AdjustableProperty.Tempo, SpeedChange); - } + public abstract void ApplyToTrack(IAdjustableAudioComponent track); public virtual void ApplyToSample(IAdjustableAudioComponent sample) { From 55c623ff02d12c6d2d2d995903f9eb707310210d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 29 Sep 2023 14:47:55 +0900 Subject: [PATCH 02/64] Apply NRT to `TestSceneSliderInput` --- .../TestSceneSliderInput.cs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs index 644524f532..4020d22910 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.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 NUnit.Framework; @@ -33,7 +31,11 @@ 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(); [Test] public void TestPressBothKeysSimultaneouslyAndReleaseOne() @@ -333,10 +335,6 @@ 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) { AddStep("load player", () => @@ -375,7 +373,7 @@ namespace osu.Game.Rulesets.Osu.Tests }; LoadScreen(currentPlayer = p); - judgementResults = new List(); + judgementResults.Clear(); }); AddUntilStep("Beatmap at 0", () => Beatmap.Value.Track.CurrentTime == 0); From 135d2497e7fe918c6a7bb9cc2fc1c542c40223c9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 29 Sep 2023 15:15:39 +0900 Subject: [PATCH 03/64] Add gameplay test coverage around last tick of slider This includes proposed changes as per https://github.com/ppy/osu/issues/22805#issuecomment-1740377493. --- .../TestSceneSliderInput.cs | 81 +++++++++++++++---- 1 file changed, 66 insertions(+), 15 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs index 4020d22910..a9d8860369 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs @@ -37,6 +37,58 @@ namespace osu.Game.Rulesets.Osu.Tests private readonly List judgementResults = new List(); + [Test] + public void TestTrackingAtTailButNotLastTick() + { + performTest(new List + { + new OsuReplayFrame { Position = Vector2.Zero, Actions = { OsuAction.LeftButton, OsuAction.RightButton }, Time = time_slider_start }, + new OsuReplayFrame { Position = new Vector2(200, 0), Actions = { OsuAction.LeftButton, OsuAction.RightButton }, Time = time_slider_start + 50 }, + new OsuReplayFrame { Position = new Vector2(200, 0), Actions = { OsuAction.LeftButton, OsuAction.RightButton }, Time = time_slider_start + 350 }, + new OsuReplayFrame { Position = new Vector2(0, 0), Actions = { OsuAction.RightButton }, Time = time_slider_start + 380 }, + }, 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), + new Vector2(0, slider_path_length), + }), + }); + + AddAssert("Full judgement awarded", assertMaxJudge); + } + + [Test] + public void TestTrackingAtLastTickButNotTail() + { + performTest(new List + { + new OsuReplayFrame { Position = Vector2.Zero, Actions = { OsuAction.LeftButton, OsuAction.RightButton }, Time = time_slider_start }, + new OsuReplayFrame { Position = new Vector2(200, 0), Actions = { OsuAction.LeftButton, OsuAction.RightButton }, Time = time_slider_start + 50 }, + new OsuReplayFrame { Position = new Vector2(200, 0), Actions = { OsuAction.LeftButton, OsuAction.RightButton }, Time = time_slider_start + 350 }, + new OsuReplayFrame { Position = new Vector2(100, 0), Actions = { OsuAction.RightButton }, Time = time_slider_start + 380 }, + }, 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), + new Vector2(0, slider_path_length), + }), + }); + + AddAssert("Full judgement awarded", assertMaxJudge); + } + [Test] public void TestPressBothKeysSimultaneouslyAndReleaseOne() { @@ -335,26 +387,25 @@ namespace osu.Game.Rulesets.Osu.Tests private bool assertMidSliderJudgementFail() => judgementResults[^2].Type == HitResult.SmallTickMiss; - private void performTest(List frames) + private void performTest(List frames, Slider? slider = 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", () => { 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 }, From e3695d2be017d7acb88b6e1d57e5aaffe3f672ed Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 29 Sep 2023 16:08:28 +0900 Subject: [PATCH 04/64] Adjust slider judgement logic to allow tracking anywhere after last tick --- .../Objects/Drawables/DrawableSlider.cs | 2 +- .../Objects/Drawables/DrawableSliderTail.cs | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs index 77e60a1690..f4138bdf91 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs @@ -256,7 +256,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) 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/DrawableSliderTail.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderTail.cs index 9fbc97c484..6344a6d30f 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,13 @@ 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; + + if (timeOffset >= 0 && Tracking) + ApplyResult(r => r.Type = r.Judgement.MaxResult); + else if (timeOffset >= -SliderEventGenerator.LAST_TICK_OFFSET) + ApplyResult(r => r.Type = r.Judgement.MinResult); } protected override void OnApply() From dd6d09189e21d7a58df891823ba1daf9e8625c83 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 29 Sep 2023 16:20:21 +0900 Subject: [PATCH 05/64] Remove usage of `LastTick` in osu! ruleset --- osu.Game.Rulesets.Osu/Mods/OsuModStrictTracking.cs | 2 +- .../Objects/Drawables/DrawableSliderTail.cs | 6 ++++-- osu.Game.Rulesets.Osu/Objects/Slider.cs | 6 +----- osu.Game.Rulesets.Osu/Objects/SliderTailCircle.cs | 5 ----- 4 files changed, 6 insertions(+), 13 deletions(-) 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/DrawableSliderTail.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderTail.cs index 6344a6d30f..7daab820dd 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderTail.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderTail.cs @@ -129,9 +129,11 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables if (userTriggered) return; - if (timeOffset >= 0 && Tracking) + // 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 >= -SliderEventGenerator.LAST_TICK_OFFSET) + else if (timeOffset >= 0) ApplyResult(r => r.Type = r.Judgement.MinResult); } 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) From 62bcb6284232833fbec4f30a8f79bfa635771ddf Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 29 Sep 2023 16:44:04 +0900 Subject: [PATCH 06/64] Document remaining pieces of `LastTick` and add back legacy prefix for generation flow --- .../Beatmaps/SliderEventGenerationTest.cs | 4 ++-- osu.Game/Rulesets/Objects/SliderEventGenerator.cs | 15 ++++++++++----- 2 files changed, 12 insertions(+), 7 deletions(-) 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..42c422a637 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,14 +88,14 @@ 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 finalSpanEndTime = Math.Max(startTime + totalDuration / 2, (finalSpanStartTime + spanDuration) + TAIL_LENIENCY); double finalProgress = (finalSpanEndTime - finalSpanStartTime) / spanDuration; if (spanCount % 2 == 0) finalProgress = 1 - finalProgress; yield return new SliderEventDescriptor { - Type = SliderEventType.LastTick, + Type = SliderEventType.LegacyLastTick, SpanIndex = finalSpanIndex, SpanStartTime = finalSpanStartTime, Time = finalSpanEndTime, @@ -183,9 +187,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 From 22cb168c0ffef3e1cef825e15f2e030135ac5039 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 29 Sep 2023 17:17:04 +0900 Subject: [PATCH 07/64] Add test coverage of tracking inside lenience period but not at tick or end --- .../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 a9d8860369..cf02e0eb32 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs @@ -37,6 +37,33 @@ namespace osu.Game.Rulesets.Osu.Tests private readonly List judgementResults = new List(); + [Test] + public void TestTrackingBetweenLastTickAndTail() + { + performTest(new List + { + new OsuReplayFrame { Position = Vector2.Zero, Actions = { OsuAction.LeftButton, OsuAction.RightButton }, Time = time_slider_start }, + new OsuReplayFrame { Position = new Vector2(200, 0), Actions = { OsuAction.LeftButton, OsuAction.RightButton }, Time = time_slider_start + 50 }, + new OsuReplayFrame { Position = new Vector2(200, 0), Actions = { OsuAction.LeftButton, OsuAction.RightButton }, Time = time_slider_start + 480 }, + new OsuReplayFrame { Position = new Vector2(0, 0), Actions = { OsuAction.LeftButton, OsuAction.RightButton }, Time = time_slider_start + 500 }, + new OsuReplayFrame { Position = new Vector2(200, 0), Actions = { OsuAction.RightButton }, Time = time_slider_start + 520 }, + }, 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), + new Vector2(0, slider_path_length), + }), + }); + + AddAssert("Full judgement awarded", assertMaxJudge); + } + [Test] public void TestTrackingAtTailButNotLastTick() { From 2410036003e75c4af046f5b15ea00f469328e679 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 29 Sep 2023 17:31:02 +0900 Subject: [PATCH 08/64] Refactor tests to be easier to visually understand --- .../TestSceneSliderInput.cs | 91 ++++++------------- 1 file changed, 27 insertions(+), 64 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs index cf02e0eb32..bacc4fb218 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs @@ -6,6 +6,7 @@ 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; @@ -37,16 +38,18 @@ namespace osu.Game.Rulesets.Osu.Tests private readonly List judgementResults = new List(); - [Test] - public void TestTrackingBetweenLastTickAndTail() + [TestCase(300, false)] + [TestCase(200, true)] + [TestCase(150, true)] + [TestCase(120, true)] + [TestCase(60, true)] + [TestCase(10, true)] + public void TestTailLeniency(float finalPosition, bool hit) { performTest(new List { new OsuReplayFrame { Position = Vector2.Zero, Actions = { OsuAction.LeftButton, OsuAction.RightButton }, Time = time_slider_start }, - new OsuReplayFrame { Position = new Vector2(200, 0), Actions = { OsuAction.LeftButton, OsuAction.RightButton }, Time = time_slider_start + 50 }, - new OsuReplayFrame { Position = new Vector2(200, 0), Actions = { OsuAction.LeftButton, OsuAction.RightButton }, Time = time_slider_start + 480 }, - new OsuReplayFrame { Position = new Vector2(0, 0), Actions = { OsuAction.LeftButton, OsuAction.RightButton }, Time = time_slider_start + 500 }, - new OsuReplayFrame { Position = new Vector2(200, 0), Actions = { OsuAction.RightButton }, Time = time_slider_start + 520 }, + new OsuReplayFrame { Position = new Vector2(finalPosition, slider_path_length * 3), Actions = { OsuAction.LeftButton, OsuAction.RightButton }, Time = time_slider_start + 20 }, }, new Slider { StartTime = time_slider_start, @@ -56,64 +59,18 @@ namespace osu.Game.Rulesets.Osu.Tests { Vector2.Zero, new Vector2(slider_path_length * 10, 0), - new Vector2(slider_path_length * 10, slider_path_length), - new Vector2(0, slider_path_length), + new Vector2(slider_path_length * 10, slider_path_length * 3), + new Vector2(0, slider_path_length * 3), }), - }); + }, 240, 1); - AddAssert("Full judgement awarded", assertMaxJudge); - } - - [Test] - public void TestTrackingAtTailButNotLastTick() - { - performTest(new List + if (hit) { - new OsuReplayFrame { Position = Vector2.Zero, Actions = { OsuAction.LeftButton, OsuAction.RightButton }, Time = time_slider_start }, - new OsuReplayFrame { Position = new Vector2(200, 0), Actions = { OsuAction.LeftButton, OsuAction.RightButton }, Time = time_slider_start + 50 }, - new OsuReplayFrame { Position = new Vector2(200, 0), Actions = { OsuAction.LeftButton, OsuAction.RightButton }, Time = time_slider_start + 350 }, - new OsuReplayFrame { Position = new Vector2(0, 0), Actions = { OsuAction.RightButton }, Time = time_slider_start + 380 }, - }, 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), - new Vector2(0, slider_path_length), - }), - }); - - AddAssert("Full judgement awarded", assertMaxJudge); - } - - [Test] - public void TestTrackingAtLastTickButNotTail() - { - performTest(new List - { - new OsuReplayFrame { Position = Vector2.Zero, Actions = { OsuAction.LeftButton, OsuAction.RightButton }, Time = time_slider_start }, - new OsuReplayFrame { Position = new Vector2(200, 0), Actions = { OsuAction.LeftButton, OsuAction.RightButton }, Time = time_slider_start + 50 }, - new OsuReplayFrame { Position = new Vector2(200, 0), Actions = { OsuAction.LeftButton, OsuAction.RightButton }, Time = time_slider_start + 350 }, - new OsuReplayFrame { Position = new Vector2(100, 0), Actions = { OsuAction.RightButton }, Time = time_slider_start + 380 }, - }, 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), - new Vector2(0, slider_path_length), - }), - }); - - AddAssert("Full judgement awarded", assertMaxJudge); + AddAssert("Tracking retained", assertMaxJudge); + AddAssert("Full judgement awarded", assertMaxJudge); + } + else + AddAssert("Tracking dropped", assertMidSliderJudgementFail); } [Test] @@ -414,7 +371,7 @@ namespace osu.Game.Rulesets.Osu.Tests private bool assertMidSliderJudgementFail() => judgementResults[^2].Type == HitResult.SmallTickMiss; - private void performTest(List frames, Slider? slider = null) + private void performTest(List frames, Slider? slider = null, double? bpm = null, int? tickRate = null) { slider ??= new Slider { @@ -430,14 +387,20 @@ namespace osu.Game.Rulesets.Osu.Tests 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 = { 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 } }); From 07207ffc322108beb76a6ca9bee9566581eec150 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 2 Oct 2023 13:31:42 +0900 Subject: [PATCH 09/64] Fix hitsounds playing too early on fast sliders --- osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs | 9 ++++++++- .../Objects/Drawables/DrawableSlider.cs | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs index bacc4fb218..d920dfa859 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs @@ -46,11 +46,13 @@ namespace osu.Game.Rulesets.Osu.Tests [TestCase(10, true)] 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 }, - }, new Slider + }, slider = new Slider { StartTime = time_slider_start, Position = new Vector2(0, 0), @@ -71,6 +73,11 @@ namespace osu.Game.Rulesets.Osu.Tests } 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] diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs index f4138bdf91..5571332076 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs @@ -256,7 +256,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables protected override void CheckForResult(bool userTriggered, double timeOffset) { - if (userTriggered || !TailCircle.Judged) + 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. From b683d55023c639f908f94a9d083563647756f379 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 2 Oct 2023 13:41:49 +0900 Subject: [PATCH 10/64] Add a couple of extra tests --- osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs index d920dfa859..ff6b68e1d2 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs @@ -44,6 +44,8 @@ namespace osu.Game.Rulesets.Osu.Tests [TestCase(120, true)] [TestCase(60, true)] [TestCase(10, true)] + [TestCase(0, true)] + [TestCase(-30, false)] public void TestTailLeniency(float finalPosition, bool hit) { Slider slider; From 589abe2c52c221ad07a4f4869672f8e4ae579711 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 2 Oct 2023 15:28:29 +0900 Subject: [PATCH 11/64] Add note about broken test --- osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs index ff6b68e1d2..68f635baa1 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs @@ -44,7 +44,7 @@ namespace osu.Game.Rulesets.Osu.Tests [TestCase(120, true)] [TestCase(60, true)] [TestCase(10, true)] - [TestCase(0, true)] + // [TestCase(0, true)] headless test doesn't run at high enough precision for this to always enter a tracking state in time. [TestCase(-30, false)] public void TestTailLeniency(float finalPosition, bool hit) { From 393ec119dd3b6db47421f7fc2ca1e373bf9d3cf8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 2 Oct 2023 13:42:30 +0900 Subject: [PATCH 12/64] Add test coverage of very short sliders --- .../TestSceneSliderInput.cs | 40 ++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs index 68f635baa1..a3b9d40601 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs @@ -1,6 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System; using System.Collections.Generic; using System.Linq; using NUnit.Framework; @@ -38,6 +39,44 @@ namespace osu.Game.Rulesets.Osu.Tests 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(80, 0)] + [TestCase(80, 1)] + [TestCase(80, 10)] + 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); + + AddAssert("Slider is longer than one frame", () => slider.Duration / (slider.RepeatCount + 1), () => Is.GreaterThan(1000 / 60f)); + AddAssert("Slider is shorter than lenience", () => slider.Duration / (slider.RepeatCount + 1), () => Is.LessThan(Math.Abs(SliderEventGenerator.TAIL_LENIENCY))); + + 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)); + } + [TestCase(300, false)] [TestCase(200, true)] [TestCase(150, true)] @@ -70,7 +109,6 @@ namespace osu.Game.Rulesets.Osu.Tests if (hit) { - AddAssert("Tracking retained", assertMaxJudge); AddAssert("Full judgement awarded", assertMaxJudge); } else From 3a124a99ce49a39448725122c08eb80012819b8c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 2 Oct 2023 14:34:26 +0900 Subject: [PATCH 13/64] Improve test output for judgement checking --- .../TestSceneSliderInput.cs | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs index a3b9d40601..e98e7f45f6 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs @@ -108,9 +108,7 @@ namespace osu.Game.Rulesets.Osu.Tests }, 240, 1); if (hit) - { - AddAssert("Full judgement awarded", assertMaxJudge); - } + assertAllMaxJudgements(); else AddAssert("Tracking dropped", assertMidSliderJudgementFail); @@ -129,7 +127,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(); } /// @@ -171,7 +169,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(); } /// @@ -192,7 +190,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(); } /// @@ -213,7 +211,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(); } /// @@ -386,7 +384,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(); } /// @@ -410,7 +408,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; From 00867593950322e5dbf46b1756a29eb5ea7c71bf Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 2 Oct 2023 15:23:22 +0900 Subject: [PATCH 14/64] Fix slider tracking state being set to `false` after slider end --- .../Objects/Drawables/DrawableSliderBall.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderBall.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderBall.cs index 47214f1e53..63e500d687 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderBall.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderBall.cs @@ -132,6 +132,9 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables { base.Update(); + if (Time.Current >= drawableSlider.HitObject.EndTime) + return; + // 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. var headCircleHitAction = GetInitialHitAction(); @@ -153,9 +156,9 @@ 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 // in valid position range - lastScreenSpaceMousePosition.HasValue && followCircleReceptor.ReceivePositionalInputAt(lastScreenSpaceMousePosition.Value) && + && lastScreenSpaceMousePosition.HasValue && followCircleReceptor.ReceivePositionalInputAt(lastScreenSpaceMousePosition.Value) && // valid action (actions?.Any(isValidTrackingAction) ?? false); From 9a3c21c3206a9853e5379783f83c38e9439f2cb3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 2 Oct 2023 15:30:33 +0900 Subject: [PATCH 15/64] Change comparison of `timeOffset` to greater-than (in line with others) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bartłomiej Dach --- osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderTail.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderTail.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderTail.cs index 7daab820dd..9edf3808c5 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderTail.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderTail.cs @@ -133,7 +133,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables // 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) + else if (timeOffset > 0) ApplyResult(r => r.Type = r.Judgement.MinResult); } From 941f26d46233902f88a4088c3bfa354064e13dee Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 3 Oct 2023 18:35:01 +0900 Subject: [PATCH 16/64] Add extra test coverage to `TestSceneOsuModAutoplay` to cover fail case Basically the slider needs to be slightly longer for this test to correctly fail in headless tests, in conjunction with the new slider tail leniency. This is due to headless tests running at a fixed frame interval, and these timings being *tight*. --- .../Mods/TestSceneOsuModAutoplay.cs | 7 ++++--- .../TestSceneSliderInput.cs | 17 ++++++++++++----- 2 files changed, 16 insertions(+), 8 deletions(-) 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/TestSceneSliderInput.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs index e98e7f45f6..9e83ca428b 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs @@ -1,7 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using System; using System.Collections.Generic; using System.Linq; using NUnit.Framework; @@ -42,9 +41,17 @@ namespace osu.Game.Rulesets.Osu.Tests // 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(80, 0)] + [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)] public void TestVeryShortSlider(float sliderLength, int repeatCount) { Slider slider; @@ -66,15 +73,15 @@ namespace osu.Game.Rulesets.Osu.Tests }), }, 240, 1); - AddAssert("Slider is longer than one frame", () => slider.Duration / (slider.RepeatCount + 1), () => Is.GreaterThan(1000 / 60f)); - AddAssert("Slider is shorter than lenience", () => slider.Duration / (slider.RepeatCount + 1), () => Is.LessThan(Math.Abs(SliderEventGenerator.TAIL_LENIENCY))); - 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)] From 9e1fec0213d8541a813300d415d1956417979c6a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 3 Oct 2023 18:36:13 +0900 Subject: [PATCH 17/64] Fix potential out-of-order tail and ticks --- .../Objects/Drawables/DrawableSliderTail.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderTail.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderTail.cs index 9edf3808c5..df55f3e73f 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderTail.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderTail.cs @@ -129,6 +129,13 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables 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) From 70ec4b060af1abd4752e483cacbab07b9c755692 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 3 Oct 2023 19:20:37 +0900 Subject: [PATCH 18/64] Rename weird diffcalc parameter name --- .../Difficulty/Evaluators/AimEvaluator.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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; From c4992d347975459823ee14ef3c07e42010470b98 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 3 Oct 2023 19:08:59 +0900 Subject: [PATCH 19/64] Fix one case of difficulty calculation no longer accounting for leniency --- .../Difficulty/Preprocessing/OsuDifficultyHitObject.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu/Difficulty/Preprocessing/OsuDifficultyHitObject.cs b/osu.Game.Rulesets.Osu/Difficulty/Preprocessing/OsuDifficultyHitObject.cs index e627c9ad67..00cb4acfb4 100644 --- a/osu.Game.Rulesets.Osu/Difficulty/Preprocessing/OsuDifficultyHitObject.cs +++ b/osu.Game.Rulesets.Osu/Difficulty/Preprocessing/OsuDifficultyHitObject.cs @@ -214,7 +214,7 @@ namespace osu.Game.Rulesets.Osu.Difficulty.Preprocessing if (slider.LazyEndPosition != null) return; - slider.LazyTravelTime = slider.NestedHitObjects[^1].StartTime - slider.StartTime; + slider.LazyTravelTime = slider.NestedHitObjects[^1].StartTime + SliderEventGenerator.TAIL_LENIENCY - slider.StartTime; double endTimeMin = slider.LazyTravelTime / slider.SpanDuration; if (endTimeMin % 2 >= 1) From 8d919912149a4853120612b67944492c27baa611 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 3 Oct 2023 20:32:46 +0900 Subject: [PATCH 20/64] Fix difficulty calculation not correct handling slider leniency anymore --- .../Preprocessing/OsuDifficultyHitObject.cs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu/Difficulty/Preprocessing/OsuDifficultyHitObject.cs b/osu.Game.Rulesets.Osu/Difficulty/Preprocessing/OsuDifficultyHitObject.cs index 00cb4acfb4..5c2c85d69e 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,15 @@ namespace osu.Game.Rulesets.Osu.Difficulty.Preprocessing if (slider.LazyEndPosition != null) return; - slider.LazyTravelTime = slider.NestedHitObjects[^1].StartTime + SliderEventGenerator.TAIL_LENIENCY - slider.StartTime; + 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.NestedHitObjects.OfType().Single().StartTime + 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 + ); + + slider.LazyTravelTime = trackingEndTime - slider.StartTime; double endTimeMin = slider.LazyTravelTime / slider.SpanDuration; if (endTimeMin % 2 >= 1) From 9361b1879c5e15667548b6d3c302c3bb43232d6c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 4 Oct 2023 12:54:13 +0900 Subject: [PATCH 21/64] Add note about early return in `DrawableSliderBall.Update` --- osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderBall.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderBall.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderBall.cs index 63e500d687..2fa2d4c0b5 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderBall.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderBall.cs @@ -132,6 +132,8 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables { base.Update(); + // 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. if (Time.Current >= drawableSlider.HitObject.EndTime) return; From cfb18e18c060f4270925b544fa545518ac48dea6 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 4 Oct 2023 13:33:06 +0900 Subject: [PATCH 22/64] Add better commenting around legacy last tick edge case --- .../Rulesets/Objects/SliderEventGenerator.cs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/osu.Game/Rulesets/Objects/SliderEventGenerator.cs b/osu.Game/Rulesets/Objects/SliderEventGenerator.cs index 42c422a637..9b8375f208 100644 --- a/osu.Game/Rulesets/Objects/SliderEventGenerator.cs +++ b/osu.Game/Rulesets/Objects/SliderEventGenerator.cs @@ -88,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) + TAIL_LENIENCY); - 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.LegacyLastTick, SpanIndex = finalSpanIndex, SpanStartTime = finalSpanStartTime, - Time = finalSpanEndTime, - PathProgress = finalProgress, + Time = legacyLastTickTime, + PathProgress = legacyLastTickProgress, }; yield return new SliderEventDescriptor From d14d885d19a018343f9aa81ca7f67e84da917865 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 4 Oct 2023 14:52:35 +0900 Subject: [PATCH 23/64] Add test coverage of very fast slider --- .../OsuDifficultyCalculatorTest.cs | 3 +++ .../Testing/Beatmaps/very-fast-slider.osu | 21 +++++++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 osu.Game.Rulesets.Osu/Resources/Testing/Beatmaps/very-fast-slider.osu 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/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: From 658b6a166f711fd719f05b2d3f746cd33ca8e944 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 4 Oct 2023 16:11:39 +0900 Subject: [PATCH 24/64] Fix regression in `Tracking` state handling It turns out multiple components depend on `Tracking` eventually becoming `false` at the end of a slider. With my previous changes, this was no longer the case (as could be seen by the legacy cursor particles test failure, and heard by slider slide taking too long to stop). --- .../Objects/Drawables/DrawableSliderBall.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderBall.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderBall.cs index 2fa2d4c0b5..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; @@ -132,11 +133,6 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables { base.Update(); - // 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. - if (Time.Current >= drawableSlider.HitObject.EndTime) - return; - // 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. var headCircleHitAction = GetInitialHitAction(); @@ -159,6 +155,9 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables Tracking = // in valid time range 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) && // valid action From 63843c79c3b3333e4cd8df844ae99257ed0579f8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 11 Oct 2023 21:12:04 +0900 Subject: [PATCH 25/64] Amend diffcalc to use something closer to the original calculation for now --- .../Preprocessing/OsuDifficultyHitObject.cs | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Difficulty/Preprocessing/OsuDifficultyHitObject.cs b/osu.Game.Rulesets.Osu/Difficulty/Preprocessing/OsuDifficultyHitObject.cs index 5c2c85d69e..f37452a79c 100644 --- a/osu.Game.Rulesets.Osu/Difficulty/Preprocessing/OsuDifficultyHitObject.cs +++ b/osu.Game.Rulesets.Osu/Difficulty/Preprocessing/OsuDifficultyHitObject.cs @@ -215,13 +215,20 @@ namespace osu.Game.Rulesets.Osu.Difficulty.Preprocessing if (slider.LazyEndPosition != null) return; - 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.NestedHitObjects.OfType().Single().StartTime + 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 - ); + // 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(Math.Max( + slider.StartTime + slider.Duration + SliderEventGenerator.TAIL_LENIENCY, + slider.StartTime + slider.Duration / 2 + ), slider.NestedHitObjects.OfType().Last().StartTime); slider.LazyTravelTime = trackingEndTime - slider.StartTime; From 5ffc25c8e847d97b678de8946676338ed8ba379b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 12 Oct 2023 03:19:43 +0900 Subject: [PATCH 26/64] Fix potential failure when slider has no ticks --- .../Difficulty/Preprocessing/OsuDifficultyHitObject.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu/Difficulty/Preprocessing/OsuDifficultyHitObject.cs b/osu.Game.Rulesets.Osu/Difficulty/Preprocessing/OsuDifficultyHitObject.cs index f37452a79c..959449fdbb 100644 --- a/osu.Game.Rulesets.Osu/Difficulty/Preprocessing/OsuDifficultyHitObject.cs +++ b/osu.Game.Rulesets.Osu/Difficulty/Preprocessing/OsuDifficultyHitObject.cs @@ -228,7 +228,7 @@ namespace osu.Game.Rulesets.Osu.Difficulty.Preprocessing double trackingEndTime = Math.Max(Math.Max( slider.StartTime + slider.Duration + SliderEventGenerator.TAIL_LENIENCY, slider.StartTime + slider.Duration / 2 - ), slider.NestedHitObjects.OfType().Last().StartTime); + ), slider.NestedHitObjects.OfType().LastOrDefault()?.StartTime ?? double.MinValue); slider.LazyTravelTime = trackingEndTime - slider.StartTime; From a3b21281e68a67fc609c719abe9bc4a488f8ee41 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 13 Oct 2023 14:25:38 +0900 Subject: [PATCH 27/64] Add reordering support to match existing diffcalc 100% --- .../Preprocessing/OsuDifficultyHitObject.cs | 41 +++++++++++++++---- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Difficulty/Preprocessing/OsuDifficultyHitObject.cs b/osu.Game.Rulesets.Osu/Difficulty/Preprocessing/OsuDifficultyHitObject.cs index 959449fdbb..488d1e2e9f 100644 --- a/osu.Game.Rulesets.Osu/Difficulty/Preprocessing/OsuDifficultyHitObject.cs +++ b/osu.Game.Rulesets.Osu/Difficulty/Preprocessing/OsuDifficultyHitObject.cs @@ -215,7 +215,7 @@ namespace osu.Game.Rulesets.Osu.Difficulty.Preprocessing if (slider.LazyEndPosition != null) return; - // This commented version is actually correct by the new lazer implementation, but intentionally held back from + // 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. @@ -225,10 +225,33 @@ namespace osu.Game.Rulesets.Osu.Difficulty.Preprocessing // slider.NestedHitObjects.LastOrDefault(n => n is not SliderTailCircle)?.StartTime ?? double.MinValue // ); - double trackingEndTime = Math.Max(Math.Max( + double trackingEndTime = Math.Max( slider.StartTime + slider.Duration + SliderEventGenerator.TAIL_LENIENCY, slider.StartTime + slider.Duration / 2 - ), slider.NestedHitObjects.OfType().LastOrDefault()?.StartTime ?? double.MinValue); + ); + + 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; @@ -239,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; @@ -252,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. @@ -279,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; } } From 937694cd14802201f1dead09df55f4d32ac49120 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 16 Oct 2023 13:35:24 +0200 Subject: [PATCH 28/64] Fix conversion test failures --- .../OsuBeatmapConversionTest.cs | 32 +++++++++++++++---- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/OsuBeatmapConversionTest.cs b/osu.Game.Rulesets.Osu.Tests/OsuBeatmapConversionTest.cs index 11e69dc133..ea07b69e78 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,13 @@ 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)); + + foreach (var obj in objects.OrderBy(cv => cv.StartTime)) + yield return obj; break; @@ -44,13 +50,25 @@ 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(); + + 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(); From b3d60c6d4fd037e28bc36b5c284aff974458e382 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Mon, 16 Oct 2023 13:40:45 +0200 Subject: [PATCH 29/64] Add inline commentary about workarounds in beatmap conversion test --- .../OsuBeatmapConversionTest.cs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/osu.Game.Rulesets.Osu.Tests/OsuBeatmapConversionTest.cs b/osu.Game.Rulesets.Osu.Tests/OsuBeatmapConversionTest.cs index ea07b69e78..3e0a86d39c 100644 --- a/osu.Game.Rulesets.Osu.Tests/OsuBeatmapConversionTest.cs +++ b/osu.Game.Rulesets.Osu.Tests/OsuBeatmapConversionTest.cs @@ -39,6 +39,14 @@ namespace osu.Game.Rulesets.Osu.Tests foreach (var nested in slider.NestedHitObjects) 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; @@ -55,6 +63,10 @@ namespace osu.Game.Rulesets.Osu.Tests 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); From d26d4b8b79dcfcce739dd0723c1cf7fb3f959dd9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 17 Oct 2023 16:42:22 +0900 Subject: [PATCH 30/64] Cache `IScrollingInfo` at a `HitObjectComposer` level automatically --- osu.Game.Rulesets.Mania/Edit/ManiaHitObjectComposer.cs | 9 +-------- osu.Game/Rulesets/Edit/HitObjectComposer.cs | 3 +++ .../Rulesets/UI/Scrolling/DrawableScrollingRuleset.cs | 2 +- .../Rulesets/UI/Scrolling/IDrawableScrollingRuleset.cs | 2 ++ 4 files changed, 7 insertions(+), 9 deletions(-) diff --git a/osu.Game.Rulesets.Mania/Edit/ManiaHitObjectComposer.cs b/osu.Game.Rulesets.Mania/Edit/ManiaHitObjectComposer.cs index 8e61baca81..7a0ec70611 100644 --- a/osu.Game.Rulesets.Mania/Edit/ManiaHitObjectComposer.cs +++ b/osu.Game.Rulesets.Mania/Edit/ManiaHitObjectComposer.cs @@ -57,16 +57,9 @@ namespace osu.Game.Rulesets.Mania.Edit protected override Playfield PlayfieldAtScreenSpacePosition(Vector2 screenSpacePosition) => Playfield.GetColumnByPosition(screenSpacePosition); - protected override DrawableRuleset CreateDrawableRuleset(Ruleset ruleset, IBeatmap beatmap, IReadOnlyList mods) - { + protected override DrawableRuleset CreateDrawableRuleset(Ruleset ruleset, IBeatmap beatmap, IReadOnlyList mods) => drawableRuleset = new DrawableManiaEditorRuleset(ruleset, beatmap, mods); - // This is the earliest we can cache the scrolling info to ourselves, before masks are added to the hierarchy and inject it - dependencies.CacheAs(drawableRuleset.ScrollingInfo); - - return drawableRuleset; - } - protected override ComposeBlueprintContainer CreateBlueprintContainer() => new ManiaBlueprintContainer(this); diff --git a/osu.Game/Rulesets/Edit/HitObjectComposer.cs b/osu.Game/Rulesets/Edit/HitObjectComposer.cs index f9a6b5083e..e1ae4b0199 100644 --- a/osu.Game/Rulesets/Edit/HitObjectComposer.cs +++ b/osu.Game/Rulesets/Edit/HitObjectComposer.cs @@ -119,6 +119,9 @@ namespace osu.Game.Rulesets.Edit return; } + if (DrawableRuleset is IDrawableScrollingRuleset scrollingRuleset) + dependencies.CacheAs(scrollingRuleset.ScrollingInfo); + dependencies.CacheAs(Playfield); InternalChildren = new Drawable[] diff --git a/osu.Game/Rulesets/UI/Scrolling/DrawableScrollingRuleset.cs b/osu.Game/Rulesets/UI/Scrolling/DrawableScrollingRuleset.cs index 6abfc6ee49..d23658ac33 100644 --- a/osu.Game/Rulesets/UI/Scrolling/DrawableScrollingRuleset.cs +++ b/osu.Game/Rulesets/UI/Scrolling/DrawableScrollingRuleset.cs @@ -81,7 +81,7 @@ namespace osu.Game.Rulesets.UI.Scrolling /// protected readonly SortedList ControlPoints = new SortedList(Comparer.Default); - protected IScrollingInfo ScrollingInfo => scrollingInfo; + public IScrollingInfo ScrollingInfo => scrollingInfo; [Cached(Type = typeof(IScrollingInfo))] private readonly LocalScrollingInfo scrollingInfo; diff --git a/osu.Game/Rulesets/UI/Scrolling/IDrawableScrollingRuleset.cs b/osu.Game/Rulesets/UI/Scrolling/IDrawableScrollingRuleset.cs index f3a3bb18bd..b348a22009 100644 --- a/osu.Game/Rulesets/UI/Scrolling/IDrawableScrollingRuleset.cs +++ b/osu.Game/Rulesets/UI/Scrolling/IDrawableScrollingRuleset.cs @@ -11,5 +11,7 @@ namespace osu.Game.Rulesets.UI.Scrolling public interface IDrawableScrollingRuleset { ScrollVisualisationMethod VisualisationMethod { get; } + + IScrollingInfo ScrollingInfo { get; } } } From 1b9acdf55c5cbeff194c4c80a3a26402ea64ab9d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 17 Oct 2023 16:42:36 +0900 Subject: [PATCH 31/64] Abstract out common implementation of `BeatSnapGrid` --- .../Edit/CatchBeatSnapGrid.cs | 173 +-------------- .../Edit/ManiaBeatSnapGrid.cs | 197 +---------------- .../Edit/Compose/Components/BeatSnapGrid.cs | 203 ++++++++++++++++++ 3 files changed, 216 insertions(+), 357 deletions(-) create mode 100644 osu.Game/Screens/Edit/Compose/Components/BeatSnapGrid.cs diff --git a/osu.Game.Rulesets.Catch/Edit/CatchBeatSnapGrid.cs b/osu.Game.Rulesets.Catch/Edit/CatchBeatSnapGrid.cs index 6862696b3a..40bd08455f 100644 --- a/osu.Game.Rulesets.Catch/Edit/CatchBeatSnapGrid.cs +++ b/osu.Game.Rulesets.Catch/Edit/CatchBeatSnapGrid.cs @@ -1,180 +1,19 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using System; using System.Collections.Generic; -using System.Linq; -using osu.Framework.Allocation; -using osu.Framework.Caching; -using osu.Framework.Graphics; -using osu.Framework.Graphics.Shapes; -using osu.Game.Graphics; +using osu.Framework.Graphics.Containers; using osu.Game.Rulesets.Catch.UI; using osu.Game.Rulesets.Edit; -using osu.Game.Rulesets.Objects; -using osu.Game.Rulesets.Objects.Drawables; -using osu.Game.Rulesets.UI.Scrolling; -using osu.Game.Screens.Edit; -using osuTK.Graphics; +using osu.Game.Screens.Edit.Compose.Components; namespace osu.Game.Rulesets.Catch.Edit { - /// - /// A grid which displays coloured beat divisor lines in proximity to the selection or placement cursor. - /// - /// - /// This class heavily borrows from osu!mania's implementation (ManiaBeatSnapGrid). - /// If further changes are to be made, they should also be applied there. - /// If the scale of the changes are large enough, abstracting may be a good path. - /// - public partial class CatchBeatSnapGrid : Component + public partial class CatchBeatSnapGrid : BeatSnapGrid { - private const double visible_range = 750; - - /// - /// The range of time values of the current selection. - /// - public (double start, double end)? SelectionTimeRange + protected override IEnumerable GetTargetContainers(HitObjectComposer composer) => new[] { - set - { - if (value == selectionTimeRange) - return; - - selectionTimeRange = value; - lineCache.Invalidate(); - } - } - - [Resolved] - private EditorBeatmap beatmap { get; set; } = null!; - - [Resolved] - private OsuColour colours { get; set; } = null!; - - [Resolved] - private BindableBeatDivisor beatDivisor { get; set; } = null!; - - private readonly Cached lineCache = new Cached(); - - private (double start, double end)? selectionTimeRange; - - private ScrollingHitObjectContainer lineContainer = null!; - - [BackgroundDependencyLoader] - private void load(HitObjectComposer composer) - { - lineContainer = new ScrollingHitObjectContainer(); - - ((CatchPlayfield)composer.Playfield).UnderlayElements.Add(lineContainer); - - beatDivisor.BindValueChanged(_ => createLines(), true); - } - - protected override void Update() - { - base.Update(); - - if (!lineCache.IsValid) - { - lineCache.Validate(); - createLines(); - } - } - - private readonly Stack availableLines = new Stack(); - - private void createLines() - { - foreach (var line in lineContainer.Objects.OfType()) - availableLines.Push(line); - - lineContainer.Clear(); - - if (selectionTimeRange == null) - return; - - var range = selectionTimeRange.Value; - - var timingPoint = beatmap.ControlPointInfo.TimingPointAt(range.start - visible_range); - - double time = timingPoint.Time; - int beat = 0; - - // progress time until in the visible range. - while (time < range.start - visible_range) - { - time += timingPoint.BeatLength / beatDivisor.Value; - beat++; - } - - while (time < range.end + visible_range) - { - var nextTimingPoint = beatmap.ControlPointInfo.TimingPointAt(time); - - // switch to the next timing point if we have reached it. - if (nextTimingPoint.Time > timingPoint.Time) - { - beat = 0; - time = nextTimingPoint.Time; - timingPoint = nextTimingPoint; - } - - Color4 colour = BindableBeatDivisor.GetColourFor( - BindableBeatDivisor.GetDivisorForBeatIndex(beat, beatDivisor.Value), colours); - - if (!availableLines.TryPop(out var line)) - line = new DrawableGridLine(); - - line.HitObject.StartTime = time; - line.Colour = colour; - - lineContainer.Add(line); - - beat++; - time += timingPoint.BeatLength / beatDivisor.Value; - } - - // required to update ScrollingHitObjectContainer's cache. - lineContainer.UpdateSubTree(); - - foreach (var line in lineContainer.Objects.OfType()) - { - time = line.HitObject.StartTime; - - if (time >= range.start && time <= range.end) - line.Alpha = 1; - else - { - double timeSeparation = time < range.start ? range.start - time : time - range.end; - line.Alpha = (float)Math.Max(0, 1 - timeSeparation / visible_range); - } - } - } - - private partial class DrawableGridLine : DrawableHitObject - { - public DrawableGridLine() - : base(new HitObject()) - { - RelativeSizeAxes = Axes.X; - Height = 2; - - AddInternal(new Box { RelativeSizeAxes = Axes.Both }); - } - - [BackgroundDependencyLoader] - private void load() - { - Origin = Anchor.BottomLeft; - Anchor = Anchor.BottomLeft; - } - - protected override void UpdateInitialTransforms() - { - // don't perform any fading – we are handling that ourselves. - LifetimeEnd = HitObject.StartTime + visible_range; - } - } + ((CatchPlayfield)composer.Playfield).UnderlayElements + }; } } diff --git a/osu.Game.Rulesets.Mania/Edit/ManiaBeatSnapGrid.cs b/osu.Game.Rulesets.Mania/Edit/ManiaBeatSnapGrid.cs index 4d45e16588..61c730912f 100644 --- a/osu.Game.Rulesets.Mania/Edit/ManiaBeatSnapGrid.cs +++ b/osu.Game.Rulesets.Mania/Edit/ManiaBeatSnapGrid.cs @@ -1,206 +1,23 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using System; using System.Collections.Generic; using System.Linq; -using osu.Framework.Allocation; -using osu.Framework.Bindables; -using osu.Framework.Caching; -using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; -using osu.Framework.Graphics.Pooling; -using osu.Framework.Graphics.Shapes; -using osu.Game.Graphics; using osu.Game.Rulesets.Edit; using osu.Game.Rulesets.Mania.UI; -using osu.Game.Rulesets.Objects; -using osu.Game.Rulesets.Objects.Drawables; -using osu.Game.Rulesets.UI.Scrolling; -using osu.Game.Screens.Edit; -using osuTK.Graphics; +using osu.Game.Screens.Edit.Compose.Components; namespace osu.Game.Rulesets.Mania.Edit { - /// - /// A grid which displays coloured beat divisor lines in proximity to the selection or placement cursor. - /// - public partial class ManiaBeatSnapGrid : CompositeComponent + public partial class ManiaBeatSnapGrid : BeatSnapGrid { - private const double visible_range = 750; - - /// - /// The range of time values of the current selection. - /// - public (double start, double end)? SelectionTimeRange + protected override IEnumerable GetTargetContainers(HitObjectComposer composer) { - set - { - if (value == selectionTimeRange) - return; - - selectionTimeRange = value; - lineCache.Invalidate(); - } - } - - [Resolved] - private EditorBeatmap beatmap { get; set; } = null!; - - [Resolved] - private OsuColour colours { get; set; } = null!; - - [Resolved] - private BindableBeatDivisor beatDivisor { get; set; } = null!; - - private readonly List grids = new List(); - - private readonly DrawablePool linesPool = new DrawablePool(50); - - private readonly Cached lineCache = new Cached(); - - private (double start, double end)? selectionTimeRange; - - [BackgroundDependencyLoader] - private void load(HitObjectComposer composer) - { - AddInternal(linesPool); - - foreach (var stage in ((ManiaPlayfield)composer.Playfield).Stages) - { - foreach (var column in stage.Columns) - { - var lineContainer = new ScrollingHitObjectContainer(); - - grids.Add(lineContainer); - column.UnderlayElements.Add(lineContainer); - } - } - - beatDivisor.BindValueChanged(_ => createLines(), true); - } - - protected override void Update() - { - base.Update(); - - if (!lineCache.IsValid) - { - lineCache.Validate(); - createLines(); - } - } - - private void createLines() - { - foreach (var grid in grids) - grid.Clear(); - - if (selectionTimeRange == null) - return; - - var range = selectionTimeRange.Value; - - var timingPoint = beatmap.ControlPointInfo.TimingPointAt(range.start - visible_range); - - double time = timingPoint.Time; - int beat = 0; - - // progress time until in the visible range. - while (time < range.start - visible_range) - { - time += timingPoint.BeatLength / beatDivisor.Value; - beat++; - } - - while (time < range.end + visible_range) - { - var nextTimingPoint = beatmap.ControlPointInfo.TimingPointAt(time); - - // switch to the next timing point if we have reached it. - if (nextTimingPoint.Time > timingPoint.Time) - { - beat = 0; - time = nextTimingPoint.Time; - timingPoint = nextTimingPoint; - } - - Color4 colour = BindableBeatDivisor.GetColourFor( - BindableBeatDivisor.GetDivisorForBeatIndex(beat, beatDivisor.Value), colours); - - foreach (var grid in grids) - { - var line = linesPool.Get(); - - line.Apply(new HitObject - { - StartTime = time - }); - - line.Colour = colour; - - grid.Add(line); - } - - beat++; - time += timingPoint.BeatLength / beatDivisor.Value; - } - - foreach (var grid in grids) - { - // required to update ScrollingHitObjectContainer's cache. - grid.UpdateSubTree(); - - foreach (var line in grid.Objects.OfType()) - { - time = line.HitObject.StartTime; - - if (time >= range.start && time <= range.end) - line.Alpha = 1; - else - { - double timeSeparation = time < range.start ? range.start - time : time - range.end; - line.Alpha = (float)Math.Max(0, 1 - timeSeparation / visible_range); - } - } - } - } - - private partial class DrawableGridLine : DrawableHitObject - { - [Resolved] - private IScrollingInfo scrollingInfo { get; set; } = null!; - - private readonly IBindable direction = new Bindable(); - - public DrawableGridLine() - : base(new HitObject()) - { - RelativeSizeAxes = Axes.X; - Height = 2; - - AddInternal(new Box { RelativeSizeAxes = Axes.Both }); - } - - [BackgroundDependencyLoader] - private void load() - { - direction.BindTo(scrollingInfo.Direction); - direction.BindValueChanged(onDirectionChanged, true); - } - - private void onDirectionChanged(ValueChangedEvent direction) - { - Origin = Anchor = direction.NewValue == ScrollingDirection.Up - ? Anchor.TopLeft - : Anchor.BottomLeft; - } - - protected override void UpdateInitialTransforms() - { - // don't perform any fading – we are handling that ourselves. - LifetimeEnd = HitObject.StartTime + visible_range; - } + return ((ManiaPlayfield)composer.Playfield) + .Stages + .SelectMany(stage => stage.Columns) + .Select(column => column.UnderlayElements); } } } diff --git a/osu.Game/Screens/Edit/Compose/Components/BeatSnapGrid.cs b/osu.Game/Screens/Edit/Compose/Components/BeatSnapGrid.cs new file mode 100644 index 0000000000..c7adaa5649 --- /dev/null +++ b/osu.Game/Screens/Edit/Compose/Components/BeatSnapGrid.cs @@ -0,0 +1,203 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System; +using System.Collections.Generic; +using System.Linq; +using osu.Framework.Allocation; +using osu.Framework.Bindables; +using osu.Framework.Caching; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; +using osu.Framework.Graphics.Pooling; +using osu.Framework.Graphics.Shapes; +using osu.Game.Graphics; +using osu.Game.Rulesets.Edit; +using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Objects.Drawables; +using osu.Game.Rulesets.UI.Scrolling; +using osuTK.Graphics; + +namespace osu.Game.Screens.Edit.Compose.Components +{ + /// + /// A grid which displays coloured beat divisor lines in proximity to the selection or placement cursor. + /// + public abstract partial class BeatSnapGrid : CompositeComponent + { + private const double visible_range = 750; + + /// + /// The range of time values of the current selection. + /// + public (double start, double end)? SelectionTimeRange + { + set + { + if (value == selectionTimeRange) + return; + + selectionTimeRange = value; + lineCache.Invalidate(); + } + } + + [Resolved] + private EditorBeatmap beatmap { get; set; } = null!; + + [Resolved] + private OsuColour colours { get; set; } = null!; + + [Resolved] + private BindableBeatDivisor beatDivisor { get; set; } = null!; + + private readonly List grids = new List(); + + private readonly DrawablePool linesPool = new DrawablePool(50); + + private readonly Cached lineCache = new Cached(); + + private (double start, double end)? selectionTimeRange; + + [BackgroundDependencyLoader] + private void load(HitObjectComposer composer) + { + AddInternal(linesPool); + + foreach (var target in GetTargetContainers(composer)) + { + var lineContainer = new ScrollingHitObjectContainer(); + + grids.Add(lineContainer); + target.Add(lineContainer); + } + + beatDivisor.BindValueChanged(_ => createLines(), true); + } + + protected abstract IEnumerable GetTargetContainers(HitObjectComposer composer); + + protected override void Update() + { + base.Update(); + + if (!lineCache.IsValid) + { + lineCache.Validate(); + createLines(); + } + } + + private void createLines() + { + foreach (var grid in grids) + grid.Clear(); + + if (selectionTimeRange == null) + return; + + var range = selectionTimeRange.Value; + + var timingPoint = beatmap.ControlPointInfo.TimingPointAt(range.start - visible_range); + + double time = timingPoint.Time; + int beat = 0; + + // progress time until in the visible range. + while (time < range.start - visible_range) + { + time += timingPoint.BeatLength / beatDivisor.Value; + beat++; + } + + while (time < range.end + visible_range) + { + var nextTimingPoint = beatmap.ControlPointInfo.TimingPointAt(time); + + // switch to the next timing point if we have reached it. + if (nextTimingPoint.Time > timingPoint.Time) + { + beat = 0; + time = nextTimingPoint.Time; + timingPoint = nextTimingPoint; + } + + Color4 colour = BindableBeatDivisor.GetColourFor( + BindableBeatDivisor.GetDivisorForBeatIndex(beat, beatDivisor.Value), colours); + + foreach (var grid in grids) + { + var line = linesPool.Get(); + + line.Apply(new HitObject + { + StartTime = time + }); + + line.Colour = colour; + + grid.Add(line); + } + + beat++; + time += timingPoint.BeatLength / beatDivisor.Value; + } + + foreach (var grid in grids) + { + // required to update ScrollingHitObjectContainer's cache. + grid.UpdateSubTree(); + + foreach (var line in grid.Objects.OfType()) + { + time = line.HitObject.StartTime; + + if (time >= range.start && time <= range.end) + line.Alpha = 1; + else + { + double timeSeparation = time < range.start ? range.start - time : time - range.end; + line.Alpha = (float)Math.Max(0, 1 - timeSeparation / visible_range); + } + } + } + } + + private partial class DrawableGridLine : DrawableHitObject + { + [Resolved] + private IScrollingInfo scrollingInfo { get; set; } = null!; + + private readonly IBindable direction = new Bindable(); + + public DrawableGridLine() + : base(new HitObject()) + { + RelativeSizeAxes = Axes.X; + Height = 2; + + AddInternal(new Box { RelativeSizeAxes = Axes.Both }); + } + + [BackgroundDependencyLoader] + private void load() + { + direction.BindTo(scrollingInfo.Direction); + direction.BindValueChanged(onDirectionChanged, true); + } + + private void onDirectionChanged(ValueChangedEvent direction) + { + Origin = Anchor = direction.NewValue == ScrollingDirection.Up + ? Anchor.TopLeft + : Anchor.BottomLeft; + } + + protected override void UpdateInitialTransforms() + { + // don't perform any fading – we are handling that ourselves. + LifetimeEnd = HitObject.StartTime + visible_range; + } + } + } +} From 2a89a257909712e79fff9578bfc5821016dacf66 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 17 Oct 2023 16:54:59 +0900 Subject: [PATCH 32/64] Add beat snap grid to osu!taiko editor Addresses https://github.com/ppy/osu/discussions/25150. --- .../Edit/TaikoBeatSnapGrid.cs | 19 ++++++++ .../Edit/TaikoHitObjectComposer.cs | 43 +++++++++++++++++++ osu.Game.Rulesets.Taiko/UI/TaikoPlayfield.cs | 11 ++++- .../Edit/Compose/Components/BeatSnapGrid.cs | 16 +++++-- 4 files changed, 85 insertions(+), 4 deletions(-) create mode 100644 osu.Game.Rulesets.Taiko/Edit/TaikoBeatSnapGrid.cs diff --git a/osu.Game.Rulesets.Taiko/Edit/TaikoBeatSnapGrid.cs b/osu.Game.Rulesets.Taiko/Edit/TaikoBeatSnapGrid.cs new file mode 100644 index 0000000000..1b6794974f --- /dev/null +++ b/osu.Game.Rulesets.Taiko/Edit/TaikoBeatSnapGrid.cs @@ -0,0 +1,19 @@ +// 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.Framework.Graphics.Containers; +using osu.Game.Rulesets.Edit; +using osu.Game.Rulesets.Taiko.UI; +using osu.Game.Screens.Edit.Compose.Components; + +namespace osu.Game.Rulesets.Taiko.Edit +{ + public partial class TaikoBeatSnapGrid : BeatSnapGrid + { + protected override IEnumerable GetTargetContainers(HitObjectComposer composer) => new[] + { + ((TaikoPlayfield)composer.Playfield).UnderlayElements + }; + } +} diff --git a/osu.Game.Rulesets.Taiko/Edit/TaikoHitObjectComposer.cs b/osu.Game.Rulesets.Taiko/Edit/TaikoHitObjectComposer.cs index 5ae4757b8f..177f548238 100644 --- a/osu.Game.Rulesets.Taiko/Edit/TaikoHitObjectComposer.cs +++ b/osu.Game.Rulesets.Taiko/Edit/TaikoHitObjectComposer.cs @@ -2,10 +2,14 @@ // See the LICENCE file in the repository root for full licence text. using System.Collections.Generic; +using System.Linq; +using osu.Framework.Allocation; +using osu.Framework.Input; using osu.Game.Beatmaps; using osu.Game.Rulesets.Edit; using osu.Game.Rulesets.Edit.Tools; using osu.Game.Rulesets.Mods; +using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Taiko.Objects; using osu.Game.Rulesets.UI; using osu.Game.Screens.Edit.Compose.Components; @@ -16,6 +20,9 @@ namespace osu.Game.Rulesets.Taiko.Edit { protected override bool ApplyHorizontalCentering => false; + private TaikoBeatSnapGrid beatSnapGrid = null!; + private InputManager inputManager = null!; + public TaikoHitObjectComposer(TaikoRuleset ruleset) : base(ruleset) { @@ -33,5 +40,41 @@ namespace osu.Game.Rulesets.Taiko.Edit protected override ComposeBlueprintContainer CreateBlueprintContainer() => new TaikoBlueprintContainer(this); + + [BackgroundDependencyLoader] + private void load() + { + AddInternal(beatSnapGrid = new TaikoBeatSnapGrid()); + } + + protected override void LoadComplete() + { + base.LoadComplete(); + + inputManager = GetContainingInputManager(); + } + + protected override void UpdateAfterChildren() + { + base.UpdateAfterChildren(); + + if (BlueprintContainer.CurrentTool is SelectTool) + { + if (EditorBeatmap.SelectedHitObjects.Any()) + { + beatSnapGrid.SelectionTimeRange = (EditorBeatmap.SelectedHitObjects.Min(h => h.StartTime), EditorBeatmap.SelectedHitObjects.Max(h => h.GetEndTime())); + } + else + beatSnapGrid.SelectionTimeRange = null; + } + else + { + var result = FindSnappedPositionAndTime(inputManager.CurrentState.Mouse.Position); + if (result.Time is double time) + beatSnapGrid.SelectionTimeRange = (time, time); + else + beatSnapGrid.SelectionTimeRange = null; + } + } } } diff --git a/osu.Game.Rulesets.Taiko/UI/TaikoPlayfield.cs b/osu.Game.Rulesets.Taiko/UI/TaikoPlayfield.cs index 23ffac1f63..31f8171290 100644 --- a/osu.Game.Rulesets.Taiko/UI/TaikoPlayfield.cs +++ b/osu.Game.Rulesets.Taiko/UI/TaikoPlayfield.cs @@ -40,6 +40,8 @@ namespace osu.Game.Rulesets.Taiko.UI /// public Bindable ClassicHitTargetPosition = new BindableBool(); + public Container UnderlayElements { get; private set; } = null!; + private Container hitExplosionContainer; private Container kiaiExplosionContainer; private JudgementContainer judgementContainer; @@ -130,7 +132,14 @@ namespace osu.Game.Rulesets.Taiko.UI { Name = "Bar line content", RelativeSizeAxes = Axes.Both, - Child = barLinePlayfield = new BarLinePlayfield(), + Children = new Drawable[] + { + UnderlayElements = new Container + { + RelativeSizeAxes = Axes.Both, + }, + barLinePlayfield = new BarLinePlayfield(), + } }, hitObjectContent = new Container { diff --git a/osu.Game/Screens/Edit/Compose/Components/BeatSnapGrid.cs b/osu.Game/Screens/Edit/Compose/Components/BeatSnapGrid.cs index c7adaa5649..766d5b5601 100644 --- a/osu.Game/Screens/Edit/Compose/Components/BeatSnapGrid.cs +++ b/osu.Game/Screens/Edit/Compose/Components/BeatSnapGrid.cs @@ -173,9 +173,6 @@ namespace osu.Game.Screens.Edit.Compose.Components public DrawableGridLine() : base(new HitObject()) { - RelativeSizeAxes = Axes.X; - Height = 2; - AddInternal(new Box { RelativeSizeAxes = Axes.Both }); } @@ -191,6 +188,19 @@ namespace osu.Game.Screens.Edit.Compose.Components Origin = Anchor = direction.NewValue == ScrollingDirection.Up ? Anchor.TopLeft : Anchor.BottomLeft; + + bool isHorizontal = direction.NewValue == ScrollingDirection.Left || direction.NewValue == ScrollingDirection.Right; + + if (isHorizontal) + { + RelativeSizeAxes = Axes.Y; + Width = 2; + } + else + { + RelativeSizeAxes = Axes.X; + Height = 2; + } } protected override void UpdateInitialTransforms() From 4381169a3fdf582414f82c0c0d3af52d85404ae6 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 17 Oct 2023 17:09:42 +0900 Subject: [PATCH 33/64] Combine selection and input handling logic for beat snap grids across all rulesets --- .../Edit/CatchHitObjectComposer.cs | 41 ++------------- .../Edit/ManiaHitObjectComposer.cs | 42 +-------------- .../Edit/TaikoHitObjectComposer.cs | 43 +--------------- osu.Game/Rulesets/Edit/HitObjectComposer.cs | 51 +++++++++++++++++-- 4 files changed, 53 insertions(+), 124 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Edit/CatchHitObjectComposer.cs b/osu.Game.Rulesets.Catch/Edit/CatchHitObjectComposer.cs index dc3a4416a5..80a9731cc4 100644 --- a/osu.Game.Rulesets.Catch/Edit/CatchHitObjectComposer.cs +++ b/osu.Game.Rulesets.Catch/Edit/CatchHitObjectComposer.cs @@ -8,7 +8,6 @@ using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Extensions.EnumExtensions; using osu.Framework.Graphics; -using osu.Framework.Input; using osu.Framework.Input.Events; using osu.Game.Beatmaps; using osu.Game.Graphics.UserInterface; @@ -32,10 +31,6 @@ namespace osu.Game.Rulesets.Catch.Edit private CatchDistanceSnapGrid distanceSnapGrid = null!; - private InputManager inputManager = null!; - - private CatchBeatSnapGrid beatSnapGrid = null!; - private readonly BindableDouble timeRangeMultiplier = new BindableDouble(1) { MinValue = 1, @@ -68,38 +63,6 @@ namespace osu.Game.Rulesets.Catch.Edit Catcher.BASE_DASH_SPEED, -Catcher.BASE_DASH_SPEED, Catcher.BASE_WALK_SPEED, -Catcher.BASE_WALK_SPEED, })); - - AddInternal(beatSnapGrid = new CatchBeatSnapGrid()); - } - - protected override void LoadComplete() - { - base.LoadComplete(); - - inputManager = GetContainingInputManager(); - } - - protected override void UpdateAfterChildren() - { - base.UpdateAfterChildren(); - - if (BlueprintContainer.CurrentTool is SelectTool) - { - if (EditorBeatmap.SelectedHitObjects.Any()) - { - beatSnapGrid.SelectionTimeRange = (EditorBeatmap.SelectedHitObjects.Min(h => h.StartTime), EditorBeatmap.SelectedHitObjects.Max(h => h.GetEndTime())); - } - else - beatSnapGrid.SelectionTimeRange = null; - } - else - { - var result = FindSnappedPositionAndTime(inputManager.CurrentState.Mouse.Position); - if (result.Time is double time) - beatSnapGrid.SelectionTimeRange = (time, time); - else - beatSnapGrid.SelectionTimeRange = null; - } } protected override double ReadCurrentDistanceSnap(HitObject before, HitObject after) @@ -174,6 +137,8 @@ namespace osu.Game.Rulesets.Catch.Edit protected override ComposeBlueprintContainer CreateBlueprintContainer() => new CatchBlueprintContainer(this); + protected override BeatSnapGrid CreateBeatSnapGrid() => new CatchBeatSnapGrid(); + private PalpableCatchHitObject? getLastSnappableHitObject(double time) { var hitObject = EditorBeatmap.HitObjects.OfType().LastOrDefault(h => h.GetEndTime() < time && !(h is BananaShower)); @@ -214,7 +179,7 @@ namespace osu.Game.Rulesets.Catch.Edit return null; } - double timeAtCursor = ((CatchPlayfield)Playfield).TimeAtScreenSpacePosition(inputManager.CurrentState.Mouse.Position); + double timeAtCursor = ((CatchPlayfield)Playfield).TimeAtScreenSpacePosition(InputManager.CurrentState.Mouse.Position); return getLastSnappableHitObject(timeAtCursor); default: diff --git a/osu.Game.Rulesets.Mania/Edit/ManiaHitObjectComposer.cs b/osu.Game.Rulesets.Mania/Edit/ManiaHitObjectComposer.cs index 7a0ec70611..91fe99f938 100644 --- a/osu.Game.Rulesets.Mania/Edit/ManiaHitObjectComposer.cs +++ b/osu.Game.Rulesets.Mania/Edit/ManiaHitObjectComposer.cs @@ -6,14 +6,12 @@ using System.Collections.Generic; using System.Linq; using osu.Framework.Allocation; -using osu.Framework.Input; using osu.Game.Beatmaps; using osu.Game.Rulesets.Edit; using osu.Game.Rulesets.Edit.Tools; using osu.Game.Rulesets.Mania.Objects; using osu.Game.Rulesets.Mania.UI; using osu.Game.Rulesets.Mods; -using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.UI; using osu.Game.Rulesets.UI.Scrolling; using osu.Game.Screens.Edit.Compose.Components; @@ -24,27 +22,12 @@ namespace osu.Game.Rulesets.Mania.Edit public partial class ManiaHitObjectComposer : ScrollingHitObjectComposer { private DrawableManiaEditorRuleset drawableRuleset; - private ManiaBeatSnapGrid beatSnapGrid; - private InputManager inputManager; public ManiaHitObjectComposer(Ruleset ruleset) : base(ruleset) { } - [BackgroundDependencyLoader] - private void load() - { - AddInternal(beatSnapGrid = new ManiaBeatSnapGrid()); - } - - protected override void LoadComplete() - { - base.LoadComplete(); - - inputManager = GetContainingInputManager(); - } - private DependencyContainer dependencies; protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnlyDependencyContainer parent) @@ -63,35 +46,14 @@ namespace osu.Game.Rulesets.Mania.Edit protected override ComposeBlueprintContainer CreateBlueprintContainer() => new ManiaBlueprintContainer(this); + protected override BeatSnapGrid CreateBeatSnapGrid() => new ManiaBeatSnapGrid(); + protected override IReadOnlyList CompositionTools => new HitObjectCompositionTool[] { new NoteCompositionTool(), new HoldNoteCompositionTool() }; - protected override void UpdateAfterChildren() - { - base.UpdateAfterChildren(); - - if (BlueprintContainer.CurrentTool is SelectTool) - { - if (EditorBeatmap.SelectedHitObjects.Any()) - { - beatSnapGrid.SelectionTimeRange = (EditorBeatmap.SelectedHitObjects.Min(h => h.StartTime), EditorBeatmap.SelectedHitObjects.Max(h => h.GetEndTime())); - } - else - beatSnapGrid.SelectionTimeRange = null; - } - else - { - var result = FindSnappedPositionAndTime(inputManager.CurrentState.Mouse.Position); - if (result.Time is double time) - beatSnapGrid.SelectionTimeRange = (time, time); - else - beatSnapGrid.SelectionTimeRange = null; - } - } - public override string ConvertSelectionToString() => string.Join(',', EditorBeatmap.SelectedHitObjects.Cast().OrderBy(h => h.StartTime).Select(h => $"{h.StartTime}|{h.Column}")); } diff --git a/osu.Game.Rulesets.Taiko/Edit/TaikoHitObjectComposer.cs b/osu.Game.Rulesets.Taiko/Edit/TaikoHitObjectComposer.cs index 177f548238..6020f6e04c 100644 --- a/osu.Game.Rulesets.Taiko/Edit/TaikoHitObjectComposer.cs +++ b/osu.Game.Rulesets.Taiko/Edit/TaikoHitObjectComposer.cs @@ -2,14 +2,10 @@ // See the LICENCE file in the repository root for full licence text. using System.Collections.Generic; -using System.Linq; -using osu.Framework.Allocation; -using osu.Framework.Input; using osu.Game.Beatmaps; using osu.Game.Rulesets.Edit; using osu.Game.Rulesets.Edit.Tools; using osu.Game.Rulesets.Mods; -using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Taiko.Objects; using osu.Game.Rulesets.UI; using osu.Game.Screens.Edit.Compose.Components; @@ -20,9 +16,6 @@ namespace osu.Game.Rulesets.Taiko.Edit { protected override bool ApplyHorizontalCentering => false; - private TaikoBeatSnapGrid beatSnapGrid = null!; - private InputManager inputManager = null!; - public TaikoHitObjectComposer(TaikoRuleset ruleset) : base(ruleset) { @@ -41,40 +34,6 @@ namespace osu.Game.Rulesets.Taiko.Edit protected override ComposeBlueprintContainer CreateBlueprintContainer() => new TaikoBlueprintContainer(this); - [BackgroundDependencyLoader] - private void load() - { - AddInternal(beatSnapGrid = new TaikoBeatSnapGrid()); - } - - protected override void LoadComplete() - { - base.LoadComplete(); - - inputManager = GetContainingInputManager(); - } - - protected override void UpdateAfterChildren() - { - base.UpdateAfterChildren(); - - if (BlueprintContainer.CurrentTool is SelectTool) - { - if (EditorBeatmap.SelectedHitObjects.Any()) - { - beatSnapGrid.SelectionTimeRange = (EditorBeatmap.SelectedHitObjects.Min(h => h.StartTime), EditorBeatmap.SelectedHitObjects.Max(h => h.GetEndTime())); - } - else - beatSnapGrid.SelectionTimeRange = null; - } - else - { - var result = FindSnappedPositionAndTime(inputManager.CurrentState.Mouse.Position); - if (result.Time is double time) - beatSnapGrid.SelectionTimeRange = (time, time); - else - beatSnapGrid.SelectionTimeRange = null; - } - } + protected override BeatSnapGrid CreateBeatSnapGrid() => new TaikoBeatSnapGrid(); } } diff --git a/osu.Game/Rulesets/Edit/HitObjectComposer.cs b/osu.Game/Rulesets/Edit/HitObjectComposer.cs index e1ae4b0199..2a859dad0d 100644 --- a/osu.Game/Rulesets/Edit/HitObjectComposer.cs +++ b/osu.Game/Rulesets/Edit/HitObjectComposer.cs @@ -7,6 +7,7 @@ using System; using System.Collections.Generic; using System.Collections.Specialized; using System.Linq; +using JetBrains.Annotations; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Extensions.EnumExtensions; @@ -76,7 +77,7 @@ namespace osu.Game.Rulesets.Edit protected readonly Container LayerBelowRuleset = new Container { RelativeSizeAxes = Axes.Both }; - private InputManager inputManager; + protected InputManager InputManager { get; private set; } private EditorRadioButtonCollection toolboxCollection; @@ -89,6 +90,9 @@ namespace osu.Game.Rulesets.Edit protected DrawableRuleset DrawableRuleset { get; private set; } + [CanBeNull] + private BeatSnapGrid beatSnapGrid; + protected HitObjectComposer(Ruleset ruleset) : base(ruleset) { @@ -204,7 +208,9 @@ namespace osu.Game.Rulesets.Edit }, } } - } + }, + // Must be constructed after drawable ruleset above. + (beatSnapGrid = CreateBeatSnapGrid()) ?? Empty(), }; toolboxCollection.Items = CompositionTools @@ -235,7 +241,7 @@ namespace osu.Game.Rulesets.Edit { base.LoadComplete(); - inputManager = GetContainingInputManager(); + InputManager = GetContainingInputManager(); hasTiming = EditorBeatmap.HasTiming.GetBoundCopy(); hasTiming.BindValueChanged(timing => @@ -269,11 +275,42 @@ namespace osu.Game.Rulesets.Edit } } + protected override void UpdateAfterChildren() + { + base.UpdateAfterChildren(); + + updateBeatSnapGrid(); + } + + private void updateBeatSnapGrid() + { + if (beatSnapGrid == null) + return; + + if (BlueprintContainer.CurrentTool is SelectTool) + { + if (EditorBeatmap.SelectedHitObjects.Any()) + { + beatSnapGrid.SelectionTimeRange = (EditorBeatmap.SelectedHitObjects.Min(h => h.StartTime), EditorBeatmap.SelectedHitObjects.Max(h => h.GetEndTime())); + } + else + beatSnapGrid.SelectionTimeRange = null; + } + else + { + var result = FindSnappedPositionAndTime(InputManager.CurrentState.Mouse.Position); + if (result.Time is double time) + beatSnapGrid.SelectionTimeRange = (time, time); + else + beatSnapGrid.SelectionTimeRange = null; + } + } + public override Playfield Playfield => drawableRulesetWrapper.Playfield; public override IEnumerable HitObjects => drawableRulesetWrapper.Playfield.AllHitObjects; - public override bool CursorInPlacementArea => drawableRulesetWrapper.Playfield.ReceivePositionalInputAt(inputManager.CurrentState.Mouse.Position); + public override bool CursorInPlacementArea => drawableRulesetWrapper.Playfield.ReceivePositionalInputAt(InputManager.CurrentState.Mouse.Position); /// /// Defines all available composition tools, listed on the left side of the editor screen as button controls. @@ -299,6 +336,12 @@ namespace osu.Game.Rulesets.Edit /// protected virtual ComposeBlueprintContainer CreateBlueprintContainer() => new ComposeBlueprintContainer(this); + /// + /// Construct an optional beat snap grid. + /// + [CanBeNull] + protected virtual BeatSnapGrid CreateBeatSnapGrid() => null; + /// /// Construct a drawable ruleset for the provided ruleset. /// From 7abd7fe658e51753a7ebc54f91d0ab042b5f7efb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 17 Oct 2023 10:26:02 +0200 Subject: [PATCH 34/64] Remove redundant explicit array type specification --- osu.Game/Rulesets/Edit/HitObjectComposer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Rulesets/Edit/HitObjectComposer.cs b/osu.Game/Rulesets/Edit/HitObjectComposer.cs index 2a859dad0d..129ab6751e 100644 --- a/osu.Game/Rulesets/Edit/HitObjectComposer.cs +++ b/osu.Game/Rulesets/Edit/HitObjectComposer.cs @@ -128,7 +128,7 @@ namespace osu.Game.Rulesets.Edit dependencies.CacheAs(Playfield); - InternalChildren = new Drawable[] + InternalChildren = new[] { PlayfieldContentContainer = new Container { From 22c1a963e722d4ce62cb24df907974fc1284d42f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 17 Oct 2023 10:27:16 +0200 Subject: [PATCH 35/64] Remove unused field --- osu.Game.Rulesets.Mania/Edit/ManiaHitObjectComposer.cs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/osu.Game.Rulesets.Mania/Edit/ManiaHitObjectComposer.cs b/osu.Game.Rulesets.Mania/Edit/ManiaHitObjectComposer.cs index 91fe99f938..61ddaffb25 100644 --- a/osu.Game.Rulesets.Mania/Edit/ManiaHitObjectComposer.cs +++ b/osu.Game.Rulesets.Mania/Edit/ManiaHitObjectComposer.cs @@ -28,11 +28,6 @@ namespace osu.Game.Rulesets.Mania.Edit { } - private DependencyContainer dependencies; - - protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnlyDependencyContainer parent) - => dependencies = new DependencyContainer(base.CreateChildDependencies(parent)); - public new ManiaPlayfield Playfield => ((ManiaPlayfield)drawableRuleset.Playfield); public IScrollingInfo ScrollingInfo => drawableRuleset.ScrollingInfo; From 2192c9f2c28e5a34cc753f62218a19026c21755c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 17 Oct 2023 10:30:01 +0200 Subject: [PATCH 36/64] Remove unused using directive --- osu.Game.Rulesets.Mania/Edit/ManiaHitObjectComposer.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game.Rulesets.Mania/Edit/ManiaHitObjectComposer.cs b/osu.Game.Rulesets.Mania/Edit/ManiaHitObjectComposer.cs index 61ddaffb25..b9db4168f4 100644 --- a/osu.Game.Rulesets.Mania/Edit/ManiaHitObjectComposer.cs +++ b/osu.Game.Rulesets.Mania/Edit/ManiaHitObjectComposer.cs @@ -5,7 +5,6 @@ using System.Collections.Generic; using System.Linq; -using osu.Framework.Allocation; using osu.Game.Beatmaps; using osu.Game.Rulesets.Edit; using osu.Game.Rulesets.Edit.Tools; From 08845ec1c691df95d457860112186b5cccb45a1d Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Tue, 17 Oct 2023 20:15:00 +0900 Subject: [PATCH 37/64] Use action for GHA diffcalc workflow permissions check --- .github/workflows/diffcalc.yml | 49 +++++++++++++++++++++------------- 1 file changed, 30 insertions(+), 19 deletions(-) diff --git a/.github/workflows/diffcalc.yml b/.github/workflows/diffcalc.yml index 5fde6e2f1a..d39dffc4b9 100644 --- a/.github/workflows/diffcalc.yml +++ b/.github/workflows/diffcalc.yml @@ -14,8 +14,8 @@ # # The workflow can be run in two ways: # 1. Via workflow dispatch. -# 2. By an owner of the repository posting a pull request or issue comment containing `!diffcalc`. -# For pull requests, the workflow will assume the pull request as the target to compare against (i.e. the `OSU_B` variable). +# 2. By an owner of the repository posting a pull request or issue comment containing `!diffcalc`. +# For pull requests, the workflow will assume the pull request as the target to compare against (i.e. the `OSU_B` variable). # Any lines in the comment of the form `KEY=VALUE` are treated as variables for the generator. # # ## Google Service Account @@ -104,21 +104,22 @@ env: COMMENT_TAG: execution-${{ github.run_id }}-${{ github.run_number }}-${{ github.run_attempt }} jobs: - wait-for-queue: - name: "Wait for previous workflows" + check-permissions: + name: Check permissions runs-on: ubuntu-latest - if: ${{ !cancelled() && (github.event_name == 'workflow_dispatch' || contains(github.event.comment.body, '!diffcalc') && github.event.comment.author_association == 'OWNER') }} - timeout-minutes: 50400 # 35 days, the maximum for jobs. + if: ${{ github.event_name == 'workflow_dispatch' || contains(github.event.comment.body, '!diffcalc') }} steps: - - uses: ahmadnassri/action-workflow-queue@v1 + - name: Check permissions + if: ${{ github.event_name != 'workflow_dispatch' }} + uses: actions-cool/check-user-permission@v2 with: - timeout: 2147483647 # Around 24 days, maximum supported. - delay: 120000 # Poll every 2 minutes. API seems fairly low on this one. + require: 'write' create-comment: name: Create PR comment + needs: check-permissions runs-on: ubuntu-latest - if: ${{ github.event_name == 'issue_comment' && github.event.issue.pull_request && contains(github.event.comment.body, '!diffcalc') && github.event.comment.author_association == 'OWNER' }} + if: ${{ github.event_name == 'issue_comment' && github.event.issue.pull_request }} steps: - name: Create comment uses: thollander/actions-comment-pull-request@v2 @@ -129,11 +130,21 @@ jobs: *This comment will update on completion* + wait-for-queue: + name: "Wait for previous workflows" + needs: check-permissions + runs-on: ubuntu-latest + timeout-minutes: 50400 # 35 days, the maximum for jobs. + steps: + - uses: ahmadnassri/action-workflow-queue@v1 + with: + timeout: 2147483647 # Around 24 days, maximum supported. + delay: 120000 # Poll every 2 minutes. API seems fairly low on this one. + directory: name: Prepare directory needs: wait-for-queue runs-on: self-hosted - if: ${{ !cancelled() && (github.event_name == 'workflow_dispatch' || contains(github.event.comment.body, '!diffcalc') && github.event.comment.author_association == 'OWNER') }} outputs: GENERATOR_DIR: ${{ steps.set-outputs.outputs.GENERATOR_DIR }} GENERATOR_ENV: ${{ steps.set-outputs.outputs.GENERATOR_ENV }} @@ -159,7 +170,6 @@ jobs: name: Setup environment needs: directory runs-on: self-hosted - if: ${{ !cancelled() && needs.directory.result == 'success' }} env: VARS_JSON: ${{ toJSON(vars) }} steps: @@ -239,7 +249,6 @@ jobs: name: Setup scores needs: [ directory, environment ] runs-on: self-hosted - if: ${{ !cancelled() && needs.environment.result == 'success' }} steps: - name: Query latest data id: query @@ -272,7 +281,6 @@ jobs: name: Setup beatmaps needs: directory runs-on: self-hosted - if: ${{ !cancelled() && needs.directory.result == 'success' }} steps: - name: Query latest data id: query @@ -305,7 +313,6 @@ jobs: needs: [ directory, environment, scores, beatmaps ] runs-on: self-hosted timeout-minutes: 720 - if: ${{ !cancelled() && needs.scores.result == 'success' && needs.beatmaps.result == 'success' }} outputs: TARGET: ${{ steps.run.outputs.TARGET }} SPREADSHEET_LINK: ${{ steps.run.outputs.SPREADSHEET_LINK }} @@ -331,17 +338,21 @@ jobs: cd "${{ needs.directory.outputs.GENERATOR_DIR }}" docker-compose down + output-cli: + name: Output info + needs: generator + runs-on: ubuntu-latest + steps: - name: Output info - if: ${{ success() }} run: | - echo "Target: ${{ steps.run.outputs.TARGET }}" - echo "Spreadsheet: ${{ steps.run.outputs.SPREADSHEET_LINK }}" + echo "Target: ${{ needs.generator.outputs.TARGET }}" + echo "Spreadsheet: ${{ needs.generator.outputs.SPREADSHEET_LINK }}" update-comment: name: Update PR comment needs: [ create-comment, generator ] runs-on: ubuntu-latest - if: ${{ github.event_name == 'issue_comment' && github.event.issue.pull_request && contains(github.event.comment.body, '!diffcalc') && github.event.comment.author_association == 'OWNER' }} + if: ${{ needs.create-comment.result == 'success' }} steps: - name: Update comment on success if: ${{ needs.generator.result == 'success' }} From 566b09ff208feb6625ec4a52324f1b9f8770eac0 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Tue, 17 Oct 2023 20:36:24 +0900 Subject: [PATCH 38/64] Delete comment on cancellation - Add always() pre-condition to force evaluation. - Message is set as it's required by the action for a non-error status code. --- .github/workflows/diffcalc.yml | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/.github/workflows/diffcalc.yml b/.github/workflows/diffcalc.yml index d39dffc4b9..a8dc16af15 100644 --- a/.github/workflows/diffcalc.yml +++ b/.github/workflows/diffcalc.yml @@ -352,7 +352,7 @@ jobs: name: Update PR comment needs: [ create-comment, generator ] runs-on: ubuntu-latest - if: ${{ needs.create-comment.result == 'success' }} + if: ${{ always() && needs.create-comment.result == 'success' }} steps: - name: Update comment on success if: ${{ needs.generator.result == 'success' }} @@ -374,3 +374,11 @@ jobs: create_if_not_exists: false message: | Difficulty calculation failed: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + + - name: Update comment on cancellation + if: ${{ needs.generator.result == 'cancelled' }} + uses: thollander/actions-comment-pull-request@v2 + with: + comment_tag: ${{ env.COMMENT_TAG }} + mode: delete + message: '.' # Appears to be required by this action for non-error status code. From 4946b437c9c5c639cfc9ea6a27aa3919ed33c250 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Tue, 17 Oct 2023 20:58:09 +0900 Subject: [PATCH 39/64] Run timeout job on self-hosted runner --- .github/workflows/diffcalc.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/diffcalc.yml b/.github/workflows/diffcalc.yml index a8dc16af15..c3d92b6d1f 100644 --- a/.github/workflows/diffcalc.yml +++ b/.github/workflows/diffcalc.yml @@ -133,13 +133,13 @@ jobs: wait-for-queue: name: "Wait for previous workflows" needs: check-permissions - runs-on: ubuntu-latest - timeout-minutes: 50400 # 35 days, the maximum for jobs. + runs-on: self-hosted + timeout-minutes: 50400 # 35 days, the maximum for jobs on self-hosted runners. steps: - uses: ahmadnassri/action-workflow-queue@v1 with: - timeout: 2147483647 # Around 24 days, maximum supported. - delay: 120000 # Poll every 2 minutes. API seems fairly low on this one. + timeout: 2147483647 # Around 24 days - the maximum supported by JS setTimeout(). + delay: 120000 # Poll every 2 minutes - the API limit seems fairly low on this one. directory: name: Prepare directory From 7479830a8ea8b21b8c6517354f35ce3fa4ddb0ff Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 18 Oct 2023 16:26:32 +0900 Subject: [PATCH 40/64] Comment flaky tests for now Same issue as other commented test (clock precision not high enough). Will probably want a solution to this at some point. --- osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs index 9e83ca428b..7f81ccddf6 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs @@ -51,7 +51,7 @@ namespace osu.Game.Rulesets.Osu.Tests [TestCase(80, 1)] [TestCase(80, 0)] [TestCase(80, 10)] - [TestCase(90, 1)] + // [TestCase(90, 1)] flaky public void TestVeryShortSlider(float sliderLength, int repeatCount) { Slider slider; @@ -87,8 +87,8 @@ namespace osu.Game.Rulesets.Osu.Tests [TestCase(300, false)] [TestCase(200, true)] [TestCase(150, true)] - [TestCase(120, true)] - [TestCase(60, true)] + // [TestCase(120, true)] flaky + // [TestCase(60, true)] flaky [TestCase(10, true)] // [TestCase(0, true)] headless test doesn't run at high enough precision for this to always enter a tracking state in time. [TestCase(-30, false)] From 43238b0cee94f28ff6b8abebb7f4bfcbbc134f93 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 18 Oct 2023 16:55:27 +0900 Subject: [PATCH 41/64] Split common functionality from rate adjust mods into helper class --- osu.Game/Rulesets/Mods/ModAdaptiveSpeed.cs | 20 +++------- osu.Game/Rulesets/Mods/ModDaycore.cs | 1 - osu.Game/Rulesets/Mods/ModDoubleTime.cs | 20 +++------- osu.Game/Rulesets/Mods/ModHalfTime.cs | 20 +++------- osu.Game/Rulesets/Mods/ModNightcore.cs | 40 +++++++++---------- osu.Game/Rulesets/Mods/ModTimeRamp.cs | 20 ++-------- osu.Game/Rulesets/Mods/RateAdjustModHelper.cs | 36 +++++++++++++++++ 7 files changed, 74 insertions(+), 83 deletions(-) create mode 100644 osu.Game/Rulesets/Mods/RateAdjustModHelper.cs diff --git a/osu.Game/Rulesets/Mods/ModAdaptiveSpeed.cs b/osu.Game/Rulesets/Mods/ModAdaptiveSpeed.cs index f62ba21827..f122baed90 100644 --- a/osu.Game/Rulesets/Mods/ModAdaptiveSpeed.cs +++ b/osu.Game/Rulesets/Mods/ModAdaptiveSpeed.cs @@ -71,7 +71,6 @@ namespace osu.Game.Rulesets.Mods // Apply a fixed rate change when missing, allowing the player to catch up when the rate is too fast. private const double rate_change_on_miss = 0.95d; - private IAdjustableAudioComponent? track; private double targetRate = 1d; /// @@ -123,24 +122,26 @@ namespace osu.Game.Rulesets.Mods /// private readonly Dictionary ratesForRewinding = new Dictionary(); + private readonly RateAdjustModHelper rateAdjustHelper; + public ModAdaptiveSpeed() { + rateAdjustHelper = new RateAdjustModHelper(SpeedChange, AdjustPitch); + InitialRate.BindValueChanged(val => { SpeedChange.Value = val.NewValue; targetRate = val.NewValue; }); - AdjustPitch.BindValueChanged(adjustPitchChanged); } public void ApplyToTrack(IAdjustableAudioComponent track) { - this.track = track; - InitialRate.TriggerChange(); - AdjustPitch.TriggerChange(); recentRates.Clear(); recentRates.AddRange(Enumerable.Repeat(InitialRate.Value, recent_rate_count)); + + rateAdjustHelper.ApplyToTrack(track); } public void ApplyToSample(IAdjustableAudioComponent sample) @@ -199,15 +200,6 @@ namespace osu.Game.Rulesets.Mods } } - private void adjustPitchChanged(ValueChangedEvent adjustPitchSetting) - { - track?.RemoveAdjustment(adjustmentForPitchSetting(adjustPitchSetting.OldValue), SpeedChange); - track?.AddAdjustment(adjustmentForPitchSetting(adjustPitchSetting.NewValue), SpeedChange); - } - - private AdjustableProperty adjustmentForPitchSetting(bool adjustPitchSettingValue) - => adjustPitchSettingValue ? AdjustableProperty.Frequency : AdjustableProperty.Tempo; - private IEnumerable getAllApplicableHitObjects(IEnumerable hitObjects) { foreach (var hitObject in hitObjects) diff --git a/osu.Game/Rulesets/Mods/ModDaycore.cs b/osu.Game/Rulesets/Mods/ModDaycore.cs index c5810cc9e5..60ce610d7d 100644 --- a/osu.Game/Rulesets/Mods/ModDaycore.cs +++ b/osu.Game/Rulesets/Mods/ModDaycore.cs @@ -39,7 +39,6 @@ namespace osu.Game.Rulesets.Mods public override void ApplyToTrack(IAdjustableAudioComponent track) { - // base.ApplyToTrack() intentionally not called (different tempo adjustment is applied) track.AddAdjustment(AdjustableProperty.Frequency, freqAdjust); track.AddAdjustment(AdjustableProperty.Tempo, tempoAdjust); } diff --git a/osu.Game/Rulesets/Mods/ModDoubleTime.cs b/osu.Game/Rulesets/Mods/ModDoubleTime.cs index df17f4f963..938c6268e2 100644 --- a/osu.Game/Rulesets/Mods/ModDoubleTime.cs +++ b/osu.Game/Rulesets/Mods/ModDoubleTime.cs @@ -27,29 +27,19 @@ namespace osu.Game.Rulesets.Mods Precision = 0.01, }; - private IAdjustableAudioComponent? track; - [SettingSource("Adjust pitch", "Should pitch be adjusted with speed")] - public virtual BindableBool AdjustPitch { get; } = new BindableBool(false); + public virtual BindableBool AdjustPitch { get; } = new BindableBool(); + + private readonly RateAdjustModHelper rateAdjustHelper; protected ModDoubleTime() { - AdjustPitch.BindValueChanged(adjustPitchChanged); + rateAdjustHelper = new RateAdjustModHelper(SpeedChange, AdjustPitch); } - private void adjustPitchChanged(ValueChangedEvent adjustPitchSetting) - { - track?.RemoveAdjustment(adjustmentForPitchSetting(adjustPitchSetting.OldValue), SpeedChange); - track?.AddAdjustment(adjustmentForPitchSetting(adjustPitchSetting.NewValue), SpeedChange); - } - - private AdjustableProperty adjustmentForPitchSetting(bool adjustPitchSettingValue) - => adjustPitchSettingValue ? AdjustableProperty.Frequency : AdjustableProperty.Tempo; - public override void ApplyToTrack(IAdjustableAudioComponent track) { - this.track = track; - AdjustPitch.TriggerChange(); + rateAdjustHelper.ApplyToTrack(track); } public override double ScoreMultiplier diff --git a/osu.Game/Rulesets/Mods/ModHalfTime.cs b/osu.Game/Rulesets/Mods/ModHalfTime.cs index 390d07d3ee..965e94efcb 100644 --- a/osu.Game/Rulesets/Mods/ModHalfTime.cs +++ b/osu.Game/Rulesets/Mods/ModHalfTime.cs @@ -27,29 +27,19 @@ namespace osu.Game.Rulesets.Mods Precision = 0.01, }; - private IAdjustableAudioComponent? track; - [SettingSource("Adjust pitch", "Should pitch be adjusted with speed")] - public virtual BindableBool AdjustPitch { get; } = new BindableBool(false); + public virtual BindableBool AdjustPitch { get; } = new BindableBool(); + + private readonly RateAdjustModHelper rateAdjustHelper; protected ModHalfTime() { - AdjustPitch.BindValueChanged(adjustPitchChanged); + rateAdjustHelper = new RateAdjustModHelper(SpeedChange, AdjustPitch); } - private void adjustPitchChanged(ValueChangedEvent adjustPitchSetting) - { - track?.RemoveAdjustment(adjustmentForPitchSetting(adjustPitchSetting.OldValue), SpeedChange); - track?.AddAdjustment(adjustmentForPitchSetting(adjustPitchSetting.NewValue), SpeedChange); - } - - private AdjustableProperty adjustmentForPitchSetting(bool adjustPitchSettingValue) - => adjustPitchSettingValue ? AdjustableProperty.Frequency : AdjustableProperty.Tempo; - public override void ApplyToTrack(IAdjustableAudioComponent track) { - this.track = track; - AdjustPitch.TriggerChange(); + rateAdjustHelper.ApplyToTrack(track); } public override double ScoreMultiplier diff --git a/osu.Game/Rulesets/Mods/ModNightcore.cs b/osu.Game/Rulesets/Mods/ModNightcore.cs index 80b9314aec..520ee6319a 100644 --- a/osu.Game/Rulesets/Mods/ModNightcore.cs +++ b/osu.Game/Rulesets/Mods/ModNightcore.cs @@ -36,27 +36,6 @@ namespace osu.Game.Rulesets.Mods Precision = 0.01, }; - public override double ScoreMultiplier - { - get - { - // Round to the nearest multiple of 0.1. - double value = (int)(SpeedChange.Value * 10) / 10.0; - - // Offset back to 0. - value -= 1; - - // Each 0.1 multiple changes score multiplier by 0.02. - value /= 5; - - return 1 + value; - } - } - } - - public abstract partial class ModNightcore : ModNightcore, IApplicableToDrawableRuleset - where TObject : HitObject - { private readonly BindableNumber tempoAdjust = new BindableDouble(1); private readonly BindableNumber freqAdjust = new BindableDouble(1); @@ -71,11 +50,28 @@ namespace osu.Game.Rulesets.Mods public override void ApplyToTrack(IAdjustableAudioComponent track) { - // base.ApplyToTrack() intentionally not called (different tempo adjustment is applied) track.AddAdjustment(AdjustableProperty.Frequency, freqAdjust); track.AddAdjustment(AdjustableProperty.Tempo, tempoAdjust); } + public override double ScoreMultiplier + { + get + { + // Round to the nearest multiple of 0.1. + double value = (int)(SpeedChange.Value * 10) / 10.0; + + // Offset back to 0. + value -= 1; + + return 1 + value; + } + } + } + + public abstract partial class ModNightcore : ModNightcore, IApplicableToDrawableRuleset + where TObject : HitObject + { public void ApplyToDrawableRuleset(DrawableRuleset drawableRuleset) { drawableRuleset.Overlays.Add(new NightcoreBeatContainer()); diff --git a/osu.Game/Rulesets/Mods/ModTimeRamp.cs b/osu.Game/Rulesets/Mods/ModTimeRamp.cs index 54ee0cd3bf..1048caa698 100644 --- a/osu.Game/Rulesets/Mods/ModTimeRamp.cs +++ b/osu.Game/Rulesets/Mods/ModTimeRamp.cs @@ -44,21 +44,20 @@ namespace osu.Game.Rulesets.Mods Precision = 0.01, }; - private IAdjustableAudioComponent? track; + private readonly RateAdjustModHelper rateAdjustHelper; protected ModTimeRamp() { + rateAdjustHelper = new RateAdjustModHelper(SpeedChange, AdjustPitch); + // for preview purpose at song select. eventually we'll want to be able to update every frame. FinalRate.BindValueChanged(_ => applyRateAdjustment(double.PositiveInfinity), true); - AdjustPitch.BindValueChanged(applyPitchAdjustment); } public void ApplyToTrack(IAdjustableAudioComponent track) { - this.track = track; - + rateAdjustHelper.ApplyToTrack(track); FinalRate.TriggerChange(); - AdjustPitch.TriggerChange(); } public void ApplyToSample(IAdjustableAudioComponent sample) @@ -95,16 +94,5 @@ namespace osu.Game.Rulesets.Mods /// Adjust the rate along the specified ramp. /// private void applyRateAdjustment(double time) => SpeedChange.Value = ApplyToRate(time); - - private void applyPitchAdjustment(ValueChangedEvent adjustPitchSetting) - { - // remove existing old adjustment - track?.RemoveAdjustment(adjustmentForPitchSetting(adjustPitchSetting.OldValue), SpeedChange); - - track?.AddAdjustment(adjustmentForPitchSetting(adjustPitchSetting.NewValue), SpeedChange); - } - - private AdjustableProperty adjustmentForPitchSetting(bool adjustPitchSettingValue) - => adjustPitchSettingValue ? AdjustableProperty.Frequency : AdjustableProperty.Tempo; } } diff --git a/osu.Game/Rulesets/Mods/RateAdjustModHelper.cs b/osu.Game/Rulesets/Mods/RateAdjustModHelper.cs new file mode 100644 index 0000000000..a54be77338 --- /dev/null +++ b/osu.Game/Rulesets/Mods/RateAdjustModHelper.cs @@ -0,0 +1,36 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using osu.Framework.Audio; +using osu.Framework.Bindables; + +namespace osu.Game.Rulesets.Mods +{ + public class RateAdjustModHelper : IApplicableToTrack + { + private readonly BindableBool? adjustPitch; + private IAdjustableAudioComponent? track; + + public RateAdjustModHelper(BindableNumber speedChange, BindableBool? adjustPitch) + { + this.adjustPitch = adjustPitch; + + // When switching between pitch adjust, we need to update adjustments to time-shift or frequency-scale. + adjustPitch?.BindValueChanged(adjustPitchSetting => + { + track?.RemoveAdjustment(adjustmentForPitchSetting(adjustPitchSetting.OldValue), speedChange); + track?.AddAdjustment(adjustmentForPitchSetting(adjustPitchSetting.NewValue), speedChange); + + AdjustableProperty adjustmentForPitchSetting(bool adjustPitchSettingValue) + => adjustPitchSettingValue ? AdjustableProperty.Frequency : AdjustableProperty.Tempo; + }); + } + + public void ApplyToTrack(IAdjustableAudioComponent track) + { + this.track = track; + + adjustPitch?.TriggerChange(); + } + } +} From e56ff33271932eeda3776034562af8a91b05b178 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 18 Oct 2023 17:23:14 +0900 Subject: [PATCH 42/64] Also share `ScoreMultiplier` implementation --- osu.Game/Rulesets/Mods/ModDaycore.cs | 17 +++---------- osu.Game/Rulesets/Mods/ModDoubleTime.cs | 17 +------------ osu.Game/Rulesets/Mods/ModHalfTime.cs | 14 +---------- osu.Game/Rulesets/Mods/ModNightcore.cs | 18 ++++--------- osu.Game/Rulesets/Mods/RateAdjustModHelper.cs | 25 +++++++++++++++++-- 5 files changed, 34 insertions(+), 57 deletions(-) diff --git a/osu.Game/Rulesets/Mods/ModDaycore.cs b/osu.Game/Rulesets/Mods/ModDaycore.cs index 60ce610d7d..0b32788bf3 100644 --- a/osu.Game/Rulesets/Mods/ModDaycore.cs +++ b/osu.Game/Rulesets/Mods/ModDaycore.cs @@ -27,9 +27,12 @@ namespace osu.Game.Rulesets.Mods private readonly BindableNumber tempoAdjust = new BindableDouble(1); private readonly BindableNumber freqAdjust = new BindableDouble(1); + private readonly RateAdjustModHelper rateAdjustHelper; protected ModDaycore() { + rateAdjustHelper = new RateAdjustModHelper(SpeedChange); + SpeedChange.BindValueChanged(val => { freqAdjust.Value = SpeedChange.Default; @@ -43,18 +46,6 @@ namespace osu.Game.Rulesets.Mods track.AddAdjustment(AdjustableProperty.Tempo, tempoAdjust); } - public override double ScoreMultiplier - { - get - { - // Round to the nearest multiple of 0.1. - double value = (int)(SpeedChange.Value * 10) / 10.0; - - // Offset back to 0. - value -= 1; - - return 1 + value; - } - } + public override double ScoreMultiplier => rateAdjustHelper.ScoreMultiplier; } } diff --git a/osu.Game/Rulesets/Mods/ModDoubleTime.cs b/osu.Game/Rulesets/Mods/ModDoubleTime.cs index 938c6268e2..f20cdcb18d 100644 --- a/osu.Game/Rulesets/Mods/ModDoubleTime.cs +++ b/osu.Game/Rulesets/Mods/ModDoubleTime.cs @@ -42,21 +42,6 @@ namespace osu.Game.Rulesets.Mods rateAdjustHelper.ApplyToTrack(track); } - public override double ScoreMultiplier - { - get - { - // Round to the nearest multiple of 0.1. - double value = (int)(SpeedChange.Value * 10) / 10.0; - - // Offset back to 0. - value -= 1; - - // Each 0.1 multiple changes score multiplier by 0.02. - value /= 5; - - return 1 + value; - } - } + public override double ScoreMultiplier => rateAdjustHelper.ScoreMultiplier; } } diff --git a/osu.Game/Rulesets/Mods/ModHalfTime.cs b/osu.Game/Rulesets/Mods/ModHalfTime.cs index 965e94efcb..4d75ee16bd 100644 --- a/osu.Game/Rulesets/Mods/ModHalfTime.cs +++ b/osu.Game/Rulesets/Mods/ModHalfTime.cs @@ -42,18 +42,6 @@ namespace osu.Game.Rulesets.Mods rateAdjustHelper.ApplyToTrack(track); } - public override double ScoreMultiplier - { - get - { - // Round to the nearest multiple of 0.1. - double value = (int)(SpeedChange.Value * 10) / 10.0; - - // Offset back to 0. - value -= 1; - - return 1 + value; - } - } + public override double ScoreMultiplier => rateAdjustHelper.ScoreMultiplier; } } diff --git a/osu.Game/Rulesets/Mods/ModNightcore.cs b/osu.Game/Rulesets/Mods/ModNightcore.cs index 520ee6319a..933c9c9fc5 100644 --- a/osu.Game/Rulesets/Mods/ModNightcore.cs +++ b/osu.Game/Rulesets/Mods/ModNightcore.cs @@ -39,8 +39,12 @@ namespace osu.Game.Rulesets.Mods private readonly BindableNumber tempoAdjust = new BindableDouble(1); private readonly BindableNumber freqAdjust = new BindableDouble(1); + private readonly RateAdjustModHelper rateAdjustHelper; + protected ModNightcore() { + rateAdjustHelper = new RateAdjustModHelper(SpeedChange); + SpeedChange.BindValueChanged(val => { freqAdjust.Value = SpeedChange.Default; @@ -54,19 +58,7 @@ namespace osu.Game.Rulesets.Mods track.AddAdjustment(AdjustableProperty.Tempo, tempoAdjust); } - public override double ScoreMultiplier - { - get - { - // Round to the nearest multiple of 0.1. - double value = (int)(SpeedChange.Value * 10) / 10.0; - - // Offset back to 0. - value -= 1; - - return 1 + value; - } - } + public override double ScoreMultiplier => rateAdjustHelper.ScoreMultiplier; } public abstract partial class ModNightcore : ModNightcore, IApplicableToDrawableRuleset diff --git a/osu.Game/Rulesets/Mods/RateAdjustModHelper.cs b/osu.Game/Rulesets/Mods/RateAdjustModHelper.cs index a54be77338..034c20fc78 100644 --- a/osu.Game/Rulesets/Mods/RateAdjustModHelper.cs +++ b/osu.Game/Rulesets/Mods/RateAdjustModHelper.cs @@ -6,13 +6,18 @@ using osu.Framework.Bindables; namespace osu.Game.Rulesets.Mods { + /// + /// Provides common functionality shared across various rate adjust mods. + /// public class RateAdjustModHelper : IApplicableToTrack { private readonly BindableBool? adjustPitch; + private readonly BindableNumber speedChange; private IAdjustableAudioComponent? track; - public RateAdjustModHelper(BindableNumber speedChange, BindableBool? adjustPitch) + public RateAdjustModHelper(BindableNumber speedChange, BindableBool? adjustPitch = null) { + this.speedChange = speedChange; this.adjustPitch = adjustPitch; // When switching between pitch adjust, we need to update adjustments to time-shift or frequency-scale. @@ -29,8 +34,24 @@ namespace osu.Game.Rulesets.Mods public void ApplyToTrack(IAdjustableAudioComponent track) { this.track = track; - adjustPitch?.TriggerChange(); } + + public double ScoreMultiplier + { + get + { + // Round to the nearest multiple of 0.1. + double value = (int)(speedChange.Value * 10) / 10.0; + + // Offset back to 0. + value -= 1; + + if (speedChange.Value >= 1) + value /= 5; + + return 1 + value; + } + } } } From 161890292fc4c18c1f8dc2cefa8233fcdf0df2dc Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 18 Oct 2023 17:38:11 +0900 Subject: [PATCH 43/64] Move audio adjustment hookup to own method for clarity --- osu.Game/Rulesets/Mods/ModAdaptiveSpeed.cs | 3 +- osu.Game/Rulesets/Mods/ModDoubleTime.cs | 3 +- osu.Game/Rulesets/Mods/ModHalfTime.cs | 3 +- osu.Game/Rulesets/Mods/ModTimeRamp.cs | 3 +- osu.Game/Rulesets/Mods/RateAdjustModHelper.cs | 35 ++++++++++++++++--- 5 files changed, 39 insertions(+), 8 deletions(-) diff --git a/osu.Game/Rulesets/Mods/ModAdaptiveSpeed.cs b/osu.Game/Rulesets/Mods/ModAdaptiveSpeed.cs index f122baed90..607e6b8399 100644 --- a/osu.Game/Rulesets/Mods/ModAdaptiveSpeed.cs +++ b/osu.Game/Rulesets/Mods/ModAdaptiveSpeed.cs @@ -126,7 +126,8 @@ namespace osu.Game.Rulesets.Mods public ModAdaptiveSpeed() { - rateAdjustHelper = new RateAdjustModHelper(SpeedChange, AdjustPitch); + rateAdjustHelper = new RateAdjustModHelper(SpeedChange); + rateAdjustHelper.HandleAudioAdjustments(AdjustPitch); InitialRate.BindValueChanged(val => { diff --git a/osu.Game/Rulesets/Mods/ModDoubleTime.cs b/osu.Game/Rulesets/Mods/ModDoubleTime.cs index f20cdcb18d..789291772d 100644 --- a/osu.Game/Rulesets/Mods/ModDoubleTime.cs +++ b/osu.Game/Rulesets/Mods/ModDoubleTime.cs @@ -34,7 +34,8 @@ namespace osu.Game.Rulesets.Mods protected ModDoubleTime() { - rateAdjustHelper = new RateAdjustModHelper(SpeedChange, AdjustPitch); + rateAdjustHelper = new RateAdjustModHelper(SpeedChange); + rateAdjustHelper.HandleAudioAdjustments(AdjustPitch); } public override void ApplyToTrack(IAdjustableAudioComponent track) diff --git a/osu.Game/Rulesets/Mods/ModHalfTime.cs b/osu.Game/Rulesets/Mods/ModHalfTime.cs index 4d75ee16bd..8b5dd39584 100644 --- a/osu.Game/Rulesets/Mods/ModHalfTime.cs +++ b/osu.Game/Rulesets/Mods/ModHalfTime.cs @@ -34,7 +34,8 @@ namespace osu.Game.Rulesets.Mods protected ModHalfTime() { - rateAdjustHelper = new RateAdjustModHelper(SpeedChange, AdjustPitch); + rateAdjustHelper = new RateAdjustModHelper(SpeedChange); + rateAdjustHelper.HandleAudioAdjustments(AdjustPitch); } public override void ApplyToTrack(IAdjustableAudioComponent track) diff --git a/osu.Game/Rulesets/Mods/ModTimeRamp.cs b/osu.Game/Rulesets/Mods/ModTimeRamp.cs index 1048caa698..d2772417db 100644 --- a/osu.Game/Rulesets/Mods/ModTimeRamp.cs +++ b/osu.Game/Rulesets/Mods/ModTimeRamp.cs @@ -48,7 +48,8 @@ namespace osu.Game.Rulesets.Mods protected ModTimeRamp() { - rateAdjustHelper = new RateAdjustModHelper(SpeedChange, AdjustPitch); + rateAdjustHelper = new RateAdjustModHelper(SpeedChange); + rateAdjustHelper.HandleAudioAdjustments(AdjustPitch); // for preview purpose at song select. eventually we'll want to be able to update every frame. FinalRate.BindValueChanged(_ => applyRateAdjustment(double.PositiveInfinity), true); diff --git a/osu.Game/Rulesets/Mods/RateAdjustModHelper.cs b/osu.Game/Rulesets/Mods/RateAdjustModHelper.cs index 034c20fc78..701bad06bc 100644 --- a/osu.Game/Rulesets/Mods/RateAdjustModHelper.cs +++ b/osu.Game/Rulesets/Mods/RateAdjustModHelper.cs @@ -1,6 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System; using osu.Framework.Audio; using osu.Framework.Bindables; @@ -11,17 +12,32 @@ namespace osu.Game.Rulesets.Mods /// public class RateAdjustModHelper : IApplicableToTrack { - private readonly BindableBool? adjustPitch; private readonly BindableNumber speedChange; + private IAdjustableAudioComponent? track; - public RateAdjustModHelper(BindableNumber speedChange, BindableBool? adjustPitch = null) + private BindableBool? adjustPitch; + + /// + /// Construct a new . + /// + /// The main speed adjust parameter which is exposed to the user. + public RateAdjustModHelper(BindableNumber speedChange) { this.speedChange = speedChange; + } + + /// + /// Setup audio track adjustments for a rate adjust mod. + /// Importantly, must be called when a track is obtained/changed for this to work. + /// + /// The "adjust pitch" setting as exposed to the user. + public void HandleAudioAdjustments(BindableBool adjustPitch) + { this.adjustPitch = adjustPitch; // When switching between pitch adjust, we need to update adjustments to time-shift or frequency-scale. - adjustPitch?.BindValueChanged(adjustPitchSetting => + adjustPitch.BindValueChanged(adjustPitchSetting => { track?.RemoveAdjustment(adjustmentForPitchSetting(adjustPitchSetting.OldValue), speedChange); track?.AddAdjustment(adjustmentForPitchSetting(adjustPitchSetting.NewValue), speedChange); @@ -31,12 +47,23 @@ namespace osu.Game.Rulesets.Mods }); } + /// + /// Should be invoked when a track is obtained / changed. + /// + /// The new track. + /// If this method is called before . public void ApplyToTrack(IAdjustableAudioComponent track) { + if (adjustPitch == null) + throw new InvalidOperationException($"Must call {nameof(HandleAudioAdjustments)} first"); + this.track = track; - adjustPitch?.TriggerChange(); + adjustPitch.TriggerChange(); } + /// + /// The score multiplier for the current . + /// public double ScoreMultiplier { get From 9907adc337dd7d4d14c2b0b0b8144695fcc475a2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 18 Oct 2023 17:39:11 +0900 Subject: [PATCH 44/64] Take in `IBindable`s and tidy up multiplier handling --- osu.Game/Rulesets/Mods/RateAdjustModHelper.cs | 50 +++++++++---------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/osu.Game/Rulesets/Mods/RateAdjustModHelper.cs b/osu.Game/Rulesets/Mods/RateAdjustModHelper.cs index 701bad06bc..ffd4de0e90 100644 --- a/osu.Game/Rulesets/Mods/RateAdjustModHelper.cs +++ b/osu.Game/Rulesets/Mods/RateAdjustModHelper.cs @@ -12,19 +12,39 @@ namespace osu.Game.Rulesets.Mods /// public class RateAdjustModHelper : IApplicableToTrack { - private readonly BindableNumber speedChange; + public readonly IBindableNumber SpeedChange; private IAdjustableAudioComponent? track; private BindableBool? adjustPitch; + /// + /// The score multiplier for the current . + /// + public double ScoreMultiplier + { + get + { + // Round to the nearest multiple of 0.1. + double value = (int)(SpeedChange.Value * 10) / 10.0; + + // Offset back to 0. + value -= 1; + + if (SpeedChange.Value >= 1) + value /= 5; + + return 1 + value; + } + } + /// /// Construct a new . /// /// The main speed adjust parameter which is exposed to the user. - public RateAdjustModHelper(BindableNumber speedChange) + public RateAdjustModHelper(IBindableNumber speedChange) { - this.speedChange = speedChange; + SpeedChange = speedChange; } /// @@ -39,8 +59,8 @@ namespace osu.Game.Rulesets.Mods // When switching between pitch adjust, we need to update adjustments to time-shift or frequency-scale. adjustPitch.BindValueChanged(adjustPitchSetting => { - track?.RemoveAdjustment(adjustmentForPitchSetting(adjustPitchSetting.OldValue), speedChange); - track?.AddAdjustment(adjustmentForPitchSetting(adjustPitchSetting.NewValue), speedChange); + track?.RemoveAdjustment(adjustmentForPitchSetting(adjustPitchSetting.OldValue), SpeedChange); + track?.AddAdjustment(adjustmentForPitchSetting(adjustPitchSetting.NewValue), SpeedChange); AdjustableProperty adjustmentForPitchSetting(bool adjustPitchSettingValue) => adjustPitchSettingValue ? AdjustableProperty.Frequency : AdjustableProperty.Tempo; @@ -60,25 +80,5 @@ namespace osu.Game.Rulesets.Mods this.track = track; adjustPitch.TriggerChange(); } - - /// - /// The score multiplier for the current . - /// - public double ScoreMultiplier - { - get - { - // Round to the nearest multiple of 0.1. - double value = (int)(speedChange.Value * 10) / 10.0; - - // Offset back to 0. - value -= 1; - - if (speedChange.Value >= 1) - value /= 5; - - return 1 + value; - } - } } } From be321e34e90ee47a2ba0076587c965ef72e0d24f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 18 Oct 2023 17:46:14 +0900 Subject: [PATCH 45/64] Attempt to fix flaky tests with egregious retry rule --- osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs index 7f81ccddf6..ce78119861 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs @@ -51,7 +51,8 @@ namespace osu.Game.Rulesets.Osu.Tests [TestCase(80, 1)] [TestCase(80, 0)] [TestCase(80, 10)] - // [TestCase(90, 1)] flaky + [TestCase(90, 1)] + [Retry(100)] // 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; @@ -87,11 +88,12 @@ namespace osu.Game.Rulesets.Osu.Tests [TestCase(300, false)] [TestCase(200, true)] [TestCase(150, true)] - // [TestCase(120, true)] flaky - // [TestCase(60, true)] flaky + [TestCase(120, true)] + [TestCase(60, true)] [TestCase(10, true)] - // [TestCase(0, true)] headless test doesn't run at high enough precision for this to always enter a tracking state in time. + [TestCase(0, true)] [TestCase(-30, false)] + [Retry(100)] // 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; From cff69d63a62c532957eb269c6eb492b441501c41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 18 Oct 2023 12:27:16 +0200 Subject: [PATCH 46/64] Add inline commentary in `Mod{Day,Night}core` about different speed adjustments --- osu.Game/Rulesets/Mods/ModDaycore.cs | 2 ++ osu.Game/Rulesets/Mods/ModNightcore.cs | 2 ++ 2 files changed, 4 insertions(+) diff --git a/osu.Game/Rulesets/Mods/ModDaycore.cs b/osu.Game/Rulesets/Mods/ModDaycore.cs index 0b32788bf3..39ebd1fe4c 100644 --- a/osu.Game/Rulesets/Mods/ModDaycore.cs +++ b/osu.Game/Rulesets/Mods/ModDaycore.cs @@ -33,6 +33,8 @@ namespace osu.Game.Rulesets.Mods { rateAdjustHelper = new RateAdjustModHelper(SpeedChange); + // intentionally not deferring the speed change handling to `RateAdjustModHelper` + // as the expected result of operation is not the same (daycore should preserve constant pitch). SpeedChange.BindValueChanged(val => { freqAdjust.Value = SpeedChange.Default; diff --git a/osu.Game/Rulesets/Mods/ModNightcore.cs b/osu.Game/Rulesets/Mods/ModNightcore.cs index 933c9c9fc5..b519ab4db7 100644 --- a/osu.Game/Rulesets/Mods/ModNightcore.cs +++ b/osu.Game/Rulesets/Mods/ModNightcore.cs @@ -45,6 +45,8 @@ namespace osu.Game.Rulesets.Mods { rateAdjustHelper = new RateAdjustModHelper(SpeedChange); + // intentionally not deferring the speed change handling to `RateAdjustModHelper` + // as the expected result of operation is not the same (nightcore should preserve constant pitch). SpeedChange.BindValueChanged(val => { freqAdjust.Value = SpeedChange.Default; From 78d3c3723dec9b46f1b271c32c4b86267555bffa Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 18 Oct 2023 20:12:14 +0900 Subject: [PATCH 47/64] Ignore instead of retry tests --- osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs index ce78119861..247aa4f445 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneSliderInput.cs @@ -52,7 +52,7 @@ namespace osu.Game.Rulesets.Osu.Tests [TestCase(80, 0)] [TestCase(80, 10)] [TestCase(90, 1)] - [Retry(100)] // headless test doesn't run at high enough precision for this to always enter a tracking state in time. + [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; @@ -93,7 +93,7 @@ namespace osu.Game.Rulesets.Osu.Tests [TestCase(10, true)] [TestCase(0, true)] [TestCase(-30, false)] - [Retry(100)] // headless test doesn't run at high enough precision for this to always enter a tracking state in time. + [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; From 14cadd1eeb23fe59733b5237f46ce98eefbda33d Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 19 Oct 2023 02:39:34 +0900 Subject: [PATCH 48/64] Pin third-party actions --- .github/workflows/diffcalc.yml | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/diffcalc.yml b/.github/workflows/diffcalc.yml index c3d92b6d1f..ce8edc1d95 100644 --- a/.github/workflows/diffcalc.yml +++ b/.github/workflows/diffcalc.yml @@ -111,7 +111,7 @@ jobs: steps: - name: Check permissions if: ${{ github.event_name != 'workflow_dispatch' }} - uses: actions-cool/check-user-permission@v2 + uses: actions-cool/check-user-permission@a0668c9aec87f3875fc56170b6452a453e9dd819 # v2.2.0 with: require: 'write' @@ -122,7 +122,7 @@ jobs: if: ${{ github.event_name == 'issue_comment' && github.event.issue.pull_request }} steps: - name: Create comment - uses: thollander/actions-comment-pull-request@v2 + uses: thollander/actions-comment-pull-request@363c6f6eae92cc5c3a66e95ba016fc771bb38943 # v2.4.2 with: comment_tag: ${{ env.COMMENT_TAG }} message: | @@ -136,7 +136,7 @@ jobs: runs-on: self-hosted timeout-minutes: 50400 # 35 days, the maximum for jobs on self-hosted runners. steps: - - uses: ahmadnassri/action-workflow-queue@v1 + - uses: ahmadnassri/action-workflow-queue@f547ac848c16a9bb1b2ed4a850e0cc5098af2cf8 # v1.1.5 with: timeout: 2147483647 # Around 24 days - the maximum supported by JS setTimeout(). delay: 120000 # Poll every 2 minutes - the API limit seems fairly low on this one. @@ -261,7 +261,7 @@ jobs: - name: Restore cache id: restore-cache - uses: maxnowack/local-cache@v1 + uses: maxnowack/local-cache@038cc090b52e4f205fbc468bf5b0756df6f68775 # v1 with: path: ${{ steps.query.outputs.DATA_NAME }}.tar.bz2 key: ${{ steps.query.outputs.DATA_NAME }} @@ -292,7 +292,7 @@ jobs: - name: Restore cache id: restore-cache - uses: maxnowack/local-cache@v1 + uses: maxnowack/local-cache@038cc090b52e4f205fbc468bf5b0756df6f68775 # v1 with: path: ${{ steps.query.outputs.DATA_NAME }}.tar.bz2 key: ${{ steps.query.outputs.DATA_NAME }} @@ -356,7 +356,7 @@ jobs: steps: - name: Update comment on success if: ${{ needs.generator.result == 'success' }} - uses: thollander/actions-comment-pull-request@v2 + uses: thollander/actions-comment-pull-request@363c6f6eae92cc5c3a66e95ba016fc771bb38943 # v2.4.2 with: comment_tag: ${{ env.COMMENT_TAG }} mode: upsert @@ -367,7 +367,7 @@ jobs: - name: Update comment on failure if: ${{ needs.generator.result == 'failure' }} - uses: thollander/actions-comment-pull-request@v2 + uses: thollander/actions-comment-pull-request@363c6f6eae92cc5c3a66e95ba016fc771bb38943 # v2.4.2 with: comment_tag: ${{ env.COMMENT_TAG }} mode: upsert @@ -377,7 +377,7 @@ jobs: - name: Update comment on cancellation if: ${{ needs.generator.result == 'cancelled' }} - uses: thollander/actions-comment-pull-request@v2 + uses: thollander/actions-comment-pull-request@363c6f6eae92cc5c3a66e95ba016fc771bb38943 # v2.4.2 with: comment_tag: ${{ env.COMMENT_TAG }} mode: delete From 832ba861bb1054aa76e056f57fceb22bc98c6dfe Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Thu, 19 Oct 2023 15:19:51 +0900 Subject: [PATCH 49/64] Fix incorrect PR url in diffcalc workflow --- .github/workflows/diffcalc.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/diffcalc.yml b/.github/workflows/diffcalc.yml index ce8edc1d95..dbb4067676 100644 --- a/.github/workflows/diffcalc.yml +++ b/.github/workflows/diffcalc.yml @@ -195,7 +195,7 @@ jobs: - name: Add pull-request environment if: ${{ github.event_name == 'issue_comment' && github.event.issue.pull_request }} run: | - sed -i "s;^OSU_B=.*$;OSU_B=${{ github.event.issue.pull_request.url }};" "${{ needs.directory.outputs.GENERATOR_ENV }}" + sed -i "s;^OSU_B=.*$;OSU_B=${{ github.event.issue.pull_request.html_url }};" "${{ needs.directory.outputs.GENERATOR_ENV }}" - name: Add comment environment if: ${{ github.event_name == 'issue_comment' }} From 3ab083b696906a03a6c6cd575ab81b8fe5541c39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 19 Oct 2023 09:56:40 +0200 Subject: [PATCH 50/64] Split `IPositionSnapProvider` from `IDistanceSnapProvider` In preparation to remove `DistancedHitObjectComposer`, split off `IPositionSnapProvider` from `IDistanceSnapProvider`. `DistancedHitObjectComposer` was not touching `IPositionSnapProvider`'s only interface member at all, it was just forwarding it for subclasses to override to their own leisure. --- .../Components/PathControlPointVisualiser.cs | 13 ++++++++----- .../Blueprints/Sliders/SliderPlacementBlueprint.cs | 9 ++++++--- .../Blueprints/Sliders/SliderSelectionBlueprint.cs | 11 +++++++---- osu.Game/Rulesets/Edit/IDistanceSnapProvider.cs | 2 +- 4 files changed, 22 insertions(+), 13 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs index f624ebc8b5..f891d23bbd 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/Components/PathControlPointVisualiser.cs @@ -47,7 +47,10 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components public Action> SplitControlPointsRequested; [Resolved(CanBeNull = true)] - private IDistanceSnapProvider snapProvider { get; set; } + private IPositionSnapProvider positionSnapProvider { get; set; } + + [Resolved(CanBeNull = true)] + private IDistanceSnapProvider distanceSnapProvider { get; set; } public PathControlPointVisualiser(T hitObject, bool allowSelection) { @@ -289,7 +292,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components { // Special handling for selections containing head control point - the position of the hit object changes which means the snapped position and time have to be taken into account Vector2 newHeadPosition = Parent!.ToScreenSpace(e.MousePosition + (dragStartPositions[0] - dragStartPositions[draggedControlPointIndex])); - var result = snapProvider?.FindSnappedPositionAndTime(newHeadPosition); + var result = positionSnapProvider?.FindSnappedPositionAndTime(newHeadPosition); Vector2 movementDelta = Parent!.ToLocalSpace(result?.ScreenSpacePosition ?? newHeadPosition) - hitObject.Position; @@ -309,7 +312,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components } else { - var result = snapProvider?.FindSnappedPositionAndTime(Parent!.ToScreenSpace(e.MousePosition), SnapType.GlobalGrids); + var result = positionSnapProvider?.FindSnappedPositionAndTime(Parent!.ToScreenSpace(e.MousePosition), SnapType.GlobalGrids); Vector2 movementDelta = Parent!.ToLocalSpace(result?.ScreenSpacePosition ?? Parent!.ToScreenSpace(e.MousePosition)) - dragStartPositions[draggedControlPointIndex] - hitObject.Position; @@ -322,7 +325,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components } // Snap the path to the current beat divisor before checking length validity. - hitObject.SnapTo(snapProvider); + hitObject.SnapTo(distanceSnapProvider); if (!hitObject.Path.HasValidLength) { @@ -332,7 +335,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders.Components hitObject.Position = oldPosition; hitObject.StartTime = oldStartTime; // Snap the path length again to undo the invalid length. - hitObject.SnapTo(snapProvider); + hitObject.SnapTo(distanceSnapProvider); return; } diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderPlacementBlueprint.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderPlacementBlueprint.cs index ba7b855511..9b6adc04cf 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderPlacementBlueprint.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderPlacementBlueprint.cs @@ -39,7 +39,10 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders private int currentSegmentLength; [Resolved(CanBeNull = true)] - private IDistanceSnapProvider snapProvider { get; set; } + private IPositionSnapProvider positionSnapProvider { get; set; } + + [Resolved(CanBeNull = true)] + private IDistanceSnapProvider distanceSnapProvider { get; set; } protected override bool IsValidForPlacement => HitObject.Path.HasValidLength; @@ -198,7 +201,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders } // Update the cursor position. - var result = snapProvider?.FindSnappedPositionAndTime(inputManager.CurrentState.Mouse.Position, state == SliderPlacementState.Body ? SnapType.GlobalGrids : SnapType.All); + var result = positionSnapProvider?.FindSnappedPositionAndTime(inputManager.CurrentState.Mouse.Position, state == SliderPlacementState.Body ? SnapType.GlobalGrids : SnapType.All); cursor.Position = ToLocalSpace(result?.ScreenSpacePosition ?? inputManager.CurrentState.Mouse.Position) - HitObject.Position; } else if (cursor != null) @@ -230,7 +233,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders private void updateSlider() { - HitObject.Path.ExpectedDistance.Value = snapProvider?.FindSnappedDistance(HitObject, (float)HitObject.Path.CalculatedDistance) ?? (float)HitObject.Path.CalculatedDistance; + HitObject.Path.ExpectedDistance.Value = distanceSnapProvider?.FindSnappedDistance(HitObject, (float)HitObject.Path.CalculatedDistance) ?? (float)HitObject.Path.CalculatedDistance; bodyPiece.UpdateFrom(HitObject); headCirclePiece.UpdateFrom(HitObject.HeadCircle); diff --git a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs index 02023decd6..80c4cee7f2 100644 --- a/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs +++ b/osu.Game.Rulesets.Osu/Edit/Blueprints/Sliders/SliderSelectionBlueprint.cs @@ -40,7 +40,10 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders protected PathControlPointVisualiser ControlPointVisualiser { get; private set; } [Resolved(CanBeNull = true)] - private IDistanceSnapProvider snapProvider { get; set; } + private IPositionSnapProvider positionSnapProvider { get; set; } + + [Resolved(CanBeNull = true)] + private IDistanceSnapProvider distanceSnapProvider { get; set; } [Resolved(CanBeNull = true)] private IPlacementHandler placementHandler { get; set; } @@ -194,7 +197,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders { if (placementControlPoint != null) { - var result = snapProvider?.FindSnappedPositionAndTime(ToScreenSpace(e.MousePosition)); + var result = positionSnapProvider?.FindSnappedPositionAndTime(ToScreenSpace(e.MousePosition)); placementControlPoint.Position = ToLocalSpace(result?.ScreenSpacePosition ?? ToScreenSpace(e.MousePosition)) - HitObject.Position; } } @@ -245,7 +248,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders // Move the control points from the insertion index onwards to make room for the insertion controlPoints.Insert(insertionIndex, pathControlPoint); - HitObject.SnapTo(snapProvider); + HitObject.SnapTo(distanceSnapProvider); return pathControlPoint; } @@ -267,7 +270,7 @@ namespace osu.Game.Rulesets.Osu.Edit.Blueprints.Sliders } // Snap the slider to the current beat divisor before checking length validity. - HitObject.SnapTo(snapProvider); + HitObject.SnapTo(distanceSnapProvider); // If there are 0 or 1 remaining control points, or the slider has an invalid length, it is in a degenerate form and should be deleted if (controlPoints.Count <= 1 || !HitObject.Path.HasValidLength) diff --git a/osu.Game/Rulesets/Edit/IDistanceSnapProvider.cs b/osu.Game/Rulesets/Edit/IDistanceSnapProvider.cs index da44b42831..af9d02f5c4 100644 --- a/osu.Game/Rulesets/Edit/IDistanceSnapProvider.cs +++ b/osu.Game/Rulesets/Edit/IDistanceSnapProvider.cs @@ -12,7 +12,7 @@ namespace osu.Game.Rulesets.Edit /// A snap provider which given a reference hit object and proposed distance from it, offers a more correct duration or distance value. /// [Cached] - public interface IDistanceSnapProvider : IPositionSnapProvider + public interface IDistanceSnapProvider { /// /// A multiplier which changes the ratio of distance travelled per time unit. From dcfd6a0a8a9ad298fe63ea988dbe6f70dae3cfad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 19 Oct 2023 11:20:10 +0200 Subject: [PATCH 51/64] Remove `DistancedHitObjectComposer` inheritance from osu! composer --- .../Editor/TestSceneOsuDistanceSnapGrid.cs | 28 +- .../Edit/OsuDistanceSnapProvider.cs | 31 ++ .../Edit/OsuHitObjectComposer.cs | 44 +-- ...tSceneHitObjectComposerDistanceSnapping.cs | 13 +- .../Editing/TestSceneDistanceSnapGrid.cs | 4 +- .../Edit/ComposerDistanceSnapProvider.cs | 311 ++++++++++++++++++ .../Edit/DistancedHitObjectComposer.cs | 4 +- .../Rulesets/Edit/IDistanceSnapProvider.cs | 2 +- 8 files changed, 392 insertions(+), 45 deletions(-) create mode 100644 osu.Game.Rulesets.Osu/Edit/OsuDistanceSnapProvider.cs create mode 100644 osu.Game/Rulesets/Edit/ComposerDistanceSnapProvider.cs diff --git a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs index 9338d5453d..b70f932913 100644 --- a/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs +++ b/osu.Game.Rulesets.Osu.Tests/Editor/TestSceneOsuDistanceSnapGrid.cs @@ -47,8 +47,7 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor [Cached] private readonly BindableBeatDivisor beatDivisor = new BindableBeatDivisor(); - [Cached(typeof(IDistanceSnapProvider))] - private readonly OsuHitObjectComposer snapProvider = new OsuHitObjectComposer(new OsuRuleset()) + private readonly TestHitObjectComposer composer = new TestHitObjectComposer { // Just used for the snap implementation, so let's hide from vision. AlwaysPresent = true, @@ -71,11 +70,18 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor base.Content.Children = new Drawable[] { editorClock = new EditorClock(editorBeatmap), - new PopoverContainer { Child = snapProvider }, + new PopoverContainer { Child = composer }, Content }; } + protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnlyDependencyContainer parent) + { + var dependencies = new DependencyContainer(base.CreateChildDependencies(parent)); + dependencies.CacheAs(composer.DistanceSnapProvider); + return dependencies; + } + protected override Container Content { get; } = new PopoverContainer { RelativeSizeAxes = Axes.Both }; [SetUp] @@ -84,7 +90,7 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor editorBeatmap.Difficulty.SliderMultiplier = 1; editorBeatmap.ControlPointInfo.Clear(); editorBeatmap.ControlPointInfo.Add(0, new TimingControlPoint { BeatLength = beat_length }); - snapProvider.DistanceSpacingMultiplier.Value = 1; + composer.DistanceSnapProvider.DistanceSpacingMultiplier.Value = 1; Children = new Drawable[] { @@ -116,7 +122,7 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor [TestCase(0.5f)] public void TestDistanceSpacing(float multiplier) { - AddStep($"set distance spacing = {multiplier}", () => snapProvider.DistanceSpacingMultiplier.Value = multiplier); + AddStep($"set distance spacing = {multiplier}", () => composer.DistanceSnapProvider.DistanceSpacingMultiplier.Value = multiplier); } [Test] @@ -153,7 +159,7 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor [TestCase(2f, beat_length * 2)] public void TestDistanceSpacingAdjust(float multiplier, float expectedDistance) { - AddStep($"Set distance spacing to {multiplier}", () => snapProvider.DistanceSpacingMultiplier.Value = multiplier); + AddStep($"Set distance spacing to {multiplier}", () => composer.DistanceSnapProvider.DistanceSpacingMultiplier.Value = multiplier); AddStep("move mouse to point", () => InputManager.MoveMouseTo(grid.ToScreenSpace(grid_position + new Vector2(beat_length, 0) * 2))); assertSnappedDistance(expectedDistance); @@ -266,5 +272,15 @@ namespace osu.Game.Rulesets.Osu.Tests.Editor cursor.Position = LastSnappedPosition = GetSnapPosition.Invoke(inputManager.CurrentState.Mouse.Position); } } + + private partial class TestHitObjectComposer : OsuHitObjectComposer + { + public new IDistanceSnapProvider DistanceSnapProvider => base.DistanceSnapProvider; + + public TestHitObjectComposer() + : base(new OsuRuleset()) + { + } + } } } diff --git a/osu.Game.Rulesets.Osu/Edit/OsuDistanceSnapProvider.cs b/osu.Game.Rulesets.Osu/Edit/OsuDistanceSnapProvider.cs new file mode 100644 index 0000000000..522943df7d --- /dev/null +++ b/osu.Game.Rulesets.Osu/Edit/OsuDistanceSnapProvider.cs @@ -0,0 +1,31 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using osu.Game.Graphics.UserInterface; +using osu.Game.Input.Bindings; +using osu.Game.Rulesets.Edit; +using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Osu.Objects; +using osuTK; + +namespace osu.Game.Rulesets.Osu.Edit +{ + public partial class OsuDistanceSnapProvider : ComposerDistanceSnapProvider + { + protected override double ReadCurrentDistanceSnap(HitObject before, HitObject after) + { + float expectedDistance = DurationToDistance(before, after.StartTime - before.GetEndTime()); + float actualDistance = Vector2.Distance(((OsuHitObject)before).EndPosition, ((OsuHitObject)after).Position); + + return actualDistance / expectedDistance; + } + + protected override bool AdjustDistanceSpacing(GlobalAction action, float amount) + { + // To allow better visualisation, ensure that the spacing grid is visible before adjusting. + DistanceSnapToggle.Value = TernaryState.True; + + return base.AdjustDistanceSpacing(action, amount); + } + } +} diff --git a/osu.Game.Rulesets.Osu/Edit/OsuHitObjectComposer.cs b/osu.Game.Rulesets.Osu/Edit/OsuHitObjectComposer.cs index fdc11be42c..0f8c960b65 100644 --- a/osu.Game.Rulesets.Osu/Edit/OsuHitObjectComposer.cs +++ b/osu.Game.Rulesets.Osu/Edit/OsuHitObjectComposer.cs @@ -17,7 +17,6 @@ using osu.Framework.Input.Events; using osu.Framework.Utils; using osu.Game.Beatmaps; using osu.Game.Graphics.UserInterface; -using osu.Game.Input.Bindings; using osu.Game.Rulesets.Edit; using osu.Game.Rulesets.Edit.Tools; using osu.Game.Rulesets.Mods; @@ -30,7 +29,7 @@ using osuTK; namespace osu.Game.Rulesets.Osu.Edit { - public partial class OsuHitObjectComposer : DistancedHitObjectComposer + public partial class OsuHitObjectComposer : HitObjectComposer { public OsuHitObjectComposer(Ruleset ruleset) : base(ruleset) @@ -49,18 +48,27 @@ namespace osu.Game.Rulesets.Osu.Edit private readonly Bindable rectangularGridSnapToggle = new Bindable(); - protected override IEnumerable CreateTernaryButtons() => base.CreateTernaryButtons().Concat(new[] - { - new TernaryButton(rectangularGridSnapToggle, "Grid Snap", () => new SpriteIcon { Icon = FontAwesome.Solid.Th }) - }); + protected override IEnumerable CreateTernaryButtons() + => base.CreateTernaryButtons() + .Concat(DistanceSnapProvider.CreateTernaryButtons()) + .Concat(new[] + { + new TernaryButton(rectangularGridSnapToggle, "Grid Snap", () => new SpriteIcon { Icon = FontAwesome.Solid.Th }) + }); private BindableList selectedHitObjects; private Bindable placementObject; + [Cached(typeof(IDistanceSnapProvider))] + protected readonly OsuDistanceSnapProvider DistanceSnapProvider = new OsuDistanceSnapProvider(); + [BackgroundDependencyLoader] private void load() { + AddInternal(DistanceSnapProvider); + DistanceSnapProvider.AttachToToolbox(RightToolbox); + // Give a bit of breathing room around the playfield content. PlayfieldContentContainer.Padding = new MarginPadding(10); @@ -81,7 +89,7 @@ namespace osu.Game.Rulesets.Osu.Edit placementObject = EditorBeatmap.PlacementObject.GetBoundCopy(); placementObject.ValueChanged += _ => updateDistanceSnapGrid(); - DistanceSnapToggle.ValueChanged += _ => updateDistanceSnapGrid(); + DistanceSnapProvider.DistanceSnapToggle.ValueChanged += _ => updateDistanceSnapGrid(); // we may be entering the screen with a selection already active updateDistanceSnapGrid(); @@ -106,14 +114,6 @@ namespace osu.Game.Rulesets.Osu.Edit private RectangularPositionSnapGrid rectangularPositionSnapGrid; - protected override double ReadCurrentDistanceSnap(HitObject before, HitObject after) - { - float expectedDistance = DurationToDistance(before, after.StartTime - before.GetEndTime()); - float actualDistance = Vector2.Distance(((OsuHitObject)before).EndPosition, ((OsuHitObject)after).Position); - - return actualDistance / expectedDistance; - } - protected override void Update() { base.Update(); @@ -143,7 +143,7 @@ namespace osu.Game.Rulesets.Osu.Edit // We want to ensure that in this particular case, the time-snapping component of distance snap is still applied. // The easiest way to ensure this is to attempt application of distance snap after a nearby object is found, and copy over // the time value if the proposed positions are roughly the same. - if (snapType.HasFlagFast(SnapType.RelativeGrids) && DistanceSnapToggle.Value == TernaryState.True && distanceSnapGrid != null) + if (snapType.HasFlagFast(SnapType.RelativeGrids) && DistanceSnapProvider.DistanceSnapToggle.Value == TernaryState.True && distanceSnapGrid != null) { (Vector2 distanceSnappedPosition, double distanceSnappedTime) = distanceSnapGrid.GetSnappedPosition(distanceSnapGrid.ToLocalSpace(snapResult.ScreenSpacePosition)); if (Precision.AlmostEquals(distanceSnapGrid.ToScreenSpace(distanceSnappedPosition), snapResult.ScreenSpacePosition, 1)) @@ -157,7 +157,7 @@ namespace osu.Game.Rulesets.Osu.Edit if (snapType.HasFlagFast(SnapType.RelativeGrids)) { - if (DistanceSnapToggle.Value == TernaryState.True && distanceSnapGrid != null) + if (DistanceSnapProvider.DistanceSnapToggle.Value == TernaryState.True && distanceSnapGrid != null) { (Vector2 pos, double time) = distanceSnapGrid.GetSnappedPosition(distanceSnapGrid.ToLocalSpace(screenSpacePosition)); @@ -220,7 +220,7 @@ namespace osu.Game.Rulesets.Osu.Edit distanceSnapGridCache.Invalidate(); distanceSnapGrid = null; - if (DistanceSnapToggle.Value != TernaryState.True) + if (DistanceSnapProvider.DistanceSnapToggle.Value != TernaryState.True) return; switch (BlueprintContainer.CurrentTool) @@ -262,14 +262,6 @@ namespace osu.Game.Rulesets.Osu.Edit base.OnKeyUp(e); } - protected override bool AdjustDistanceSpacing(GlobalAction action, float amount) - { - // To allow better visualisation, ensure that the spacing grid is visible before adjusting. - DistanceSnapToggle.Value = TernaryState.True; - - return base.AdjustDistanceSpacing(action, amount); - } - private bool gridSnapMomentary; private void handleToggleViaKey(KeyboardEvent key) diff --git a/osu.Game.Tests/Editing/TestSceneHitObjectComposerDistanceSnapping.cs b/osu.Game.Tests/Editing/TestSceneHitObjectComposerDistanceSnapping.cs index 6b78eea009..463287fb35 100644 --- a/osu.Game.Tests/Editing/TestSceneHitObjectComposerDistanceSnapping.cs +++ b/osu.Game.Tests/Editing/TestSceneHitObjectComposerDistanceSnapping.cs @@ -3,7 +3,6 @@ using NUnit.Framework; using osu.Framework.Allocation; -using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Cursor; @@ -230,25 +229,25 @@ namespace osu.Game.Tests.Editing } private void assertSnapDistance(float expectedDistance, HitObject? referenceObject, bool includeSliderVelocity) - => AddAssert($"distance is {expectedDistance}", () => composer.GetBeatSnapDistanceAt(referenceObject ?? new HitObject(), includeSliderVelocity), () => Is.EqualTo(expectedDistance).Within(Precision.FLOAT_EPSILON)); + => AddAssert($"distance is {expectedDistance}", () => composer.DistanceSnapProvider.GetBeatSnapDistanceAt(referenceObject ?? new HitObject(), includeSliderVelocity), () => Is.EqualTo(expectedDistance).Within(Precision.FLOAT_EPSILON)); private void assertDurationToDistance(double duration, float expectedDistance, HitObject? referenceObject = null) - => AddAssert($"duration = {duration} -> distance = {expectedDistance}", () => composer.DurationToDistance(referenceObject ?? new HitObject(), duration), () => Is.EqualTo(expectedDistance).Within(Precision.FLOAT_EPSILON)); + => AddAssert($"duration = {duration} -> distance = {expectedDistance}", () => composer.DistanceSnapProvider.DurationToDistance(referenceObject ?? new HitObject(), duration), () => Is.EqualTo(expectedDistance).Within(Precision.FLOAT_EPSILON)); private void assertDistanceToDuration(float distance, double expectedDuration, HitObject? referenceObject = null) - => AddAssert($"distance = {distance} -> duration = {expectedDuration}", () => composer.DistanceToDuration(referenceObject ?? new HitObject(), distance), () => Is.EqualTo(expectedDuration).Within(Precision.FLOAT_EPSILON)); + => AddAssert($"distance = {distance} -> duration = {expectedDuration}", () => composer.DistanceSnapProvider.DistanceToDuration(referenceObject ?? new HitObject(), distance), () => Is.EqualTo(expectedDuration).Within(Precision.FLOAT_EPSILON)); private void assertSnappedDuration(float distance, double expectedDuration, HitObject? referenceObject = null) - => AddAssert($"distance = {distance} -> duration = {expectedDuration} (snapped)", () => composer.FindSnappedDuration(referenceObject ?? new HitObject(), distance), () => Is.EqualTo(expectedDuration).Within(Precision.FLOAT_EPSILON)); + => AddAssert($"distance = {distance} -> duration = {expectedDuration} (snapped)", () => composer.DistanceSnapProvider.FindSnappedDuration(referenceObject ?? new HitObject(), distance), () => Is.EqualTo(expectedDuration).Within(Precision.FLOAT_EPSILON)); private void assertSnappedDistance(float distance, float expectedDistance, HitObject? referenceObject = null) - => AddAssert($"distance = {distance} -> distance = {expectedDistance} (snapped)", () => composer.FindSnappedDistance(referenceObject ?? new HitObject(), distance), () => Is.EqualTo(expectedDistance).Within(Precision.FLOAT_EPSILON)); + => AddAssert($"distance = {distance} -> distance = {expectedDistance} (snapped)", () => composer.DistanceSnapProvider.FindSnappedDistance(referenceObject ?? new HitObject(), distance), () => Is.EqualTo(expectedDistance).Within(Precision.FLOAT_EPSILON)); private partial class TestHitObjectComposer : OsuHitObjectComposer { public new EditorBeatmap EditorBeatmap => base.EditorBeatmap; - public new Bindable DistanceSpacingMultiplier => base.DistanceSpacingMultiplier; + public new IDistanceSnapProvider DistanceSnapProvider => base.DistanceSnapProvider; public TestHitObjectComposer() : base(new OsuRuleset()) diff --git a/osu.Game.Tests/Visual/Editing/TestSceneDistanceSnapGrid.cs b/osu.Game.Tests/Visual/Editing/TestSceneDistanceSnapGrid.cs index 70e4420a45..f2a015402a 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneDistanceSnapGrid.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneDistanceSnapGrid.cs @@ -187,11 +187,9 @@ namespace osu.Game.Tests.Visual.Editing private class SnapProvider : IDistanceSnapProvider { - public SnapResult FindSnappedPositionAndTime(Vector2 screenSpacePosition, SnapType snapType = SnapType.AllGrids) => new SnapResult(screenSpacePosition, 0); - public Bindable DistanceSpacingMultiplier { get; } = new BindableDouble(1); - IBindable IDistanceSnapProvider.DistanceSpacingMultiplier => DistanceSpacingMultiplier; + Bindable IDistanceSnapProvider.DistanceSpacingMultiplier => DistanceSpacingMultiplier; public float GetBeatSnapDistanceAt(HitObject referenceObject, bool useReferenceSliderVelocity = true) => beat_snap_distance; diff --git a/osu.Game/Rulesets/Edit/ComposerDistanceSnapProvider.cs b/osu.Game/Rulesets/Edit/ComposerDistanceSnapProvider.cs new file mode 100644 index 0000000000..0b1809e7d9 --- /dev/null +++ b/osu.Game/Rulesets/Edit/ComposerDistanceSnapProvider.cs @@ -0,0 +1,311 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; +using osu.Framework.Allocation; +using osu.Framework.Bindables; +using osu.Framework.Extensions; +using osu.Framework.Extensions.LocalisationExtensions; +using osu.Framework.Graphics; +using osu.Framework.Graphics.Sprites; +using osu.Framework.Input.Bindings; +using osu.Framework.Input.Events; +using osu.Framework.Localisation; +using osu.Framework.Utils; +using osu.Game.Configuration; +using osu.Game.Graphics.UserInterface; +using osu.Game.Input.Bindings; +using osu.Game.Overlays; +using osu.Game.Overlays.OSD; +using osu.Game.Overlays.Settings.Sections; +using osu.Game.Rulesets.Objects; +using osu.Game.Rulesets.Objects.Types; +using osu.Game.Rulesets.UI; +using osu.Game.Screens.Edit; +using osu.Game.Screens.Edit.Components.TernaryButtons; + +namespace osu.Game.Rulesets.Edit +{ + public abstract partial class ComposerDistanceSnapProvider : Component, IDistanceSnapProvider, IScrollBindingHandler + { + private const float adjust_step = 0.1f; + + public BindableDouble DistanceSpacingMultiplier { get; } = new BindableDouble(1.0) + { + MinValue = 0.1, + MaxValue = 6.0, + Precision = 0.01, + }; + + Bindable IDistanceSnapProvider.DistanceSpacingMultiplier => DistanceSpacingMultiplier; + + private ExpandableSlider> distanceSpacingSlider = null!; + private ExpandableButton currentDistanceSpacingButton = null!; + + [Resolved] + private Playfield playfield { get; set; } = null!; + + [Resolved] + private EditorClock editorClock { get; set; } = null!; + + [Resolved] + private EditorBeatmap editorBeatmap { get; set; } = null!; + + [Resolved] + private IBeatSnapProvider beatSnapProvider { get; set; } = null!; + + [Resolved] + private OnScreenDisplay? onScreenDisplay { get; set; } + + public readonly Bindable DistanceSnapToggle = new Bindable(); + + private bool distanceSnapMomentary; + + private EditorToolboxGroup? toolboxGroup; + + public void AttachToToolbox(ExpandingToolboxContainer toolboxContainer) + { + if (toolboxGroup != null) + throw new InvalidOperationException($"{nameof(AttachToToolbox)} may be called only once for a single {nameof(ComposerDistanceSnapProvider)} instance."); + + toolboxContainer.Add(toolboxGroup = new EditorToolboxGroup("snapping") + { + Alpha = DistanceSpacingMultiplier.Disabled ? 0 : 1, + Children = new Drawable[] + { + distanceSpacingSlider = new ExpandableSlider> + { + KeyboardStep = adjust_step, + // Manual binding in LoadComplete to handle one-way event flow. + Current = DistanceSpacingMultiplier.GetUnboundCopy(), + }, + currentDistanceSpacingButton = new ExpandableButton + { + Action = () => + { + (HitObject before, HitObject after)? objects = getObjectsOnEitherSideOfCurrentTime(); + + Debug.Assert(objects != null); + + DistanceSpacingMultiplier.Value = ReadCurrentDistanceSnap(objects.Value.before, objects.Value.after); + DistanceSnapToggle.Value = TernaryState.True; + }, + RelativeSizeAxes = Axes.X, + } + } + }); + + if (DistanceSpacingMultiplier.Disabled) + { + distanceSpacingSlider.Hide(); + return; + } + + DistanceSpacingMultiplier.Value = editorBeatmap.BeatmapInfo.DistanceSpacing; + DistanceSpacingMultiplier.BindValueChanged(multiplier => + { + distanceSpacingSlider.ContractedLabelText = $"D. S. ({multiplier.NewValue:0.##x})"; + distanceSpacingSlider.ExpandedLabelText = $"Distance Spacing ({multiplier.NewValue:0.##x})"; + + if (multiplier.NewValue != multiplier.OldValue) + onScreenDisplay?.Display(new DistanceSpacingToast(multiplier.NewValue.ToLocalisableString(@"0.##x"), multiplier)); + + editorBeatmap.BeatmapInfo.DistanceSpacing = multiplier.NewValue; + }, true); + + // Manual binding to handle enabling distance spacing when the slider is interacted with. + distanceSpacingSlider.Current.BindValueChanged(spacing => + { + DistanceSpacingMultiplier.Value = spacing.NewValue; + DistanceSnapToggle.Value = TernaryState.True; + }); + DistanceSpacingMultiplier.BindValueChanged(spacing => distanceSpacingSlider.Current.Value = spacing.NewValue); + } + + private (HitObject before, HitObject after)? getObjectsOnEitherSideOfCurrentTime() + { + HitObject? lastBefore = playfield.HitObjectContainer.AliveObjects.LastOrDefault(h => h.HitObject.StartTime < editorClock.CurrentTime)?.HitObject; + + if (lastBefore == null) + return null; + + HitObject? firstAfter = playfield.HitObjectContainer.AliveObjects.FirstOrDefault(h => h.HitObject.StartTime >= editorClock.CurrentTime)?.HitObject; + + if (firstAfter == null) + return null; + + if (lastBefore == firstAfter) + return null; + + return (lastBefore, firstAfter); + } + + protected abstract double ReadCurrentDistanceSnap(HitObject before, HitObject after); + + protected override void Update() + { + base.Update(); + + (HitObject before, HitObject after)? objects = getObjectsOnEitherSideOfCurrentTime(); + + double currentSnap = objects == null + ? 0 + : ReadCurrentDistanceSnap(objects.Value.before, objects.Value.after); + + if (currentSnap > DistanceSpacingMultiplier.MinValue) + { + currentDistanceSpacingButton.Enabled.Value = currentDistanceSpacingButton.Expanded.Value + && !DistanceSpacingMultiplier.Disabled + && !Precision.AlmostEquals(currentSnap, DistanceSpacingMultiplier.Value, DistanceSpacingMultiplier.Precision / 2); + currentDistanceSpacingButton.ContractedLabelText = $"current {currentSnap:N2}x"; + currentDistanceSpacingButton.ExpandedLabelText = $"Use current ({currentSnap:N2}x)"; + } + else + { + currentDistanceSpacingButton.Enabled.Value = false; + currentDistanceSpacingButton.ContractedLabelText = string.Empty; + currentDistanceSpacingButton.ExpandedLabelText = "Use current (unavailable)"; + } + } + + public IEnumerable CreateTernaryButtons() => new[] + { + new TernaryButton(DistanceSnapToggle, "Distance Snap", () => new SpriteIcon { Icon = FontAwesome.Solid.Ruler }) + }; + + protected override bool OnKeyDown(KeyDownEvent e) + { + if (e.Repeat) + return false; + + handleToggleViaKey(e); + return base.OnKeyDown(e); + } + + protected override void OnKeyUp(KeyUpEvent e) + { + handleToggleViaKey(e); + base.OnKeyUp(e); + } + + private void handleToggleViaKey(KeyboardEvent key) + { + bool altPressed = key.AltPressed; + + if (altPressed != distanceSnapMomentary) + { + distanceSnapMomentary = altPressed; + DistanceSnapToggle.Value = DistanceSnapToggle.Value == TernaryState.False ? TernaryState.True : TernaryState.False; + } + } + + public virtual bool OnPressed(KeyBindingPressEvent e) + { + switch (e.Action) + { + case GlobalAction.EditorIncreaseDistanceSpacing: + case GlobalAction.EditorDecreaseDistanceSpacing: + return AdjustDistanceSpacing(e.Action, adjust_step); + } + + return false; + } + + public virtual void OnReleased(KeyBindingReleaseEvent e) + { + } + + public bool OnScroll(KeyBindingScrollEvent e) + { + switch (e.Action) + { + case GlobalAction.EditorIncreaseDistanceSpacing: + case GlobalAction.EditorDecreaseDistanceSpacing: + return AdjustDistanceSpacing(e.Action, e.ScrollAmount * adjust_step); + } + + return false; + } + + protected virtual bool AdjustDistanceSpacing(GlobalAction action, float amount) + { + if (DistanceSpacingMultiplier.Disabled) + return false; + + if (action == GlobalAction.EditorIncreaseDistanceSpacing) + DistanceSpacingMultiplier.Value += amount; + else if (action == GlobalAction.EditorDecreaseDistanceSpacing) + DistanceSpacingMultiplier.Value -= amount; + + DistanceSnapToggle.Value = TernaryState.True; + return true; + } + + #region IDistanceSnapProvider + + public virtual float GetBeatSnapDistanceAt(HitObject referenceObject, bool useReferenceSliderVelocity = true) + { + return (float)(100 * (useReferenceSliderVelocity && referenceObject is IHasSliderVelocity hasSliderVelocity ? hasSliderVelocity.SliderVelocityMultiplier : 1) * editorBeatmap.Difficulty.SliderMultiplier * 1 + / beatSnapProvider.BeatDivisor); + } + + public virtual float DurationToDistance(HitObject referenceObject, double duration) + { + double beatLength = beatSnapProvider.GetBeatLengthAtTime(referenceObject.StartTime); + return (float)(duration / beatLength * GetBeatSnapDistanceAt(referenceObject)); + } + + public virtual double DistanceToDuration(HitObject referenceObject, float distance) + { + double beatLength = beatSnapProvider.GetBeatLengthAtTime(referenceObject.StartTime); + return distance / GetBeatSnapDistanceAt(referenceObject) * beatLength; + } + + public virtual double FindSnappedDuration(HitObject referenceObject, float distance) + => beatSnapProvider.SnapTime(referenceObject.StartTime + DistanceToDuration(referenceObject, distance), referenceObject.StartTime) - referenceObject.StartTime; + + public virtual float FindSnappedDistance(HitObject referenceObject, float distance) + { + double startTime = referenceObject.StartTime; + + double actualDuration = startTime + DistanceToDuration(referenceObject, distance); + + double snappedEndTime = beatSnapProvider.SnapTime(actualDuration, startTime); + + double beatLength = beatSnapProvider.GetBeatLengthAtTime(startTime); + + // we don't want to exceed the actual duration and snap to a point in the future. + // as we are snapping to beat length via SnapTime (which will round-to-nearest), check for snapping in the forward direction and reverse it. + if (snappedEndTime > actualDuration + 1) + snappedEndTime -= beatLength; + + return DurationToDistance(referenceObject, snappedEndTime - startTime); + } + + #endregion + + private partial class DistanceSpacingToast : Toast + { + private readonly ValueChangedEvent change; + + public DistanceSpacingToast(LocalisableString value, ValueChangedEvent change) + : base(getAction(change).GetLocalisableDescription(), value, string.Empty) + { + this.change = change; + } + + [BackgroundDependencyLoader] + private void load(OsuConfigManager config) + { + ShortcutText.Text = config.LookupKeyBindings(getAction(change)).ToUpper(); + } + + private static GlobalAction getAction(ValueChangedEvent change) => change.NewValue - change.OldValue > 0 + ? GlobalAction.EditorIncreaseDistanceSpacing + : GlobalAction.EditorDecreaseDistanceSpacing; + } + } +} diff --git a/osu.Game/Rulesets/Edit/DistancedHitObjectComposer.cs b/osu.Game/Rulesets/Edit/DistancedHitObjectComposer.cs index 70b79c30f0..938e2d9011 100644 --- a/osu.Game/Rulesets/Edit/DistancedHitObjectComposer.cs +++ b/osu.Game/Rulesets/Edit/DistancedHitObjectComposer.cs @@ -44,7 +44,7 @@ namespace osu.Game.Rulesets.Edit Precision = 0.01, }; - IBindable IDistanceSnapProvider.DistanceSpacingMultiplier => DistanceSpacingMultiplier; + Bindable IDistanceSnapProvider.DistanceSpacingMultiplier => DistanceSpacingMultiplier; private ExpandableSlider> distanceSpacingSlider; private ExpandableButton currentDistanceSpacingButton; @@ -52,7 +52,7 @@ namespace osu.Game.Rulesets.Edit [Resolved(canBeNull: true)] private OnScreenDisplay onScreenDisplay { get; set; } - protected readonly Bindable DistanceSnapToggle = new Bindable(); + public readonly Bindable DistanceSnapToggle = new Bindable(); private bool distanceSnapMomentary; diff --git a/osu.Game/Rulesets/Edit/IDistanceSnapProvider.cs b/osu.Game/Rulesets/Edit/IDistanceSnapProvider.cs index af9d02f5c4..380038eadf 100644 --- a/osu.Game/Rulesets/Edit/IDistanceSnapProvider.cs +++ b/osu.Game/Rulesets/Edit/IDistanceSnapProvider.cs @@ -19,7 +19,7 @@ namespace osu.Game.Rulesets.Edit /// Importantly, this is provided for manual usage, and not multiplied into any of the methods exposed by this interface. /// /// - IBindable DistanceSpacingMultiplier { get; } + Bindable DistanceSpacingMultiplier { get; } /// /// Retrieves the distance between two points within a timing point that are one beat length apart. From 31849192c39f555de03d34f1dc95cd24dce2834b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 19 Oct 2023 11:42:40 +0200 Subject: [PATCH 52/64] Remove `DistancedHitObjectComposer` inheritance from catch composer --- .../Edit/CatchDistanceSnapProvider.cs | 26 +++++++++++ .../Edit/CatchHitObjectComposer.cs | 44 ++++++++++--------- 2 files changed, 49 insertions(+), 21 deletions(-) create mode 100644 osu.Game.Rulesets.Catch/Edit/CatchDistanceSnapProvider.cs diff --git a/osu.Game.Rulesets.Catch/Edit/CatchDistanceSnapProvider.cs b/osu.Game.Rulesets.Catch/Edit/CatchDistanceSnapProvider.cs new file mode 100644 index 0000000000..c3103bd204 --- /dev/null +++ b/osu.Game.Rulesets.Catch/Edit/CatchDistanceSnapProvider.cs @@ -0,0 +1,26 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System; +using osu.Game.Rulesets.Catch.Objects; +using osu.Game.Rulesets.Edit; +using osu.Game.Rulesets.Objects; + +namespace osu.Game.Rulesets.Catch.Edit +{ + public partial class CatchDistanceSnapProvider : ComposerDistanceSnapProvider + { + protected override double ReadCurrentDistanceSnap(HitObject before, HitObject after) + { + // osu!catch's distance snap implementation is limited, in that a custom spacing cannot be specified. + // Therefore this functionality is not currently used. + // + // The implementation below is probably correct but should be checked if/when exposed via controls. + + float expectedDistance = DurationToDistance(before, after.StartTime - before.GetEndTime()); + float actualDistance = Math.Abs(((CatchHitObject)before).EffectiveX - ((CatchHitObject)after).EffectiveX); + + return actualDistance / expectedDistance; + } + } +} diff --git a/osu.Game.Rulesets.Catch/Edit/CatchHitObjectComposer.cs b/osu.Game.Rulesets.Catch/Edit/CatchHitObjectComposer.cs index dc3a4416a5..50bc9ec157 100644 --- a/osu.Game.Rulesets.Catch/Edit/CatchHitObjectComposer.cs +++ b/osu.Game.Rulesets.Catch/Edit/CatchHitObjectComposer.cs @@ -1,7 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using System; using System.Collections.Generic; using System.Linq; using osu.Framework.Allocation; @@ -9,6 +8,7 @@ using osu.Framework.Bindables; using osu.Framework.Extensions.EnumExtensions; using osu.Framework.Graphics; using osu.Framework.Input; +using osu.Framework.Input.Bindings; using osu.Framework.Input.Events; using osu.Game.Beatmaps; using osu.Game.Graphics.UserInterface; @@ -20,13 +20,14 @@ using osu.Game.Rulesets.Edit.Tools; using osu.Game.Rulesets.Mods; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.UI; +using osu.Game.Screens.Edit.Components.TernaryButtons; using osu.Game.Screens.Edit.Compose.Components; using osuTK; namespace osu.Game.Rulesets.Catch.Edit { // we're also a ScrollingHitObjectComposer candidate, but can't be everything can we? - public partial class CatchHitObjectComposer : DistancedHitObjectComposer + public partial class CatchHitObjectComposer : HitObjectComposer, IKeyBindingHandler { private const float distance_snap_radius = 50; @@ -42,6 +43,9 @@ namespace osu.Game.Rulesets.Catch.Edit MaxValue = 10, }; + [Cached(typeof(IDistanceSnapProvider))] + protected readonly CatchDistanceSnapProvider DistanceSnapProvider = new CatchDistanceSnapProvider(); + public CatchHitObjectComposer(CatchRuleset ruleset) : base(ruleset) { @@ -50,8 +54,11 @@ namespace osu.Game.Rulesets.Catch.Edit [BackgroundDependencyLoader] private void load() { + AddInternal(DistanceSnapProvider); + DistanceSnapProvider.AttachToToolbox(RightToolbox); + // todo: enable distance spacing once catch supports applying it to its existing distance snap grid implementation. - DistanceSpacingMultiplier.Disabled = true; + DistanceSnapProvider.DistanceSpacingMultiplier.Disabled = true; LayerBelowRuleset.Add(new PlayfieldBorder { @@ -72,6 +79,10 @@ namespace osu.Game.Rulesets.Catch.Edit AddInternal(beatSnapGrid = new CatchBeatSnapGrid()); } + protected override IEnumerable CreateTernaryButtons() + => base.CreateTernaryButtons() + .Concat(DistanceSnapProvider.CreateTernaryButtons()); + protected override void LoadComplete() { base.LoadComplete(); @@ -102,19 +113,6 @@ namespace osu.Game.Rulesets.Catch.Edit } } - protected override double ReadCurrentDistanceSnap(HitObject before, HitObject after) - { - // osu!catch's distance snap implementation is limited, in that a custom spacing cannot be specified. - // Therefore this functionality is not currently used. - // - // The implementation below is probably correct but should be checked if/when exposed via controls. - - float expectedDistance = DurationToDistance(before, after.StartTime - before.GetEndTime()); - float actualDistance = Math.Abs(((CatchHitObject)before).EffectiveX - ((CatchHitObject)after).EffectiveX); - - return actualDistance / expectedDistance; - } - protected override void Update() { base.Update(); @@ -122,7 +120,7 @@ namespace osu.Game.Rulesets.Catch.Edit updateDistanceSnapGrid(); } - public override bool OnPressed(KeyBindingPressEvent e) + public bool OnPressed(KeyBindingPressEvent e) { switch (e.Action) { @@ -131,14 +129,18 @@ namespace osu.Game.Rulesets.Catch.Edit // May be worth considering standardising "zoom" behaviour with what the timeline uses (ie. alt-wheel) but that may cause new conflicts. case GlobalAction.IncreaseScrollSpeed: this.TransformBindableTo(timeRangeMultiplier, timeRangeMultiplier.Value - 1, 200, Easing.OutQuint); - break; + return true; case GlobalAction.DecreaseScrollSpeed: this.TransformBindableTo(timeRangeMultiplier, timeRangeMultiplier.Value + 1, 200, Easing.OutQuint); - break; + return true; } - return base.OnPressed(e); + return false; + } + + public void OnReleased(KeyBindingReleaseEvent e) + { } protected override DrawableRuleset CreateDrawableRuleset(Ruleset ruleset, IBeatmap beatmap, IReadOnlyList mods) => @@ -224,7 +226,7 @@ namespace osu.Game.Rulesets.Catch.Edit private void updateDistanceSnapGrid() { - if (DistanceSnapToggle.Value != TernaryState.True) + if (DistanceSnapProvider.DistanceSnapToggle.Value != TernaryState.True) { distanceSnapGrid.Hide(); return; From 144ef5a87c2b12d4fe525d15945b6ee0d844ff75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 19 Oct 2023 11:43:11 +0200 Subject: [PATCH 53/64] Remove `DistancedHitObjectComposer` --- .../Edit/DistancedHitObjectComposer.cs | 305 ------------------ 1 file changed, 305 deletions(-) delete mode 100644 osu.Game/Rulesets/Edit/DistancedHitObjectComposer.cs diff --git a/osu.Game/Rulesets/Edit/DistancedHitObjectComposer.cs b/osu.Game/Rulesets/Edit/DistancedHitObjectComposer.cs deleted file mode 100644 index 938e2d9011..0000000000 --- a/osu.Game/Rulesets/Edit/DistancedHitObjectComposer.cs +++ /dev/null @@ -1,305 +0,0 @@ -// 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.Diagnostics; -using System.Linq; -using osu.Framework.Allocation; -using osu.Framework.Bindables; -using osu.Framework.Extensions; -using osu.Framework.Extensions.LocalisationExtensions; -using osu.Framework.Graphics; -using osu.Framework.Graphics.Sprites; -using osu.Framework.Input.Bindings; -using osu.Framework.Input.Events; -using osu.Framework.Localisation; -using osu.Framework.Utils; -using osu.Game.Configuration; -using osu.Game.Graphics.UserInterface; -using osu.Game.Input.Bindings; -using osu.Game.Overlays; -using osu.Game.Overlays.OSD; -using osu.Game.Overlays.Settings.Sections; -using osu.Game.Rulesets.Objects; -using osu.Game.Rulesets.Objects.Types; -using osu.Game.Screens.Edit.Components.TernaryButtons; - -namespace osu.Game.Rulesets.Edit -{ - /// - /// Represents a for rulesets with the concept of distances between objects. - /// - /// The base type of supported objects. - public abstract partial class DistancedHitObjectComposer : HitObjectComposer, IDistanceSnapProvider, IScrollBindingHandler - where TObject : HitObject - { - private const float adjust_step = 0.1f; - - public BindableDouble DistanceSpacingMultiplier { get; } = new BindableDouble(1.0) - { - MinValue = 0.1, - MaxValue = 6.0, - Precision = 0.01, - }; - - Bindable IDistanceSnapProvider.DistanceSpacingMultiplier => DistanceSpacingMultiplier; - - private ExpandableSlider> distanceSpacingSlider; - private ExpandableButton currentDistanceSpacingButton; - - [Resolved(canBeNull: true)] - private OnScreenDisplay onScreenDisplay { get; set; } - - public readonly Bindable DistanceSnapToggle = new Bindable(); - - private bool distanceSnapMomentary; - - protected DistancedHitObjectComposer(Ruleset ruleset) - : base(ruleset) - { - } - - [BackgroundDependencyLoader] - private void load() - { - RightToolbox.Add(new EditorToolboxGroup("snapping") - { - Alpha = DistanceSpacingMultiplier.Disabled ? 0 : 1, - Children = new Drawable[] - { - distanceSpacingSlider = new ExpandableSlider> - { - KeyboardStep = adjust_step, - // Manual binding in LoadComplete to handle one-way event flow. - Current = DistanceSpacingMultiplier.GetUnboundCopy(), - }, - currentDistanceSpacingButton = new ExpandableButton - { - Action = () => - { - (HitObject before, HitObject after)? objects = getObjectsOnEitherSideOfCurrentTime(); - - Debug.Assert(objects != null); - - DistanceSpacingMultiplier.Value = ReadCurrentDistanceSnap(objects.Value.before, objects.Value.after); - DistanceSnapToggle.Value = TernaryState.True; - }, - RelativeSizeAxes = Axes.X, - } - } - }); - } - - private (HitObject before, HitObject after)? getObjectsOnEitherSideOfCurrentTime() - { - HitObject lastBefore = Playfield.HitObjectContainer.AliveObjects.LastOrDefault(h => h.HitObject.StartTime < EditorClock.CurrentTime)?.HitObject; - - if (lastBefore == null) - return null; - - HitObject firstAfter = Playfield.HitObjectContainer.AliveObjects.FirstOrDefault(h => h.HitObject.StartTime >= EditorClock.CurrentTime)?.HitObject; - - if (firstAfter == null) - return null; - - if (lastBefore == firstAfter) - return null; - - return (lastBefore, firstAfter); - } - - protected abstract double ReadCurrentDistanceSnap(HitObject before, HitObject after); - - protected override void Update() - { - base.Update(); - - (HitObject before, HitObject after)? objects = getObjectsOnEitherSideOfCurrentTime(); - - double currentSnap = objects == null - ? 0 - : ReadCurrentDistanceSnap(objects.Value.before, objects.Value.after); - - if (currentSnap > DistanceSpacingMultiplier.MinValue) - { - currentDistanceSpacingButton.Enabled.Value = currentDistanceSpacingButton.Expanded.Value - && !DistanceSpacingMultiplier.Disabled - && !Precision.AlmostEquals(currentSnap, DistanceSpacingMultiplier.Value, DistanceSpacingMultiplier.Precision / 2); - currentDistanceSpacingButton.ContractedLabelText = $"current {currentSnap:N2}x"; - currentDistanceSpacingButton.ExpandedLabelText = $"Use current ({currentSnap:N2}x)"; - } - else - { - currentDistanceSpacingButton.Enabled.Value = false; - currentDistanceSpacingButton.ContractedLabelText = string.Empty; - currentDistanceSpacingButton.ExpandedLabelText = "Use current (unavailable)"; - } - } - - protected override void LoadComplete() - { - base.LoadComplete(); - - if (DistanceSpacingMultiplier.Disabled) - { - distanceSpacingSlider.Hide(); - return; - } - - DistanceSpacingMultiplier.Value = EditorBeatmap.BeatmapInfo.DistanceSpacing; - DistanceSpacingMultiplier.BindValueChanged(multiplier => - { - distanceSpacingSlider.ContractedLabelText = $"D. S. ({multiplier.NewValue:0.##x})"; - distanceSpacingSlider.ExpandedLabelText = $"Distance Spacing ({multiplier.NewValue:0.##x})"; - - if (multiplier.NewValue != multiplier.OldValue) - onScreenDisplay?.Display(new DistanceSpacingToast(multiplier.NewValue.ToLocalisableString(@"0.##x"), multiplier)); - - EditorBeatmap.BeatmapInfo.DistanceSpacing = multiplier.NewValue; - }, true); - - // Manual binding to handle enabling distance spacing when the slider is interacted with. - distanceSpacingSlider.Current.BindValueChanged(spacing => - { - DistanceSpacingMultiplier.Value = spacing.NewValue; - DistanceSnapToggle.Value = TernaryState.True; - }); - DistanceSpacingMultiplier.BindValueChanged(spacing => distanceSpacingSlider.Current.Value = spacing.NewValue); - } - - protected override IEnumerable CreateTernaryButtons() => base.CreateTernaryButtons().Concat(new[] - { - new TernaryButton(DistanceSnapToggle, "Distance Snap", () => new SpriteIcon { Icon = FontAwesome.Solid.Ruler }) - }); - - protected override bool OnKeyDown(KeyDownEvent e) - { - if (e.Repeat) - return false; - - handleToggleViaKey(e); - return base.OnKeyDown(e); - } - - protected override void OnKeyUp(KeyUpEvent e) - { - handleToggleViaKey(e); - base.OnKeyUp(e); - } - - private void handleToggleViaKey(KeyboardEvent key) - { - bool altPressed = key.AltPressed; - - if (altPressed != distanceSnapMomentary) - { - distanceSnapMomentary = altPressed; - DistanceSnapToggle.Value = DistanceSnapToggle.Value == TernaryState.False ? TernaryState.True : TernaryState.False; - } - } - - public virtual bool OnPressed(KeyBindingPressEvent e) - { - switch (e.Action) - { - case GlobalAction.EditorIncreaseDistanceSpacing: - case GlobalAction.EditorDecreaseDistanceSpacing: - return AdjustDistanceSpacing(e.Action, adjust_step); - } - - return false; - } - - public virtual void OnReleased(KeyBindingReleaseEvent e) - { - } - - public bool OnScroll(KeyBindingScrollEvent e) - { - switch (e.Action) - { - case GlobalAction.EditorIncreaseDistanceSpacing: - case GlobalAction.EditorDecreaseDistanceSpacing: - return AdjustDistanceSpacing(e.Action, e.ScrollAmount * adjust_step); - } - - return false; - } - - protected virtual bool AdjustDistanceSpacing(GlobalAction action, float amount) - { - if (DistanceSpacingMultiplier.Disabled) - return false; - - if (action == GlobalAction.EditorIncreaseDistanceSpacing) - DistanceSpacingMultiplier.Value += amount; - else if (action == GlobalAction.EditorDecreaseDistanceSpacing) - DistanceSpacingMultiplier.Value -= amount; - - DistanceSnapToggle.Value = TernaryState.True; - return true; - } - - public virtual float GetBeatSnapDistanceAt(HitObject referenceObject, bool useReferenceSliderVelocity = true) - { - return (float)(100 * (useReferenceSliderVelocity && referenceObject is IHasSliderVelocity hasSliderVelocity ? hasSliderVelocity.SliderVelocityMultiplier : 1) * EditorBeatmap.Difficulty.SliderMultiplier * 1 - / BeatSnapProvider.BeatDivisor); - } - - public virtual float DurationToDistance(HitObject referenceObject, double duration) - { - double beatLength = BeatSnapProvider.GetBeatLengthAtTime(referenceObject.StartTime); - return (float)(duration / beatLength * GetBeatSnapDistanceAt(referenceObject)); - } - - public virtual double DistanceToDuration(HitObject referenceObject, float distance) - { - double beatLength = BeatSnapProvider.GetBeatLengthAtTime(referenceObject.StartTime); - return distance / GetBeatSnapDistanceAt(referenceObject) * beatLength; - } - - public virtual double FindSnappedDuration(HitObject referenceObject, float distance) - => BeatSnapProvider.SnapTime(referenceObject.StartTime + DistanceToDuration(referenceObject, distance), referenceObject.StartTime) - referenceObject.StartTime; - - public virtual float FindSnappedDistance(HitObject referenceObject, float distance) - { - double startTime = referenceObject.StartTime; - - double actualDuration = startTime + DistanceToDuration(referenceObject, distance); - - double snappedEndTime = BeatSnapProvider.SnapTime(actualDuration, startTime); - - double beatLength = BeatSnapProvider.GetBeatLengthAtTime(startTime); - - // we don't want to exceed the actual duration and snap to a point in the future. - // as we are snapping to beat length via SnapTime (which will round-to-nearest), check for snapping in the forward direction and reverse it. - if (snappedEndTime > actualDuration + 1) - snappedEndTime -= beatLength; - - return DurationToDistance(referenceObject, snappedEndTime - startTime); - } - - private partial class DistanceSpacingToast : Toast - { - private readonly ValueChangedEvent change; - - public DistanceSpacingToast(LocalisableString value, ValueChangedEvent change) - : base(getAction(change).GetLocalisableDescription(), value, string.Empty) - { - this.change = change; - } - - [BackgroundDependencyLoader] - private void load(OsuConfigManager config) - { - ShortcutText.Text = config.LookupKeyBindings(getAction(change)).ToUpper(); - } - - private static GlobalAction getAction(ValueChangedEvent change) => change.NewValue - change.OldValue > 0 - ? GlobalAction.EditorIncreaseDistanceSpacing - : GlobalAction.EditorDecreaseDistanceSpacing; - } - } -} From 0c4e74c82dbc3423cb461cb73c2e1bd15867257c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 19 Oct 2023 11:43:31 +0200 Subject: [PATCH 54/64] Inherit `ScrollingHitObjectComposer` in catch --- osu.Game.Rulesets.Catch/Edit/CatchHitObjectComposer.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Edit/CatchHitObjectComposer.cs b/osu.Game.Rulesets.Catch/Edit/CatchHitObjectComposer.cs index 50bc9ec157..04dd4827b6 100644 --- a/osu.Game.Rulesets.Catch/Edit/CatchHitObjectComposer.cs +++ b/osu.Game.Rulesets.Catch/Edit/CatchHitObjectComposer.cs @@ -26,8 +26,7 @@ using osuTK; namespace osu.Game.Rulesets.Catch.Edit { - // we're also a ScrollingHitObjectComposer candidate, but can't be everything can we? - public partial class CatchHitObjectComposer : HitObjectComposer, IKeyBindingHandler + public partial class CatchHitObjectComposer : ScrollingHitObjectComposer, IKeyBindingHandler { private const float distance_snap_radius = 50; From 013b5fa916d819ec7a8a93b1692d4aa027934a67 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 19 Oct 2023 23:54:34 +0900 Subject: [PATCH 55/64] Move beat snap grid implementation details to `ScrollingHitObjectComposer` --- .../Edit/CatchHitObjectComposer.cs | 34 -------------- osu.Game/Rulesets/Edit/HitObjectComposer.cs | 42 ----------------- .../Edit/ScrollingHitObjectComposer.cs | 46 +++++++++++++++++++ 3 files changed, 46 insertions(+), 76 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Edit/CatchHitObjectComposer.cs b/osu.Game.Rulesets.Catch/Edit/CatchHitObjectComposer.cs index 2ccf99c6e6..9b258841b2 100644 --- a/osu.Game.Rulesets.Catch/Edit/CatchHitObjectComposer.cs +++ b/osu.Game.Rulesets.Catch/Edit/CatchHitObjectComposer.cs @@ -40,8 +40,6 @@ namespace osu.Game.Rulesets.Catch.Edit [Cached(typeof(IDistanceSnapProvider))] protected readonly CatchDistanceSnapProvider DistanceSnapProvider = new CatchDistanceSnapProvider(); - private BeatSnapGrid beatSnapGrid = null!; - public CatchHitObjectComposer(CatchRuleset ruleset) : base(ruleset) { @@ -71,44 +69,12 @@ namespace osu.Game.Rulesets.Catch.Edit Catcher.BASE_DASH_SPEED, -Catcher.BASE_DASH_SPEED, Catcher.BASE_WALK_SPEED, -Catcher.BASE_WALK_SPEED, })); - - AddInternal(beatSnapGrid = new CatchBeatSnapGrid()); } protected override IEnumerable CreateTernaryButtons() => base.CreateTernaryButtons() .Concat(DistanceSnapProvider.CreateTernaryButtons()); - protected override void UpdateAfterChildren() - { - base.UpdateAfterChildren(); - - if (BlueprintContainer.CurrentTool is SelectTool) - { - if (EditorBeatmap.SelectedHitObjects.Any()) - { - beatSnapGrid.SelectionTimeRange = (EditorBeatmap.SelectedHitObjects.Min(h => h.StartTime), EditorBeatmap.SelectedHitObjects.Max(h => h.GetEndTime())); - } - else - beatSnapGrid.SelectionTimeRange = null; - } - else - { - var result = FindSnappedPositionAndTime(InputManager.CurrentState.Mouse.Position); - if (result.Time is double time) - beatSnapGrid.SelectionTimeRange = (time, time); - else - beatSnapGrid.SelectionTimeRange = null; - } - } - - protected override void Update() - { - base.Update(); - - updateDistanceSnapGrid(); - } - public bool OnPressed(KeyBindingPressEvent e) { switch (e.Action) diff --git a/osu.Game/Rulesets/Edit/HitObjectComposer.cs b/osu.Game/Rulesets/Edit/HitObjectComposer.cs index 129ab6751e..b68af49222 100644 --- a/osu.Game/Rulesets/Edit/HitObjectComposer.cs +++ b/osu.Game/Rulesets/Edit/HitObjectComposer.cs @@ -90,9 +90,6 @@ namespace osu.Game.Rulesets.Edit protected DrawableRuleset DrawableRuleset { get; private set; } - [CanBeNull] - private BeatSnapGrid beatSnapGrid; - protected HitObjectComposer(Ruleset ruleset) : base(ruleset) { @@ -209,8 +206,6 @@ namespace osu.Game.Rulesets.Edit } } }, - // Must be constructed after drawable ruleset above. - (beatSnapGrid = CreateBeatSnapGrid()) ?? Empty(), }; toolboxCollection.Items = CompositionTools @@ -275,37 +270,6 @@ namespace osu.Game.Rulesets.Edit } } - protected override void UpdateAfterChildren() - { - base.UpdateAfterChildren(); - - updateBeatSnapGrid(); - } - - private void updateBeatSnapGrid() - { - if (beatSnapGrid == null) - return; - - if (BlueprintContainer.CurrentTool is SelectTool) - { - if (EditorBeatmap.SelectedHitObjects.Any()) - { - beatSnapGrid.SelectionTimeRange = (EditorBeatmap.SelectedHitObjects.Min(h => h.StartTime), EditorBeatmap.SelectedHitObjects.Max(h => h.GetEndTime())); - } - else - beatSnapGrid.SelectionTimeRange = null; - } - else - { - var result = FindSnappedPositionAndTime(InputManager.CurrentState.Mouse.Position); - if (result.Time is double time) - beatSnapGrid.SelectionTimeRange = (time, time); - else - beatSnapGrid.SelectionTimeRange = null; - } - } - public override Playfield Playfield => drawableRulesetWrapper.Playfield; public override IEnumerable HitObjects => drawableRulesetWrapper.Playfield.AllHitObjects; @@ -336,12 +300,6 @@ namespace osu.Game.Rulesets.Edit /// protected virtual ComposeBlueprintContainer CreateBlueprintContainer() => new ComposeBlueprintContainer(this); - /// - /// Construct an optional beat snap grid. - /// - [CanBeNull] - protected virtual BeatSnapGrid CreateBeatSnapGrid() => null; - /// /// Construct a drawable ruleset for the provided ruleset. /// diff --git a/osu.Game/Rulesets/Edit/ScrollingHitObjectComposer.cs b/osu.Game/Rulesets/Edit/ScrollingHitObjectComposer.cs index cb92ebbd15..eb73cef01a 100644 --- a/osu.Game/Rulesets/Edit/ScrollingHitObjectComposer.cs +++ b/osu.Game/Rulesets/Edit/ScrollingHitObjectComposer.cs @@ -1,6 +1,7 @@ // 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 osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; @@ -8,9 +9,11 @@ using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Sprites; using osu.Game.Configuration; using osu.Game.Graphics.UserInterface; +using osu.Game.Rulesets.Edit.Tools; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.UI.Scrolling; using osu.Game.Screens.Edit.Components.TernaryButtons; +using osu.Game.Screens.Edit.Compose.Components; using osuTK; namespace osu.Game.Rulesets.Edit @@ -21,6 +24,13 @@ namespace osu.Game.Rulesets.Edit private readonly Bindable showSpeedChanges = new Bindable(); private Bindable configShowSpeedChanges = null!; + private BeatSnapGrid? beatSnapGrid; + + /// + /// Construct an optional beat snap grid. + /// + protected virtual BeatSnapGrid? CreateBeatSnapGrid() => null; + protected ScrollingHitObjectComposer(Ruleset ruleset) : base(ruleset) { @@ -57,6 +67,42 @@ namespace osu.Game.Rulesets.Edit configShowSpeedChanges.Value = enabled; }, true); } + + beatSnapGrid = CreateBeatSnapGrid(); + + if (beatSnapGrid != null) + AddInternal(beatSnapGrid); + } + + protected override void UpdateAfterChildren() + { + base.UpdateAfterChildren(); + + updateBeatSnapGrid(); + } + + private void updateBeatSnapGrid() + { + if (beatSnapGrid == null) + return; + + if (BlueprintContainer.CurrentTool is SelectTool) + { + if (EditorBeatmap.SelectedHitObjects.Any()) + { + beatSnapGrid.SelectionTimeRange = (EditorBeatmap.SelectedHitObjects.Min(h => h.StartTime), EditorBeatmap.SelectedHitObjects.Max(h => h.GetEndTime())); + } + else + beatSnapGrid.SelectionTimeRange = null; + } + else + { + var result = FindSnappedPositionAndTime(InputManager.CurrentState.Mouse.Position); + if (result.Time is double time) + beatSnapGrid.SelectionTimeRange = (time, time); + else + beatSnapGrid.SelectionTimeRange = null; + } } } } From 74b86349d58e5169e52bbdc70cef3dec81579a74 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 19 Oct 2023 23:57:36 +0900 Subject: [PATCH 56/64] Tidy up `CatchHitObjectComposer` --- .../Edit/CatchHitObjectComposer.cs | 56 ++++++------------- 1 file changed, 17 insertions(+), 39 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Edit/CatchHitObjectComposer.cs b/osu.Game.Rulesets.Catch/Edit/CatchHitObjectComposer.cs index 9b258841b2..6f0ee260ab 100644 --- a/osu.Game.Rulesets.Catch/Edit/CatchHitObjectComposer.cs +++ b/osu.Game.Rulesets.Catch/Edit/CatchHitObjectComposer.cs @@ -10,7 +10,6 @@ using osu.Framework.Graphics; using osu.Framework.Input.Bindings; using osu.Framework.Input.Events; using osu.Game.Beatmaps; -using osu.Game.Graphics.UserInterface; using osu.Game.Input.Bindings; using osu.Game.Rulesets.Catch.Objects; using osu.Game.Rulesets.Catch.UI; @@ -75,6 +74,23 @@ namespace osu.Game.Rulesets.Catch.Edit => base.CreateTernaryButtons() .Concat(DistanceSnapProvider.CreateTernaryButtons()); + protected override DrawableRuleset CreateDrawableRuleset(Ruleset ruleset, IBeatmap beatmap, IReadOnlyList mods) => + new DrawableCatchEditorRuleset(ruleset, beatmap, mods) + { + TimeRangeMultiplier = { BindTarget = timeRangeMultiplier, } + }; + + protected override ComposeBlueprintContainer CreateBlueprintContainer() => new CatchBlueprintContainer(this); + + protected override BeatSnapGrid CreateBeatSnapGrid() => new CatchBeatSnapGrid(); + + protected override IReadOnlyList CompositionTools => new HitObjectCompositionTool[] + { + new FruitCompositionTool(), + new JuiceStreamCompositionTool(), + new BananaShowerCompositionTool() + }; + public bool OnPressed(KeyBindingPressEvent e) { switch (e.Action) @@ -98,19 +114,6 @@ namespace osu.Game.Rulesets.Catch.Edit { } - protected override DrawableRuleset CreateDrawableRuleset(Ruleset ruleset, IBeatmap beatmap, IReadOnlyList mods) => - new DrawableCatchEditorRuleset(ruleset, beatmap, mods) - { - TimeRangeMultiplier = { BindTarget = timeRangeMultiplier, } - }; - - protected override IReadOnlyList CompositionTools => new HitObjectCompositionTool[] - { - new FruitCompositionTool(), - new JuiceStreamCompositionTool(), - new BananaShowerCompositionTool() - }; - public override SnapResult FindSnappedPositionAndTime(Vector2 screenSpacePosition, SnapType snapType = SnapType.All) { var result = base.FindSnappedPositionAndTime(screenSpacePosition, snapType); @@ -129,10 +132,6 @@ namespace osu.Game.Rulesets.Catch.Edit return result; } - protected override ComposeBlueprintContainer CreateBlueprintContainer() => new CatchBlueprintContainer(this); - - protected override BeatSnapGrid CreateBeatSnapGrid() => new CatchBeatSnapGrid(); - private PalpableCatchHitObject? getLastSnappableHitObject(double time) { var hitObject = EditorBeatmap.HitObjects.OfType().LastOrDefault(h => h.GetEndTime() < time && !(h is BananaShower)); @@ -180,26 +179,5 @@ namespace osu.Game.Rulesets.Catch.Edit return null; } } - - private void updateDistanceSnapGrid() - { - if (DistanceSnapProvider.DistanceSnapToggle.Value != TernaryState.True) - { - distanceSnapGrid.Hide(); - return; - } - - var sourceHitObject = getDistanceSnapGridSourceHitObject(); - - if (sourceHitObject == null) - { - distanceSnapGrid.Hide(); - return; - } - - distanceSnapGrid.Show(); - distanceSnapGrid.StartTime = sourceHitObject.GetEndTime(); - distanceSnapGrid.StartX = sourceHitObject.EffectiveX; - } } } From cc1f1d2270043f452653bd82ea70e702ba41eacd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 19 Oct 2023 19:47:49 +0200 Subject: [PATCH 57/64] Fix Floating Fruits not flipping playfield properly Regressed by https://github.com/ppy/osu/pull/25070. --- .../Mods/TestSceneCatchModFloatingFruits.cs | 21 +++++++++++++++++++ .../Mods/CatchModFloatingFruits.cs | 5 +---- 2 files changed, 22 insertions(+), 4 deletions(-) create mode 100644 osu.Game.Rulesets.Catch.Tests/Mods/TestSceneCatchModFloatingFruits.cs diff --git a/osu.Game.Rulesets.Catch.Tests/Mods/TestSceneCatchModFloatingFruits.cs b/osu.Game.Rulesets.Catch.Tests/Mods/TestSceneCatchModFloatingFruits.cs new file mode 100644 index 0000000000..73579d1c22 --- /dev/null +++ b/osu.Game.Rulesets.Catch.Tests/Mods/TestSceneCatchModFloatingFruits.cs @@ -0,0 +1,21 @@ +// 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.Game.Rulesets.Catch.Mods; +using osu.Game.Tests.Visual; + +namespace osu.Game.Rulesets.Catch.Tests.Mods +{ + public partial class TestSceneCatchModFloatingFruits : ModTestScene + { + protected override Ruleset CreatePlayerRuleset() => new CatchRuleset(); + + [Test] + public void TestFloating() => CreateModTest(new ModTestData + { + Mod = new CatchModFloatingFruits(), + PassCondition = () => true + }); + } +} diff --git a/osu.Game.Rulesets.Catch/Mods/CatchModFloatingFruits.cs b/osu.Game.Rulesets.Catch/Mods/CatchModFloatingFruits.cs index dd6757eac9..9d88c90576 100644 --- a/osu.Game.Rulesets.Catch/Mods/CatchModFloatingFruits.cs +++ b/osu.Game.Rulesets.Catch/Mods/CatchModFloatingFruits.cs @@ -1,7 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using osu.Framework.Graphics; using osu.Framework.Graphics.Sprites; using osu.Framework.Localisation; using osu.Game.Rulesets.Catch.Objects; @@ -21,10 +20,8 @@ namespace osu.Game.Rulesets.Catch.Mods public void ApplyToDrawableRuleset(DrawableRuleset drawableRuleset) { - drawableRuleset.PlayfieldAdjustmentContainer.Anchor = Anchor.Centre; - drawableRuleset.PlayfieldAdjustmentContainer.Origin = Anchor.Centre; - drawableRuleset.PlayfieldAdjustmentContainer.Scale = new Vector2(1, -1); + drawableRuleset.PlayfieldAdjustmentContainer.Y = 1 - drawableRuleset.PlayfieldAdjustmentContainer.Y; } } } From 2c6bf9e3469e4c004a41508dd802cad9b3882c61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 19 Oct 2023 20:45:47 +0200 Subject: [PATCH 58/64] Remove unused using directive --- osu.Game/Rulesets/Edit/HitObjectComposer.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game/Rulesets/Edit/HitObjectComposer.cs b/osu.Game/Rulesets/Edit/HitObjectComposer.cs index b68af49222..07e5869e28 100644 --- a/osu.Game/Rulesets/Edit/HitObjectComposer.cs +++ b/osu.Game/Rulesets/Edit/HitObjectComposer.cs @@ -7,7 +7,6 @@ using System; using System.Collections.Generic; using System.Collections.Specialized; using System.Linq; -using JetBrains.Annotations; using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Extensions.EnumExtensions; From d73650331d05a11af6527627d54083d96e70acc8 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Fri, 20 Oct 2023 03:26:12 +0900 Subject: [PATCH 59/64] Isolate diffcalc workflow runs --- .github/workflows/diffcalc.yml | 35 ++++++++++++---------------------- 1 file changed, 12 insertions(+), 23 deletions(-) diff --git a/.github/workflows/diffcalc.yml b/.github/workflows/diffcalc.yml index dbb4067676..67180f7ec2 100644 --- a/.github/workflows/diffcalc.yml +++ b/.github/workflows/diffcalc.yml @@ -101,7 +101,7 @@ permissions: pull-requests: write env: - COMMENT_TAG: execution-${{ github.run_id }}-${{ github.run_number }}-${{ github.run_attempt }} + EXECUTION_ID: execution-${{ github.run_id }}-${{ github.run_number }}-${{ github.run_attempt }} jobs: check-permissions: @@ -124,26 +124,15 @@ jobs: - name: Create comment uses: thollander/actions-comment-pull-request@363c6f6eae92cc5c3a66e95ba016fc771bb38943 # v2.4.2 with: - comment_tag: ${{ env.COMMENT_TAG }} + comment_tag: ${{ env.EXECUTION_ID }} message: | Difficulty calculation queued -- please wait! (${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}) *This comment will update on completion* - wait-for-queue: - name: "Wait for previous workflows" - needs: check-permissions - runs-on: self-hosted - timeout-minutes: 50400 # 35 days, the maximum for jobs on self-hosted runners. - steps: - - uses: ahmadnassri/action-workflow-queue@f547ac848c16a9bb1b2ed4a850e0cc5098af2cf8 # v1.1.5 - with: - timeout: 2147483647 # Around 24 days - the maximum supported by JS setTimeout(). - delay: 120000 # Poll every 2 minutes - the API limit seems fairly low on this one. - directory: name: Prepare directory - needs: wait-for-queue + needs: check-permissions runs-on: self-hosted outputs: GENERATOR_DIR: ${{ steps.set-outputs.outputs.GENERATOR_DIR }} @@ -156,15 +145,15 @@ jobs: - name: Checkout diffcalc-sheet-generator uses: actions/checkout@v3 with: - path: 'diffcalc-sheet-generator' + path: ${{ env.EXECUTION_ID }} repository: 'smoogipoo/diffcalc-sheet-generator' - name: Set outputs id: set-outputs run: | - echo "GENERATOR_DIR=${{ github.workspace }}/diffcalc-sheet-generator" >> "${GITHUB_OUTPUT}" - echo "GENERATOR_ENV=${{ github.workspace }}/diffcalc-sheet-generator/.env" >> "${GITHUB_OUTPUT}" - echo "GOOGLE_CREDS_FILE=${{ github.workspace }}/diffcalc-sheet-generator/google-credentials.json" >> "${GITHUB_OUTPUT}" + echo "GENERATOR_DIR=${{ github.workspace }}/${{ env.EXECUTION_ID }}" >> "${GITHUB_OUTPUT}" + echo "GENERATOR_ENV=${{ github.workspace }}/${{ env.EXECUTION_ID }}/.env" >> "${GITHUB_OUTPUT}" + echo "GOOGLE_CREDS_FILE=${{ github.workspace }}/${{ env.EXECUTION_ID }}/google-credentials.json" >> "${GITHUB_OUTPUT}" environment: name: Setup environment @@ -176,7 +165,7 @@ jobs: - name: Add base environment run: | # Required by diffcalc-sheet-generator - cp '${{ github.workspace }}/diffcalc-sheet-generator/.env.sample' "${{ needs.directory.outputs.GENERATOR_ENV }}" + cp '${{ env.GENERATOR_DIR }}/.env.sample' "${{ needs.directory.outputs.GENERATOR_ENV }}" # Add Google credentials echo '${{ secrets.DIFFCALC_GOOGLE_CREDENTIALS }}' | base64 -d > "${{ needs.directory.outputs.GOOGLE_CREDS_FILE }}" @@ -336,7 +325,7 @@ jobs: if: ${{ always() }} run: | cd "${{ needs.directory.outputs.GENERATOR_DIR }}" - docker-compose down + docker-compose down -v output-cli: name: Output info @@ -358,7 +347,7 @@ jobs: if: ${{ needs.generator.result == 'success' }} uses: thollander/actions-comment-pull-request@363c6f6eae92cc5c3a66e95ba016fc771bb38943 # v2.4.2 with: - comment_tag: ${{ env.COMMENT_TAG }} + comment_tag: ${{ env.EXECUTION_ID }} mode: upsert create_if_not_exists: false message: | @@ -369,7 +358,7 @@ jobs: if: ${{ needs.generator.result == 'failure' }} uses: thollander/actions-comment-pull-request@363c6f6eae92cc5c3a66e95ba016fc771bb38943 # v2.4.2 with: - comment_tag: ${{ env.COMMENT_TAG }} + comment_tag: ${{ env.EXECUTION_ID }} mode: upsert create_if_not_exists: false message: | @@ -379,6 +368,6 @@ jobs: if: ${{ needs.generator.result == 'cancelled' }} uses: thollander/actions-comment-pull-request@363c6f6eae92cc5c3a66e95ba016fc771bb38943 # v2.4.2 with: - comment_tag: ${{ env.COMMENT_TAG }} + comment_tag: ${{ env.EXECUTION_ID }} mode: delete message: '.' # Appears to be required by this action for non-error status code. From 634323e71ee5ffa1aaa3c2ab0a5b77185f81536f Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Fri, 20 Oct 2023 11:24:16 +0900 Subject: [PATCH 60/64] Fix wrong variable --- .github/workflows/diffcalc.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/diffcalc.yml b/.github/workflows/diffcalc.yml index 67180f7ec2..6523903bb1 100644 --- a/.github/workflows/diffcalc.yml +++ b/.github/workflows/diffcalc.yml @@ -165,7 +165,7 @@ jobs: - name: Add base environment run: | # Required by diffcalc-sheet-generator - cp '${{ env.GENERATOR_DIR }}/.env.sample' "${{ needs.directory.outputs.GENERATOR_ENV }}" + cp '${{ needs.directory.outputs.GENERATOR_DIR }}/.env.sample' "${{ needs.directory.outputs.GENERATOR_ENV }}" # Add Google credentials echo '${{ secrets.DIFFCALC_GOOGLE_CREDENTIALS }}' | base64 -d > "${{ needs.directory.outputs.GOOGLE_CREDS_FILE }}" From 6cf0cbf3a9042d1a39e9ed034f2431bd56ace9a2 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Fri, 20 Oct 2023 11:32:11 +0900 Subject: [PATCH 61/64] Remove repo checkout step --- .github/workflows/diffcalc.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.github/workflows/diffcalc.yml b/.github/workflows/diffcalc.yml index 6523903bb1..11778b446b 100644 --- a/.github/workflows/diffcalc.yml +++ b/.github/workflows/diffcalc.yml @@ -139,9 +139,6 @@ jobs: GENERATOR_ENV: ${{ steps.set-outputs.outputs.GENERATOR_ENV }} GOOGLE_CREDS_FILE: ${{ steps.set-outputs.outputs.GOOGLE_CREDS_FILE }} steps: - - name: Checkout - uses: actions/checkout@v3 - - name: Checkout diffcalc-sheet-generator uses: actions/checkout@v3 with: From e3bbf293336facc07ccb8ecc76f64d27fb29bc01 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Fri, 20 Oct 2023 11:34:08 +0900 Subject: [PATCH 62/64] Add cleanup step --- .github/workflows/diffcalc.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/workflows/diffcalc.yml b/.github/workflows/diffcalc.yml index 11778b446b..99d8f25cc2 100644 --- a/.github/workflows/diffcalc.yml +++ b/.github/workflows/diffcalc.yml @@ -334,6 +334,14 @@ jobs: echo "Target: ${{ needs.generator.outputs.TARGET }}" echo "Spreadsheet: ${{ needs.generator.outputs.SPREADSHEET_LINK }}" + cleanup: + name: Cleanup + needs: [ directory, generator ] + if: ${{ always() }} + runs-on: self-hosted + run: | + rm -rf "${{ needs.directory.outputs.GENERATOR_DIR }}" + update-comment: name: Update PR comment needs: [ create-comment, generator ] From 6c61894514749dd1f545bee47aa7233045c364ca Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Fri, 20 Oct 2023 11:35:31 +0900 Subject: [PATCH 63/64] Cleanup in a step --- .github/workflows/diffcalc.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/diffcalc.yml b/.github/workflows/diffcalc.yml index 99d8f25cc2..c5bac64ff2 100644 --- a/.github/workflows/diffcalc.yml +++ b/.github/workflows/diffcalc.yml @@ -339,8 +339,10 @@ jobs: needs: [ directory, generator ] if: ${{ always() }} runs-on: self-hosted - run: | - rm -rf "${{ needs.directory.outputs.GENERATOR_DIR }}" + steps: + - name: Cleanup + run: | + rm -rf "${{ needs.directory.outputs.GENERATOR_DIR }}" update-comment: name: Update PR comment From f36a671eb41dce6dd08169891e8992962930aa9f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 20 Oct 2023 15:38:00 +0900 Subject: [PATCH 64/64] Fix a couple of silly inspections --- osu.Game.Tests/Database/RealmTest.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game.Tests/Database/RealmTest.cs b/osu.Game.Tests/Database/RealmTest.cs index d2779e3038..2812a97268 100644 --- a/osu.Game.Tests/Database/RealmTest.cs +++ b/osu.Game.Tests/Database/RealmTest.cs @@ -35,6 +35,7 @@ namespace osu.Game.Tests.Database Logger.Log($"Running test using realm file {testStorage.GetFullPath(realm.Filename)}"); testAction(realm, testStorage); + // ReSharper disable once DisposeOnUsingVariable realm.Dispose(); Logger.Log($"Final database size: {getFileSize(testStorage, realm)}"); @@ -58,6 +59,7 @@ namespace osu.Game.Tests.Database Logger.Log($"Running test using realm file {testStorage.GetFullPath(realm.Filename)}"); await testAction(realm, testStorage); + // ReSharper disable once DisposeOnUsingVariable realm.Dispose(); Logger.Log($"Final database size: {getFileSize(testStorage, realm)}");