Merge branch 'master' into fix-replay-download-button-always-being-disabled

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

@@ -110,7 +110,7 @@ namespace osu.Game.Rulesets.Osu.Tests
                 this.hasColours = hasColours;
             }
 
-            protected override ISkin GetSkin() => new TestBeatmapSkin(BeatmapInfo, hasColours);
+            protected override IBeatmapSkin GetSkin() => new TestBeatmapSkin(BeatmapInfo, hasColours);
         }
 
         private class TestBeatmapSkin : LegacyBeatmapSkin

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

@@ -29,12 +29,12 @@ namespace osu.Game.Rulesets.Osu.Tests
     public class TestSceneSkinFallbacks : TestSceneOsuPlayer
     {
         private readonly TestSource testUserSkin;
-        private readonly TestSource testBeatmapSkin;
+        private readonly BeatmapTestSource testBeatmapSkin;
 
         public TestSceneSkinFallbacks()
         {
             testUserSkin = new TestSource("user");
-            testBeatmapSkin = new TestSource("beatmap");
+            testBeatmapSkin = new BeatmapTestSource();
         }
 
         [Test]
@@ -80,15 +80,15 @@ namespace osu.Game.Rulesets.Osu.Tests
 
         public class CustomSkinWorkingBeatmap : ClockBackedTestWorkingBeatmap
         {
-            private readonly ISkinSource skin;
+            private readonly IBeatmapSkin skin;
 
-            public CustomSkinWorkingBeatmap(IBeatmap beatmap, Storyboard storyboard, IFrameBasedClock frameBasedClock, AudioManager audio, ISkinSource skin)
+            public CustomSkinWorkingBeatmap(IBeatmap beatmap, Storyboard storyboard, IFrameBasedClock frameBasedClock, AudioManager audio, IBeatmapSkin skin)
                 : base(beatmap, storyboard, frameBasedClock, audio)
             {
                 this.skin = skin;
             }
 
-            protected override ISkin GetSkin() => skin;
+            protected override IBeatmapSkin GetSkin() => skin;
         }
 
         public class SkinProvidingPlayer : TestPlayer
@@ -112,6 +112,14 @@ namespace osu.Game.Rulesets.Osu.Tests
             }
         }
 
+        private class BeatmapTestSource : TestSource, IBeatmapSkin
+        {
+            public BeatmapTestSource()
+                : base("beatmap")
+            {
+            }
+        }
+
         public class TestSource : ISkinSource
         {
             private readonly string identifier;

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

@@ -9,6 +9,7 @@ using osu.Game.Beatmaps.ControlPoints;
 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 @@ namespace osu.Game.Rulesets.Osu.Tests
             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/LegacyOldStyleSpinner.cs

@@ -22,11 +22,11 @@ namespace osu.Game.Rulesets.Osu.Skinning
     {
         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 @@ namespace osu.Game.Rulesets.Osu.Skinning
 
             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,
+                    new Sprite
-                    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
                     {
-                        Texture = source.GetTexture("spinner-metre"),
+                        Anchor = Anchor.Centre,
-                        Anchor = Anchor.BottomCentre,
+                        Origin = Anchor.Centre,
-                        Origin = Anchor.BottomCentre,
+                        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 @@ namespace osu.Game.Rulesets.Osu.Skinning
         {
             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 @@ namespace osu.Game.Rulesets.Osu.Skinning
             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.Tests/Gameplay/TestSceneStoryboardSamples.cs

@@ -116,7 +116,7 @@ namespace osu.Game.Tests.Gameplay
             AddAssert("sample playback rate matches mod rates", () => sample.Channel.AggregateFrequency.Value == expectedRate);
         }
 
-        private class TestSkin : LegacySkin
+        private class TestSkin : LegacySkin, IBeatmapSkin
         {
             public TestSkin(string resourceName, AudioManager audioManager)
                 : base(DefaultLegacySkin.Info, new TestResourceStore(resourceName), audioManager, "skin.ini")
@@ -156,7 +156,7 @@ namespace osu.Game.Tests.Gameplay
                 this.audio = audio;
             }
 
-            protected override ISkin GetSkin() => new TestSkin("test-sample", audio);
+            protected override IBeatmapSkin GetSkin() => new TestSkin("test-sample", audio);
         }
 
         private class TestDrawableStoryboardSample : DrawableStoryboardSample

osu.Game/Beatmaps/BeatmapManager_WorkingBeatmap.cs

@@ -140,7 +140,7 @@ namespace osu.Game.Beatmaps
                 return storyboard;
             }
 
-            protected override ISkin GetSkin()
+            protected override IBeatmapSkin GetSkin()
             {
                 try
                 {

osu.Game/Beatmaps/IWorkingBeatmap.cs

@@ -42,9 +42,9 @@ namespace osu.Game.Beatmaps
         Storyboard Storyboard { get; }
 
         /// <summary>
-        /// Retrieves the <see cref="Skin"/> which this <see cref="WorkingBeatmap"/> provides.
+        /// Retrieves the <see cref="IBeatmapSkin"/> which this <see cref="WorkingBeatmap"/> provides.
         /// </summary>
-        ISkin Skin { get; }
+        IBeatmapSkin Skin { get; }
 
         /// <summary>
         /// Constructs a playable <see cref="IBeatmap"/> from <see cref="Beatmap"/> using the applicable converters for a specific <see cref="RulesetInfo"/>.

osu.Game/Beatmaps/WorkingBeatmap.cs

@@ -44,7 +44,7 @@ namespace osu.Game.Beatmaps
             background = new RecyclableLazy<Texture>(GetBackground, BackgroundStillValid);
             waveform = new RecyclableLazy<Waveform>(GetWaveform);
             storyboard = new RecyclableLazy<Storyboard>(GetStoryboard);
-            skin = new RecyclableLazy<ISkin>(GetSkin);
+            skin = new RecyclableLazy<IBeatmapSkin>(GetSkin);
 
             total_count.Value++;
         }
@@ -275,10 +275,10 @@ namespace osu.Game.Beatmaps
         private readonly RecyclableLazy<Storyboard> storyboard;
 
         public bool SkinLoaded => skin.IsResultAvailable;
-        public ISkin Skin => skin.Value;
+        public IBeatmapSkin Skin => skin.Value;
 
-        protected virtual ISkin GetSkin() => new DefaultSkin();
+        protected virtual IBeatmapSkin GetSkin() => new DefaultBeatmapSkin();
-        private readonly RecyclableLazy<ISkin> skin;
+        private readonly RecyclableLazy<IBeatmapSkin> skin;
 
         /// <summary>
         /// Transfer pieces of a beatmap to a new one, where possible, to save on loading.

osu.Game/Skinning/BeatmapSkinProvidingContainer.cs

@@ -11,7 +11,7 @@ namespace osu.Game.Skinning
     /// <summary>
     /// A container which overrides existing skin options with beatmap-local values.
     /// </summary>
-    public class BeatmapSkinProvidingContainer : SkinProvidingContainer
+    public class BeatmapSkinProvidingContainer : SkinProvidingContainer, IBeatmapSkin
     {
         private readonly Bindable<bool> beatmapSkins = new Bindable<bool>();
         private readonly Bindable<bool> beatmapHitsounds = new Bindable<bool>();
@@ -21,7 +21,7 @@ namespace osu.Game.Skinning
         protected override bool AllowTextureLookup(string componentName) => beatmapSkins.Value;
         protected override bool AllowSampleLookup(ISampleInfo componentName) => beatmapHitsounds.Value;
 
-        public BeatmapSkinProvidingContainer(ISkin skin)
+        public BeatmapSkinProvidingContainer(IBeatmapSkin skin)
             : base(skin)
         {
         }

osu.Game/Skinning/DefaultBeatmapSkin.cs

@@ -0,0 +1,9 @@
+// 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.
+
+namespace osu.Game.Skinning
+{
+    public class DefaultBeatmapSkin : DefaultSkin, IBeatmapSkin
+    {
+    }
+}

osu.Game/Skinning/IBeatmapSkin.cs

@@ -0,0 +1,12 @@
+// 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.
+
+namespace osu.Game.Skinning
+{
+    /// <summary>
+    /// Marker interface for skins that originate from beatmaps.
+    /// </summary>
+    public interface IBeatmapSkin : ISkin
+    {
+    }
+}

osu.Game/Skinning/LegacyBeatmapSkin.cs

@@ -11,7 +11,7 @@ using osu.Game.Rulesets.Objects.Legacy;
 
 namespace osu.Game.Skinning
 {
-    public class LegacyBeatmapSkin : LegacySkin
+    public class LegacyBeatmapSkin : LegacySkin, IBeatmapSkin
     {
         protected override bool AllowManiaSkin => false;
         protected override bool UseCustomSampleBanks => true;

osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs

@@ -188,7 +188,7 @@ namespace osu.Game.Tests.Beatmaps
                 this.resourceStore = resourceStore;
             }
 
-            protected override ISkin GetSkin() => new LegacyBeatmapSkin(skinBeatmapInfo, resourceStore, AudioManager);
+            protected override IBeatmapSkin GetSkin() => new LegacyBeatmapSkin(skinBeatmapInfo, resourceStore, AudioManager);
         }
     }
 }