Merge branch 'master' into beatmapskin-interface

osu.Game.Rulesets.Catch/Skinning/CatchLegacySkinTransformer.cs

@@ -6,6 +6,7 @@
 using osu.Framework.Graphics;
 using osu.Game.Skinning;
 using osuTK;
+using osuTK.Graphics;
 
 namespace osu.Game.Rulesets.Catch.Skinning
 {
@@ -61,7 +62,12 @@ public override IBindable<TValue> GetConfig<TLookup, TValue>(TLookup lookup)
             switch (lookup)
             {
                 case CatchSkinColour colour:
-                    return Source.GetConfig<SkinCustomColourLookup, TValue>(new SkinCustomColourLookup(colour));
+                    var result = (Bindable<Color4>)Source.GetConfig<SkinCustomColourLookup, TValue>(new SkinCustomColourLookup(colour));
+                    if (result == null)
+                        return null;
+
+                    result.Value = LegacyColourCompatibility.DisallowZeroAlpha(result.Value);
+                    return (IBindable<TValue>)result;
             }
 
             return Source.GetConfig<TLookup, TValue>(lookup);

osu.Game.Rulesets.Catch/Skinning/LegacyFruitPiece.cs

@@ -40,7 +40,6 @@ private void load(DrawableHitObject drawableObject, ISkinSource skin)
                 colouredSprite = new Sprite
                 {
                     Texture = skin.GetTexture(lookupName),
-                    Colour = drawableObject.AccentColour.Value,
                     Anchor = Anchor.Centre,
                     Origin = Anchor.Centre,
                 },
@@ -76,7 +75,7 @@ protected override void LoadComplete()
         {
             base.LoadComplete();
 
-            accentColour.BindValueChanged(colour => colouredSprite.Colour = colour.NewValue, true);
+            accentColour.BindValueChanged(colour => colouredSprite.Colour = LegacyColourCompatibility.DisallowZeroAlpha(colour.NewValue), true);
         }
     }
 }

osu.Game.Rulesets.Mania/Skinning/LegacyColumnBackground.cs

@@ -49,7 +49,7 @@ private void load(ISkinSource skin, IScrollingInfo scrollingInfo)
                     {
                         Anchor = Anchor.BottomCentre,
                         Origin = Anchor.BottomCentre,
-                        Colour = lightColour,
+                        Colour = LegacyColourCompatibility.DisallowZeroAlpha(lightColour),
                         Texture = skin.GetTexture(lightImage),
                         RelativeSizeAxes = Axes.X,
                         Width = 1,

osu.Game.Rulesets.Mania/Skinning/LegacyHitTarget.cs

@@ -51,7 +51,7 @@ private void load(ISkinSource skin, IScrollingInfo scrollingInfo)
                         Anchor = Anchor.CentreLeft,
                         RelativeSizeAxes = Axes.X,
                         Height = 1,
-                        Colour = lineColour,
+                        Colour = LegacyColourCompatibility.DisallowZeroAlpha(lineColour),
                         Alpha = showJudgementLine ? 0.9f : 0
                     }
                 }

osu.Game.Rulesets.Mania/Skinning/LegacyStageBackground.cs

@@ -106,37 +106,38 @@ private void load(ISkinSource skin)
 
                 InternalChildren = new Drawable[]
                 {
-                    new Container
+                    LegacyColourCompatibility.ApplyWithDoubledAlpha(new Box
                     {
-                        RelativeSizeAxes = Axes.Both,
-                        Child = new Box
-                        {
-                            RelativeSizeAxes = Axes.Both,
-                            Colour = backgroundColour
-                        },
-                    },
+                        RelativeSizeAxes = Axes.Both
+                    }, backgroundColour),
                     new HitTargetInsetContainer
                     {
                         RelativeSizeAxes = Axes.Both,
                         Children = new[]
                         {
-                            new Box
+                            new Container
                             {
                                 RelativeSizeAxes = Axes.Y,
                                 Width = leftLineWidth,
                                 Scale = new Vector2(0.740f, 1),
-                                Colour = lineColour,
-                                Alpha = hasLeftLine ? 1 : 0
+                                Alpha = hasLeftLine ? 1 : 0,
+                                Child = LegacyColourCompatibility.ApplyWithDoubledAlpha(new Box
+                                {
+                                    RelativeSizeAxes = Axes.Both
+                                }, lineColour)
                             },
-                            new Box
+                            new Container
                             {
                                 Anchor = Anchor.TopRight,
                                 Origin = Anchor.TopRight,
                                 RelativeSizeAxes = Axes.Y,
                                 Width = rightLineWidth,
                                 Scale = new Vector2(0.740f, 1),
-                                Colour = lineColour,
-                                Alpha = hasRightLine ? 1 : 0
+                                Alpha = hasRightLine ? 1 : 0,
+                                Child = LegacyColourCompatibility.ApplyWithDoubledAlpha(new Box
+                                {
+                                    RelativeSizeAxes = Axes.Both
+                                }, lineColour)
                             },
                         }
                     }

osu.Game.Rulesets.Osu.Tests/TestSceneSpinner.cs

@@ -9,6 +9,7 @@
 using osu.Game.Rulesets.Mods;
 using osu.Game.Rulesets.Osu.Objects;
 using osu.Game.Rulesets.Osu.Objects.Drawables;
+using osuTK;
 
 namespace osu.Game.Rulesets.Osu.Tests
 {
@@ -62,7 +63,8 @@ private Drawable testSingle(float circleSize, bool auto = false, double length =
             drawableSpinner = new TestDrawableSpinner(spinner, auto)
             {
                 Anchor = Anchor.Centre,
-                Depth = depthIndex++
+                Depth = depthIndex++,
+                Scale = new Vector2(0.75f)
             };
 
             foreach (var mod in SelectedMods.Value.OfType<IApplicableToDrawableHitObjects>())

osu.Game.Rulesets.Osu/Skinning/LegacyMainCirclePiece.cs

@@ -59,7 +59,6 @@ private void load(DrawableHitObject drawableObject)
                         hitCircleSprite = new Sprite
                         {
                             Texture = getTextureWithFallback(string.Empty),
-                            Colour = drawableObject.AccentColour.Value,
                             Anchor = Anchor.Centre,
                             Origin = Anchor.Centre,
                         },
@@ -107,7 +106,7 @@ protected override void LoadComplete()
             base.LoadComplete();
 
             state.BindValueChanged(updateState, true);
-            accentColour.BindValueChanged(colour => hitCircleSprite.Colour = colour.NewValue, true);
+            accentColour.BindValueChanged(colour => hitCircleSprite.Colour = LegacyColourCompatibility.DisallowZeroAlpha(colour.NewValue), true);
             indexInCurrentCombo.BindValueChanged(index => hitCircleText.Text = (index.NewValue + 1).ToString(), true);
         }
 

osu.Game.Rulesets.Osu/Skinning/LegacyOldStyleSpinner.cs

@@ -22,11 +22,11 @@ public class LegacyOldStyleSpinner : CompositeDrawable
     {
         private DrawableSpinner drawableSpinner;
         private Sprite disc;
+        private Sprite metreSprite;
         private Container metre;
 
-        private const float background_y_offset = 20;
-
         private const float sprite_scale = 1 / 1.6f;
+        private const float final_metre_height = 692 * sprite_scale;
 
         [BackgroundDependencyLoader]
         private void load(ISkinSource source, DrawableHitObject drawableObject)
@@ -35,50 +35,58 @@ private void load(ISkinSource source, DrawableHitObject drawableObject)
 
             RelativeSizeAxes = Axes.Both;
 
-            InternalChildren = new Drawable[]
+            InternalChild = new Container
             {
-                new Sprite
+                // the old-style spinner relied heavily on absolute screen-space coordinate values.
+                // wrap everything in a container simulating absolute coords to preserve alignment
+                // as there are skins that depend on it.
+                Width = 640,
+                Height = 480,
+                Anchor = Anchor.Centre,
+                Origin = Anchor.Centre,
+                Children = new Drawable[]
                 {
-                    Anchor = Anchor.BottomCentre,
-                    Origin = Anchor.BottomCentre,
-                    Texture = source.GetTexture("spinner-background"),
-                    Y = background_y_offset,
-                    Scale = new Vector2(sprite_scale)
-                },
-                disc = new Sprite
-                {
-                    Anchor = Anchor.Centre,
-                    Origin = Anchor.Centre,
-                    Texture = source.GetTexture("spinner-circle"),
-                    Scale = new Vector2(sprite_scale)
-                },
-                metre = new Container
-                {
-                    Anchor = Anchor.BottomCentre,
-                    Origin = Anchor.BottomCentre,
-                    Y = background_y_offset,
-                    Masking = true,
-                    Child = new Sprite
+                    new Sprite
                     {
-                        Texture = source.GetTexture("spinner-metre"),
-                        Anchor = Anchor.BottomCentre,
-                        Origin = Anchor.BottomCentre,
+                        Anchor = Anchor.Centre,
+                        Origin = Anchor.Centre,
+                        Texture = source.GetTexture("spinner-background"),
+                        Scale = new Vector2(sprite_scale)
                     },
-                    Scale = new Vector2(0.625f)
+                    disc = new Sprite
+                    {
+                        Anchor = Anchor.Centre,
+                        Origin = Anchor.Centre,
+                        Texture = source.GetTexture("spinner-circle"),
+                        Scale = new Vector2(sprite_scale)
+                    },
+                    metre = new Container
+                    {
+                        AutoSizeAxes = Axes.Both,
+                        // this anchor makes no sense, but that's what stable uses.
+                        Anchor = Anchor.TopLeft,
+                        Origin = Anchor.TopLeft,
+                        // adjustment for stable (metre has additional offset)
+                        Margin = new MarginPadding { Top = 20 },
+                        Masking = true,
+                        Child = metreSprite = new Sprite
+                        {
+                            Texture = source.GetTexture("spinner-metre"),
+                            Anchor = Anchor.TopLeft,
+                            Origin = Anchor.TopLeft,
+                            Scale = new Vector2(0.625f)
+                        }
+                    }
                 }
             };
         }
 
-        private Vector2 metreFinalSize;
-
         protected override void LoadComplete()
         {
             base.LoadComplete();
 
             this.FadeOut();
             drawableSpinner.State.BindValueChanged(updateStateTransforms, true);
-
-            metreFinalSize = metre.Size = metre.Child.Size;
         }
 
         private void updateStateTransforms(ValueChangedEvent<ArmedState> state)
@@ -93,7 +101,16 @@ protected override void Update()
         {
             base.Update();
             disc.Rotation = drawableSpinner.RotationTracker.Rotation;
-            metre.Height = getMetreHeight(drawableSpinner.Progress);
+
+            // careful: need to call this exactly once for all calculations in a frame
+            // as the function has a random factor in it
+            var metreHeight = getMetreHeight(drawableSpinner.Progress);
+
+            // hack to make the metre blink up from below than down from above.
+            // move down the container to be able to apply masking for the metre,
+            // and then move the sprite back up the same amount to keep its position absolute.
+            metre.Y = final_metre_height - metreHeight;
+            metreSprite.Y = -metre.Y;
         }
 
         private const int total_bars = 10;
@@ -108,7 +125,7 @@ private float getMetreHeight(float progress)
             if (RNG.NextBool(((int)progress % 10) / 10f))
                 barCount++;
 
-            return (float)barCount / total_bars * metreFinalSize.Y;
+            return (float)barCount / total_bars * final_metre_height;
         }
     }
 }

osu.Game.Rulesets.Osu/Skinning/LegacySliderBall.cs

@@ -28,7 +28,7 @@ public LegacySliderBall(Drawable animationContent)
         [BackgroundDependencyLoader]
         private void load(ISkinSource skin, DrawableHitObject drawableObject)
         {
-            animationContent.Colour = skin.GetConfig<OsuSkinColour, Color4>(OsuSkinColour.SliderBall)?.Value ?? Color4.White;
+            var ballColour = skin.GetConfig<OsuSkinColour, Color4>(OsuSkinColour.SliderBall)?.Value ?? Color4.White;
 
             InternalChildren = new[]
             {
@@ -39,11 +39,11 @@ private void load(ISkinSource skin, DrawableHitObject drawableObject)
                     Texture = skin.GetTexture("sliderb-nd"),
                     Colour = new Color4(5, 5, 5, 255),
                 },
-                animationContent.With(d =>
+                LegacyColourCompatibility.ApplyWithDoubledAlpha(animationContent.With(d =>
                 {
                     d.Anchor = Anchor.Centre;
                     d.Origin = Anchor.Centre;
-                }),
+                }), ballColour),
                 layerSpec = new Sprite
                 {
                     Anchor = Anchor.Centre,

osu.Game.Rulesets.Taiko/Skinning/LegacyCirclePiece.cs

@@ -90,7 +90,7 @@ public Color4 AccentColour
 
         private void updateAccentColour()
         {
-            backgroundLayer.Colour = accentColour;
+            backgroundLayer.Colour = LegacyColourCompatibility.DisallowZeroAlpha(accentColour);
         }
     }
 }

osu.Game.Rulesets.Taiko/Skinning/LegacyDrumRoll.cs

@@ -76,9 +76,11 @@ public Color4 AccentColour
 
         private void updateAccentColour()
         {
-            headCircle.AccentColour = accentColour;
-            body.Colour = accentColour;
-            end.Colour = accentColour;
+            var colour = LegacyColourCompatibility.DisallowZeroAlpha(accentColour);
+
+            headCircle.AccentColour = colour;
+            body.Colour = colour;
+            end.Colour = colour;
         }
     }
 }

osu.Game.Rulesets.Taiko/Skinning/LegacyHit.cs

@@ -2,6 +2,7 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using osu.Framework.Allocation;
+using osu.Game.Skinning;
 using osuTK.Graphics;
 
 namespace osu.Game.Rulesets.Taiko.Skinning
@@ -18,9 +19,10 @@ public LegacyHit(TaikoSkinComponents component)
         [BackgroundDependencyLoader]
         private void load()
         {
-            AccentColour = component == TaikoSkinComponents.CentreHit
-                ? new Color4(235, 69, 44, 255)
-                : new Color4(67, 142, 172, 255);
+            AccentColour = LegacyColourCompatibility.DisallowZeroAlpha(
+                component == TaikoSkinComponents.CentreHit
+                    ? new Color4(235, 69, 44, 255)
+                    : new Color4(67, 142, 172, 255));
         }
     }
 }

osu.Game.Tests/Resources/skin-zero-alpha-colour.ini

@@ -1,5 +0,0 @@
-[General]
-Version: latest
-
-[Colours]
-Combo1: 255,255,255,0

osu.Game.Tests/Skins/LegacySkinDecoderTest.cs

@@ -108,15 +108,5 @@ public void TestDecodeNoVersion()
             using (var stream = new LineBufferedReader(resStream))
                 Assert.That(decoder.Decode(stream).LegacyVersion, Is.EqualTo(1.0m));
         }
-
-        [Test]
-        public void TestDecodeColourWithZeroAlpha()
-        {
-            var decoder = new LegacySkinDecoder();
-
-            using (var resStream = TestResources.OpenResource("skin-zero-alpha-colour.ini"))
-            using (var stream = new LineBufferedReader(resStream))
-                Assert.That(decoder.Decode(stream).ComboColours[0].A, Is.EqualTo(1.0f));
-        }
     }
 }

osu.Game/Beatmaps/Formats/LegacyDecoder.cs

@@ -104,10 +104,6 @@ protected void HandleColours<TModel>(TModel output, string line)
             try
             {
                 byte alpha = split.Length == 4 ? byte.Parse(split[3]) : (byte)255;
-
-                if (alpha == 0)
-                    alpha = 255;
-
                 colour = new Color4(byte.Parse(split[0]), byte.Parse(split[1]), byte.Parse(split[2]), alpha);
             }
             catch

osu.Game/Skinning/LegacyColourCompatibility.cs

@@ -0,0 +1,46 @@
+// 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.Graphics;
+using osuTK.Graphics;
+
+namespace osu.Game.Skinning
+{
+    /// <summary>
+    /// Compatibility methods to convert osu!stable colours to osu!lazer-compatible ones. Should be used for legacy skins only.
+    /// </summary>
+    public static class LegacyColourCompatibility
+    {
+        /// <summary>
+        /// Forces an alpha of 1 if a given <see cref="Color4"/> is fully transparent.
+        /// </summary>
+        /// <remarks>
+        /// This is equivalent to setting colour post-constructor in osu!stable.
+        /// </remarks>
+        /// <param name="colour">The <see cref="Color4"/> to disallow zero alpha on.</param>
+        /// <returns>The resultant <see cref="Color4"/>.</returns>
+        public static Color4 DisallowZeroAlpha(Color4 colour)
+        {
+            if (colour.A == 0)
+                colour.A = 1;
+            return colour;
+        }
+
+        /// <summary>
+        /// Applies a <see cref="Color4"/> to a <see cref="Drawable"/>, doubling the alpha value into the <see cref="Drawable.Alpha"/> property.
+        /// </summary>
+        /// <remarks>
+        /// This is equivalent to setting colour in the constructor in osu!stable.
+        /// </remarks>
+        /// <param name="drawable">The <see cref="Drawable"/> to apply the colour to.</param>
+        /// <param name="colour">The <see cref="Color4"/> to apply.</param>
+        /// <returns>The given <paramref name="drawable"/>.</returns>
+        public static T ApplyWithDoubledAlpha<T>(T drawable, Color4 colour)
+            where T : Drawable
+        {
+            drawable.Alpha = colour.A;
+            drawable.Colour = DisallowZeroAlpha(colour);
+            return drawable;
+        }
+    }
+}