diff --git a/osu.Android.props b/osu.Android.props index 81f1a3e259..eb5e209e29 100644 --- a/osu.Android.props +++ b/osu.Android.props @@ -52,7 +52,7 @@ - + diff --git a/osu.Game.Tests/Database/BeatmapImporterTests.cs b/osu.Game.Tests/Database/BeatmapImporterTests.cs index d4956e97e0..de2bbeb791 100644 --- a/osu.Game.Tests/Database/BeatmapImporterTests.cs +++ b/osu.Game.Tests/Database/BeatmapImporterTests.cs @@ -1,6 +1,8 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +#nullable enable + using System; using System.Collections.Generic; using System.Diagnostics; @@ -12,7 +14,6 @@ using NUnit.Framework; using osu.Framework.Extensions; using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Logging; -using osu.Framework.Platform; using osu.Game.Beatmaps; using osu.Game.Database; using osu.Game.Extensions; @@ -28,8 +29,6 @@ using SharpCompress.Archives.Zip; using SharpCompress.Common; using SharpCompress.Writers.Zip; -#nullable enable - namespace osu.Game.Tests.Database { [TestFixture] @@ -394,7 +393,6 @@ namespace osu.Game.Tests.Database } [Test] - [Ignore("intentionally broken by import optimisations")] public void TestImportThenImportWithChangedFile() { RunTestWithRealmAsync(async (realm, storage) => @@ -491,7 +489,6 @@ namespace osu.Game.Tests.Database } [Test] - [Ignore("intentionally broken by import optimisations")] public void TestImportCorruptThenImport() { RunTestWithRealmAsync(async (realm, storage) => @@ -503,16 +500,18 @@ namespace osu.Game.Tests.Database var firstFile = imported.Files.First(); + var fileStorage = storage.GetStorageForDirectory("files"); + long originalLength; - using (var stream = storage.GetStream(firstFile.File.GetStoragePath())) + using (var stream = fileStorage.GetStream(firstFile.File.GetStoragePath())) originalLength = stream.Length; - using (var stream = storage.CreateFileSafely(firstFile.File.GetStoragePath())) + using (var stream = fileStorage.CreateFileSafely(firstFile.File.GetStoragePath())) stream.WriteByte(0); var importedSecondTime = await LoadOszIntoStore(importer, realm.Realm); - using (var stream = storage.GetStream(firstFile.File.GetStoragePath())) + using (var stream = fileStorage.GetStream(firstFile.File.GetStoragePath())) Assert.AreEqual(stream.Length, originalLength, "Corruption was not fixed on second import"); // check the newly "imported" beatmap is actually just the restored previous import. since it matches hash. @@ -622,7 +621,7 @@ namespace osu.Game.Tests.Database using var importer = new BeatmapModelManager(realm, storage); using var store = new RealmRulesetStore(realm, storage); - var imported = await LoadOszIntoStore(importer, realm.Realm); + var imported = await LoadOszIntoStore(importer, realm.Realm, batchImport: true); deleteBeatmapSet(imported, realm.Realm); @@ -678,7 +677,7 @@ namespace osu.Game.Tests.Database { RunTestWithRealmAsync(async (realm, storage) => { - using var importer = new NonOptimisedBeatmapImporter(realm, storage); + using var importer = new BeatmapModelManager(realm, storage); using var store = new RealmRulesetStore(realm, storage); var imported = await LoadOszIntoStore(importer, realm.Realm); @@ -960,11 +959,11 @@ namespace osu.Game.Tests.Database return realm.All().FirstOrDefault(beatmapSet => beatmapSet.ID == importedSet!.ID); } - public static async Task LoadOszIntoStore(BeatmapImporter importer, Realm realm, string? path = null, bool virtualTrack = false) + public static async Task LoadOszIntoStore(BeatmapImporter importer, Realm realm, string? path = null, bool virtualTrack = false, bool batchImport = false) { string? temp = path ?? TestResources.GetTestBeatmapForImport(virtualTrack); - var importedSet = await importer.Import(new ImportTask(temp)); + var importedSet = await importer.Import(new ImportTask(temp), batchImport); Assert.NotNull(importedSet); Debug.Assert(importedSet != null); @@ -1081,15 +1080,5 @@ namespace osu.Game.Tests.Database Assert.Fail(failureMessage); } - - public class NonOptimisedBeatmapImporter : BeatmapImporter - { - public NonOptimisedBeatmapImporter(RealmAccess realm, Storage storage) - : base(realm, storage) - { - } - - protected override bool HasCustomHashFunction => true; - } } } diff --git a/osu.Game.Tests/Database/LegacyBeatmapImporterTest.cs b/osu.Game.Tests/Database/LegacyBeatmapImporterTest.cs new file mode 100644 index 0000000000..2af46374fe --- /dev/null +++ b/osu.Game.Tests/Database/LegacyBeatmapImporterTest.cs @@ -0,0 +1,73 @@ +// 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; +using System.IO; +using System.Linq; +using NUnit.Framework; +using osu.Framework.Platform; +using osu.Framework.Testing; +using osu.Game.Database; +using osu.Game.IO; + +namespace osu.Game.Tests.Database +{ + [TestFixture] + public class LegacyBeatmapImporterTest + { + private readonly TestLegacyBeatmapImporter importer = new TestLegacyBeatmapImporter(); + + [Test] + public void TestSongsSubdirectories() + { + using (var storage = new TemporaryNativeStorage("stable-songs-folder")) + { + var songsStorage = storage.GetStorageForDirectory(StableStorage.STABLE_DEFAULT_SONGS_PATH); + + // normal beatmap folder + var beatmap1 = songsStorage.GetStorageForDirectory("beatmap1"); + createFile(beatmap1, "beatmap.osu"); + + // songs subdirectory + var subdirectory = songsStorage.GetStorageForDirectory("subdirectory"); + createFile(subdirectory, Path.Combine("beatmap2", "beatmap.osu")); + createFile(subdirectory, Path.Combine("beatmap3", "beatmap.osu")); + createFile(subdirectory, Path.Combine("sub-subdirectory", "beatmap4", "beatmap.osu")); + + // songs subdirectory with system file + var subdirectory2 = songsStorage.GetStorageForDirectory("subdirectory2"); + createFile(subdirectory2, ".DS_Store"); + createFile(subdirectory2, Path.Combine("beatmap5", "beatmap.osu")); + createFile(subdirectory2, Path.Combine("beatmap6", "beatmap.osu")); + + // empty songs subdirectory + songsStorage.GetStorageForDirectory("subdirectory3"); + + string[] paths = importer.GetStableImportPaths(songsStorage).ToArray(); + Assert.That(paths.Length, Is.EqualTo(6)); + Assert.That(paths.Contains(songsStorage.GetFullPath("beatmap1"))); + Assert.That(paths.Contains(songsStorage.GetFullPath(Path.Combine("subdirectory", "beatmap2")))); + Assert.That(paths.Contains(songsStorage.GetFullPath(Path.Combine("subdirectory", "beatmap3")))); + Assert.That(paths.Contains(songsStorage.GetFullPath(Path.Combine("subdirectory", "sub-subdirectory", "beatmap4")))); + Assert.That(paths.Contains(songsStorage.GetFullPath(Path.Combine("subdirectory2", "beatmap5")))); + Assert.That(paths.Contains(songsStorage.GetFullPath(Path.Combine("subdirectory2", "beatmap6")))); + } + + static void createFile(Storage storage, string path) + { + using (var stream = storage.CreateFileSafely(path)) + stream.WriteByte(0); + } + } + + private class TestLegacyBeatmapImporter : LegacyBeatmapImporter + { + public TestLegacyBeatmapImporter() + : base(null) + { + } + + public new IEnumerable GetStableImportPaths(Storage storage) => base.GetStableImportPaths(storage); + } + } +} diff --git a/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs b/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs index 278acce3e5..4f255ddd00 100644 --- a/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs +++ b/osu.Game.Tests/Online/TestSceneOnlinePlayBeatmapAvailabilityTracker.cs @@ -225,10 +225,10 @@ namespace osu.Game.Tests.Online this.testBeatmapManager = testBeatmapManager; } - public override Live Import(BeatmapSetInfo item, ArchiveReader archive = null, CancellationToken cancellationToken = default) + public override Live Import(BeatmapSetInfo item, ArchiveReader archive = null, bool batchImport = false, CancellationToken cancellationToken = default) { testBeatmapManager.AllowImport.Task.WaitSafely(); - return (testBeatmapManager.CurrentImport = base.Import(item, archive, cancellationToken)); + return (testBeatmapManager.CurrentImport = base.Import(item, archive, batchImport, cancellationToken)); } } } diff --git a/osu.Game.Tests/Visual/Editing/TestSceneComposerSelection.cs b/osu.Game.Tests/Visual/Editing/TestSceneComposerSelection.cs index dddd9f07ab..17ca9da8f8 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneComposerSelection.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneComposerSelection.cs @@ -10,6 +10,7 @@ using osu.Framework.Graphics.UserInterface; using osu.Game.Beatmaps; using osu.Game.Graphics.UserInterface; using osu.Game.Rulesets; +using osu.Game.Rulesets.Edit; using osu.Game.Rulesets.Objects; using osu.Game.Rulesets.Osu; using osu.Game.Rulesets.Osu.Objects; @@ -61,6 +62,21 @@ namespace osu.Game.Tests.Visual.Editing AddUntilStep("context menu is visible", () => contextMenuContainer.ChildrenOfType().Single().State == MenuState.Open); } + [Test] + public void TestSelectAndShowContextMenuOutsideBounds() + { + var addedObject = new HitCircle { StartTime = 100, Position = OsuPlayfield.BASE_SIZE }; + AddStep("add hitobject", () => EditorBeatmap.Add(addedObject)); + + AddStep("descale blueprint container", () => this.ChildrenOfType().Single().Scale = new Vector2(0.5f)); + AddStep("move mouse to bottom-right", () => InputManager.MoveMouseTo(blueprintContainer.ToScreenSpace(blueprintContainer.LayoutRectangle.BottomRight + new Vector2(10)))); + + AddStep("right click", () => InputManager.Click(MouseButton.Right)); + + AddUntilStep("hitobject selected", () => EditorBeatmap.SelectedHitObjects.Single() == addedObject); + AddUntilStep("context menu is visible", () => contextMenuContainer.ChildrenOfType().Single().State == MenuState.Open); + } + [Test] public void TestNudgeSelection() { diff --git a/osu.Game.Tests/Visual/Menus/TestSceneToolbar.cs b/osu.Game.Tests/Visual/Menus/TestSceneToolbar.cs index eb77453199..f230e48b11 100644 --- a/osu.Game.Tests/Visual/Menus/TestSceneToolbar.cs +++ b/osu.Game.Tests/Visual/Menus/TestSceneToolbar.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System.Linq; +using JetBrains.Annotations; using Moq; using NUnit.Framework; using osu.Framework.Allocation; @@ -13,6 +14,7 @@ using osu.Framework.Graphics.Shapes; using osu.Framework.Testing; using osu.Game.Graphics.Containers; using osu.Game.Overlays; +using osu.Game.Overlays.Notifications; using osu.Game.Overlays.Toolbar; using osu.Game.Rulesets; using osuTK.Graphics; @@ -28,21 +30,44 @@ namespace osu.Game.Tests.Visual.Menus [Resolved] private IRulesetStore rulesets { get; set; } - private readonly Mock notifications = new Mock(); + [Cached] + private readonly NowPlayingOverlay nowPlayingOverlay = new NowPlayingOverlay + { + Anchor = Anchor.TopRight, + Origin = Anchor.TopRight, + Y = Toolbar.HEIGHT, + }; + + [Cached] + private readonly VolumeOverlay volumeOverlay = new VolumeOverlay + { + Anchor = Anchor.CentreLeft, + Origin = Anchor.CentreLeft, + }; + + private readonly Mock notifications = new Mock(); private readonly BindableInt unreadNotificationCount = new BindableInt(); [BackgroundDependencyLoader] private void load() { - Dependencies.CacheAs(notifications.Object); + Dependencies.CacheAs(notifications.Object); notifications.SetupGet(n => n.UnreadCount).Returns(unreadNotificationCount); } [SetUp] public void SetUp() => Schedule(() => { - Child = toolbar = new TestToolbar { State = { Value = Visibility.Visible } }; + Remove(nowPlayingOverlay); + Remove(volumeOverlay); + + Children = new Drawable[] + { + nowPlayingOverlay, + volumeOverlay, + toolbar = new TestToolbar { State = { Value = Visibility.Visible } }, + }; }); [Test] @@ -122,9 +147,51 @@ namespace osu.Game.Tests.Visual.Menus AddAssert("not scrolled", () => scroll.Current == 0); } + [Test] + public void TestVolumeControlViaMusicButtonScroll() + { + AddStep("hover toolbar music button", () => InputManager.MoveMouseTo(this.ChildrenOfType().Single())); + + AddStep("reset volume", () => Audio.Volume.Value = 1); + + AddRepeatStep("scroll down", () => InputManager.ScrollVerticalBy(-10), 5); + AddAssert("volume lowered down", () => Audio.Volume.Value < 1); + AddRepeatStep("scroll up", () => InputManager.ScrollVerticalBy(10), 5); + AddAssert("volume raised up", () => Audio.Volume.Value == 1); + } + + [Test] + public void TestVolumeControlViaMusicButtonArrowKeys() + { + AddStep("hover toolbar music button", () => InputManager.MoveMouseTo(this.ChildrenOfType().Single())); + + AddStep("reset volume", () => Audio.Volume.Value = 1); + + AddRepeatStep("arrow down", () => InputManager.Key(Key.Down), 5); + AddAssert("volume lowered down", () => Audio.Volume.Value < 1); + AddRepeatStep("arrow up", () => InputManager.Key(Key.Up), 5); + AddAssert("volume raised up", () => Audio.Volume.Value == 1); + } + public class TestToolbar : Toolbar { public new Bindable OverlayActivationMode => base.OverlayActivationMode as Bindable; } + + // interface mocks break hot reload, mocking this stub implementation instead works around it. + // see: https://github.com/moq/moq4/issues/1252 + [UsedImplicitly] + public class TestNotificationOverlay : INotificationOverlay + { + public virtual void Post(Notification notification) + { + } + + public virtual void Hide() + { + } + + public virtual IBindable UnreadCount => null; + } } } diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 51f4d23bb4..e156c2b430 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -353,11 +353,11 @@ namespace osu.Game.Beatmaps public Task>> Import(ProgressNotification notification, params ImportTask[] tasks) => beatmapModelManager.Import(notification, tasks); - public Task?> Import(ImportTask task, bool lowPriority = false, CancellationToken cancellationToken = default) => beatmapModelManager.Import(task, lowPriority, cancellationToken); + public Task?> Import(ImportTask task, bool batchImport = false, CancellationToken cancellationToken = default) => beatmapModelManager.Import(task, batchImport, cancellationToken); - public Task?> Import(ArchiveReader archive, bool lowPriority = false, CancellationToken cancellationToken = default) => beatmapModelManager.Import(archive, lowPriority, cancellationToken); + public Task?> Import(ArchiveReader archive, bool batchImport = false, CancellationToken cancellationToken = default) => beatmapModelManager.Import(archive, batchImport, cancellationToken); - public Live? Import(BeatmapSetInfo item, ArchiveReader? archive = null, CancellationToken cancellationToken = default) => beatmapModelManager.Import(item, archive, cancellationToken); + public Live? Import(BeatmapSetInfo item, ArchiveReader? archive = null, CancellationToken cancellationToken = default) => beatmapModelManager.Import(item, archive, false, cancellationToken); public IEnumerable HandledExtensions => beatmapModelManager.HandledExtensions; diff --git a/osu.Game/Database/LegacyBeatmapImporter.cs b/osu.Game/Database/LegacyBeatmapImporter.cs index 97f6eba6c2..3477312154 100644 --- a/osu.Game/Database/LegacyBeatmapImporter.cs +++ b/osu.Game/Database/LegacyBeatmapImporter.cs @@ -1,6 +1,9 @@ // 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; +using System.Linq; +using osu.Framework.IO.Stores; using osu.Framework.Platform; using osu.Game.Beatmaps; using osu.Game.IO; @@ -13,6 +16,24 @@ namespace osu.Game.Database protected override Storage PrepareStableStorage(StableStorage stableStorage) => stableStorage.GetSongStorage(); + protected override IEnumerable GetStableImportPaths(Storage storage) + { + foreach (string directory in storage.GetDirectories(string.Empty)) + { + var directoryStorage = storage.GetStorageForDirectory(directory); + + if (!directoryStorage.GetFiles(string.Empty).ExcludeSystemFileNames().Any()) + { + // if a directory doesn't contain files, attempt looking for beatmaps inside of that directory. + // this is a special behaviour in stable for beatmaps only, see https://github.com/ppy/osu/issues/18615. + foreach (string subDirectory in GetStableImportPaths(directoryStorage)) + yield return subDirectory; + } + else + yield return storage.GetFullPath(directory); + } + } + public LegacyBeatmapImporter(IModelImporter importer) : base(importer) { diff --git a/osu.Game/IO/StableStorage.cs b/osu.Game/IO/StableStorage.cs index 84b7da91fc..d4ff8fcdda 100644 --- a/osu.Game/IO/StableStorage.cs +++ b/osu.Game/IO/StableStorage.cs @@ -14,7 +14,7 @@ namespace osu.Game.IO /// public class StableStorage : DesktopStorage { - private const string stable_default_songs_path = "Songs"; + public const string STABLE_DEFAULT_SONGS_PATH = "Songs"; private readonly DesktopGameHost host; private readonly Lazy songsPath; @@ -62,7 +62,7 @@ namespace osu.Game.IO } } - return GetFullPath(stable_default_songs_path); + return GetFullPath(STABLE_DEFAULT_SONGS_PATH); } } } diff --git a/osu.Game/Overlays/Toolbar/ToolbarMusicButton.cs b/osu.Game/Overlays/Toolbar/ToolbarMusicButton.cs index 0f5e8e5456..d59127d61f 100644 --- a/osu.Game/Overlays/Toolbar/ToolbarMusicButton.cs +++ b/osu.Game/Overlays/Toolbar/ToolbarMusicButton.cs @@ -2,24 +2,131 @@ // See the LICENCE file in the repository root for full licence text. using osu.Framework.Allocation; +using osu.Framework.Audio; +using osu.Framework.Bindables; +using osu.Framework.Extensions.Color4Extensions; using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; +using osu.Framework.Graphics.Shapes; +using osu.Framework.Graphics.Transforms; +using osu.Framework.Input.Events; +using osu.Framework.Threading; using osu.Game.Input.Bindings; +using osuTK.Graphics; +using osuTK.Input; namespace osu.Game.Overlays.Toolbar { public class ToolbarMusicButton : ToolbarOverlayToggleButton { + private Circle volumeBar; + protected override Anchor TooltipAnchor => Anchor.TopRight; public ToolbarMusicButton() { Hotkey = GlobalAction.ToggleNowPlaying; + AutoSizeAxes = Axes.X; } [BackgroundDependencyLoader(true)] private void load(NowPlayingOverlay music) { StateContainer = music; + + Flow.Padding = new MarginPadding { Horizontal = Toolbar.HEIGHT / 4 }; + Flow.Add(volumeDisplay = new Container + { + Anchor = Anchor.CentreLeft, + Origin = Anchor.CentreLeft, + Width = 3f, + Height = IconContainer.Height, + Margin = new MarginPadding { Horizontal = 2.5f }, + Masking = true, + Children = new[] + { + new Circle + { + RelativeSizeAxes = Axes.Both, + Colour = Color4.White.Opacity(0.25f), + }, + volumeBar = new Circle + { + RelativeSizeAxes = Axes.Both, + Height = 0f, + Anchor = Anchor.BottomLeft, + Origin = Anchor.BottomLeft, + Colour = Color4.White, + } + } + }); + } + + [Resolved] + private AudioManager audio { get; set; } + + [Resolved(canBeNull: true)] + private VolumeOverlay volume { get; set; } + + private IBindable globalVolume; + private Container volumeDisplay; + + protected override void LoadComplete() + { + base.LoadComplete(); + + globalVolume = audio.Volume.GetBoundCopy(); + globalVolume.BindValueChanged(v => volumeBar.ResizeHeightTo((float)v.NewValue, 200, Easing.OutQuint), true); + } + + protected override bool OnKeyDown(KeyDownEvent e) + { + switch (e.Key) + { + case Key.Up: + focusForAdjustment(); + volume?.Adjust(GlobalAction.IncreaseVolume); + return true; + + case Key.Down: + focusForAdjustment(); + volume?.Adjust(GlobalAction.DecreaseVolume); + return true; + } + + return base.OnKeyDown(e); + } + + protected override bool OnScroll(ScrollEvent e) + { + focusForAdjustment(); + volume?.Adjust(GlobalAction.IncreaseVolume, e.ScrollDelta.Y, e.IsPrecise); + return true; + } + + private void focusForAdjustment() + { + volume?.FocusMasterVolume(); + expandVolumeBarTemporarily(); + } + + private TransformSequence expandTransform; + private ScheduledDelegate contractTransform; + + private void expandVolumeBarTemporarily() + { + // avoid starting a new transform if one is already active. + if (expandTransform == null) + { + expandTransform = volumeDisplay.ResizeWidthTo(6, 500, Easing.OutQuint); + expandTransform.Finally(_ => expandTransform = null); + } + + contractTransform?.Cancel(); + contractTransform = Scheduler.AddDelayed(() => + { + volumeDisplay.ResizeWidthTo(3f, 500, Easing.OutQuint); + }, 1000); } } } diff --git a/osu.Game/Overlays/VolumeOverlay.cs b/osu.Game/Overlays/VolumeOverlay.cs index 9d2ed3f837..46ea45491e 100644 --- a/osu.Game/Overlays/VolumeOverlay.cs +++ b/osu.Game/Overlays/VolumeOverlay.cs @@ -140,11 +140,16 @@ namespace osu.Game.Overlays private ScheduledDelegate popOutDelegate; + public void FocusMasterVolume() + { + volumeMeters.Select(volumeMeterMaster); + } + public override void Show() { // Focus on the master meter as a default if previously hidden if (State.Value == Visibility.Hidden) - volumeMeters.Select(volumeMeterMaster); + FocusMasterVolume(); if (State.Value == Visibility.Visible) schedulePopOut(); diff --git a/osu.Game/Scoring/ScoreManager.cs b/osu.Game/Scoring/ScoreManager.cs index d42bf1b454..e857d2109f 100644 --- a/osu.Game/Scoring/ScoreManager.cs +++ b/osu.Game/Scoring/ScoreManager.cs @@ -280,7 +280,7 @@ namespace osu.Game.Scoring public Task>> Import(ProgressNotification notification, params ImportTask[] tasks) => scoreModelManager.Import(notification, tasks); - public Live Import(ScoreInfo item, ArchiveReader archive = null, bool lowPriority = false, CancellationToken cancellationToken = default) => scoreModelManager.Import(item, archive, cancellationToken); + public Live Import(ScoreInfo item, ArchiveReader archive = null, bool batchImport = false, CancellationToken cancellationToken = default) => scoreModelManager.Import(item, archive, batchImport, cancellationToken); public bool IsAvailableLocally(ScoreInfo model) => scoreModelManager.IsAvailableLocally(model); diff --git a/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs b/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs index d56dc176f6..fbec80a63b 100644 --- a/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs +++ b/osu.Game/Screens/Edit/Compose/Components/BlueprintContainer.cs @@ -106,6 +106,13 @@ namespace osu.Game.Screens.Edit.Compose.Components /// protected virtual bool AllowDeselectionDuringDrag => true; + /// + /// Positional input must be received outside the container's bounds, + /// in order to handle blueprints which are partially offscreen. + /// + /// + public override bool ReceivePositionalInputAt(Vector2 screenSpacePos) => true; + protected override bool OnMouseDown(MouseDownEvent e) { bool selectionPerformed = performMouseDownActions(e); diff --git a/osu.Game/Screens/Edit/Compose/Components/ComposeBlueprintContainer.cs b/osu.Game/Screens/Edit/Compose/Components/ComposeBlueprintContainer.cs index 68be20720d..0be2cb4462 100644 --- a/osu.Game/Screens/Edit/Compose/Components/ComposeBlueprintContainer.cs +++ b/osu.Game/Screens/Edit/Compose/Components/ComposeBlueprintContainer.cs @@ -30,8 +30,6 @@ namespace osu.Game.Screens.Edit.Compose.Components /// public class ComposeBlueprintContainer : EditorBlueprintContainer { - public override bool ReceivePositionalInputAt(Vector2 screenSpacePos) => true; - private readonly Container placementBlueprintContainer; protected new EditorSelectionHandler SelectionHandler => (EditorSelectionHandler)base.SelectionHandler; diff --git a/osu.Game/Screens/Edit/Compose/Components/SelectionHandler.cs b/osu.Game/Screens/Edit/Compose/Components/SelectionHandler.cs index 78b98a3649..e5020afcde 100644 --- a/osu.Game/Screens/Edit/Compose/Components/SelectionHandler.cs +++ b/osu.Game/Screens/Edit/Compose/Components/SelectionHandler.cs @@ -97,6 +97,13 @@ namespace osu.Game.Screens.Edit.Compose.Components #region User Input Handling + /// + /// Positional input must be received outside the container's bounds, + /// in order to handle blueprints which are partially offscreen. + /// + /// + public override bool ReceivePositionalInputAt(Vector2 screenSpacePos) => true; + /// /// Handles the selected items being moved. /// diff --git a/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineBlueprintContainer.cs b/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineBlueprintContainer.cs index a9e9ef5001..1c0c2fb215 100644 --- a/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineBlueprintContainer.cs +++ b/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineBlueprintContainer.cs @@ -33,9 +33,6 @@ namespace osu.Game.Screens.Edit.Compose.Components.Timeline private Bindable placement; private SelectionBlueprint placementBlueprint; - // We want children within the timeline to be interactable - public override bool ReceivePositionalInputAt(Vector2 screenSpacePos) => timeline.ScreenSpaceDrawQuad.Contains(screenSpacePos); - public TimelineBlueprintContainer(HitObjectComposer composer) : base(composer) { diff --git a/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineSelectionHandler.cs b/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineSelectionHandler.cs index e98cf8332f..da12ab521f 100644 --- a/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineSelectionHandler.cs +++ b/osu.Game/Screens/Edit/Compose/Components/Timeline/TimelineSelectionHandler.cs @@ -6,23 +6,16 @@ using System.Collections.Generic; using System.Diagnostics; using System.Linq; using JetBrains.Annotations; -using osu.Framework.Allocation; using osu.Framework.Input.Events; using osu.Game.Input.Bindings; using osu.Game.Rulesets.Edit; using osu.Game.Rulesets.Objects; -using osuTK; using osuTK.Input; namespace osu.Game.Screens.Edit.Compose.Components.Timeline { internal class TimelineSelectionHandler : EditorSelectionHandler { - [Resolved] - private Timeline timeline { get; set; } - - public override bool ReceivePositionalInputAt(Vector2 screenSpacePos) => timeline.ScreenSpaceDrawQuad.Contains(screenSpacePos); - // for now we always allow movement. snapping is provided by the Timeline's "distance" snap implementation public override bool HandleMovement(MoveSelectionEvent moveEvent) => true; diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index 7ec922987b..f296386d34 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -276,13 +276,10 @@ namespace osu.Game.Skinning public Task>> Import(ProgressNotification notification, params ImportTask[] tasks) => skinModelManager.Import(notification, tasks); - public Task> Import(ImportTask task, bool lowPriority = false, CancellationToken cancellationToken = default) => skinModelManager.Import(task, lowPriority, cancellationToken); + public Task> Import(ImportTask task, bool batchImport = false, CancellationToken cancellationToken = default) => skinModelManager.Import(task, batchImport, cancellationToken); - public Task> Import(ArchiveReader archive, bool lowPriority = false, CancellationToken cancellationToken = default) => - skinModelManager.Import(archive, lowPriority, cancellationToken); - - public Live Import(SkinInfo item, ArchiveReader archive = null, bool lowPriority = false, CancellationToken cancellationToken = default) => - skinModelManager.Import(item, archive, cancellationToken); + public Task> Import(ArchiveReader archive, bool batchImport = false, CancellationToken cancellationToken = default) => + skinModelManager.Import(archive, batchImport, cancellationToken); #endregion diff --git a/osu.Game/Skinning/SkinModelManager.cs b/osu.Game/Skinning/SkinModelManager.cs index 23813e8eb2..0a18449800 100644 --- a/osu.Game/Skinning/SkinModelManager.cs +++ b/osu.Game/Skinning/SkinModelManager.cs @@ -46,8 +46,6 @@ namespace osu.Game.Skinning private const string unknown_creator_string = @"Unknown"; - protected override bool HasCustomHashFunction => true; - protected override void Populate(SkinInfo model, ArchiveReader? archive, Realm realm, CancellationToken cancellationToken = default) { var skinInfoFile = model.Files.SingleOrDefault(f => f.Filename == skin_info_file); diff --git a/osu.Game/Stores/RealmArchiveModelImporter.cs b/osu.Game/Stores/RealmArchiveModelImporter.cs index 82b77bfb05..7f31bc66be 100644 --- a/osu.Game/Stores/RealmArchiveModelImporter.cs +++ b/osu.Game/Stores/RealmArchiveModelImporter.cs @@ -32,12 +32,16 @@ namespace osu.Game.Stores public abstract class RealmArchiveModelImporter : IModelImporter where TModel : RealmObject, IHasRealmFiles, IHasGuidPrimaryKey, ISoftDelete { + /// + /// The maximum number of concurrent imports to run per import scheduler. + /// private const int import_queue_request_concurrency = 1; /// - /// The size of a batch import operation before considering it a lower priority operation. + /// The minimum number of items in a single import call in order for the import to be processed as a batch. + /// Batch imports will apply optimisations preferring speed over consistency when detecting changes in already-imported items. /// - private const int low_priority_import_batch_size = 1; + private const int minimum_items_considered_batch_import = 10; /// /// A singleton scheduler shared by all . @@ -49,11 +53,11 @@ namespace osu.Game.Stores private static readonly ThreadedTaskScheduler import_scheduler = new ThreadedTaskScheduler(import_queue_request_concurrency, nameof(RealmArchiveModelImporter)); /// - /// A second scheduler for lower priority imports. + /// A second scheduler for batch imports. /// For simplicity, these will just run in parallel with normal priority imports, but a future refactor would see this implemented via a custom scheduler/queue. /// See https://gist.github.com/peppy/f0e118a14751fc832ca30dd48ba3876b for an incomplete version of this. /// - private static readonly ThreadedTaskScheduler import_scheduler_low_priority = new ThreadedTaskScheduler(import_queue_request_concurrency, nameof(RealmArchiveModelImporter)); + private static readonly ThreadedTaskScheduler import_scheduler_batch = new ThreadedTaskScheduler(import_queue_request_concurrency, nameof(RealmArchiveModelImporter)); public virtual IEnumerable HandledExtensions => new[] { @".zip" }; @@ -105,7 +109,7 @@ namespace osu.Game.Stores var imported = new List>(); - bool isLowPriorityImport = tasks.Length > low_priority_import_batch_size; + bool isBatchImport = tasks.Length >= minimum_items_considered_batch_import; try { @@ -115,7 +119,7 @@ namespace osu.Game.Stores try { - var model = await Import(task, isLowPriorityImport, notification.CancellationToken).ConfigureAwait(false); + var model = await Import(task, isBatchImport, notification.CancellationToken).ConfigureAwait(false); lock (imported) { @@ -178,16 +182,16 @@ namespace osu.Game.Stores /// Note that this bypasses the UI flow and should only be used for special cases or testing. /// /// The containing data about the to import. - /// Whether this is a low priority import. + /// Whether this import is part of a larger batch. /// An optional cancellation token. /// The imported model, if successful. - public async Task?> Import(ImportTask task, bool lowPriority = false, CancellationToken cancellationToken = default) + public async Task?> Import(ImportTask task, bool batchImport = false, CancellationToken cancellationToken = default) { cancellationToken.ThrowIfCancellationRequested(); Live? import; using (ArchiveReader reader = task.GetReader()) - import = await Import(reader, lowPriority, cancellationToken).ConfigureAwait(false); + import = await Import(reader, batchImport, cancellationToken).ConfigureAwait(false); // We may or may not want to delete the file depending on where it is stored. // e.g. reconstructing/repairing database with items from default storage. @@ -210,9 +214,9 @@ namespace osu.Game.Stores /// Silently import an item from an . /// /// The archive to be imported. - /// Whether this is a low priority import. + /// Whether this import is part of a larger batch. /// An optional cancellation token. - public async Task?> Import(ArchiveReader archive, bool lowPriority = false, CancellationToken cancellationToken = default) + public async Task?> Import(ArchiveReader archive, bool batchImport = false, CancellationToken cancellationToken = default) { cancellationToken.ThrowIfCancellationRequested(); @@ -235,10 +239,10 @@ namespace osu.Game.Stores return null; } - var scheduledImport = Task.Factory.StartNew(() => Import(model, archive, cancellationToken), + var scheduledImport = Task.Factory.StartNew(() => Import(model, archive, batchImport, cancellationToken), cancellationToken, TaskCreationOptions.HideScheduler, - lowPriority ? import_scheduler_low_priority : import_scheduler); + batchImport ? import_scheduler_batch : import_scheduler); return await scheduledImport.ConfigureAwait(false); } @@ -248,106 +252,104 @@ namespace osu.Game.Stores /// /// The model to be imported. /// An optional archive to use for model population. + /// If true, imports will be skipped before they begin, given an existing model matches on hash and filenames. Should generally only be used for large batch imports, as it may defy user expectations when updating an existing model. /// An optional cancellation token. - public virtual Live? Import(TModel item, ArchiveReader? archive = null, CancellationToken cancellationToken = default) + public virtual Live? Import(TModel item, ArchiveReader? archive = null, bool batchImport = false, CancellationToken cancellationToken = default) => Realm.Run(realm => { - return Realm.Run(realm => + cancellationToken.ThrowIfCancellationRequested(); + + bool checkedExisting = false; + TModel? existing = null; + + if (batchImport && archive != null) { - cancellationToken.ThrowIfCancellationRequested(); + // this is a fast bail condition to improve large import performance. + item.Hash = computeHashFast(archive); - bool checkedExisting = false; - TModel? existing = null; + checkedExisting = true; + existing = CheckForExisting(item, realm); - if (archive != null && !HasCustomHashFunction) + if (existing != null) { - // this is a fast bail condition to improve large import performance. - item.Hash = computeHashFast(archive); + // bare minimum comparisons + // + // note that this should really be checking filesizes on disk (of existing files) for some degree of sanity. + // or alternatively doing a faster hash check. either of these require database changes and reprocessing of existing files. + if (CanSkipImport(existing, item) && + getFilenames(existing.Files).SequenceEqual(getShortenedFilenames(archive).Select(p => p.shortened).OrderBy(f => f)) && + checkAllFilesExist(existing)) + { + LogForModel(item, @$"Found existing (optimised) {HumanisedModelName} for {item} (ID {existing.ID}) – skipping import."); - checkedExisting = true; - existing = CheckForExisting(item, realm); + using (var transaction = realm.BeginWrite()) + { + UndeleteForReuse(existing); + transaction.Commit(); + } + + return existing.ToLive(Realm); + } + + LogForModel(item, @"Found existing (optimised) but failed pre-check."); + } + } + + try + { + LogForModel(item, @"Beginning import..."); + + // TODO: do we want to make the transaction this local? not 100% sure, will need further investigation. + using (var transaction = realm.BeginWrite()) + { + if (archive != null) + // TODO: look into rollback of file additions (or delayed commit). + item.Files.AddRange(createFileInfos(archive, Files, realm)); + + item.Hash = ComputeHash(item); + + // TODO: we may want to run this outside of the transaction. + Populate(item, archive, realm, cancellationToken); + + if (!checkedExisting) + existing = CheckForExisting(item, realm); if (existing != null) { - // bare minimum comparisons - // - // note that this should really be checking filesizes on disk (of existing files) for some degree of sanity. - // or alternatively doing a faster hash check. either of these require database changes and reprocessing of existing files. - if (CanSkipImport(existing, item) && - getFilenames(existing.Files).SequenceEqual(getShortenedFilenames(archive).Select(p => p.shortened).OrderBy(f => f)) && - checkAllFilesExist(existing)) + if (CanReuseExisting(existing, item)) { - LogForModel(item, @$"Found existing (optimised) {HumanisedModelName} for {item} (ID {existing.ID}) – skipping import."); + LogForModel(item, @$"Found existing {HumanisedModelName} for {item} (ID {existing.ID}) – skipping import."); - using (var transaction = realm.BeginWrite()) - { - UndeleteForReuse(existing); - transaction.Commit(); - } + UndeleteForReuse(existing); + transaction.Commit(); return existing.ToLive(Realm); } - LogForModel(item, @"Found existing (optimised) but failed pre-check."); - } - } + LogForModel(item, @"Found existing but failed re-use check."); - try - { - LogForModel(item, @"Beginning import..."); - - // TODO: do we want to make the transaction this local? not 100% sure, will need further investigation. - using (var transaction = realm.BeginWrite()) - { - if (archive != null) - // TODO: look into rollback of file additions (or delayed commit). - item.Files.AddRange(createFileInfos(archive, Files, realm)); - - item.Hash = ComputeHash(item); - - // TODO: we may want to run this outside of the transaction. - Populate(item, archive, realm, cancellationToken); - - if (!checkedExisting) - existing = CheckForExisting(item, realm); - - if (existing != null) - { - if (CanReuseExisting(existing, item)) - { - LogForModel(item, @$"Found existing {HumanisedModelName} for {item} (ID {existing.ID}) – skipping import."); - - UndeleteForReuse(existing); - transaction.Commit(); - - return existing.ToLive(Realm); - } - - LogForModel(item, @"Found existing but failed re-use check."); - - existing.DeletePending = true; - } - - PreImport(item, realm); - - // import to store - realm.Add(item); - - transaction.Commit(); + existing.DeletePending = true; } - LogForModel(item, @"Import successfully completed!"); - } - catch (Exception e) - { - if (!(e is TaskCanceledException)) - LogForModel(item, @"Database import or population failed and has been rolled back.", e); + PreImport(item, realm); - throw; + // import to store + realm.Add(item); + + transaction.Commit(); } - return (Live?)item.ToLive(Realm); - }); - } + LogForModel(item, @"Import successfully completed!"); + } + catch (Exception e) + { + if (!(e is TaskCanceledException)) + LogForModel(item, @"Database import or population failed and has been rolled back.", e); + + throw; + } + + return (Live?)item.ToLive(Realm); + }); /// /// Any file extensions which should be included in hash creation. @@ -375,19 +377,13 @@ namespace osu.Game.Stores Logger.Log($"{prefix} {message}", LoggingTarget.Database); } - /// - /// Whether the implementation overrides with a custom implementation. - /// Custom hash implementations must bypass the early exit in the import flow (see usage). - /// - protected virtual bool HasCustomHashFunction => false; - /// /// Create a SHA-2 hash from the provided archive based on file content of all files matching . /// /// /// In the case of no matching files, a hash will be generated from the passed archive's . /// - protected virtual string ComputeHash(TModel item) + protected string ComputeHash(TModel item) { // for now, concatenate all hashable files in the set to create a unique hash. MemoryStream hashable = new MemoryStream(); diff --git a/osu.Game/Tests/Visual/EditorTestScene.cs b/osu.Game/Tests/Visual/EditorTestScene.cs index 46f31ae53b..8e9bb6ff78 100644 --- a/osu.Game/Tests/Visual/EditorTestScene.cs +++ b/osu.Game/Tests/Visual/EditorTestScene.cs @@ -147,7 +147,7 @@ namespace osu.Game.Tests.Visual protected override BeatmapModelManager CreateBeatmapModelManager(Storage storage, RealmAccess realm, RulesetStore rulesets, BeatmapOnlineLookupQueue onlineLookupQueue) { - return new TestBeatmapModelManager(storage, realm, onlineLookupQueue); + return new BeatmapModelManager(realm, storage, onlineLookupQueue); } protected override WorkingBeatmapCache CreateWorkingBeatmapCache(AudioManager audioManager, IResourceStore resources, IResourceStore storage, WorkingBeatmap defaultBeatmap, GameHost host) @@ -181,17 +181,6 @@ namespace osu.Game.Tests.Visual => testBeatmapManager.TestBeatmap; } - internal class TestBeatmapModelManager : BeatmapModelManager - { - public TestBeatmapModelManager(Storage storage, RealmAccess databaseAccess, BeatmapOnlineLookupQueue beatmapOnlineLookupQueue) - : base(databaseAccess, storage, beatmapOnlineLookupQueue) - { - } - - protected override string ComputeHash(BeatmapSetInfo item) - => string.Empty; - } - public override void Save(BeatmapInfo info, IBeatmap beatmapContent, ISkin beatmapSkin = null) { // don't actually care about saving for this context. diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index f8ff29d58a..a0bf43e06b 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -36,7 +36,7 @@ runtime; build; native; contentfiles; analyzers; buildtransitive - + diff --git a/osu.iOS.props b/osu.iOS.props index 929df0736b..6f76bd654a 100644 --- a/osu.iOS.props +++ b/osu.iOS.props @@ -61,7 +61,7 @@ - + @@ -84,7 +84,7 @@ - +