Merge branch 'master' into overlays/profile/previous-usernames

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<OsuHitObject>
+            {
+                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<ReplayFrame>
+            {
+                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<OsuHitObject>
+            {
+                new HitCircle
+                {
+                    StartTime = time_first_circle,
+                    Position = positionFirstCircle,
+                },
+                new HitCircle
+                {
+                    StartTime = time_second_circle,
+                    Position = positionSecondCircle,
+                },
+            };
+
+            performTest(hitObjects, new List<ReplayFrame>
+            {
+                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 },
             });

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;
             }
         }
 
+        /// <summary>
+        /// 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".
+        /// </summary>
+        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) =>

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;
                         }
 

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;
 
         /// <summary>

osu.Game/Skinning/LegacySkin.cs

@@ -30,13 +30,7 @@ namespace osu.Game.Skinning
 {
     public class LegacySkin : Skin
     {
-        /// <summary>
+        protected virtual bool AllowManiaConfigLookups => true;
-        /// Whether texture for the keys exists.
-        /// Used to determine if the mania ruleset is skinned.
-        /// </summary>
-        private readonly Lazy<bool> hasKeyTexture;
-
-        protected virtual bool AllowManiaSkin => hasKeyTexture.Value;
 
         /// <summary>
         /// 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<byte[]>? 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<bool>(() => this.GetAnimation(
-                lookupForMania<string>(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<TValue>(getCustomColour(Configuration, customColour.Lookup.ToString() ?? string.Empty));
 
                     case LegacyManiaSkinConfigurationLookup maniaLookup:
-                        if (!AllowManiaSkin)
+                        if (!AllowManiaConfigLookups)
                             break;
 
                         var result = lookupForMania<TValue>(maniaLookup);