Apply reviews, delete playerloader test

osu.Game.Tests/Visual/TestCaseLogoFacadeContainer.cs

@@ -9,7 +9,6 @@ using NUnit.Framework;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Framework.Graphics;
-using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Shapes;
 using osu.Framework.Graphics.UserInterface;
 using osu.Game.Configuration;
@@ -24,14 +23,14 @@ using osuTK.Graphics;
 
 namespace osu.Game.Tests.Visual
 {
-    public class TestCaseFacadeContainer : ScreenTestCase
+    public class TestCaseLogoFacadeContainer : ScreenTestCase
     {
         public override IReadOnlyList<Type> RequiredTypes => new[]
         {
             typeof(PlayerLoader),
             typeof(Player),
-            typeof(Facade),
+            typeof(LogoFacadeContainer.Facade),
-            typeof(FacadeContainer),
+            typeof(LogoFacadeContainer),
             typeof(ButtonSystem),
             typeof(ButtonSystemState),
             typeof(Menu),
@@ -43,7 +42,7 @@ namespace osu.Game.Tests.Visual
 
         private readonly Bindable<float> uiScale = new Bindable<float>();
 
-        public TestCaseFacadeContainer()
+        public TestCaseLogoFacadeContainer()
         {
             Add(logo = new OsuLogo());
         }
@@ -63,20 +62,7 @@ namespace osu.Game.Tests.Visual
             AddStep("Move facade to random position", () => LoadScreen(new TestScreen(randomPositions)));
         }
 
-        [Test]
+        private class TestLogoFacadeContainer : LogoFacadeContainer
-        public void PlayerLoaderTest()
-        {
-            AddToggleStep("Toggle mods", b => { Beatmap.Value.Mods.Value = b ? Beatmap.Value.Mods.Value.Concat(new[] { new OsuModNoFail() }) : Enumerable.Empty<Mod>(); });
-            AddStep("Add new playerloader", () => LoadScreen(new TestPlayerLoader(() => new TestPlayer
-            {
-                AllowPause = false,
-                AllowLeadIn = false,
-                AllowResults = false,
-                Ready = false
-            })));
-        }
-
-        private class TestFacadeContainer : FacadeContainer
         {
             protected override Facade CreateFacade() => new Facade
             {
@@ -92,8 +78,8 @@ namespace osu.Game.Tests.Visual
 
         private class TestScreen : OsuScreen
         {
-            private TestFacadeContainer facadeContainer;
+            private TestLogoFacadeContainer logoFacadeContainer;
-            private Facade facadeFlowComponent;
+            private LogoFacadeContainer.Facade facadeFlowComponent;
             private readonly bool randomPositions;
 
             public TestScreen(bool randomPositions = false)
@@ -104,8 +90,8 @@ namespace osu.Game.Tests.Visual
             [BackgroundDependencyLoader]
             private void load()
             {
-                InternalChild = facadeContainer = new TestFacadeContainer();
+                InternalChild = logoFacadeContainer = new TestLogoFacadeContainer();
-                facadeContainer.Child = facadeFlowComponent = facadeContainer.Facade;
+                logoFacadeContainer.Child = facadeFlowComponent = logoFacadeContainer.LogoFacade;
             }
 
             protected override void LogoArriving(OsuLogo logo, bool resuming)
@@ -113,15 +99,15 @@ namespace osu.Game.Tests.Visual
                 base.LogoArriving(logo, resuming);
                 logo.FadeIn(350);
                 logo.ScaleTo(new Vector2(0.15f), 350, Easing.In);
-                facadeContainer.SetLogo(logo, 0.3f, 1000, Easing.InOutQuint);
+                logoFacadeContainer.SetLogo(logo, 0.3f, 1000, Easing.InOutQuint);
-                facadeContainer.Tracking = true;
+                logoFacadeContainer.Tracking = true;
                 moveLogoFacade();
             }
 
             protected override void LogoExiting(OsuLogo logo)
             {
                 base.LogoExiting(logo);
-                facadeContainer.Tracking = false;
+                logoFacadeContainer.Tracking = false;
             }
 
             private void moveLogoFacade()
@@ -136,39 +122,5 @@ namespace osu.Game.Tests.Visual
                     Schedule(moveLogoFacade);
             }
         }
-
-        private class FacadeFlowComponent : FillFlowContainer
-        {
-            [BackgroundDependencyLoader]
-            private void load(Facade facade)
-            {
-                facade.Anchor = Anchor.TopCentre;
-                facade.Origin = Anchor.TopCentre;
-                Child = facade;
-            }
-        }
-
-        private class TestPlayerLoader : PlayerLoader
-        {
-            public TestPlayerLoader(Func<Player> player)
-                : base(player)
-            {
-            }
-
-            protected override FacadeContainer CreateFacadeContainer() => new TestFacadeContainer();
-        }
-
-        private class TestPlayer : Player
-        {
-            public bool Ready;
-
-            [BackgroundDependencyLoader]
-            private void load(CancellationToken token)
-            {
-                // Never finish loading
-                while (!Ready && !token.IsCancellationRequested)
-                    Thread.Sleep(1);
-            }
-        }
     }
 }

osu.Game/Graphics/Containers/FacadeContainer.cs

@@ -1,102 +0,0 @@
-// 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 osu.Framework.Graphics.Containers;
-using osu.Framework.MathUtils;
-using osu.Game.Screens.Menu;
-using osuTK;
-
-namespace osu.Game.Graphics.Containers
-{
-    /// <summary>
-    /// A container that creates a <see cref="Facade"/> to be used by its children.
-    /// This container also updates the position and size of the Facade, and contains logic for tracking an <see cref="OsuLogo"/> on the Facade's position.
-    /// </summary>
-    public class FacadeContainer : Container
-    {
-        protected virtual Facade CreateFacade() => new Facade();
-
-        public Facade Facade { get; }
-
-        /// <summary>
-        /// Whether or not the logo assigned to this FacadeContainer should be tracking the position its facade.
-        /// </summary>
-        public bool Tracking;
-
-        private OsuLogo logo;
-        private float facadeScale;
-        private Vector2 startPosition;
-        private Easing easing;
-        private double startTime;
-        private double duration;
-
-        public FacadeContainer()
-        {
-            Facade = CreateFacade();
-        }
-
-        /// <summary>
-        /// Assign the logo that should track the Facade's position, as well as how it should transform to its initial position.
-        /// </summary>
-        /// <param name="logo"> The instance of the logo to be used for tracking. </param>
-        /// <param name="facadeScale"> The scale of the facade. </param>
-        /// <param name="duration"> The duration of the initial transform. Default is instant.</param>
-        /// <param name="easing"> The easing type of the initial transform. </param>
-        public void SetLogo(OsuLogo logo, float facadeScale, double duration = 0, Easing easing = Easing.None)
-        {
-            if (logo != null)
-            {
-                this.logo = logo;
-            }
-
-            this.facadeScale = facadeScale;
-            this.duration = duration;
-            this.easing = easing;
-        }
-
-        private Vector2 logoTrackingPosition => logo.Parent.ToLocalSpace(Facade.ScreenSpaceDrawQuad.Centre);
-
-        protected override void UpdateAfterChildren()
-        {
-            base.UpdateAfterChildren();
-
-            if (logo == null || !Tracking)
-                return;
-
-            Facade.Size = new Vector2(logo.SizeForFlow * facadeScale);
-
-            if (Facade.IsLoaded && logo.Position != logoTrackingPosition)
-            {
-                // Required for the correct position of the logo to be set with respect to logoTrackingPosition
-                logo.RelativePositionAxes = Axes.None;
-
-                // If this is our first update since tracking has started, initialize our starting values for interpolation
-                if (startTime == 0)
-                {
-                    startTime = Time.Current;
-                    startPosition = logo.Position;
-                }
-
-                var endTime = startTime + duration;
-                var remainingDuration = endTime - Time.Current;
-
-                // If our transform should be instant, our position should already be at logoTrackingPosition, thus set the blend to 0.
-                // If we are already past when the transform should be finished playing, set the blend to 0 so that the logo is always at the position of the facade.
-                var blend = duration > 0 && remainingDuration > 0
-                    ? (float)Interpolation.ApplyEasing(easing, remainingDuration / duration)
-                    : 0;
-
-                // Interpolate the position of the logo, where blend 0 is the position of the Facade, and blend 1 is where the logo was when it first began interpolating.
-                logo.Position = Vector2.Lerp(logoTrackingPosition, startPosition, blend);
-            }
-        }
-    }
-}
-
-/// <summary>
-/// A placeholder container that serves as a dummy object to denote another object's location and size.
-/// </summary>
-public class Facade : Container
-{
-}

osu.Game/Graphics/Containers/LogoFacadeContainer.cs

@@ -0,0 +1,100 @@
+// 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;
+using osu.Framework.Graphics;
+using osu.Framework.Graphics.Containers;
+using osu.Framework.MathUtils;
+using osu.Game.Screens.Menu;
+using osuTK;
+
+namespace osu.Game.Graphics.Containers
+{
+    /// <summary>
+    /// A container that creates a <see cref="Facade"/> to be used to update and track the position of an <see cref="OsuLogo"/>.
+    /// </summary>
+    public class LogoFacadeContainer : Container
+    {
+        protected virtual Facade CreateFacade() => new Facade();
+
+        public Facade LogoFacade { get; }
+
+        /// <summary>
+        /// Whether or not the logo assigned to this FacadeContainer should be tracking the position its facade.
+        /// </summary>
+        public bool Tracking = false;
+
+        private OsuLogo logo;
+        private float facadeScale;
+        private Vector2 startPosition;
+        private Easing easing;
+        private double? startTime;
+        private double duration;
+
+        public LogoFacadeContainer()
+        {
+            LogoFacade = CreateFacade();
+        }
+
+        /// <summary>
+        /// Assign the logo that should track the Facade's position, as well as how it should transform to its initial position.
+        /// </summary>
+        /// <param name="logo">The instance of the logo to be used for tracking.</param>
+        /// <param name="facadeScale">The scale of the facade. Does not actually affect the logo itself.</param>
+        /// <param name="duration">The duration of the initial transform. Default is instant.</param>
+        /// <param name="easing">The easing type of the initial transform.</param>
+        public void SetLogo(OsuLogo logo, float facadeScale, double duration = 0, Easing easing = Easing.None)
+        {
+            this.logo = logo ?? throw new ArgumentNullException(nameof(logo));
+            this.facadeScale = facadeScale;
+            this.duration = duration;
+            this.easing = easing;
+        }
+
+        private Vector2 logoTrackingPosition => logo.Parent.ToLocalSpace(LogoFacade.ScreenSpaceDrawQuad.Centre);
+
+        protected override void UpdateAfterChildren()
+        {
+            base.UpdateAfterChildren();
+
+            if (logo == null || !Tracking)
+                return;
+
+            LogoFacade.Size = new Vector2(logo.SizeForFlow * facadeScale);
+
+            if (LogoFacade.Parent != null && logo.Position != logoTrackingPosition)
+            {
+                // Required for the correct position of the logo to be set with respect to logoTrackingPosition
+                logo.RelativePositionAxes = Axes.None;
+
+                // If this is our first update since tracking has started, initialize our starting values for interpolation
+                if (startTime == null)
+                {
+                    startTime = Time.Current;
+                    startPosition = logo.Position;
+                }
+
+                if (duration != 0)
+                {
+                    double elapsedDuration = Time.Current - startTime ?? 0;
+
+                    var mount = (float)Interpolation.ApplyEasing(easing, Math.Min(elapsedDuration / duration, 1));
+
+                    // Interpolate the position of the logo, where mount 0 is where the logo was when it first began interpolating, and mount 1 is the target location.
+                    logo.Position = Vector2.Lerp(startPosition, logoTrackingPosition, mount);
+                }
+                else
+                {
+                    logo.Position = logoTrackingPosition;
+                }
+            }
+        }
+
+        /// <summary>
+        /// A placeholder container that serves as a dummy object to denote another object's location and size.
+        /// </summary>
+        public class Facade : Container
+        {
+        }
+    }
+}

osu.Game/Screens/Menu/ButtonSystem.cs

@@ -48,6 +48,10 @@ namespace osu.Game.Screens.Menu
 
         private OsuLogo logo;
 
+        /// <summary>
+        /// Assign the <see cref="OsuLogo"/> that this ButtonSystem should manage the position of.
+        /// </summary>
+        /// <param name="logo">The instance of the logo to be assigned. If null, we are suspending from the screen that uses this ButtonSystem.</param>
         public void SetOsuLogo(OsuLogo logo)
         {
             this.logo = logo;
@@ -55,7 +59,7 @@ namespace osu.Game.Screens.Menu
             if (this.logo != null)
             {
                 this.logo.Action = onOsuLogo;
-                facadeContainer.SetLogo(logo, 0.5f);
+                logoFacadeContainer.SetLogo(logo, 0.5f);
 
                 // osuLogo.SizeForFlow relies on loading to be complete.
                 buttonArea.Flow.Position = new Vector2(WEDGE_WIDTH * 2 - (BUTTON_WIDTH + this.logo.SizeForFlow / 4), 0);
@@ -64,9 +68,8 @@ namespace osu.Game.Screens.Menu
             }
             else
             {
-                // If logo is null, we are suspending from the screen that uses this ButtonSystem.
                 // We should stop tracking as the facade is now out of scope.
-                facadeContainer.Tracking = false;
+                logoFacadeContainer.Tracking = false;
             }
         }
 
@@ -79,13 +82,13 @@ namespace osu.Game.Screens.Menu
 
         private SampleChannel sampleBack;
 
-        private readonly FacadeContainer facadeContainer;
+        private readonly LogoFacadeContainer logoFacadeContainer;
 
         public ButtonSystem()
         {
             RelativeSizeAxes = Axes.Both;
 
-            Child = facadeContainer = new FacadeContainer
+            Child = logoFacadeContainer = new LogoFacadeContainer
             {
                 RelativeSizeAxes = Axes.Both,
                 Child = buttonArea = new ButtonArea()
@@ -98,10 +101,10 @@ namespace osu.Game.Screens.Menu
                 {
                     VisibleState = ButtonSystemState.Play,
                 },
-                facadeContainer.Facade
+                logoFacadeContainer.LogoFacade
             });
 
-            buttonArea.Flow.CentreTarget = facadeContainer.Facade;
+            buttonArea.Flow.CentreTarget = logoFacadeContainer.LogoFacade;
         }
 
         [Resolved(CanBeNull = true)]
@@ -267,7 +270,7 @@ namespace osu.Game.Screens.Menu
                     logoDelayedAction?.Cancel();
                     logoDelayedAction = Scheduler.AddDelayed(() =>
                         {
-                            facadeContainer.Tracking = false;
+                            logoFacadeContainer.Tracking = false;
 
                             game?.Toolbar.Hide();
 
@@ -295,7 +298,7 @@ namespace osu.Game.Screens.Menu
                             logoDelayedAction?.Cancel();
                             logoDelayedAction = Scheduler.AddDelayed(() =>
                             {
-                                facadeContainer.Tracking = true;
+                                logoFacadeContainer.Tracking = true;
 
                                 if (impact)
                                     logo.Impact();
@@ -305,14 +308,14 @@ namespace osu.Game.Screens.Menu
                             break;
                         default:
                             logo.ClearTransforms(targetMember: nameof(Position));
-                            facadeContainer.Tracking = true;
+                            logoFacadeContainer.Tracking = true;
                             logo.ScaleTo(0.5f, 200, Easing.OutQuint);
                             break;
                     }
 
                     break;
                 case ButtonSystemState.EnteringMode:
-                    facadeContainer.Tracking = true;
+                    logoFacadeContainer.Tracking = true;
                     break;
             }
         }

osu.Game/Screens/Play/PlayerLoader.cs

@@ -33,9 +33,7 @@ namespace osu.Game.Screens.Play
 
         private Player player;
 
-        private FacadeContainer facadeContainer;
+        private LogoFacadeContainer content;
-
-        protected virtual FacadeContainer CreateFacadeContainer() => new FacadeContainer();
 
         private BeatmapMetadataDisplay info;
 
@@ -62,30 +60,32 @@ namespace osu.Game.Screens.Play
         [BackgroundDependencyLoader]
         private void load()
         {
-            InternalChild = facadeContainer = CreateFacadeContainer();
+            InternalChild = content = new LogoFacadeContainer
-            facadeContainer.Anchor = Anchor.Centre;
-            facadeContainer.Origin = Anchor.Centre;
-            facadeContainer.RelativeSizeAxes = Axes.Both;
-            facadeContainer.Children = new Drawable[]
             {
-                info = new BeatmapMetadataDisplay(Beatmap.Value, facadeContainer.Facade)
+                Anchor = Anchor.Centre,
+                Origin = Anchor.Centre,
+                RelativeSizeAxes = Axes.Both,
+                Children = new Drawable[]
                 {
-                    Alpha = 0,
+                    info = new BeatmapMetadataDisplay(Beatmap.Value, content.LogoFacade)
-                    Anchor = Anchor.Centre,
-                    Origin = Anchor.Centre,
-                },
-                new FillFlowContainer<PlayerSettingsGroup>
-                {
-                    Anchor = Anchor.TopRight,
-                    Origin = Anchor.TopRight,
-                    AutoSizeAxes = Axes.Both,
-                    Direction = FillDirection.Vertical,
-                    Spacing = new Vector2(0, 20),
-                    Margin = new MarginPadding(25),
-                    Children = new PlayerSettingsGroup[]
                     {
-                        VisualSettings = new VisualSettings(),
+                        Alpha = 0,
-                        new InputSettings()
+                        Anchor = Anchor.Centre,
+                        Origin = Anchor.Centre,
+                    },
+                    new FillFlowContainer<PlayerSettingsGroup>
+                    {
+                        Anchor = Anchor.TopRight,
+                        Origin = Anchor.TopRight,
+                        AutoSizeAxes = Axes.Both,
+                        Direction = FillDirection.Vertical,
+                        Spacing = new Vector2(0, 20),
+                        Margin = new MarginPadding(25),
+                        Children = new PlayerSettingsGroup[]
+                        {
+                            VisualSettings = new VisualSettings(),
+                            new InputSettings()
+                        }
                     }
                 }
             };
@@ -122,21 +122,21 @@ namespace osu.Game.Screens.Play
 
         private void contentIn()
         {
-            facadeContainer.ScaleTo(1, 650, Easing.OutQuint);
+            content.ScaleTo(1, 650, Easing.OutQuint);
-            facadeContainer.FadeInFromZero(400);
+            content.FadeInFromZero(400);
         }
 
         private void contentOut()
         {
-            facadeContainer.ScaleTo(0.7f, 300, Easing.InQuint);
+            content.ScaleTo(0.7f, 300, Easing.InQuint);
-            facadeContainer.FadeOut(250);
+            content.FadeOut(250);
         }
 
         public override void OnEntering(IScreen last)
         {
             base.OnEntering(last);
 
-            facadeContainer.ScaleTo(0.7f);
+            content.ScaleTo(0.7f);
             Background?.FadeColour(Color4.White, 800, Easing.OutQuint);
 
             contentIn();
@@ -155,15 +155,15 @@ namespace osu.Game.Screens.Play
             logo.MoveTo(new Vector2(0.5f), duration, Easing.In);
             logo.FadeIn(350);
 
-            facadeContainer.SetLogo(logo, 0.3f, 500, Easing.InOutQuint);
+            content.SetLogo(logo, 0.3f, 500, Easing.InOutExpo);
 
-            Scheduler.AddDelayed(() => facadeContainer.Tracking = true, duration);
+            Scheduler.AddDelayed(() => content.Tracking = true, resuming ? 0 : 500);
         }
 
         protected override void LogoExiting(OsuLogo logo)
         {
             base.LogoExiting(logo);
-            facadeContainer.Tracking = false;
+            content.Tracking = false;
         }
 
         protected override void LoadComplete()
@@ -238,7 +238,7 @@ namespace osu.Game.Screens.Play
 
         public override bool OnExiting(IScreen next)
         {
-            facadeContainer.ScaleTo(0.7f, 150, Easing.InQuint);
+            content.ScaleTo(0.7f, 150, Easing.InQuint);
             this.FadeOut(150);
             cancelLoad();
 
@@ -310,7 +310,7 @@ namespace osu.Game.Screens.Play
             }
 
             private readonly WorkingBeatmap beatmap;
-            private readonly Facade facade;
+            private readonly LogoFacadeContainer.Facade facade;
             private LoadingAnimation loading;
             private Sprite backgroundSprite;
             private ModDisplay modDisplay;
@@ -332,7 +332,7 @@ namespace osu.Game.Screens.Play
                 }
             }
 
-            public BeatmapMetadataDisplay(WorkingBeatmap beatmap, Facade facade)
+            public BeatmapMetadataDisplay(WorkingBeatmap beatmap, LogoFacadeContainer.Facade facade)
             {
                 this.beatmap = beatmap;
                 this.facade = facade;