Merge branch 'master' into fix-listing-terminal-breakage

osu.Game.Tests/Visual/Gameplay/TestScenePlayerScoreSubmission.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Linq;
+using System.Threading;
 using System.Threading.Tasks;
 using NUnit.Framework;
 using osu.Framework.Screens;
@@ -36,7 +37,9 @@ namespace osu.Game.Tests.Visual.Gameplay
 
         protected override bool HasCustomSteps => true;
 
-        protected override TestPlayer CreatePlayer(Ruleset ruleset) => new NonImportingPlayer(false);
+        protected override TestPlayer CreatePlayer(Ruleset ruleset) => new FakeImportingPlayer(false);
+
+        protected new FakeImportingPlayer Player => (FakeImportingPlayer)base.Player;
 
         protected override Ruleset CreatePlayerRuleset() => createCustomRuleset?.Invoke() ?? new OsuRuleset();
 
@@ -207,6 +210,25 @@ namespace osu.Game.Tests.Visual.Gameplay
             AddAssert("ensure failing submission", () => Player.SubmittedScore?.ScoreInfo.Passed == false);
         }
 
+        [Test]
+        public void TestSubmissionOnExitDuringImport()
+        {
+            prepareTokenResponse(true);
+
+            createPlayerTest();
+            AddStep("block imports", () => Player.AllowImportCompletion.Wait());
+
+            AddUntilStep("wait for token request", () => Player.TokenCreationRequested);
+
+            addFakeHit();
+
+            AddUntilStep("wait for import to start", () => Player.ScoreImportStarted);
+
+            AddStep("exit", () => Player.Exit());
+            AddStep("allow import to proceed", () => Player.AllowImportCompletion.Release(1));
+            AddAssert("ensure submission", () => Player.SubmittedScore != null && Player.ImportedScore != null);
+        }
+
         [Test]
         public void TestNoSubmissionOnLocalBeatmap()
         {
@@ -288,15 +310,26 @@ namespace osu.Game.Tests.Visual.Gameplay
             });
         }
 
-        private class NonImportingPlayer : TestPlayer
+        protected class FakeImportingPlayer : TestPlayer
         {
-            public NonImportingPlayer(bool allowPause = true, bool showResults = true, bool pauseOnFocusLost = false)
+            public bool ScoreImportStarted { get; set; }
+            public SemaphoreSlim AllowImportCompletion { get; }
+            public Score ImportedScore { get; private set; }
+
+            public FakeImportingPlayer(bool allowPause = true, bool showResults = true, bool pauseOnFocusLost = false)
                 : base(allowPause, showResults, pauseOnFocusLost)
             {
+                AllowImportCompletion = new SemaphoreSlim(1);
             }
 
-            protected override Task ImportScore(Score score)
+            protected override async Task ImportScore(Score score)
             {
+                ScoreImportStarted = true;
+
+                await AllowImportCompletion.WaitAsync().ConfigureAwait(false);
+
+                ImportedScore = score;
+
                 // It was discovered that Score members could sometimes be half-populated.
                 // In particular, the RulesetID property could be set to 0 even on non-osu! maps.
                 // We want to test that the state of that property is consistent in this test.
@@ -311,8 +344,7 @@ namespace osu.Game.Tests.Visual.Gameplay
                 // In the above instance, if a ScoreInfo with Ruleset = {mania} and RulesetID = 0 is attached to an EF context,
                 // RulesetID WILL BE SILENTLY SET TO THE CORRECT VALUE of 3.
                 //
-                // For the above reasons, importing is disabled in this test.
+                // For the above reasons, actual importing is disabled in this test.
-                return Task.CompletedTask;
             }
         }
     }

osu.Game/Skinning/Editor/SkinEditorOverlay.cs

@@ -1,7 +1,6 @@
 // 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 JetBrains.Annotations;
 using osu.Framework.Bindables;
 using osu.Framework.Graphics;
@@ -67,18 +66,34 @@ namespace osu.Game.Skinning.Editor
 
         public override void Show()
         {
-            // base call intentionally omitted.
+            // base call intentionally omitted as we have custom behaviour.
-            if (skinEditor == null)
+
+            if (skinEditor != null)
             {
-                skinEditor = new SkinEditor(target);
-                skinEditor.State.BindValueChanged(editorVisibilityChanged);
-
-                Debug.Assert(skinEditor != null);
-
-                LoadComponentAsync(skinEditor, AddInternal);
-            }
-            else
                 skinEditor.Show();
+                return;
+            }
+
+            var editor = new SkinEditor(target);
+            editor.State.BindValueChanged(editorVisibilityChanged);
+
+            skinEditor = editor;
+
+            // Schedule ensures that if `Show` is called before this overlay is loaded,
+            // it will not throw (LoadComponentAsync requires the load target to be in a loaded state).
+            Schedule(() =>
+            {
+                if (editor != skinEditor)
+                    return;
+
+                LoadComponentAsync(editor, _ =>
+                {
+                    if (editor != skinEditor)
+                        return;
+
+                    AddInternal(editor);
+                });
+            });
         }
 
         private void editorVisibilityChanged(ValueChangedEvent<Visibility> visibility)