From 738ce0f6894953445cd9287ff00fbdde8a0454c4 Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Wed, 1 Sep 2021 19:34:57 +0900
Subject: [PATCH 1/5] Fix repeat arrows being hidden beneath head circles in
 legacy skins

Aims to make minimal changes to `DrawableSlider` itself. I'm not super
happy about the slider ball being moved above the head circle, but it
*is* what people are used to so no one except for me is going to
complain.

Supersedes and closes https://github.com/ppy/osu/pull/14561.
---
 .../Objects/Drawables/DrawableSlider.cs       | 11 ++++-
 .../Objects/Drawables/DrawableSliderHead.cs   |  2 +-
 .../Objects/Drawables/DrawableSliderRepeat.cs |  2 +-
 .../Skinning/Legacy/LegacyMainCirclePiece.cs  | 40 +++++++----------
 .../Skinning/Legacy/LegacyReverseArrow.cs     | 44 +++++++++++++++++++
 .../Legacy/LegacySliderHeadHitCircle.cs       | 30 +++++++++++++
 .../Legacy/OsuLegacySkinTransformer.cs        |  8 +++-
 7 files changed, 110 insertions(+), 27 deletions(-)
 create mode 100644 osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyReverseArrow.cs
 create mode 100644 osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderHeadHitCircle.cs

diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs
index 0bec33bf77..0e1d1043e3 100644
--- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs
+++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs
@@ -32,6 +32,12 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
         public SliderBall Ball { get; private set; }
         public SkinnableDrawable Body { get; private set; }
 
+        /// <summary>
+        /// A target container which can be used to add top level elements to the slider's display.
+        /// Intended to be used for proxy purposes only.
+        /// </summary>
+        public Container OverlayElementContainer { get; private set; }
+
         public override bool DisplayResult => !HitObject.OnlyJudgeNestedObjects;
 
         [CanBeNull]
@@ -65,6 +71,8 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
                 tailContainer = new Container<DrawableSliderTail> { RelativeSizeAxes = Axes.Both },
                 tickContainer = new Container<DrawableSliderTick> { RelativeSizeAxes = Axes.Both },
                 repeatContainer = new Container<DrawableSliderRepeat> { RelativeSizeAxes = Axes.Both },
+                headContainer = new Container<DrawableSliderHead> { RelativeSizeAxes = Axes.Both },
+                OverlayElementContainer = new Container { RelativeSizeAxes = Axes.Both, },
                 Ball = new SliderBall(this)
                 {
                     GetInitialHitAction = () => HeadCircle.HitAction,
@@ -72,7 +80,6 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
                     AlwaysPresent = true,
                     Alpha = 0
                 },
-                headContainer = new Container<DrawableSliderHead> { RelativeSizeAxes = Axes.Both },
                 slidingSample = new PausableSkinnableSound { Looping = true }
             };
 
@@ -179,6 +186,8 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
             tailContainer.Clear(false);
             repeatContainer.Clear(false);
             tickContainer.Clear(false);
+
+            OverlayElementContainer.Clear();
         }
 
         protected override DrawableHitObject CreateNestedHitObject(HitObject hitObject)
diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderHead.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderHead.cs
index 01c0d988ee..2b026e6840 100644
--- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderHead.cs
+++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderHead.cs
@@ -18,7 +18,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
         [CanBeNull]
         public Slider Slider => DrawableSlider?.HitObject;
 
-        protected DrawableSlider DrawableSlider => (DrawableSlider)ParentHitObject;
+        public DrawableSlider DrawableSlider => (DrawableSlider)ParentHitObject;
 
         public override bool DisplayResult => HitObject?.JudgeAsNormalHitCircle ?? base.DisplayResult;
 
diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderRepeat.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderRepeat.cs
index 4a2a18ffd6..673211ac6c 100644
--- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderRepeat.cs
+++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSliderRepeat.cs
@@ -22,7 +22,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
         [CanBeNull]
         public Slider Slider => DrawableSlider?.HitObject;
 
-        protected DrawableSlider DrawableSlider => (DrawableSlider)ParentHitObject;
+        public DrawableSlider DrawableSlider => (DrawableSlider)ParentHitObject;
 
         private double animDuration;
 
diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyMainCirclePiece.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyMainCirclePiece.cs
index 7a210324d7..3afd814174 100644
--- a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyMainCirclePiece.cs
+++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyMainCirclePiece.cs
@@ -33,9 +33,9 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy
             Size = new Vector2(OsuHitObject.OBJECT_RADIUS * 2);
         }
 
-        private Container circleSprites;
         private Drawable hitCircleSprite;
-        private Drawable hitCircleOverlay;
+
+        protected Drawable HitCircleOverlay { get; private set; }
 
         private SkinnableSpriteText hitCircleText;
 
@@ -70,28 +70,19 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy
             // expected behaviour in this scenario is not showing the overlay, rather than using hitcircleoverlay.png (potentially from the default/fall-through skin).
             Texture overlayTexture = getTextureWithFallback("overlay");
 
-            InternalChildren = new Drawable[]
+            InternalChildren = new[]
             {
-                circleSprites = new Container
+                hitCircleSprite = new KiaiFlashingSprite
                 {
+                    Texture = baseTexture,
+                    Anchor = Anchor.Centre,
+                    Origin = Anchor.Centre,
+                },
+                HitCircleOverlay = new KiaiFlashingSprite
+                {
+                    Texture = overlayTexture,
                     Anchor = Anchor.Centre,
                     Origin = Anchor.Centre,
-                    RelativeSizeAxes = Axes.Both,
-                    Children = new[]
-                    {
-                        hitCircleSprite = new KiaiFlashingSprite
-                        {
-                            Texture = baseTexture,
-                            Anchor = Anchor.Centre,
-                            Origin = Anchor.Centre,
-                        },
-                        hitCircleOverlay = new KiaiFlashingSprite
-                        {
-                            Texture = overlayTexture,
-                            Anchor = Anchor.Centre,
-                            Origin = Anchor.Centre,
-                        }
-                    }
                 },
             };
 
@@ -111,7 +102,7 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy
             bool overlayAboveNumber = skin.GetConfig<OsuSkinConfiguration, bool>(OsuSkinConfiguration.HitCircleOverlayAboveNumber)?.Value ?? true;
 
             if (overlayAboveNumber)
-                AddInternal(hitCircleOverlay.CreateProxy());
+                ChangeInternalChildDepth(HitCircleOverlay, float.MinValue);
 
             accentColour.BindTo(drawableObject.AccentColour);
             indexInCurrentCombo.BindTo(drawableOsuObject.IndexInCurrentComboBindable);
@@ -153,8 +144,11 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy
                 switch (state)
                 {
                     case ArmedState.Hit:
-                        circleSprites.FadeOut(legacy_fade_duration, Easing.Out);
-                        circleSprites.ScaleTo(1.4f, legacy_fade_duration, Easing.Out);
+                        hitCircleSprite.FadeOut(legacy_fade_duration, Easing.Out);
+                        hitCircleSprite.ScaleTo(1.4f, legacy_fade_duration, Easing.Out);
+
+                        HitCircleOverlay.FadeOut(legacy_fade_duration, Easing.Out);
+                        HitCircleOverlay.ScaleTo(1.4f, legacy_fade_duration, Easing.Out);
 
                         if (hasNumber)
                         {
diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyReverseArrow.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyReverseArrow.cs
new file mode 100644
index 0000000000..b6956693b6
--- /dev/null
+++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyReverseArrow.cs
@@ -0,0 +1,44 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using osu.Framework.Allocation;
+using osu.Framework.Graphics;
+using osu.Framework.Graphics.Containers;
+using osu.Game.Rulesets.Objects.Drawables;
+using osu.Game.Rulesets.Osu.Objects.Drawables;
+using osu.Game.Skinning;
+
+namespace osu.Game.Rulesets.Osu.Skinning.Legacy
+{
+    public class LegacyReverseArrow : CompositeDrawable
+    {
+        private ISkin skin { get; set; }
+
+        [Resolved(canBeNull: true)]
+        private DrawableHitObject drawableHitObject { get; set; }
+
+        public LegacyReverseArrow(ISkin skin)
+        {
+            this.skin = skin;
+        }
+
+        [BackgroundDependencyLoader]
+        private void load()
+        {
+            AutoSizeAxes = Axes.Both;
+
+            string lookupName = new OsuSkinComponent(OsuSkinComponents.ReverseArrow).LookupName;
+
+            InternalChild = skin.GetAnimation(lookupName, true, true) ?? Drawable.Empty();
+        }
+
+        protected override void LoadComplete()
+        {
+            base.LoadComplete();
+
+            // see logic in LegacySliderHeadHitCircle.
+            (drawableHitObject as DrawableSliderRepeat)?.DrawableSlider
+                                                       .OverlayElementContainer.Add(CreateProxy());
+        }
+    }
+}
diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderHeadHitCircle.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderHeadHitCircle.cs
new file mode 100644
index 0000000000..83ebdafa50
--- /dev/null
+++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderHeadHitCircle.cs
@@ -0,0 +1,30 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using osu.Framework.Allocation;
+using osu.Framework.Graphics;
+using osu.Game.Rulesets.Objects.Drawables;
+using osu.Game.Rulesets.Osu.Objects.Drawables;
+
+namespace osu.Game.Rulesets.Osu.Skinning.Legacy
+{
+    public class LegacySliderHeadHitCircle : LegacyMainCirclePiece
+    {
+        [Resolved(canBeNull: true)]
+        private DrawableHitObject drawableHitObject { get; set; }
+
+        public LegacySliderHeadHitCircle()
+            : base("sliderstartcircle")
+        {
+        }
+
+        protected override void LoadComplete()
+        {
+            base.LoadComplete();
+
+            // see logic in LegacyReverseArrow.
+            (drawableHitObject as DrawableSliderHead)?.DrawableSlider
+                                                     .OverlayElementContainer.Add(HitCircleOverlay.CreateProxy().With(d => d.Depth = float.MinValue));
+        }
+    }
+}
diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs
index 41b0a88f11..8df8001d42 100644
--- a/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs
+++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/OsuLegacySkinTransformer.cs
@@ -67,7 +67,13 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy
 
                     case OsuSkinComponents.SliderHeadHitCircle:
                         if (hasHitCircle.Value)
-                            return new LegacyMainCirclePiece("sliderstartcircle");
+                            return new LegacySliderHeadHitCircle();
+
+                        return null;
+
+                    case OsuSkinComponents.ReverseArrow:
+                        if (hasHitCircle.Value)
+                            return new LegacyReverseArrow(this);
 
                         return null;
 

From 79438c19a45eecb5c0983e6a14efb98539e3686c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= <dach.bartlomiej@gmail.com>
Date: Sat, 18 Sep 2021 16:27:30 +0200
Subject: [PATCH 2/5] Fix slider parts not reproxying after first hitobject
 freed

---
 .../Objects/Drawables/DrawableSlider.cs       |  2 +-
 .../Skinning/Legacy/LegacyReverseArrow.cs     | 29 +++++++++++++++++--
 .../Legacy/LegacySliderHeadHitCircle.cs       | 27 +++++++++++++++--
 3 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs
index 0e1d1043e3..3acec4498d 100644
--- a/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs
+++ b/osu.Game.Rulesets.Osu/Objects/Drawables/DrawableSlider.cs
@@ -187,7 +187,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
             repeatContainer.Clear(false);
             tickContainer.Clear(false);
 
-            OverlayElementContainer.Clear();
+            OverlayElementContainer.Clear(false);
         }
 
         protected override DrawableHitObject CreateNestedHitObject(HitObject hitObject)
diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyReverseArrow.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyReverseArrow.cs
index b6956693b6..9e4bd258bd 100644
--- a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyReverseArrow.cs
+++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyReverseArrow.cs
@@ -1,6 +1,7 @@
 // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
+using System.Diagnostics;
 using osu.Framework.Allocation;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
@@ -17,12 +18,14 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy
         [Resolved(canBeNull: true)]
         private DrawableHitObject drawableHitObject { get; set; }
 
+        private Drawable proxy;
+
         public LegacyReverseArrow(ISkin skin)
         {
             this.skin = skin;
         }
 
-        [BackgroundDependencyLoader]
+        [BackgroundDependencyLoader(true)]
         private void load()
         {
             AutoSizeAxes = Axes.Both;
@@ -36,9 +39,29 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy
         {
             base.LoadComplete();
 
+            proxy = CreateProxy();
+
+            if (drawableHitObject != null)
+            {
+                drawableHitObject.HitObjectApplied += onHitObjectApplied;
+                onHitObjectApplied(drawableHitObject);
+            }
+        }
+
+        private void onHitObjectApplied(DrawableHitObject drawableObject)
+        {
+            Debug.Assert(proxy.Parent == null);
+
             // see logic in LegacySliderHeadHitCircle.
-            (drawableHitObject as DrawableSliderRepeat)?.DrawableSlider
-                                                       .OverlayElementContainer.Add(CreateProxy());
+            (drawableObject as DrawableSliderRepeat)?.DrawableSlider
+                                                    .OverlayElementContainer.Add(proxy);
+        }
+
+        protected override void Dispose(bool isDisposing)
+        {
+            base.Dispose(isDisposing);
+            if (drawableHitObject != null)
+                drawableHitObject.HitObjectApplied -= onHitObjectApplied;
         }
     }
 }
diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderHeadHitCircle.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderHeadHitCircle.cs
index 83ebdafa50..13ba42ba50 100644
--- a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderHeadHitCircle.cs
+++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacySliderHeadHitCircle.cs
@@ -1,6 +1,7 @@
 // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
+using System.Diagnostics;
 using osu.Framework.Allocation;
 using osu.Framework.Graphics;
 using osu.Game.Rulesets.Objects.Drawables;
@@ -13,6 +14,8 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy
         [Resolved(canBeNull: true)]
         private DrawableHitObject drawableHitObject { get; set; }
 
+        private Drawable proxiedHitCircleOverlay;
+
         public LegacySliderHeadHitCircle()
             : base("sliderstartcircle")
         {
@@ -21,10 +24,30 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy
         protected override void LoadComplete()
         {
             base.LoadComplete();
+            proxiedHitCircleOverlay = HitCircleOverlay.CreateProxy();
+
+            if (drawableHitObject != null)
+            {
+                drawableHitObject.HitObjectApplied += onHitObjectApplied;
+                onHitObjectApplied(drawableHitObject);
+            }
+        }
+
+        private void onHitObjectApplied(DrawableHitObject drawableObject)
+        {
+            Debug.Assert(proxiedHitCircleOverlay.Parent == null);
 
             // see logic in LegacyReverseArrow.
-            (drawableHitObject as DrawableSliderHead)?.DrawableSlider
-                                                     .OverlayElementContainer.Add(HitCircleOverlay.CreateProxy().With(d => d.Depth = float.MinValue));
+            (drawableObject as DrawableSliderHead)?.DrawableSlider
+                                                  .OverlayElementContainer.Add(proxiedHitCircleOverlay.With(d => d.Depth = float.MinValue));
+        }
+
+        protected override void Dispose(bool isDisposing)
+        {
+            base.Dispose(isDisposing);
+
+            if (drawableHitObject != null)
+                drawableHitObject.HitObjectApplied -= onHitObjectApplied;
         }
     }
 }

From 59657aca9a5e42bef22d93ceb8053557f8bbf8cd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= <dach.bartlomiej@gmail.com>
Date: Sat, 18 Sep 2021 16:28:25 +0200
Subject: [PATCH 3/5] Remove redundant qualifier

---
 osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyReverseArrow.cs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyReverseArrow.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyReverseArrow.cs
index 9e4bd258bd..298079fa6c 100644
--- a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyReverseArrow.cs
+++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyReverseArrow.cs
@@ -32,7 +32,7 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy
 
             string lookupName = new OsuSkinComponent(OsuSkinComponents.ReverseArrow).LookupName;
 
-            InternalChild = skin.GetAnimation(lookupName, true, true) ?? Drawable.Empty();
+            InternalChild = skin.GetAnimation(lookupName, true, true) ?? Empty();
         }
 
         protected override void LoadComplete()

From c23354bb67fc63d54e490d195436d5adfa1441a4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= <dach.bartlomiej@gmail.com>
Date: Sat, 18 Sep 2021 16:28:44 +0200
Subject: [PATCH 4/5] Remove unused setter

---
 osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyReverseArrow.cs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyReverseArrow.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyReverseArrow.cs
index 298079fa6c..ab774a2b67 100644
--- a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyReverseArrow.cs
+++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyReverseArrow.cs
@@ -13,7 +13,7 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy
 {
     public class LegacyReverseArrow : CompositeDrawable
     {
-        private ISkin skin { get; set; }
+        private ISkin skin { get; }
 
         [Resolved(canBeNull: true)]
         private DrawableHitObject drawableHitObject { get; set; }

From 36237398fa307f6939306df065e897b4683cf740 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= <dach.bartlomiej@gmail.com>
Date: Sat, 18 Sep 2021 18:24:36 +0200
Subject: [PATCH 5/5] Remove accidental leftover nullable BDL spec

---
 osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyReverseArrow.cs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyReverseArrow.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyReverseArrow.cs
index ab774a2b67..fd7bfe7e60 100644
--- a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyReverseArrow.cs
+++ b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyReverseArrow.cs
@@ -25,7 +25,7 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy
             this.skin = skin;
         }
 
-        [BackgroundDependencyLoader(true)]
+        [BackgroundDependencyLoader]
         private void load()
         {
             AutoSizeAxes = Axes.Both;