Merge branch 'master' into realm-ruleset-keybinding-short-name

CodeAnalysis/BannedSymbols.txt

@@ -8,4 +8,5 @@ M:osu.Framework.Graphics.Sprites.SpriteText.#ctor;Use OsuSpriteText.
 M:osu.Framework.Bindables.IBindableList`1.GetBoundCopy();Fails on iOS. Use manual ctor + BindTo instead. (see https://github.com/mono/mono/issues/19900)
 T:Microsoft.EntityFrameworkCore.Internal.EnumerableExtensions;Don't use internal extension methods.
 T:Microsoft.EntityFrameworkCore.Internal.TypeExtensions;Don't use internal extension methods.
-M:System.Enum.HasFlag(System.Enum);Use osu.Framework.Extensions.EnumExtensions.HasFlagFast<T>() instead.
+T:NuGet.Packaging.CollectionExtensions;Don't use internal extension methods.
+M:System.Enum.HasFlag(System.Enum);Use osu.Framework.Extensions.EnumExtensions.HasFlagFast<T>() instead.

osu.Game.Tests/Visual/Settings/TestSceneSettingsNumberBox.cs

@@ -0,0 +1,83 @@
+// 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.Linq;
+using NUnit.Framework;
+using osu.Framework.Testing;
+using osu.Game.Graphics.UserInterface;
+using osu.Game.Overlays.Settings;
+
+namespace osu.Game.Tests.Visual.Settings
+{
+    public class TestSceneSettingsNumberBox : OsuTestScene
+    {
+        private SettingsNumberBox numberBox;
+        private OsuTextBox textBox;
+
+        [SetUpSteps]
+        public void SetUpSteps()
+        {
+            AddStep("create number box", () => Child = numberBox = new SettingsNumberBox());
+            AddStep("get inner text box", () => textBox = numberBox.ChildrenOfType<OsuTextBox>().Single());
+        }
+
+        [Test]
+        public void TestLargeInteger()
+        {
+            AddStep("set current to 1,000,000,000", () => numberBox.Current.Value = 1_000_000_000);
+            AddAssert("text box text is correct", () => textBox.Text == "1000000000");
+        }
+
+        [Test]
+        public void TestUserInput()
+        {
+            inputText("42");
+            currentValueIs(42);
+            currentTextIs("42");
+
+            inputText(string.Empty);
+            currentValueIs(null);
+            currentTextIs(string.Empty);
+
+            inputText("555");
+            currentValueIs(555);
+            currentTextIs("555");
+
+            inputText("-4444");
+            // attempting to input the minus will raise an input error, the rest will pass through fine.
+            currentValueIs(4444);
+            currentTextIs("4444");
+
+            // checking the upper bound.
+            inputText(int.MaxValue.ToString());
+            currentValueIs(int.MaxValue);
+            currentTextIs(int.MaxValue.ToString());
+
+            inputText(smallestOverflowValue.ToString());
+            currentValueIs(int.MaxValue);
+            currentTextIs(int.MaxValue.ToString());
+
+            inputText("0");
+            currentValueIs(0);
+            currentTextIs("0");
+
+            // checking that leading zeroes are stripped.
+            inputText("00");
+            currentValueIs(0);
+            currentTextIs("0");
+
+            inputText("01");
+            currentValueIs(1);
+            currentTextIs("1");
+        }
+
+        private void inputText(string text) => AddStep($"set textbox text to {text}", () => textBox.Text = text);
+        private void currentValueIs(int? value) => AddAssert($"current value is {value?.ToString() ?? "null"}", () => numberBox.Current.Value == value);
+        private void currentTextIs(string value) => AddAssert($"current text is {value}", () => textBox.Text == value);
+
+        /// <summary>
+        /// The smallest number that overflows <see langword="int"/>.
+        /// </summary>
+        private static long smallestOverflowValue => 1L + int.MaxValue;
+    }
+}

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

@@ -38,7 +38,7 @@ namespace osu.Game.Tournament.Tests.NonVisual
         {
             using (HeadlessGameHost host = new HeadlessGameHost(nameof(TestCustomDirectory))) // don't use clean run as we are writing a config file.
             {
-                string osuDesktopStorage = basePath(nameof(TestCustomDirectory));
+                string osuDesktopStorage = PrepareBasePath(nameof(TestCustomDirectory));
                 const string custom_tournament = "custom";
 
                 // need access before the game has constructed its own storage yet.
@@ -60,6 +60,15 @@ namespace osu.Game.Tournament.Tests.NonVisual
                 finally
                 {
                     host.Exit();
+
+                    try
+                    {
+                        if (Directory.Exists(osuDesktopStorage))
+                            Directory.Delete(osuDesktopStorage, true);
+                    }
+                    catch
+                    {
+                    }
                 }
             }
         }
@@ -69,7 +78,7 @@ namespace osu.Game.Tournament.Tests.NonVisual
         {
             using (HeadlessGameHost host = new HeadlessGameHost(nameof(TestMigration))) // don't use clean run as we are writing test files for migration.
             {
-                string osuRoot = basePath(nameof(TestMigration));
+                string osuRoot = PrepareBasePath(nameof(TestMigration));
                 string configFile = Path.Combine(osuRoot, "tournament.ini");
 
                 if (File.Exists(configFile))
@@ -136,18 +145,29 @@ namespace osu.Game.Tournament.Tests.NonVisual
                 }
                 finally
                 {
+                    host.Exit();
+
                     try
                     {
-                        host.Storage.Delete("tournament.ini");
+                        if (Directory.Exists(osuRoot))
-                        host.Storage.DeleteDirectory("tournaments");
+                            Directory.Delete(osuRoot, true);
+                    }
+                    catch
+                    {
                     }
-                    catch { }
-
-                    host.Exit();
                 }
             }
         }
 
-        private string basePath(string testInstance) => Path.Combine(RuntimeInfo.StartupDirectory, "headless", testInstance);
+        public static string PrepareBasePath(string testInstance)
+        {
+            string basePath = Path.Combine(RuntimeInfo.StartupDirectory, "headless", testInstance);
+
+            // manually clean before starting in case there are left-over files at the test site.
+            if (Directory.Exists(basePath))
+                Directory.Delete(basePath, true);
+
+            return basePath;
+        }
     }
 }

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

@@ -3,7 +3,6 @@
 
 using System.IO;
 using NUnit.Framework;
-using osu.Framework;
 using osu.Framework.Allocation;
 using osu.Framework.Platform;
 using osu.Game.Tournament.IO;
@@ -20,7 +19,7 @@ namespace osu.Game.Tournament.Tests.NonVisual
             // don't use clean run because files are being written before osu! launches.
             using (HeadlessGameHost host = new HeadlessGameHost(nameof(CheckIPCLocation)))
             {
-                string basePath = Path.Combine(RuntimeInfo.StartupDirectory, "headless", nameof(CheckIPCLocation));
+                string basePath = CustomTourneyDirectoryTest.PrepareBasePath(nameof(CheckIPCLocation));
 
                 // Set up a fake IPC client for the IPC Storage to switch to.
                 string testStableInstallDirectory = Path.Combine(basePath, "stable-ce");
@@ -42,9 +41,16 @@ namespace osu.Game.Tournament.Tests.NonVisual
                 }
                 finally
                 {
-                    host.Storage.DeleteDirectory(testStableInstallDirectory);
-                    host.Storage.DeleteDirectory("tournaments");
                     host.Exit();
+
+                    try
+                    {
+                        if (Directory.Exists(basePath))
+                            Directory.Delete(basePath, true);
+                    }
+                    catch
+                    {
+                    }
                 }
             }
         }

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

@@ -20,7 +20,7 @@ namespace osu.Game.Tournament.Tests.NonVisual
             return tournament;
         }
 
-        public static void WaitForOrAssert(Func<bool> result, string failureMessage, int timeout = 90000)
+        public static void WaitForOrAssert(Func<bool> result, string failureMessage, int timeout = 30000)
         {
             Task task = Task.Run(() =>
             {

osu.Game/Database/RealmContextFactory.cs

@@ -297,7 +297,7 @@ namespace osu.Game.Database
         }
 
         private string? getRulesetShortNameFromLegacyID(long rulesetId) =>
-            efContextFactory?.Get().RulesetInfo.First(r => r.ID == rulesetId)?.ShortName;
+            efContextFactory?.Get().RulesetInfo.FirstOrDefault(r => r.ID == rulesetId)?.ShortName;
 
         /// <summary>
         /// Flush any active contexts and block any further writes.

osu.Game/Extensions/CollectionExtensions.cs

@@ -0,0 +1,22 @@
+// 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.Generic;
+
+namespace osu.Game.Extensions
+{
+    public static class CollectionExtensions
+    {
+        public static void AddRange<T>(this ICollection<T> collection, IEnumerable<T> items)
+        {
+            // List<T> has a potentially more optimal path to adding a range.
+            if (collection is List<T> list)
+                list.AddRange(items);
+            else
+            {
+                foreach (T obj in items)
+                    collection.Add(obj);
+            }
+        }
+    }
+}

osu.Game/Overlays/Settings/SettingsNumberBox.cs

@@ -35,7 +35,6 @@ namespace osu.Game.Overlays.Settings
                 {
                     numberBox = new OutlinedNumberBox
                     {
-                        LengthLimit = 9, // limited to less than a value that could overflow int32 backing.
                         Margin = new MarginPadding { Top = 5 },
                         RelativeSizeAxes = Axes.X,
                         CommitOnFocusLost = true
@@ -44,12 +43,19 @@ namespace osu.Game.Overlays.Settings
 
                 numberBox.Current.BindValueChanged(e =>
                 {
-                    int? value = null;
+                    if (string.IsNullOrEmpty(e.NewValue))
+                    {
+                        Current.Value = null;
+                        return;
+                    }
 
                     if (int.TryParse(e.NewValue, out int intVal))
-                        value = intVal;
+                        Current.Value = intVal;
+                    else
+                        numberBox.NotifyInputError();
 
-                    current.Value = value;
+                    // trigger Current again to either restore the previous text box value, or to reformat the new value via .ToString().
+                    Current.TriggerChange();
                 });
 
                 Current.BindValueChanged(e =>
@@ -62,6 +68,8 @@ namespace osu.Game.Overlays.Settings
         private class OutlinedNumberBox : OutlinedTextBox
         {
             protected override bool CanAddCharacter(char character) => char.IsNumber(character);
+
+            public new void NotifyInputError() => base.NotifyInputError();
         }
     }
 }

osu.Game/Stores/BeatmapImporter.cs

@@ -7,7 +7,6 @@ using System.IO;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
-using NuGet.Packaging;
 using osu.Framework.Audio.Track;
 using osu.Framework.Extensions;
 using osu.Framework.Extensions.IEnumerableExtensions;

osu.Game/Stores/RealmArchiveModelImporter.cs

@@ -8,7 +8,6 @@ using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using Humanizer;
-using NuGet.Packaging;
 using osu.Framework.Extensions;
 using osu.Framework.Extensions.IEnumerableExtensions;
 using osu.Framework.Logging;

osu.Game/Tests/Visual/OsuTestScene.cs

@@ -43,13 +43,6 @@ namespace osu.Game.Tests.Visual
 
         protected new OsuScreenDependencies Dependencies { get; private set; }
 
-        private DrawableRulesetDependencies rulesetDependencies;
-
-        private Lazy<Storage> localStorage;
-        protected Storage LocalStorage => localStorage.Value;
-
-        private Lazy<DatabaseContextFactory> contextFactory;
-
         protected IResourceStore<byte[]> Resources;
 
         protected IAPIProvider API
@@ -65,8 +58,6 @@ namespace osu.Game.Tests.Visual
 
         private DummyAPIAccess dummyAPI;
 
-        protected DatabaseContextFactory ContextFactory => contextFactory.Value;
-
         /// <summary>
         /// Whether this test scene requires real-world API access.
         /// If true, this will bypass the local <see cref="DummyAPIAccess"/> and use the <see cref="OsuGameBase"/> provided one.
@@ -74,15 +65,42 @@ namespace osu.Game.Tests.Visual
         protected virtual bool UseOnlineAPI => false;
 
         /// <summary>
-        /// When running headless, there is an opportunity to use the host storage rather than creating a second isolated one.
+        /// A database context factory to be used by test runs. Can be isolated and reset by setting <see cref="UseFreshStoragePerRun"/> to <c>true</c>.
-        /// This is because the host is recycled per TestScene execution in headless at an nunit level.
         /// </summary>
-        private Storage isolatedHostStorage;
+        /// <remarks>
+        /// In interactive runs (ie. VisualTests) this will use the user's database if <see cref="UseFreshStoragePerRun"/> is not set to <c>true</c>.
+        /// </remarks>
+        protected DatabaseContextFactory ContextFactory => contextFactory.Value;
+
+        private Lazy<DatabaseContextFactory> contextFactory;
+
+        /// <summary>
+        /// Whether a fresh storage should be initialised per test (method) run.
+        /// </summary>
+        /// <remarks>
+        /// By default (ie. if not set to <c>true</c>):
+        /// - in interactive runs, the user's storage will be used
+        /// - in headless runs, a shared temporary storage will be used per test class.
+        /// </remarks>
+        protected virtual bool UseFreshStoragePerRun => false;
+
+        /// <summary>
+        /// A storage to be used by test runs. Can be isolated by setting <see cref="UseFreshStoragePerRun"/> to <c>true</c>.
+        /// </summary>
+        /// <remarks>
+        /// In interactive runs (ie. VisualTests) this will use the user's storage if <see cref="UseFreshStoragePerRun"/> is not set to <c>true</c>.
+        /// </remarks>
+        protected Storage LocalStorage => localStorage.Value;
+
+        private Lazy<Storage> localStorage;
+
+        private Storage headlessHostStorage;
+
+        private DrawableRulesetDependencies rulesetDependencies;
 
         protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnlyDependencyContainer parent)
         {
-            if (!UseFreshStoragePerRun)
+            headlessHostStorage = (parent.Get<GameHost>() as HeadlessGameHost)?.Storage;
-                isolatedHostStorage = (parent.Get<GameHost>() as HeadlessGameHost)?.Storage;
 
             Resources = parent.Get<OsuGameBase>().Resources;
 
@@ -90,11 +108,6 @@ namespace osu.Game.Tests.Visual
             {
                 var factory = new DatabaseContextFactory(LocalStorage);
 
-                // only reset the database if not using the host storage.
-                // if we reset the host storage, it will delete global key bindings.
-                if (isolatedHostStorage == null)
-                    factory.ResetDatabase();
-
                 using (var usage = factory.Get())
                     usage.Migrate();
                 return factory;
@@ -138,8 +151,6 @@ namespace osu.Game.Tests.Visual
             base.Content.Add(content = new DrawSizePreservingFillContainer());
         }
 
-        protected virtual bool UseFreshStoragePerRun => false;
-
         public virtual void RecycleLocalStorage(bool isDisposing)
         {
             if (localStorage?.IsValueCreated == true)
@@ -154,8 +165,16 @@ namespace osu.Game.Tests.Visual
                 }
             }
 
-            localStorage =
+            localStorage = new Lazy<Storage>(() =>
-                new Lazy<Storage>(() => isolatedHostStorage ?? new TemporaryNativeStorage($"{GetType().Name}-{Guid.NewGuid()}"));
+            {
+                // When running headless, there is an opportunity to use the host storage rather than creating a second isolated one.
+                // This is because the host is recycled per TestScene execution in headless at an nunit level.
+                // Importantly, we can't use this optimisation when `UseFreshStoragePerRun` is true, as it doesn't reset per test method.
+                if (!UseFreshStoragePerRun && headlessHostStorage != null)
+                    return headlessHostStorage;
+
+                return new TemporaryNativeStorage($"{GetType().Name}-{Guid.NewGuid()}");
+            });
         }
 
         [Resolved]