diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneLegacyHitPolicy.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneLegacyHitPolicy.cs index 8c311cce91..a2ef72fe57 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneLegacyHitPolicy.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneLegacyHitPolicy.cs @@ -506,7 +506,6 @@ namespace osu.Game.Rulesets.Osu.Tests } [Test] - [Ignore("Currently broken, first attempt at fixing broke even harder. See https://github.com/ppy/osu/issues/24743.")] public void TestInputDoesNotFallThroughOverlappingSliders() { const double time_first_slider = 1000; @@ -550,12 +549,103 @@ namespace osu.Game.Rulesets.Osu.Tests addJudgementOffsetAssert("first slider head", () => ((Slider)hitObjects[0]).HeadCircle, 0); addJudgementAssert(hitObjects[1], HitResult.Miss); // the slider head of the first slider prevents the second slider's head from being hit, so the judgement offset should be very late. + // this is not strictly done by the hit policy implementation itself (see `OsuModClassic.blockInputToObjectsUnderSliderHead()`), + // but we're testing this here anyways to just keep everything related to input handling and note lock in one place. addJudgementOffsetAssert("second slider head", () => ((Slider)hitObjects[1]).HeadCircle, referenceHitWindows.WindowFor(HitResult.Meh)); addClickActionAssert(0, ClickAction.Hit); } [Test] - public void TestOverlappingObjectsDontBlockEachOtherWhenFullyFadedOut() + public void TestOverlappingSlidersDontBlockEachOtherWhenFullyJudged() + { + const double time_first_slider = 1000; + const double time_second_slider = 1600; + Vector2 positionFirstSlider = new Vector2(100, 50); + Vector2 positionSecondSlider = new Vector2(100, 80); + var midpoint = (positionFirstSlider + positionSecondSlider) / 2; + + var hitObjects = new List + { + new Slider + { + StartTime = time_first_slider, + Position = positionFirstSlider, + Path = new SliderPath(PathType.Linear, new[] + { + Vector2.Zero, + new Vector2(25, 0), + }) + }, + new Slider + { + StartTime = time_second_slider, + Position = positionSecondSlider, + Path = new SliderPath(PathType.Linear, new[] + { + Vector2.Zero, + new Vector2(25, 0), + }) + } + }; + + performTest(hitObjects, new List + { + new OsuReplayFrame { Time = time_first_slider, Position = midpoint, Actions = { OsuAction.RightButton } }, + new OsuReplayFrame { Time = time_first_slider + 25, Position = midpoint }, + // this frame doesn't do anything on lazer, but is REQUIRED for correct playback on stable, + // because stable during replay playback only updates game state _when it encounters a replay frame_ + new OsuReplayFrame { Time = 1250, Position = midpoint }, + new OsuReplayFrame { Time = time_second_slider + 50, Position = midpoint, Actions = { OsuAction.LeftButton } }, + new OsuReplayFrame { Time = time_second_slider + 75, Position = midpoint }, + }); + + addJudgementAssert(hitObjects[0], HitResult.Ok); + addJudgementOffsetAssert("first slider head", () => ((Slider)hitObjects[0]).HeadCircle, 0); + addJudgementAssert(hitObjects[1], HitResult.Ok); + addJudgementOffsetAssert("second slider head", () => ((Slider)hitObjects[1]).HeadCircle, 50); + addClickActionAssert(0, ClickAction.Hit); + addClickActionAssert(1, ClickAction.Hit); + } + + [Test] + public void TestOverlappingHitCirclesDontBlockEachOtherWhenBothVisible() + { + const double time_first_circle = 1000; + const double time_second_circle = 1200; + Vector2 positionFirstCircle = new Vector2(100); + Vector2 positionSecondCircle = new Vector2(120); + var midpoint = (positionFirstCircle + positionSecondCircle) / 2; + + var hitObjects = new List + { + new HitCircle + { + StartTime = time_first_circle, + Position = positionFirstCircle, + }, + new HitCircle + { + StartTime = time_second_circle, + Position = positionSecondCircle, + }, + }; + + performTest(hitObjects, new List + { + new OsuReplayFrame { Time = time_first_circle, Position = midpoint, Actions = { OsuAction.LeftButton } }, + new OsuReplayFrame { Time = time_first_circle + 25, Position = midpoint }, + new OsuReplayFrame { Time = time_first_circle + 50, Position = midpoint, Actions = { OsuAction.RightButton } }, + }); + + addJudgementAssert(hitObjects[0], HitResult.Great); + addJudgementOffsetAssert(hitObjects[0], 0); + + addJudgementAssert(hitObjects[1], HitResult.Meh); + addJudgementOffsetAssert(hitObjects[1], -150); + } + + [Test] + public void TestOverlappingHitCirclesDontBlockEachOtherWhenFullyFadedOut() { const double time_first_circle = 1000; const double time_second_circle = 1200; @@ -586,8 +676,10 @@ namespace osu.Game.Rulesets.Osu.Tests { new OsuReplayFrame { Time = time_first_circle, Position = positionFirstCircle, Actions = { OsuAction.LeftButton } }, new OsuReplayFrame { Time = time_first_circle + 50, Position = positionFirstCircle }, + new OsuReplayFrame { Time = time_second_circle - 50, Position = positionSecondCircle }, new OsuReplayFrame { Time = time_second_circle, Position = positionSecondCircle, Actions = { OsuAction.LeftButton } }, new OsuReplayFrame { Time = time_second_circle + 50, Position = positionSecondCircle }, + new OsuReplayFrame { Time = time_third_circle - 50, Position = positionFirstCircle }, new OsuReplayFrame { Time = time_third_circle, Position = positionFirstCircle, Actions = { OsuAction.LeftButton } }, new OsuReplayFrame { Time = time_third_circle + 50, Position = positionFirstCircle }, }); diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModClassic.cs b/osu.Game.Rulesets.Osu/Mods/OsuModClassic.cs index 5dbf23f7ea..0148ec1987 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModClassic.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModClassic.cs @@ -74,6 +74,10 @@ namespace osu.Game.Rulesets.Osu.Mods head.TrackFollowCircle = !NoSliderHeadMovement.Value; if (FadeHitCircleEarly.Value && !usingHiddenFading) applyEarlyFading(head); + + if (ClassicNoteLock.Value) + blockInputToObjectsUnderSliderHead(head); + break; case DrawableSliderTail tail: @@ -83,10 +87,27 @@ namespace osu.Game.Rulesets.Osu.Mods case DrawableHitCircle circle: if (FadeHitCircleEarly.Value && !usingHiddenFading) applyEarlyFading(circle); + break; } } + /// + /// On stable, slider heads that have already been hit block input from reaching objects that may be underneath them + /// until the sliders they're part of have been fully judged. + /// The purpose of this method is to restore that behaviour. + /// In order to avoid introducing yet another confusing config option, this behaviour is roped into the general notion of "note lock". + /// + private static void blockInputToObjectsUnderSliderHead(DrawableSliderHead slider) + { + var oldHitAction = slider.HitArea.Hit; + slider.HitArea.Hit = () => + { + oldHitAction?.Invoke(); + return !slider.DrawableSlider.AllJudged; + }; + } + private void applyEarlyFading(DrawableHitCircle circle) { circle.ApplyCustomUpdateState += (dho, state) => diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableHitCircle.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableHitCircle.cs index 932f6d3fff..6beed0294d 100644 --- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableHitCircle.cs +++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableHitCircle.cs @@ -261,7 +261,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables case OsuAction.RightButton: if (IsHovered && (Hit?.Invoke() ?? false)) { - HitAction = e.Action; + HitAction ??= e.Action; return true; } diff --git a/osu.Game/Skinning/LegacyBeatmapSkin.cs b/osu.Game/Skinning/LegacyBeatmapSkin.cs index 83dcc3895d..90eb5fa013 100644 --- a/osu.Game/Skinning/LegacyBeatmapSkin.cs +++ b/osu.Game/Skinning/LegacyBeatmapSkin.cs @@ -19,7 +19,7 @@ namespace osu.Game.Skinning { public class LegacyBeatmapSkin : LegacySkin { - protected override bool AllowManiaSkin => false; + protected override bool AllowManiaConfigLookups => false; protected override bool UseCustomSampleBanks => true; /// diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index 98e2d3e862..76190d0abe 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -30,13 +30,7 @@ namespace osu.Game.Skinning { public class LegacySkin : Skin { - /// - /// Whether texture for the keys exists. - /// Used to determine if the mania ruleset is skinned. - /// - private readonly Lazy hasKeyTexture; - - protected virtual bool AllowManiaSkin => hasKeyTexture.Value; + protected virtual bool AllowManiaConfigLookups => true; /// /// Whether this skin can use samples with a custom bank (custom sample set in stable terminology). @@ -62,10 +56,6 @@ namespace osu.Game.Skinning protected LegacySkin(SkinInfo skin, IStorageResourceProvider? resources, IResourceStore? storage, string configurationFilename = @"skin.ini") : base(skin, resources, storage, configurationFilename) { - // todo: this shouldn't really be duplicated here (from ManiaLegacySkinTransformer). we need to come up with a better solution. - hasKeyTexture = new Lazy(() => this.GetAnimation( - lookupForMania(new LegacyManiaSkinConfigurationLookup(4, LegacyManiaSkinConfigurationLookups.KeyImage, 0))?.Value ?? "mania-key1", true, - true) != null); } protected override void ParseConfigurationStream(Stream stream) @@ -115,7 +105,7 @@ namespace osu.Game.Skinning return SkinUtils.As(getCustomColour(Configuration, customColour.Lookup.ToString() ?? string.Empty)); case LegacyManiaSkinConfigurationLookup maniaLookup: - if (!AllowManiaSkin) + if (!AllowManiaConfigLookups) break; var result = lookupForMania(maniaLookup);