Merge branch 'master' into dont-throw-on-missing-drawings

osu.Game.Tests/Visual/UserInterface/TestSceneLabelledColourPalette.cs

@@ -53,15 +53,15 @@ namespace osu.Game.Tests.Visual.UserInterface
 
                 component.Colours.AddRange(new[]
                 {
-                    Color4.DarkRed,
+                    Colour4.DarkRed,
-                    Color4.Aquamarine,
+                    Colour4.Aquamarine,
-                    Color4.Goldenrod,
+                    Colour4.Goldenrod,
-                    Color4.Gainsboro
+                    Colour4.Gainsboro
                 });
             });
         }
 
-        private Color4 randomColour() => new Color4(
+        private Colour4 randomColour() => new Color4(
             RNG.NextSingle(),
             RNG.NextSingle(),
             RNG.NextSingle(),

osu.Game.Tournament.Tests/NonVisual/CustomTourneyDirectoryTest.cs

@@ -1,21 +1,18 @@
 // 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 System.IO;
-using System.Threading;
-using System.Threading.Tasks;
 using NUnit.Framework;
 using osu.Framework;
 using osu.Framework.Allocation;
 using osu.Framework.Platform;
-using osu.Game.Tournament.Configuration;
 using osu.Game.Tests;
+using osu.Game.Tournament.Configuration;
 
 namespace osu.Game.Tournament.Tests.NonVisual
 {
     [TestFixture]
-    public class CustomTourneyDirectoryTest
+    public class CustomTourneyDirectoryTest : TournamentHostTest
     {
         [Test]
         public void TestDefaultDirectory()
@@ -24,7 +21,7 @@ namespace osu.Game.Tournament.Tests.NonVisual
             {
                 try
                 {
-                    var osu = loadOsu(host);
+                    var osu = LoadTournament(host);
                     var storage = osu.Dependencies.Get<Storage>();
 
                     Assert.That(storage.GetFullPath("."), Is.EqualTo(Path.Combine(host.Storage.GetFullPath("."), "tournaments", "default")));
@@ -54,7 +51,7 @@ namespace osu.Game.Tournament.Tests.NonVisual
 
                 try
                 {
-                    var osu = loadOsu(host);
+                    var osu = LoadTournament(host);
 
                     storage = osu.Dependencies.Get<Storage>();
 
@@ -111,7 +108,7 @@ namespace osu.Game.Tournament.Tests.NonVisual
 
                 try
                 {
-                    var osu = loadOsu(host);
+                    var osu = LoadTournament(host);
 
                     var storage = osu.Dependencies.Get<Storage>();
 
@@ -151,25 +148,6 @@ namespace osu.Game.Tournament.Tests.NonVisual
             }
         }
 
-        private TournamentGameBase loadOsu(GameHost host)
-        {
-            var osu = new TournamentGameBase();
-            Task.Run(() => host.Run(osu))
-                .ContinueWith(t => Assert.Fail($"Host threw exception {t.Exception}"), TaskContinuationOptions.OnlyOnFaulted);
-            waitForOrAssert(() => osu.IsLoaded, @"osu! failed to start in a reasonable amount of time");
-            return osu;
-        }
-
-        private static void waitForOrAssert(Func<bool> result, string failureMessage, int timeout = 90000)
-        {
-            Task task = Task.Run(() =>
-            {
-                while (!result()) Thread.Sleep(200);
-            });
-
-            Assert.IsTrue(task.Wait(timeout), failureMessage);
-        }
-
         private string basePath(string testInstance) => Path.Combine(RuntimeInfo.StartupDirectory, "headless", testInstance);
     }
 }

osu.Game.Tournament.Tests/NonVisual/DataLoadTest.cs

@@ -0,0 +1,45 @@
+// 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.IO;
+using NUnit.Framework;
+using osu.Framework.Allocation;
+using osu.Framework.Platform;
+using osu.Game.Rulesets;
+using osu.Game.Tests;
+
+namespace osu.Game.Tournament.Tests.NonVisual
+{
+    public class DataLoadTest : TournamentHostTest
+    {
+        [Test]
+        public void TestUnavailableRuleset()
+        {
+            using (HeadlessGameHost host = new CleanRunHeadlessGameHost(nameof(TestUnavailableRuleset)))
+            {
+                try
+                {
+                    var osu = new TestTournament();
+
+                    LoadTournament(host, osu);
+                    var storage = osu.Dependencies.Get<Storage>();
+
+                    Assert.That(storage.GetFullPath("."), Is.EqualTo(Path.Combine(host.Storage.GetFullPath("."), "tournaments", "default")));
+                }
+                finally
+                {
+                    host.Exit();
+                }
+            }
+        }
+
+        public class TestTournament : TournamentGameBase
+        {
+            [BackgroundDependencyLoader]
+            private void load()
+            {
+                Ruleset.Value = new RulesetInfo(); // not available
+            }
+        }
+    }
+}

osu.Game.Tournament.Tests/NonVisual/IPCLocationTest.cs

@@ -1,10 +1,7 @@
 // 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 System.IO;
-using System.Threading;
-using System.Threading.Tasks;
 using NUnit.Framework;
 using osu.Framework;
 using osu.Framework.Allocation;
@@ -15,7 +12,7 @@ using osu.Game.Tournament.IPC;
 namespace osu.Game.Tournament.Tests.NonVisual
 {
     [TestFixture]
-    public class IPCLocationTest
+    public class IPCLocationTest : TournamentHostTest
     {
         [Test]
         public void CheckIPCLocation()
@@ -34,11 +31,11 @@ namespace osu.Game.Tournament.Tests.NonVisual
 
                 try
                 {
-                    var osu = loadOsu(host);
+                    var osu = LoadTournament(host);
                     TournamentStorage storage = (TournamentStorage)osu.Dependencies.Get<Storage>();
                     FileBasedIPC ipc = null;
 
-                    waitForOrAssert(() => (ipc = osu.Dependencies.Get<MatchIPCInfo>() as FileBasedIPC) != null, @"ipc could not be populated in a reasonable amount of time");
+                    WaitForOrAssert(() => (ipc = osu.Dependencies.Get<MatchIPCInfo>() as FileBasedIPC) != null, @"ipc could not be populated in a reasonable amount of time");
 
                     Assert.True(ipc.SetIPCLocation(testStableInstallDirectory));
                     Assert.True(storage.AllTournaments.Exists("stable.json"));
@@ -51,24 +48,5 @@ namespace osu.Game.Tournament.Tests.NonVisual
                 }
             }
         }
-
-        private TournamentGameBase loadOsu(GameHost host)
-        {
-            var osu = new TournamentGameBase();
-            Task.Run(() => host.Run(osu))
-                .ContinueWith(t => Assert.Fail($"Host threw exception {t.Exception}"), TaskContinuationOptions.OnlyOnFaulted);
-            waitForOrAssert(() => osu.IsLoaded, @"osu! failed to start in a reasonable amount of time");
-            return osu;
-        }
-
-        private static void waitForOrAssert(Func<bool> result, string failureMessage, int timeout = 90000)
-        {
-            Task task = Task.Run(() =>
-            {
-                while (!result()) Thread.Sleep(200);
-            });
-
-            Assert.IsTrue(task.Wait(timeout), failureMessage);
-        }
     }
 }

osu.Game.Tournament.Tests/NonVisual/TournamentHostTest.cs

@@ -0,0 +1,33 @@
+// 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 System.Threading;
+using System.Threading.Tasks;
+using NUnit.Framework;
+using osu.Framework.Platform;
+
+namespace osu.Game.Tournament.Tests.NonVisual
+{
+    public abstract class TournamentHostTest
+    {
+        public static TournamentGameBase LoadTournament(GameHost host, TournamentGameBase tournament = null)
+        {
+            tournament ??= new TournamentGameBase();
+            Task.Run(() => host.Run(tournament))
+                .ContinueWith(t => Assert.Fail($"Host threw exception {t.Exception}"), TaskContinuationOptions.OnlyOnFaulted);
+            WaitForOrAssert(() => tournament.IsLoaded, @"osu! failed to start in a reasonable amount of time");
+            return tournament;
+        }
+
+        public static void WaitForOrAssert(Func<bool> result, string failureMessage, int timeout = 90000)
+        {
+            Task task = Task.Run(() =>
+            {
+                while (!result()) Thread.Sleep(200);
+            });
+
+            Assert.IsTrue(task.Wait(timeout), failureMessage);
+        }
+    }
+}

osu.Game.Tournament/TournamentGameBase.cs

@@ -66,7 +66,9 @@ namespace osu.Game.Tournament
             }
 
             ladder ??= new LadderInfo();
-            ladder.Ruleset.Value ??= RulesetStore.AvailableRulesets.First();
+
+            ladder.Ruleset.Value = RulesetStore.GetRuleset(ladder.Ruleset.Value?.ShortName)
+                                   ?? RulesetStore.AvailableRulesets.First();
 
             bool addedInfo = false;
 

osu.Game/Graphics/UserInterfaceV2/ColourDisplay.cs

@@ -3,30 +3,31 @@
 
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
-using osu.Framework.Extensions.Color4Extensions;
+using osu.Framework.Extensions;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
+using osu.Framework.Graphics.Cursor;
 using osu.Framework.Graphics.Shapes;
 using osu.Framework.Graphics.UserInterface;
 using osu.Framework.Localisation;
+using osu.Game.Graphics.Containers;
 using osu.Game.Graphics.Sprites;
 using osuTK;
-using osuTK.Graphics;
 
 namespace osu.Game.Graphics.UserInterfaceV2
 {
     /// <summary>
     /// A component which displays a colour along with related description text.
     /// </summary>
-    public class ColourDisplay : CompositeDrawable, IHasCurrentValue<Color4>
+    public class ColourDisplay : CompositeDrawable, IHasCurrentValue<Colour4>, IHasPopover
     {
-        private readonly BindableWithCurrent<Color4> current = new BindableWithCurrent<Color4>();
+        private readonly BindableWithCurrent<Colour4> current = new BindableWithCurrent<Colour4>();
 
         private Box fill;
         private OsuSpriteText colourHexCode;
         private OsuSpriteText colourName;
 
-        public Bindable<Color4> Current
+        public Bindable<Colour4> Current
         {
             get => current.Current;
             set => current.Current = value;
@@ -62,10 +63,11 @@ namespace osu.Game.Graphics.UserInterfaceV2
                 Spacing = new Vector2(0, 10),
                 Children = new Drawable[]
                 {
-                    new CircularContainer
+                    new OsuClickableContainer
                     {
                         RelativeSizeAxes = Axes.X,
                         Height = 100,
+                        CornerRadius = 50,
                         Masking = true,
                         Children = new Drawable[]
                         {
@@ -79,7 +81,8 @@ namespace osu.Game.Graphics.UserInterfaceV2
                                 Origin = Anchor.Centre,
                                 Font = OsuFont.Default.With(size: 12)
                             }
-                        }
+                        },
+                        Action = this.ShowPopover
                     },
                     colourName = new OsuSpriteText
                     {
@@ -103,5 +106,13 @@ namespace osu.Game.Graphics.UserInterfaceV2
             colourHexCode.Text = current.Value.ToHex();
             colourHexCode.Colour = OsuColour.ForegroundTextColourFor(current.Value);
         }
+
+        public Popover GetPopover() => new OsuPopover(false)
+        {
+            Child = new OsuColourPicker
+            {
+                Current = { BindTarget = Current }
+            }
+        };
     }
 }

osu.Game/Graphics/UserInterfaceV2/ColourPalette.cs

@@ -1,6 +1,7 @@
 // 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.Collections.Specialized;
 using System.Linq;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
@@ -8,7 +9,6 @@ using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Game.Graphics.Sprites;
 using osuTK;
-using osuTK.Graphics;
 
 namespace osu.Game.Graphics.UserInterfaceV2
 {
@@ -17,7 +17,7 @@ namespace osu.Game.Graphics.UserInterfaceV2
     /// </summary>
     public class ColourPalette : CompositeDrawable
     {
-        public BindableList<Color4> Colours { get; } = new BindableList<Color4>();
+        public BindableList<Colour4> Colours { get; } = new BindableList<Colour4>();
 
         private string colourNamePrefix = "Colour";
 
@@ -73,14 +73,17 @@ namespace osu.Game.Graphics.UserInterfaceV2
         {
             base.LoadComplete();
 
-            Colours.BindCollectionChanged((_, __) => updatePalette(), true);
+            Colours.BindCollectionChanged((_, args) => updatePalette(args), true);
             FinishTransforms(true);
         }
 
         private const int fade_duration = 200;
 
-        private void updatePalette()
+        private void updatePalette(NotifyCollectionChangedEventArgs args)
         {
+            if (args.Action == NotifyCollectionChangedAction.Replace)
+                return;
+
             palette.Clear();
 
             if (Colours.Any())
@@ -94,12 +97,18 @@ namespace osu.Game.Graphics.UserInterfaceV2
                 placeholder.FadeIn(fade_duration, Easing.OutQuint);
             }
 
-            foreach (var item in Colours)
+            for (int i = 0; i < Colours.Count; ++i)
             {
-                palette.Add(new ColourDisplay
+                // copy to avoid accesses to modified closure.
+                int colourIndex = i;
+                ColourDisplay display;
+
+                palette.Add(display = new ColourDisplay
                 {
-                    Current = { Value = item }
+                    Current = { Value = Colours[colourIndex] }
                 });
+
+                display.Current.BindValueChanged(colour => Colours[colourIndex] = colour.NewValue);
             }
 
             reindexItems();

osu.Game/Graphics/UserInterfaceV2/LabelledColourPalette.cs

@@ -2,7 +2,7 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using osu.Framework.Bindables;
-using osuTK.Graphics;
+using osu.Framework.Graphics;
 
 namespace osu.Game.Graphics.UserInterfaceV2
 {
@@ -13,7 +13,7 @@ namespace osu.Game.Graphics.UserInterfaceV2
         {
         }
 
-        public BindableList<Color4> Colours => Component.Colours;
+        public BindableList<Colour4> Colours => Component.Colours;
 
         public string ColourNamePrefix
         {

osu.Game/Online/API/APIRequest.cs

@@ -86,8 +86,6 @@ namespace osu.Game.Online.API
         /// </summary>
         private APIRequestCompletionState completionState;
 
-        private Action pendingFailure;
-
         public void Perform(IAPIProvider api)
         {
             if (!(api is APIAccess apiAccess))
@@ -99,29 +97,23 @@ namespace osu.Game.Online.API
             API = apiAccess;
             User = apiAccess.LocalUser.Value;
 
-            if (checkAndScheduleFailure())
+            if (isFailing) return;
-                return;
 
             WebRequest = CreateWebRequest();
             WebRequest.Failed += Fail;
             WebRequest.AllowRetryOnTimeout = false;
             WebRequest.AddHeader("Authorization", $"Bearer {API.AccessToken}");
 
-            if (checkAndScheduleFailure())
+            if (isFailing) return;
-                return;
 
-            if (!WebRequest.Aborted) // could have been aborted by a Cancel() call
+            Logger.Log($@"Performing request {this}", LoggingTarget.Network);
-            {
+            WebRequest.Perform();
-                Logger.Log($@"Performing request {this}", LoggingTarget.Network);
-                WebRequest.Perform();
-            }
 
-            if (checkAndScheduleFailure())
+            if (isFailing) return;
-                return;
 
             PostProcess();
 
-            API.Schedule(TriggerSuccess);
+            TriggerSuccess();
         }
 
         /// <summary>
@@ -141,7 +133,10 @@ namespace osu.Game.Online.API
                 completionState = APIRequestCompletionState.Completed;
             }
 
-            Success?.Invoke();
+            if (API == null)
+                Success?.Invoke();
+            else
+                API.Schedule(() => Success?.Invoke());
         }
 
         internal void TriggerFailure(Exception e)
@@ -154,7 +149,10 @@ namespace osu.Game.Online.API
                 completionState = APIRequestCompletionState.Failed;
             }
 
-            Failure?.Invoke(e);
+            if (API == null)
+                Failure?.Invoke(e);
+            else
+                API.Schedule(() => Failure?.Invoke(e));
         }
 
         public void Cancel() => Fail(new OperationCanceledException(@"Request cancelled"));
@@ -163,59 +161,47 @@ namespace osu.Game.Online.API
         {
             lock (completionStateLock)
             {
-                // while it doesn't matter if code following this check is run more than once,
-                // this avoids unnecessarily performing work where we are already sure the user has been informed.
                 if (completionState != APIRequestCompletionState.Waiting)
                     return;
-            }
 
-            WebRequest?.Abort();
+                WebRequest?.Abort();
 
-            // in the case of a cancellation we don't care about whether there's an error in the response.
+                // in the case of a cancellation we don't care about whether there's an error in the response.
-            if (!(e is OperationCanceledException))
+                if (!(e is OperationCanceledException))
-            {
-                string responseString = WebRequest?.GetResponseString();
-
-                // naive check whether there's an error in the response to avoid unnecessary JSON deserialisation.
-                if (!string.IsNullOrEmpty(responseString) && responseString.Contains(@"""error"""))
                 {
-                    try
+                    string responseString = WebRequest?.GetResponseString();
-                    {
+
-                        // attempt to decode a displayable error string.
+                    // naive check whether there's an error in the response to avoid unnecessary JSON deserialisation.
-                        var error = JsonConvert.DeserializeObject<DisplayableError>(responseString);
+                    if (!string.IsNullOrEmpty(responseString) && responseString.Contains(@"""error"""))
-                        if (error != null)
-                            e = new APIException(error.ErrorMessage, e);
-                    }
-                    catch
                     {
+                        try
+                        {
+                            // attempt to decode a displayable error string.
+                            var error = JsonConvert.DeserializeObject<DisplayableError>(responseString);
+                            if (error != null)
+                                e = new APIException(error.ErrorMessage, e);
+                        }
+                        catch
+                        {
+                        }
                     }
                 }
-            }
 
-            Logger.Log($@"Failing request {this} ({e})", LoggingTarget.Network);
+                Logger.Log($@"Failing request {this} ({e})", LoggingTarget.Network);
-            pendingFailure = () => TriggerFailure(e);
+                TriggerFailure(e);
-            checkAndScheduleFailure();
+            }
         }
 
         /// <summary>
-        /// Checked for cancellation or error. Also queues up the Failed event if we can.
+        /// Whether this request is in a failing or failed state.
         /// </summary>
-        /// <returns>Whether we are in a failed or cancelled state.</returns>
+        private bool isFailing
-        private bool checkAndScheduleFailure()
         {
-            lock (completionStateLock)
+            get
             {
-                if (pendingFailure == null)
+                lock (completionStateLock)
                     return completionState == APIRequestCompletionState.Failed;
             }
-
-            if (API == null)
-                pendingFailure();
-            else
-                API.Schedule(pendingFailure);
-
-            pendingFailure = null;
-            return true;
         }
 
         private class DisplayableError

osu.Game/OsuGameBase.cs

@@ -479,7 +479,7 @@ namespace osu.Game
             if (r.NewValue?.Available != true)
             {
                 // reject the change if the ruleset is not available.
-                Ruleset.Value = r.OldValue;
+                Ruleset.Value = r.OldValue?.Available == true ? r.OldValue : RulesetStore.AvailableRulesets.First();
                 return;
             }
 

osu.Game/Screens/Edit/Setup/ColoursSection.cs

@@ -2,6 +2,7 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using System.Collections.Generic;
+using System.Linq;
 using osu.Framework.Allocation;
 using osu.Framework.Graphics;
 using osu.Framework.Localisation;
@@ -32,7 +33,7 @@ namespace osu.Game.Screens.Edit.Setup
 
             var colours = Beatmap.BeatmapSkin?.GetConfig<GlobalSkinColours, IReadOnlyList<Color4>>(GlobalSkinColours.ComboColours)?.Value;
             if (colours != null)
-                comboColours.Colours.AddRange(colours);
+                comboColours.Colours.AddRange(colours.Select(c => (Colour4)c));
         }
     }
 }