From 0a45aa80cb011e658dc1d7c3504d2fe4ef12f079 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 27 Jan 2022 16:16:49 +0900 Subject: [PATCH 01/20] Remove unnecessary double-schedule in `UpdateBeatmapSet` --- osu.Game/Screens/Select/BeatmapCarousel.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index dff2c598c3..94e520ba1c 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -311,13 +311,10 @@ namespace osu.Game.Screens.Select itemsCache.Invalidate(); - Schedule(() => - { - if (!Scroll.UserScrolling) - ScrollToSelected(true); + if (!Scroll.UserScrolling) + ScrollToSelected(true); - BeatmapSetsChanged?.Invoke(); - }); + BeatmapSetsChanged?.Invoke(); }); /// From 449e9bcf5c123e1e1c92534f8f3a710412a5028d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 27 Jan 2022 16:17:38 +0900 Subject: [PATCH 02/20] Ensure beatmap carousel scroll position is maintained during deletion operations --- osu.Game/Screens/Select/BeatmapCarousel.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 94e520ba1c..c27915c383 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -286,6 +286,9 @@ namespace osu.Game.Screens.Select root.RemoveChild(existingSet); itemsCache.Invalidate(); + + if (!Scroll.UserScrolling) + ScrollToSelected(true); }); public void UpdateBeatmapSet(BeatmapSetInfo beatmapSet) => Schedule(() => From f2cecad83b2e1df2b2e15c5cd49e04534cf55dca Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 27 Jan 2022 16:38:02 +0900 Subject: [PATCH 03/20] Add failing test coverage showing carousel deletions don't keep scroll position --- .../SongSelect/TestSceneBeatmapCarousel.cs | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs index 0298c3bea9..2f2001c4f2 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs @@ -9,6 +9,7 @@ using osu.Framework.Allocation; using osu.Framework.Extensions.IEnumerableExtensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; +using osu.Framework.Utils; using osu.Game.Beatmaps; using osu.Game.Configuration; using osu.Game.Rulesets; @@ -40,6 +41,36 @@ namespace osu.Game.Tests.Visual.SongSelect this.rulesets = rulesets; } + [Test] + public void TestScrollPositionMaintainedOnAdd() + { + loadBeatmaps(count: 1, randomDifficulties: false); + + for (int i = 0; i < 10; i++) + { + AddRepeatStep("Add some sets", () => carousel.UpdateBeatmapSet(TestResources.CreateTestBeatmapSetInfo()), 4); + + checkSelectionIsCentered(); + } + } + + [Test] + public void TestScrollPositionMaintainedOnDelete() + { + loadBeatmaps(count: 50, randomDifficulties: false); + + for (int i = 0; i < 10; i++) + { + AddRepeatStep("Remove some sets", () => + carousel.RemoveBeatmapSet(carousel.Items.Select(item => item.Item) + .OfType() + .OrderBy(item => item.GetHashCode()) + .First(item => item.State.Value != CarouselItemState.Selected && item.Visible).BeatmapSet), 4); + + checkSelectionIsCentered(); + } + } + [Test] public void TestManyPanels() { @@ -813,6 +844,18 @@ namespace osu.Game.Tests.Visual.SongSelect carousel.Items.Count(s => (diff ? s.Item is CarouselBeatmap : s.Item is CarouselBeatmapSet) && s.Item.Visible) == count); } + private void checkSelectionIsCentered() + { + AddAssert("Selected panel is centered", () => + { + return Precision.AlmostEquals( + carousel.ScreenSpaceDrawQuad.Centre, + carousel.Items + .First(i => i.Item.State.Value == CarouselItemState.Selected) + .ScreenSpaceDrawQuad.Centre, 100); + }); + } + private void checkNoSelection() => AddAssert("Selection is null", () => currentSelection == null); private void nextRandom() => From 942ea896f1dfab080c624e57b8c90b30844e56ba Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 28 Jan 2022 01:20:32 +0900 Subject: [PATCH 04/20] Skip scores missing beatmaps during realm migration --- osu.Game/Database/EFToRealmMigrator.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game/Database/EFToRealmMigrator.cs b/osu.Game/Database/EFToRealmMigrator.cs index 9fb0130619..a53704df9d 100644 --- a/osu.Game/Database/EFToRealmMigrator.cs +++ b/osu.Game/Database/EFToRealmMigrator.cs @@ -374,12 +374,12 @@ namespace osu.Game.Database log($"Migrated {written}/{count} scores..."); } - var beatmap = r.All().First(b => b.Hash == score.BeatmapInfo.Hash); + var beatmap = r.All().FirstOrDefault(b => b.Hash == score.BeatmapInfo.Hash); var ruleset = r.Find(score.Ruleset.ShortName); - if (ruleset == null) + if (beatmap == null || ruleset == null) { - log($"Skipping {++missing} scores with missing ruleset"); + log($"Skipping {++missing} scores with missing ruleset or beatmap"); continue; } From 9deeaee40481e7702c4b0e143ec177e755928dd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 27 Jan 2022 16:57:38 +0100 Subject: [PATCH 05/20] Fix tournament client not loading Caused by a `LoadComponentsAsync()` call being fired from a worker thread, which will throw exceptions since the recent addition of safety checks around that method. --- osu.Game.Tournament/TournamentGame.cs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/osu.Game.Tournament/TournamentGame.cs b/osu.Game.Tournament/TournamentGame.cs index 5d613894d4..7967f54b49 100644 --- a/osu.Game.Tournament/TournamentGame.cs +++ b/osu.Game.Tournament/TournamentGame.cs @@ -61,18 +61,15 @@ namespace osu.Game.Tournament loadingSpinner.Show(); - BracketLoadTask.ContinueWith(t => + BracketLoadTask.ContinueWith(t => Schedule(() => { if (t.IsFaulted) { - Schedule(() => - { - loadingSpinner.Hide(); - loadingSpinner.Expire(); + loadingSpinner.Hide(); + loadingSpinner.Expire(); - Logger.Error(t.Exception, "Couldn't load bracket with error"); - Add(new WarningBox($"Your {BRACKET_FILENAME} file could not be parsed. Please check runtime.log for more details.")); - }); + Logger.Error(t.Exception, "Couldn't load bracket with error"); + Add(new WarningBox($"Your {BRACKET_FILENAME} file could not be parsed. Please check runtime.log for more details.")); return; } @@ -143,7 +140,7 @@ namespace osu.Game.Tournament windowMode.Value = WindowMode.Windowed; }), true); }); - }); + })); } } } From 7dc3940dee206b6ebec790078c4eefa6ee78b464 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 27 Jan 2022 21:41:09 +0100 Subject: [PATCH 06/20] Add test coverage for preserving legacy beatmap info defaults --- .../Formats/LegacyBeatmapDecoderTest.cs | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs index 1edd21b5a9..3a6051ed43 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs @@ -794,5 +794,32 @@ namespace osu.Game.Tests.Beatmaps.Formats Assert.That(path.Distance, Is.EqualTo(1)); } } + + [Test] + public void TestLegacyDefaultsPreserved() + { + var decoder = new LegacyBeatmapDecoder { ApplyOffsets = false }; + + using (var memoryStream = new MemoryStream()) + using (var stream = new LineBufferedReader(memoryStream)) + { + var decoded = decoder.Decode(stream); + + Assert.Multiple(() => + { + Assert.That(decoded.BeatmapInfo.AudioLeadIn, Is.EqualTo(0)); + Assert.That(decoded.BeatmapInfo.StackLeniency, Is.EqualTo(0.7f)); + Assert.That(decoded.BeatmapInfo.SpecialStyle, Is.False); + Assert.That(decoded.BeatmapInfo.LetterboxInBreaks, Is.False); + Assert.That(decoded.BeatmapInfo.WidescreenStoryboard, Is.False); + Assert.That(decoded.BeatmapInfo.EpilepsyWarning, Is.False); + Assert.That(decoded.BeatmapInfo.SamplesMatchPlaybackRate, Is.False); + Assert.That(decoded.BeatmapInfo.Countdown, Is.EqualTo(CountdownType.Normal)); + Assert.That(decoded.BeatmapInfo.CountdownOffset, Is.EqualTo(0)); + Assert.That(decoded.BeatmapInfo.Metadata.PreviewTime, Is.EqualTo(-1)); + Assert.That(decoded.BeatmapInfo.Ruleset.OnlineID, Is.EqualTo(0)); + }); + } + } } } From 1b8136e3e0fa023a613dd0b66602b9b943db83d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 27 Jan 2022 21:41:30 +0100 Subject: [PATCH 07/20] Change some `BeatmapInfo` defaults in a backwards compatible manner --- osu.Game/Beatmaps/BeatmapInfo.cs | 4 ++-- osu.Game/Beatmaps/BeatmapManager.cs | 9 +-------- osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs | 15 +++++++++++++++ 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapInfo.cs b/osu.Game/Beatmaps/BeatmapInfo.cs index 3e6e33f1d0..e4bfd768b7 100644 --- a/osu.Game/Beatmaps/BeatmapInfo.cs +++ b/osu.Game/Beatmaps/BeatmapInfo.cs @@ -99,11 +99,11 @@ namespace osu.Game.Beatmaps public bool LetterboxInBreaks { get; set; } - public bool WidescreenStoryboard { get; set; } + public bool WidescreenStoryboard { get; set; } = true; public bool EpilepsyWarning { get; set; } - public bool SamplesMatchPlaybackRate { get; set; } + public bool SamplesMatchPlaybackRate { get; set; } = true; public double DistanceSpacing { get; set; } diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 414b7cd12b..e4fdb3d471 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -90,14 +90,7 @@ namespace osu.Game.Beatmaps { Beatmaps = { - new BeatmapInfo - { - Difficulty = new BeatmapDifficulty(), - Ruleset = ruleset, - Metadata = metadata, - WidescreenStoryboard = true, - SamplesMatchPlaybackRate = true, - } + new BeatmapInfo(ruleset, new BeatmapDifficulty(), metadata) } }; diff --git a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs index 35d1cefeca..01ba0fcc9f 100644 --- a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs @@ -56,6 +56,8 @@ namespace osu.Game.Beatmaps.Formats this.beatmap = beatmap; this.beatmap.BeatmapInfo.BeatmapVersion = FormatVersion; + applyLegacyDefaults(this.beatmap.BeatmapInfo); + base.ParseStreamInto(stream, beatmap); flushPendingPoints(); @@ -70,6 +72,19 @@ namespace osu.Game.Beatmaps.Formats hitObject.ApplyDefaults(this.beatmap.ControlPointInfo, this.beatmap.Difficulty); } + /// + /// Some `BeatmapInfo` members have default values that differ from the default values used by stable. + /// In addition, legacy beatmaps will sometimes not contain some configuration keys, in which case + /// the legacy default values should be used. + /// This method's intention is to restore those legacy defaults. + /// See also: https://osu.ppy.sh/wiki/en/Client/File_formats/Osu_%28file_format%29 + /// + private void applyLegacyDefaults(BeatmapInfo beatmapInfo) + { + beatmapInfo.WidescreenStoryboard = false; + beatmapInfo.SamplesMatchPlaybackRate = false; + } + protected override bool ShouldSkipLine(string line) => base.ShouldSkipLine(line) || line.StartsWith(' ') || line.StartsWith('_'); protected override void ParseLine(Beatmap beatmap, Section section, string line) From 6674567af1d678f33eb7c54920e3f11cba7301a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 27 Jan 2022 21:51:51 +0100 Subject: [PATCH 08/20] Use -1 as the default preview time globally in metadata --- osu.Game/Beatmaps/BeatmapMetadata.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Beatmaps/BeatmapMetadata.cs b/osu.Game/Beatmaps/BeatmapMetadata.cs index cb38373bd3..1514d3af7a 100644 --- a/osu.Game/Beatmaps/BeatmapMetadata.cs +++ b/osu.Game/Beatmaps/BeatmapMetadata.cs @@ -39,7 +39,7 @@ namespace osu.Game.Beatmaps /// The time in milliseconds to begin playing the track for preview purposes. /// If -1, the track should begin playing at 40% of its length. /// - public int PreviewTime { get; set; } + public int PreviewTime { get; set; } = -1; public string AudioFile { get; set; } = string.Empty; public string BackgroundFile { get; set; } = string.Empty; From c44af4853d6aabb82006d7944e146d82c06401fd Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 28 Jan 2022 13:43:53 +0900 Subject: [PATCH 09/20] Add thread safety to `PollingComponent.Poll` implementations --- .../Components/ListingPollingComponent.cs | 14 ++++++++------ .../Components/SelectionPollingComponent.cs | 15 +++++++++------ 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Components/ListingPollingComponent.cs b/osu.Game/Screens/OnlinePlay/Components/ListingPollingComponent.cs index fcf7767958..666d425f62 100644 --- a/osu.Game/Screens/OnlinePlay/Components/ListingPollingComponent.cs +++ b/osu.Game/Screens/OnlinePlay/Components/ListingPollingComponent.cs @@ -33,7 +33,7 @@ namespace osu.Game.Screens.OnlinePlay.Components }); } - private GetRoomsRequest pollReq; + private GetRoomsRequest lastPollRequest; protected override Task Poll() { @@ -45,10 +45,11 @@ namespace osu.Game.Screens.OnlinePlay.Components var tcs = new TaskCompletionSource(); - pollReq?.Cancel(); - pollReq = new GetRoomsRequest(Filter.Value.Status, Filter.Value.Category); + lastPollRequest?.Cancel(); - pollReq.Success += result => + var req = new GetRoomsRequest(Filter.Value.Status, Filter.Value.Category); + + req.Success += result => { foreach (var existing in RoomManager.Rooms.ToArray()) { @@ -66,10 +67,11 @@ namespace osu.Game.Screens.OnlinePlay.Components tcs.SetResult(true); }; - pollReq.Failure += _ => tcs.SetResult(false); + req.Failure += _ => tcs.SetResult(false); - API.Queue(pollReq); + API.Queue(req); + lastPollRequest = req; return tcs.Task; } } diff --git a/osu.Game/Screens/OnlinePlay/Components/SelectionPollingComponent.cs b/osu.Game/Screens/OnlinePlay/Components/SelectionPollingComponent.cs index 22842fbb9e..e05bdf8c8e 100644 --- a/osu.Game/Screens/OnlinePlay/Components/SelectionPollingComponent.cs +++ b/osu.Game/Screens/OnlinePlay/Components/SelectionPollingComponent.cs @@ -18,7 +18,7 @@ namespace osu.Game.Screens.OnlinePlay.Components this.room = room; } - private GetRoomRequest pollReq; + private GetRoomRequest lastPollRequest; protected override Task Poll() { @@ -30,19 +30,22 @@ namespace osu.Game.Screens.OnlinePlay.Components var tcs = new TaskCompletionSource(); - pollReq?.Cancel(); - pollReq = new GetRoomRequest(room.RoomID.Value.Value); + lastPollRequest?.Cancel(); - pollReq.Success += result => + var req = new GetRoomRequest(room.RoomID.Value.Value); + + req.Success += result => { result.RemoveExpiredPlaylistItems(); RoomManager.AddOrUpdateRoom(result); tcs.SetResult(true); }; - pollReq.Failure += _ => tcs.SetResult(false); + req.Failure += _ => tcs.SetResult(false); - API.Queue(pollReq); + API.Queue(req); + + lastPollRequest = req; return tcs.Task; } From c953a5d5034416bb32ee2caada7c0a74f0b4551a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 28 Jan 2022 13:44:09 +0900 Subject: [PATCH 10/20] Ensure `PollingComponent.Poll` is always called from the update thread Not strictly required since all `Poll` implementations are now threadsafe, but extra safety is never a bad thing? --- osu.Game/Online/PollingComponent.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/osu.Game/Online/PollingComponent.cs b/osu.Game/Online/PollingComponent.cs index 243be8da44..5eddb3b49d 100644 --- a/osu.Game/Online/PollingComponent.cs +++ b/osu.Game/Online/PollingComponent.cs @@ -2,8 +2,10 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Diagnostics; using System.Threading.Tasks; using osu.Framework.Bindables; +using osu.Framework.Development; using osu.Framework.Graphics.Containers; using osu.Framework.Threading; @@ -66,6 +68,8 @@ namespace osu.Game.Online private void doPoll() { + Debug.Assert(ThreadSafety.IsUpdateThread); + scheduledPoll = null; pollingActive = true; Poll().ContinueWith(_ => pollComplete()); @@ -96,13 +100,13 @@ namespace osu.Game.Online if (!lastTimePolled.HasValue) { - doPoll(); + Scheduler.AddOnce(doPoll); return; } if (Time.Current - lastTimePolled.Value > TimeBetweenPolls.Value) { - doPoll(); + Scheduler.AddOnce(doPoll); return; } From 91be77ad3d61f844c997681cfdf5b2643115bd43 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 28 Jan 2022 14:01:10 +0900 Subject: [PATCH 11/20] Fix null ref in `ComposeScreen` when ruleset doesn't provide a composer --- osu.Game/Screens/Edit/Compose/ComposeScreen.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/osu.Game/Screens/Edit/Compose/ComposeScreen.cs b/osu.Game/Screens/Edit/Compose/ComposeScreen.cs index 2bdf59b21c..2cde962b12 100644 --- a/osu.Game/Screens/Edit/Compose/ComposeScreen.cs +++ b/osu.Game/Screens/Edit/Compose/ComposeScreen.cs @@ -82,6 +82,11 @@ namespace osu.Game.Screens.Edit.Compose protected override void LoadComplete() { base.LoadComplete(); + + // May be null in the case of a ruleset that doesn't have editor support, see CreateMainContent(). + if (composer == null) + return; + EditorBeatmap.SelectedHitObjects.BindCollectionChanged((_, __) => updateClipboardActionAvailability()); clipboard.BindValueChanged(_ => updateClipboardActionAvailability()); composer.OnLoadComplete += _ => updateClipboardActionAvailability(); From b3856c900541e1af68aa8e4a5c55b9a03f62356f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 28 Jan 2022 14:01:31 +0900 Subject: [PATCH 12/20] Fix editor crashing on custom rulesets due to `ChangeHandler` not being supported As per https://github.com/ppy/osu/discussions/16668, even without proper saving support some ruleset developers do want to work on the editor. This brings things back into a workable state. --- osu.Game/Screens/Edit/Editor.cs | 46 +++++++++++++++++++++++++++------ 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/osu.Game/Screens/Edit/Editor.cs b/osu.Game/Screens/Edit/Editor.cs index 4812da98f4..2fead84deb 100644 --- a/osu.Game/Screens/Edit/Editor.cs +++ b/osu.Game/Screens/Edit/Editor.cs @@ -28,6 +28,8 @@ using osu.Game.Graphics.UserInterface; using osu.Game.Input.Bindings; using osu.Game.Online.API; using osu.Game.Overlays; +using osu.Game.Overlays.Notifications; +using osu.Game.Rulesets; using osu.Game.Rulesets.Edit; using osu.Game.Screens.Edit.Components; using osu.Game.Screens.Edit.Components.Menus; @@ -61,7 +63,16 @@ namespace osu.Game.Screens.Edit public override bool? AllowTrackAdjustments => false; - protected bool HasUnsavedChanges => lastSavedHash != changeHandler.CurrentStateHash; + protected bool HasUnsavedChanges + { + get + { + if (!canSave) + return false; + + return lastSavedHash != changeHandler?.CurrentStateHash; + } + } [Resolved] private BeatmapManager beatmapManager { get; set; } @@ -72,10 +83,15 @@ namespace osu.Game.Screens.Edit [Resolved(canBeNull: true)] private DialogOverlay dialogOverlay { get; set; } + [Resolved(canBeNull: true)] + private NotificationOverlay notifications { get; set; } + public IBindable SamplePlaybackDisabled => samplePlaybackDisabled; private readonly Bindable samplePlaybackDisabled = new Bindable(); + private bool canSave; + private bool exitConfirmed; private string lastSavedHash; @@ -92,6 +108,8 @@ namespace osu.Game.Screens.Edit private IBeatmap playableBeatmap; private EditorBeatmap editorBeatmap; + + [CanBeNull] // Should be non-null once it can support custom rulesets. private EditorChangeHandler changeHandler; private EditorMenuBar menuBar; @@ -172,8 +190,14 @@ namespace osu.Game.Screens.Edit AddInternal(editorBeatmap = new EditorBeatmap(playableBeatmap, loadableBeatmap.GetSkin(), loadableBeatmap.BeatmapInfo)); dependencies.CacheAs(editorBeatmap); - changeHandler = new EditorChangeHandler(editorBeatmap); - dependencies.CacheAs(changeHandler); + + canSave = editorBeatmap.BeatmapInfo.Ruleset.CreateInstance() is ILegacyRuleset; + + if (canSave) + { + changeHandler = new EditorChangeHandler(editorBeatmap); + dependencies.CacheAs(changeHandler); + } beatDivisor.Value = editorBeatmap.BeatmapInfo.BeatDivisor; beatDivisor.BindValueChanged(divisor => editorBeatmap.BeatmapInfo.BeatDivisor = divisor.NewValue); @@ -311,8 +335,8 @@ namespace osu.Game.Screens.Edit } }); - changeHandler.CanUndo.BindValueChanged(v => undoMenuItem.Action.Disabled = !v.NewValue, true); - changeHandler.CanRedo.BindValueChanged(v => redoMenuItem.Action.Disabled = !v.NewValue, true); + changeHandler?.CanUndo.BindValueChanged(v => undoMenuItem.Action.Disabled = !v.NewValue, true); + changeHandler?.CanRedo.BindValueChanged(v => redoMenuItem.Action.Disabled = !v.NewValue, true); menuBar.Mode.ValueChanged += onModeChanged; } @@ -353,6 +377,12 @@ namespace osu.Game.Screens.Edit protected void Save() { + if (!canSave) + { + notifications?.Post(new SimpleErrorNotification { Text = "Saving is not supported for this ruleset yet, sorry!" }); + return; + } + // no longer new after first user-triggered save. isNewBeatmap = false; @@ -648,9 +678,9 @@ namespace osu.Game.Screens.Edit #endregion - protected void Undo() => changeHandler.RestoreState(-1); + protected void Undo() => changeHandler?.RestoreState(-1); - protected void Redo() => changeHandler.RestoreState(1); + protected void Redo() => changeHandler?.RestoreState(1); private void resetTrack(bool seekToStart = false) { @@ -761,7 +791,7 @@ namespace osu.Game.Screens.Edit private void updateLastSavedHash() { - lastSavedHash = changeHandler.CurrentStateHash; + lastSavedHash = changeHandler?.CurrentStateHash; } private List createFileMenuItems() From b7d8c9bf067f64021a4648ecfd413648258f373f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 28 Jan 2022 14:18:10 +0900 Subject: [PATCH 13/20] Fix a couple of cases of incorrect equality checks in the case both values are null --- .../BeatmapSet/Scores/TopScoreStatisticsSection.cs | 3 +++ .../Screens/Select/Leaderboards/BeatmapLeaderboard.cs | 8 ++++++++ 2 files changed, 11 insertions(+) diff --git a/osu.Game/Overlays/BeatmapSet/Scores/TopScoreStatisticsSection.cs b/osu.Game/Overlays/BeatmapSet/Scores/TopScoreStatisticsSection.cs index 1f3f73a60a..ec795cf6b2 100644 --- a/osu.Game/Overlays/BeatmapSet/Scores/TopScoreStatisticsSection.cs +++ b/osu.Game/Overlays/BeatmapSet/Scores/TopScoreStatisticsSection.cs @@ -107,6 +107,9 @@ namespace osu.Game.Overlays.BeatmapSet.Scores { set { + if (score == null && value == null) + return; + if (score?.Equals(value) == true) return; diff --git a/osu.Game/Screens/Select/Leaderboards/BeatmapLeaderboard.cs b/osu.Game/Screens/Select/Leaderboards/BeatmapLeaderboard.cs index 2070e53257..31cbe91f5c 100644 --- a/osu.Game/Screens/Select/Leaderboards/BeatmapLeaderboard.cs +++ b/osu.Game/Screens/Select/Leaderboards/BeatmapLeaderboard.cs @@ -38,6 +38,14 @@ namespace osu.Game.Screens.Select.Leaderboards get => beatmapInfo; set { + if (beatmapInfo == null && value == null) + { + // always null scores to ensure a correct initial display. + // see weird `scoresLoadedOnce` logic in base implementation. + Scores = null; + return; + } + if (beatmapInfo?.Equals(value) == true) return; From f32d56e213fa26566b38ca12bb44cf9d8f0f0100 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 28 Jan 2022 14:48:17 +0900 Subject: [PATCH 14/20] Bring `HoldForMenuButton` tests up-to-date in code quality --- .../Gameplay/TestSceneHoldForMenuButton.cs | 37 ++++++++++++------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneHoldForMenuButton.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneHoldForMenuButton.cs index 235842acc9..e0765ab5fb 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneHoldForMenuButton.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneHoldForMenuButton.cs @@ -3,9 +3,10 @@ using System.Linq; using NUnit.Framework; -using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Sprites; +using osu.Framework.Testing; +using osu.Game.Graphics.Containers; using osu.Game.Screens.Play.HUD; using osuTK; using osuTK.Input; @@ -19,28 +20,35 @@ namespace osu.Game.Tests.Visual.Gameplay protected override double TimePerAction => 100; // required for the early exit test, since hold-to-confirm delay is 200ms - [BackgroundDependencyLoader] - private void load() + private HoldForMenuButton holdForMenuButton; + + [SetUpSteps] + public void SetUpSteps() { - HoldForMenuButton holdForMenuButton; - - Add(holdForMenuButton = new HoldForMenuButton + AddStep("create button", () => { - Origin = Anchor.BottomRight, - Anchor = Anchor.BottomRight, - Action = () => exitAction = true + exitAction = false; + + Child = holdForMenuButton = new HoldForMenuButton + { + Scale = new Vector2(2), + Origin = Anchor.CentreRight, + Anchor = Anchor.CentreRight, + Action = () => exitAction = true + }; }); + } - var text = holdForMenuButton.Children.OfType().First(); - + [Test] + public void TestMovementAndTrigger() + { AddStep("Trigger text fade in", () => InputManager.MoveMouseTo(holdForMenuButton)); - AddUntilStep("Text visible", () => text.IsPresent && !exitAction); + AddUntilStep("Text visible", () => getSpriteText().IsPresent && !exitAction); AddStep("Trigger text fade out", () => InputManager.MoveMouseTo(Vector2.One)); - AddUntilStep("Text is not visible", () => !text.IsPresent && !exitAction); + AddUntilStep("Text is not visible", () => !getSpriteText().IsPresent && !exitAction); AddStep("Trigger exit action", () => { - exitAction = false; InputManager.MoveMouseTo(holdForMenuButton); InputManager.PressButton(MouseButton.Left); }); @@ -50,6 +58,7 @@ namespace osu.Game.Tests.Visual.Gameplay AddStep("Trigger exit action", () => InputManager.PressButton(MouseButton.Left)); AddUntilStep($"{nameof(holdForMenuButton.Action)} was triggered", () => exitAction); + AddStep("Release", () => InputManager.ReleaseButton(MouseButton.Left)); } } } From 28c8e07e3f223aaaa15fe840fc8ee375c31dc48d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 28 Jan 2022 14:48:34 +0900 Subject: [PATCH 15/20] Ensure hold for menu button fades out if the cursor is never moved Closes https://github.com/ppy/osu/discussions/16669. --- .../Visual/Gameplay/TestSceneHoldForMenuButton.cs | 10 ++++++++++ osu.Game/Screens/Play/HUD/HoldForMenuButton.cs | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneHoldForMenuButton.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneHoldForMenuButton.cs index e0765ab5fb..feec06f372 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneHoldForMenuButton.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneHoldForMenuButton.cs @@ -60,5 +60,15 @@ namespace osu.Game.Tests.Visual.Gameplay AddUntilStep($"{nameof(holdForMenuButton.Action)} was triggered", () => exitAction); AddStep("Release", () => InputManager.ReleaseButton(MouseButton.Left)); } + + [Test] + public void TestFadeOnNoInput() + { + AddStep("move mouse away", () => InputManager.MoveMouseTo(Vector2.One)); + AddUntilStep("wait for text fade out", () => !getSpriteText().IsPresent); + AddUntilStep("wait for button fade out", () => holdForMenuButton.Alpha < 0.1f); + } + + private SpriteText getSpriteText() => holdForMenuButton.Children.OfType().First(); } } diff --git a/osu.Game/Screens/Play/HUD/HoldForMenuButton.cs b/osu.Game/Screens/Play/HUD/HoldForMenuButton.cs index 5a7ef786d3..430f001427 100644 --- a/osu.Game/Screens/Play/HUD/HoldForMenuButton.cs +++ b/osu.Game/Screens/Play/HUD/HoldForMenuButton.cs @@ -80,7 +80,7 @@ namespace osu.Game.Screens.Play.HUD base.LoadComplete(); } - private float positionalAdjust; + private float positionalAdjust = 1; // Start at 1 to handle the case where a user never send positional input. protected override bool OnMouseMove(MouseMoveEvent e) { From 32f9299fe082f7347aebfceef7aa6f8dd017f2f6 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Fri, 28 Jan 2022 15:26:29 +0900 Subject: [PATCH 16/20] Remove unused using --- osu.Game.Tests/Visual/Gameplay/TestSceneHoldForMenuButton.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneHoldForMenuButton.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneHoldForMenuButton.cs index feec06f372..ddb0872541 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneHoldForMenuButton.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneHoldForMenuButton.cs @@ -6,7 +6,6 @@ using NUnit.Framework; using osu.Framework.Graphics; using osu.Framework.Graphics.Sprites; using osu.Framework.Testing; -using osu.Game.Graphics.Containers; using osu.Game.Screens.Play.HUD; using osuTK; using osuTK.Input; From cb7ae413feea2e53fd4b5aa3b59167ae0a398c13 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 28 Jan 2022 15:50:13 +0900 Subject: [PATCH 17/20] Ensure test game is always active --- osu.Game/Tests/Visual/OsuGameTestScene.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/osu.Game/Tests/Visual/OsuGameTestScene.cs b/osu.Game/Tests/Visual/OsuGameTestScene.cs index ebbd9bb5dd..dabf49c15e 100644 --- a/osu.Game/Tests/Visual/OsuGameTestScene.cs +++ b/osu.Game/Tests/Visual/OsuGameTestScene.cs @@ -158,6 +158,14 @@ namespace osu.Game.Tests.Visual Dependencies.Get().SetValue(Static.MutedAudioNotificationShownOnce, true); } + + protected override void Update() + { + base.Update(); + + // when running in visual tests and the window loses focus, we generally don't want the game to pause. + ((Bindable)IsActive).Value = true; + } } public class TestLoader : Loader From 778eebc94df6b2ff8a068b014aaee05de8bd594b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 28 Jan 2022 15:50:22 +0900 Subject: [PATCH 18/20] Add test coverage of local score import and deletion --- .../Navigation/TestSceneScreenNavigation.cs | 90 +++++++++++++++---- osu.Game/Tests/Visual/OsuGameTestScene.cs | 3 + 2 files changed, 74 insertions(+), 19 deletions(-) diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs index 89dca77af4..6d89feef52 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs @@ -1,6 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System; using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; @@ -10,16 +11,19 @@ using osu.Framework.Screens; using osu.Framework.Testing; using osu.Game.Beatmaps; using osu.Game.Graphics.UserInterface; +using osu.Game.Online.Leaderboards; using osu.Game.Overlays; using osu.Game.Overlays.Mods; using osu.Game.Overlays.Toolbar; using osu.Game.Rulesets.Mods; using osu.Game.Rulesets.Osu.Mods; +using osu.Game.Scoring; using osu.Game.Screens.Menu; using osu.Game.Screens.OnlinePlay.Lounge; using osu.Game.Screens.Play; using osu.Game.Screens.Ranking; using osu.Game.Screens.Select; +using osu.Game.Screens.Select.Leaderboards; using osu.Game.Screens.Select.Options; using osu.Game.Tests.Beatmaps.IO; using osuTK; @@ -96,35 +100,52 @@ namespace osu.Game.Tests.Visual.Navigation [Test] public void TestRetryFromResults() { - Player player = null; - ResultsScreen results = null; + var getOriginalPlayer = playToResults(); - IWorkingBeatmap beatmap() => Game.Beatmap.Value; + AddStep("attempt to retry", () => ((ResultsScreen)Game.ScreenStack.CurrentScreen).ChildrenOfType().First().Action()); + AddUntilStep("wait for player", () => Game.ScreenStack.CurrentScreen != getOriginalPlayer() && Game.ScreenStack.CurrentScreen is Player); + } - Screens.Select.SongSelect songSelect = null; - PushAndConfirm(() => songSelect = new TestPlaySongSelect()); - AddUntilStep("wait for song select", () => songSelect.BeatmapSetsLoaded); + [Test] + public void TestDeleteScoreAfterPlaying() + { + playToResults(); - AddStep("import beatmap", () => BeatmapImportHelper.LoadQuickOszIntoOsu(Game).WaitSafely()); + ScoreInfo score = null; + LeaderboardScore scorePanel = null; - AddUntilStep("wait for selected", () => !Game.Beatmap.IsDefault); + AddStep("get score", () => score = ((ResultsScreen)Game.ScreenStack.CurrentScreen).Score); - AddStep("set mods", () => Game.SelectedMods.Value = new Mod[] { new OsuModNoFail(), new OsuModDoubleTime { SpeedChange = { Value = 2 } } }); + AddAssert("ensure score is databased", () => Game.Realm.Run(r => r.Find(score.ID)?.DeletePending == false)); - AddStep("press enter", () => InputManager.Key(Key.Enter)); + AddStep("press back button", () => Game.ChildrenOfType().First().Action()); - AddUntilStep("wait for player", () => + AddStep("show local scores", () => Game.ChildrenOfType().First().Current.Value = new BeatmapDetailAreaLeaderboardTabItem(BeatmapLeaderboardScope.Local)); + + AddUntilStep("wait for score displayed", () => (scorePanel = Game.ChildrenOfType().FirstOrDefault(s => s.Score.Equals(score))) != null); + + AddStep("right click panel", () => { - // dismiss any notifications that may appear (ie. muted notification). - clickMouseInCentre(); - return (player = Game.ScreenStack.CurrentScreen as Player) != null; + InputManager.MoveMouseTo(scorePanel); + InputManager.Click(MouseButton.Right); }); - AddUntilStep("wait for track playing", () => beatmap().Track.IsRunning); - AddStep("seek to near end", () => player.ChildrenOfType().First().Seek(beatmap().Beatmap.HitObjects[^1].StartTime - 1000)); - AddUntilStep("wait for pass", () => (results = Game.ScreenStack.CurrentScreen as ResultsScreen) != null && results.IsLoaded); - AddStep("attempt to retry", () => results.ChildrenOfType().First().Action()); - AddUntilStep("wait for player", () => Game.ScreenStack.CurrentScreen != player && Game.ScreenStack.CurrentScreen is Player); + AddStep("click delete", () => + { + var dropdownItem = Game + .ChildrenOfType().First() + .ChildrenOfType().First() + .ChildrenOfType().First(i => i.Item.Text.ToString() == "Delete"); + + InputManager.MoveMouseTo(dropdownItem); + InputManager.Click(MouseButton.Left); + }); + + AddStep("confirm deletion", () => InputManager.Key(Key.Number1)); + + AddAssert("ensure score is pending deletion", () => Game.Realm.Run(r => r.Find(score.ID)?.DeletePending == true)); + + AddUntilStep("wait for score panel removal", () => scorePanel.Parent == null); } [TestCase(true)] @@ -432,6 +453,37 @@ namespace osu.Game.Tests.Visual.Navigation AddStep("test dispose doesn't crash", () => Game.Dispose()); } + private Func playToResults() + { + Player player = null; + + IWorkingBeatmap beatmap() => Game.Beatmap.Value; + + Screens.Select.SongSelect songSelect = null; + PushAndConfirm(() => songSelect = new TestPlaySongSelect()); + AddUntilStep("wait for song select", () => songSelect.BeatmapSetsLoaded); + + AddStep("import beatmap", () => BeatmapImportHelper.LoadQuickOszIntoOsu(Game).WaitSafely()); + + AddUntilStep("wait for selected", () => !Game.Beatmap.IsDefault); + + AddStep("set mods", () => Game.SelectedMods.Value = new Mod[] { new OsuModNoFail(), new OsuModDoubleTime { SpeedChange = { Value = 2 } } }); + + AddStep("press enter", () => InputManager.Key(Key.Enter)); + + AddUntilStep("wait for player", () => + { + // dismiss any notifications that may appear (ie. muted notification). + clickMouseInCentre(); + return (player = Game.ScreenStack.CurrentScreen as Player) != null; + }); + + AddUntilStep("wait for track playing", () => beatmap().Track.IsRunning); + AddStep("seek to near end", () => player.ChildrenOfType().First().Seek(beatmap().Beatmap.HitObjects[^1].StartTime - 1000)); + AddUntilStep("wait for pass", () => (Game.ScreenStack.CurrentScreen as ResultsScreen)?.IsLoaded == true); + return () => player; + } + private void clickMouseInCentre() { InputManager.MoveMouseTo(Game.ScreenSpaceDrawQuad.Centre); diff --git a/osu.Game/Tests/Visual/OsuGameTestScene.cs b/osu.Game/Tests/Visual/OsuGameTestScene.cs index dabf49c15e..3b8d9a4cd1 100644 --- a/osu.Game/Tests/Visual/OsuGameTestScene.cs +++ b/osu.Game/Tests/Visual/OsuGameTestScene.cs @@ -14,6 +14,7 @@ using osu.Framework.Screens; using osu.Framework.Testing; using osu.Game.Beatmaps; using osu.Game.Configuration; +using osu.Game.Database; using osu.Game.Graphics.UserInterface; using osu.Game.Online.API; using osu.Game.Overlays; @@ -111,6 +112,8 @@ namespace osu.Game.Tests.Visual public new ScreenStack ScreenStack => base.ScreenStack; + public RealmAccess Realm => Dependencies.Get(); + public new BackButton BackButton => base.BackButton; public new BeatmapManager BeatmapManager => base.BeatmapManager; From 2453bf5ed0b8b3660c2a109ad2720ba125b1fc2c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 28 Jan 2022 15:54:51 +0900 Subject: [PATCH 19/20] Add test coverage of the same thing but via "clear all scores" button --- .../Navigation/TestSceneScreenNavigation.cs | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs index 6d89feef52..3be7cf7c5c 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs @@ -106,6 +106,35 @@ namespace osu.Game.Tests.Visual.Navigation AddUntilStep("wait for player", () => Game.ScreenStack.CurrentScreen != getOriginalPlayer() && Game.ScreenStack.CurrentScreen is Player); } + [Test] + public void TestDeleteAllScoresAfterPlaying() + { + playToResults(); + + ScoreInfo score = null; + LeaderboardScore scorePanel = null; + + AddStep("get score", () => score = ((ResultsScreen)Game.ScreenStack.CurrentScreen).Score); + + AddAssert("ensure score is databased", () => Game.Realm.Run(r => r.Find(score.ID)?.DeletePending == false)); + + AddStep("press back button", () => Game.ChildrenOfType().First().Action()); + + AddStep("show local scores", () => Game.ChildrenOfType().First().Current.Value = new BeatmapDetailAreaLeaderboardTabItem(BeatmapLeaderboardScope.Local)); + + AddUntilStep("wait for score displayed", () => (scorePanel = Game.ChildrenOfType().FirstOrDefault(s => s.Score.Equals(score))) != null); + + AddStep("open options", () => InputManager.Key(Key.F3)); + + AddStep("choose clear all scores", () => InputManager.Key(Key.Number4)); + + AddStep("confirm deletion", () => InputManager.Key(Key.Number1)); + + AddAssert("ensure score is pending deletion", () => Game.Realm.Run(r => r.Find(score.ID)?.DeletePending == true)); + + AddUntilStep("wait for score panel removal", () => scorePanel.Parent == null); + } + [Test] public void TestDeleteScoreAfterPlaying() { From 0d3ac4fd9c72db8eb2049d07652375c0de419f0c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 28 Jan 2022 15:08:39 +0900 Subject: [PATCH 20/20] Fix delete local scores crashing the game --- osu.Game/Scoring/ScoreManager.cs | 9 +++++++++ osu.Game/Screens/Select/BeatmapClearScoresDialog.cs | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/osu.Game/Scoring/ScoreManager.cs b/osu.Game/Scoring/ScoreManager.cs index 8f665224ee..6f9cce2d3c 100644 --- a/osu.Game/Scoring/ScoreManager.cs +++ b/osu.Game/Scoring/ScoreManager.cs @@ -266,6 +266,15 @@ namespace osu.Game.Scoring }); } + public void Delete(BeatmapInfo beatmap, bool silent = false) + { + realm.Run(r => + { + var beatmapScores = r.Find(beatmap.ID).Scores.ToList(); + scoreModelManager.Delete(beatmapScores, silent); + }); + } + public void Delete(List items, bool silent = false) { scoreModelManager.Delete(items, silent); diff --git a/osu.Game/Screens/Select/BeatmapClearScoresDialog.cs b/osu.Game/Screens/Select/BeatmapClearScoresDialog.cs index 774d3b4b28..4a16be4a3a 100644 --- a/osu.Game/Screens/Select/BeatmapClearScoresDialog.cs +++ b/osu.Game/Screens/Select/BeatmapClearScoresDialog.cs @@ -28,7 +28,7 @@ namespace osu.Game.Screens.Select Text = @"Yes. Please.", Action = () => { - Task.Run(() => scoreManager.Delete(s => !s.DeletePending && s.BeatmapInfo.ID == beatmapInfo.ID)) + Task.Run(() => scoreManager.Delete(beatmapInfo)) .ContinueWith(_ => onCompletion); } },