diff --git a/CodeAnalysis/BannedSymbols.txt b/CodeAnalysis/BannedSymbols.txt index ea3e25142c..b72803482d 100644 --- a/CodeAnalysis/BannedSymbols.txt +++ b/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() instead. \ No newline at end of file +T:NuGet.Packaging.CollectionExtensions;Don't use internal extension methods. +M:System.Enum.HasFlag(System.Enum);Use osu.Framework.Extensions.EnumExtensions.HasFlagFast() instead. diff --git a/osu.Game.Tests/Visual/Settings/TestSceneSettingsNumberBox.cs b/osu.Game.Tests/Visual/Settings/TestSceneSettingsNumberBox.cs new file mode 100644 index 0000000000..334a814688 --- /dev/null +++ b/osu.Game.Tests/Visual/Settings/TestSceneSettingsNumberBox.cs @@ -0,0 +1,83 @@ +// Copyright (c) ppy Pty Ltd . 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().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); + + /// + /// The smallest number that overflows . + /// + private static long smallestOverflowValue => 1L + int.MaxValue; + } +} diff --git a/osu.Game.Tournament.Tests/NonVisual/CustomTourneyDirectoryTest.cs b/osu.Game.Tournament.Tests/NonVisual/CustomTourneyDirectoryTest.cs index fcc9f44f0c..94472a8bc7 100644 --- a/osu.Game.Tournament.Tests/NonVisual/CustomTourneyDirectoryTest.cs +++ b/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"); - host.Storage.DeleteDirectory("tournaments"); + if (Directory.Exists(osuRoot)) + 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; + } } } diff --git a/osu.Game.Tournament.Tests/NonVisual/IPCLocationTest.cs b/osu.Game.Tournament.Tests/NonVisual/IPCLocationTest.cs index eaa009c180..db89855db7 100644 --- a/osu.Game.Tournament.Tests/NonVisual/IPCLocationTest.cs +++ b/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 + { + } } } } diff --git a/osu.Game.Tournament.Tests/NonVisual/TournamentHostTest.cs b/osu.Game.Tournament.Tests/NonVisual/TournamentHostTest.cs index 319a768e65..bf99f69b2a 100644 --- a/osu.Game.Tournament.Tests/NonVisual/TournamentHostTest.cs +++ b/osu.Game.Tournament.Tests/NonVisual/TournamentHostTest.cs @@ -20,7 +20,7 @@ namespace osu.Game.Tournament.Tests.NonVisual return tournament; } - public static void WaitForOrAssert(Func result, string failureMessage, int timeout = 90000) + public static void WaitForOrAssert(Func result, string failureMessage, int timeout = 30000) { Task task = Task.Run(() => { diff --git a/osu.Game/Database/RealmContextFactory.cs b/osu.Game/Database/RealmContextFactory.cs index 411cfc52ee..0a21f67422 100644 --- a/osu.Game/Database/RealmContextFactory.cs +++ b/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; /// /// Flush any active contexts and block any further writes. diff --git a/osu.Game/Extensions/CollectionExtensions.cs b/osu.Game/Extensions/CollectionExtensions.cs new file mode 100644 index 0000000000..473dc4b8f4 --- /dev/null +++ b/osu.Game/Extensions/CollectionExtensions.cs @@ -0,0 +1,22 @@ +// Copyright (c) ppy Pty Ltd . 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(this ICollection collection, IEnumerable items) + { + // List has a potentially more optimal path to adding a range. + if (collection is List list) + list.AddRange(items); + else + { + foreach (T obj in items) + collection.Add(obj); + } + } + } +} diff --git a/osu.Game/Overlays/Settings/SettingsNumberBox.cs b/osu.Game/Overlays/Settings/SettingsNumberBox.cs index 545f1050b2..cbe9f7fc64 100644 --- a/osu.Game/Overlays/Settings/SettingsNumberBox.cs +++ b/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(); } } } diff --git a/osu.Game/Stores/BeatmapImporter.cs b/osu.Game/Stores/BeatmapImporter.cs index dad2b29dd0..32f0cd3d7a 100644 --- a/osu.Game/Stores/BeatmapImporter.cs +++ b/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; diff --git a/osu.Game/Stores/RealmArchiveModelImporter.cs b/osu.Game/Stores/RealmArchiveModelImporter.cs index 5b8c73b218..6370d4ebe4 100644 --- a/osu.Game/Stores/RealmArchiveModelImporter.cs +++ b/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; diff --git a/osu.Game/Tests/Visual/OsuTestScene.cs b/osu.Game/Tests/Visual/OsuTestScene.cs index fc7bc324ca..eedf266bbe 100644 --- a/osu.Game/Tests/Visual/OsuTestScene.cs +++ b/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 localStorage; - protected Storage LocalStorage => localStorage.Value; - - private Lazy contextFactory; - protected IResourceStore Resources; protected IAPIProvider API @@ -65,8 +58,6 @@ namespace osu.Game.Tests.Visual private DummyAPIAccess dummyAPI; - protected DatabaseContextFactory ContextFactory => contextFactory.Value; - /// /// Whether this test scene requires real-world API access. /// If true, this will bypass the local and use the provided one. @@ -74,15 +65,42 @@ namespace osu.Game.Tests.Visual protected virtual bool UseOnlineAPI => false; /// - /// 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. + /// A database context factory to be used by test runs. Can be isolated and reset by setting to true. /// - private Storage isolatedHostStorage; + /// + /// In interactive runs (ie. VisualTests) this will use the user's database if is not set to true. + /// + protected DatabaseContextFactory ContextFactory => contextFactory.Value; + + private Lazy contextFactory; + + /// + /// Whether a fresh storage should be initialised per test (method) run. + /// + /// + /// By default (ie. if not set to true): + /// - in interactive runs, the user's storage will be used + /// - in headless runs, a shared temporary storage will be used per test class. + /// + protected virtual bool UseFreshStoragePerRun => false; + + /// + /// A storage to be used by test runs. Can be isolated by setting to true. + /// + /// + /// In interactive runs (ie. VisualTests) this will use the user's storage if is not set to true. + /// + protected Storage LocalStorage => localStorage.Value; + + private Lazy localStorage; + + private Storage headlessHostStorage; + + private DrawableRulesetDependencies rulesetDependencies; protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnlyDependencyContainer parent) { - if (!UseFreshStoragePerRun) - isolatedHostStorage = (parent.Get() as HeadlessGameHost)?.Storage; + headlessHostStorage = (parent.Get() as HeadlessGameHost)?.Storage; Resources = parent.Get().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 = - new Lazy(() => isolatedHostStorage ?? new TemporaryNativeStorage($"{GetType().Name}-{Guid.NewGuid()}")); + localStorage = new Lazy(() => + { + // 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]