Merge pull request #2334 from peppy/async-screens

Make screens always load async (fixes stutter when entering editor)

osu-framework

@@ -1 +1 @@
-Subproject commit f1751c27ffe2c5febece74129368596b5ad3a4e2
+Subproject commit 61e676094d25436bb9e8858946f65c43d15d8e01

osu.Game.Tests/Visual/TestCaseBeatmapInfoWedge.cs

@@ -110,11 +110,13 @@ namespace osu.Game.Tests.Visual
 
         private void selectBeatmap(string name)
         {
-            var infoBefore = infoWedge.Info;
+            BeatmapInfoWedge.BufferedWedgeInfo infoBefore = null;
 
             AddStep($"select {name} beatmap", () =>
             {
-                beatmap.Value = new TestWorkingBeatmap(beatmaps.First(b => b.BeatmapInfo.Ruleset.ShortName == name));
+                infoBefore = infoWedge.Info;
+                WorkingBeatmap bm = new TestWorkingBeatmap(beatmaps.First(b => b.BeatmapInfo.Ruleset.ShortName == name));
+                beatmap.Value = bm;
                 infoWedge.UpdateBeatmap(beatmap);
             });
 

osu.Game.Tests/Visual/TestCasePlayerLoader.cs

@@ -0,0 +1,24 @@
+// Copyright (c) 2007-2018 ppy Pty Ltd <contact@ppy.sh>.
+// Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE
+
+using osu.Framework.Allocation;
+using osu.Game.Beatmaps;
+using osu.Game.Screens.Play;
+
+namespace osu.Game.Tests.Visual
+{
+    public class TestCasePlayerLoader : OsuTestCase
+    {
+        [BackgroundDependencyLoader]
+        private void load(OsuGameBase game)
+        {
+            AddStep("load dummy beatmap", () => Add(new PlayerLoader(new Player
+            {
+                InitialBeatmap = new DummyWorkingBeatmap(game),
+                AllowPause = false,
+                AllowLeadIn = false,
+                AllowResults = false,
+            })));
+        }
+    }
+}

osu.Game.Tests/Visual/TestCaseUserProfile.cs

@@ -5,7 +5,9 @@ using System;
 using System.Collections.Generic;
 using System.Linq;
 using NUnit.Framework;
+using osu.Framework.Allocation;
 using osu.Game.Graphics.UserInterface;
+using osu.Game.Online.API;
 using osu.Game.Overlays;
 using osu.Game.Overlays.Profile;
 using osu.Game.Overlays.Profile.Header;
@@ -17,6 +19,7 @@ namespace osu.Game.Tests.Visual
     public class TestCaseUserProfile : OsuTestCase
     {
         private readonly TestUserProfileOverlay profile;
+        private APIAccess api;
 
         public override IReadOnlyList<Type> RequiredTypes => new[]
         {
@@ -32,6 +35,12 @@ namespace osu.Game.Tests.Visual
             Add(profile = new TestUserProfileOverlay());
         }
 
+        [BackgroundDependencyLoader]
+        private void load(APIAccess api)
+        {
+            this.api = api;
+        }
+
         protected override void LoadComplete()
         {
             base.LoadComplete();
@@ -79,9 +88,10 @@ namespace osu.Game.Tests.Visual
             {
                 Username = @"peppy",
                 Id = 2,
+                IsSupporter = true,
                 Country = new Country { FullName = @"Australia", FlagName = @"AU" },
                 CoverUrl = @"https://osu.ppy.sh/images/headers/profile-covers/c3.jpg"
-            }));
+            }, api.IsLoggedIn));
 
             checkSupporterTag(true);
 
@@ -91,7 +101,7 @@ namespace osu.Game.Tests.Visual
                 Id = 3103765,
                 Country = new Country { FullName = @"Japan", FlagName = @"JP" },
                 CoverUrl = @"https://osu.ppy.sh/images/headers/profile-covers/c6.jpg"
-            }));
+            }, api.IsLoggedIn));
 
             AddStep("Hide", profile.Hide);
             AddStep("Show without reload", profile.Show);

osu.Game/IO/Archives/ArchiveReader.cs

@@ -1,14 +1,13 @@
 // Copyright (c) 2007-2018 ppy Pty Ltd <contact@ppy.sh>.
 // Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE
 
-using System;
 using System.Collections.Generic;
 using System.IO;
 using osu.Framework.IO.Stores;
 
 namespace osu.Game.IO.Archives
 {
-    public abstract class ArchiveReader : IDisposable, IResourceStore<byte[]>
+    public abstract class ArchiveReader : IResourceStore<byte[]>
     {
         /// <summary>
         /// Opens a stream for reading a specific file from this archive.

osu.Game/OsuGameBase.cs

@@ -188,6 +188,20 @@ namespace osu.Game
             FileStore.Cleanup();
 
             AddInternal(api);
+
+            GlobalActionContainer globalBinding;
+
+            CursorOverrideContainer = new CursorOverrideContainer { RelativeSizeAxes = Axes.Both };
+            CursorOverrideContainer.Child = globalBinding = new GlobalActionContainer(this)
+            {
+                RelativeSizeAxes = Axes.Both,
+                Child = content = new OsuTooltipContainer(CursorOverrideContainer.Cursor) { RelativeSizeAxes = Axes.Both }
+            };
+
+            base.Content.Add(new DrawSizePreservingFillContainer { Child = CursorOverrideContainer });
+
+            KeyBindingStore.Register(globalBinding);
+            dependencies.Cache(globalBinding);
         }
 
         private void runMigrations()
@@ -217,20 +231,6 @@ namespace osu.Game
         {
             base.LoadComplete();
 
-            GlobalActionContainer globalBinding;
-
-            CursorOverrideContainer = new CursorOverrideContainer { RelativeSizeAxes = Axes.Both };
-            CursorOverrideContainer.Child = globalBinding = new GlobalActionContainer(this)
-            {
-                RelativeSizeAxes = Axes.Both,
-                Child = content = new OsuTooltipContainer(CursorOverrideContainer.Cursor) { RelativeSizeAxes = Axes.Both }
-            };
-
-            base.Content.Add(new DrawSizePreservingFillContainer { Child = CursorOverrideContainer });
-
-            KeyBindingStore.Register(globalBinding);
-            dependencies.Cache(globalBinding);
-
             // TODO: This is temporary until we reimplement the local FPS display.
             // It's just to allow end-users to access the framework FPS display without knowing the shortcut key.
             fpsDisplayVisible = LocalConfig.GetBindable<bool>(OsuSetting.ShowFpsDisplay);

osu.Game/Screens/BackgroundScreen.cs

@@ -26,14 +26,14 @@ namespace osu.Game.Screens
             return false;
         }
 
-        public override bool Push(Screen screen)
+        public override void Push(Screen screen)
         {
             // When trying to push a non-loaded screen, load it asynchronously and re-invoke Push
             // once it's done.
             if (screen.LoadState == LoadState.NotLoaded)
             {
                 LoadComponentAsync(screen, d => Push((BackgroundScreen)d));
-                return true;
+                return;
             }
 
             // Make sure the in-progress loading is complete before pushing the screen.
@@ -41,8 +41,6 @@ namespace osu.Game.Screens
                 Thread.Sleep(1);
 
             base.Push(screen);
-
-            return true;
         }
 
         protected override void Update()

osu.Game/Screens/Edit/Editor.cs

@@ -178,7 +178,7 @@ namespace osu.Game.Screens.Edit
             }
 
             currentScreen.Beatmap.BindTo(Beatmap);
-            screenContainer.Add(currentScreen);
+            LoadComponentAsync(currentScreen, screenContainer.Add);
         }
 
         protected override bool OnWheel(InputState state)

osu.Game/Screens/Play/Player.cs

@@ -77,7 +77,7 @@ namespace osu.Game.Screens.Play
         private DrawableStoryboard storyboard;
         private Container storyboardContainer;
 
-        private bool loadedSuccessfully => RulesetContainer?.Objects.Any() == true;
+        public bool LoadedBeatmapSuccessfully => RulesetContainer?.Objects.Any() == true;
 
         [BackgroundDependencyLoader]
         private void load(AudioManager audio, APIAccess api, OsuConfigManager config)
@@ -86,10 +86,7 @@ namespace osu.Game.Screens.Play
 
             WorkingBeatmap working = Beatmap.Value;
             if (working is DummyWorkingBeatmap)
-            {
-                Exit();
                 return;
-            }
 
             sampleRestart = audio.Sample.Get(@"Gameplay/restart");
 
@@ -122,14 +119,15 @@ namespace osu.Game.Screens.Play
                 }
 
                 if (!RulesetContainer.Objects.Any())
-                    throw new InvalidOperationException("Beatmap contains no hit objects!");
+                {
+                    Logger.Error(new InvalidOperationException("Beatmap contains no hit objects!"), "Beatmap contains no hit objects!");
+                    return;
+                }
             }
             catch (Exception e)
             {
                 Logger.Error(e, "Could not load beatmap sucessfully!");
-
                 //couldn't load, hard abort!
-                Exit();
                 return;
             }
 
@@ -293,7 +291,7 @@ namespace osu.Game.Screens.Play
         {
             base.OnEntering(last);
 
-            if (!loadedSuccessfully)
+            if (!LoadedBeatmapSuccessfully)
                 return;
 
             Content.Alpha = 0;
@@ -343,7 +341,7 @@ namespace osu.Game.Screens.Play
                 return base.OnExiting(next);
             }
 
-            if (loadedSuccessfully)
+            if (LoadedBeatmapSuccessfully)
                 pauseContainer?.Pause();
 
             return true;

osu.Game/Screens/Play/PlayerLoader.cs

@@ -159,14 +159,14 @@ namespace osu.Game.Screens.Play
 
                         loadTask = null;
 
-                        if (!Push(player))
-                            Exit();
+                        //By default, we want to load the player and never be returned to.
+                        //Note that this may change if the player we load requested a re-run.
+                        ValidForResume = false;
+
+                        if (player.LoadedBeatmapSuccessfully)
+                            Push(player);
                         else
-                        {
-                            //By default, we want to load the player and never be returned to.
-                            //Note that this may change if the player we load requested a re-run.
-                            ValidForResume = false;
-                        }
+                            Exit();
                     });
                 }, 500);
             }

osu.Game/Screens/Select/Carousel/CarouselGroup.cs

@@ -16,6 +16,12 @@ namespace osu.Game.Screens.Select.Carousel
 
         protected List<CarouselItem> InternalChildren = new List<CarouselItem>();
 
+        /// <summary>
+        /// Used to assign a monotonically increasing ID to children as they are added. This member is
+        /// incremented whenever a child is added.
+        /// </summary>
+        private ulong currentChildID;
+
         public override List<DrawableCarouselItem> Drawables
         {
             get
@@ -39,6 +45,7 @@ namespace osu.Game.Screens.Select.Carousel
         public virtual void AddChild(CarouselItem i)
         {
             i.State.ValueChanged += v => ChildItemStateChanged(i, v);
+            i.ChildID = ++currentChildID;
             InternalChildren.Add(i);
         }
 

osu.Game/Screens/Select/Carousel/CarouselItem.cs

@@ -31,6 +31,8 @@ namespace osu.Game.Screens.Select.Carousel
             }
         }
 
+        private int creationOrder;
+
         protected CarouselItem()
         {
             drawableRepresentation = new Lazy<DrawableCarouselItem>(CreateDrawableRepresentation);
@@ -44,13 +46,18 @@ namespace osu.Game.Screens.Select.Carousel
 
         private readonly Lazy<DrawableCarouselItem> drawableRepresentation;
 
+        /// <summary>
+        /// Used as a default sort method for <see cref="CarouselItem"/>s of differing types.
+        /// </summary>
+        internal ulong ChildID;
+
         protected abstract DrawableCarouselItem CreateDrawableRepresentation();
 
         public virtual void Filter(FilterCriteria criteria)
         {
         }
 
-        public virtual int CompareTo(FilterCriteria criteria, CarouselItem other) => GetHashCode().CompareTo(other.GetHashCode());
+        public virtual int CompareTo(FilterCriteria criteria, CarouselItem other) => ChildID.CompareTo(other.ChildID);
     }
 
     public enum CarouselItemState

osu.Game/Skinning/LegacySkin.cs

@@ -113,6 +113,31 @@ namespace osu.Game.Skinning
                 string path = getPathForFile(name);
                 return path == null ? null : underlyingStore.Get(path);
             }
+
+            #region IDisposable Support
+
+            private bool isDisposed;
+
+            protected virtual void Dispose(bool disposing)
+            {
+                if (!isDisposed)
+                {
+                    isDisposed = true;
+                }
+            }
+
+            ~LegacySkinResourceStore()
+            {
+                Dispose(false);
+            }
+
+            public void Dispose()
+            {
+                Dispose(true);
+                GC.SuppressFinalize(this);
+            }
+
+            #endregion
         }
     }
 }

osu.Game/Tests/Visual/OsuTestCase.cs

@@ -15,18 +15,14 @@ namespace osu.Game.Tests.Visual
         {
             protected override string MainResourceFile => File.Exists(base.MainResourceFile) ? base.MainResourceFile : Assembly.GetExecutingAssembly().Location;
 
-            private readonly TestCaseTestRunner.TestRunner runner;
+            private TestCaseTestRunner.TestRunner runner;
 
-            public OsuTestCaseTestRunner()
+            protected override void LoadAsyncComplete()
             {
-                runner = new TestCaseTestRunner.TestRunner();
-            }
-
-            protected override void LoadComplete()
-            {
-                base.LoadComplete();
-
-                Add(runner);
+                // this has to be run here rather than LoadComplete because
+                // TestCase.cs is checking the IsLoaded state (on another thread) and expects
+                // the runner to be loaded at that point.
+                Add(runner = new TestCaseTestRunner.TestRunner());
             }
 
             public void RunTestBlocking(TestCase test) => runner.RunTestBlocking(test);