From 90a7dd771107f0de718a1fefc286c2d946ee99c8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 25 Jan 2022 14:30:00 +0900 Subject: [PATCH 01/12] In gameplay bindings test, ensure a selection is made before attempting to enter gameplay --- .../Visual/Navigation/TestSceneChangeAndUseGameplayBindings.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneChangeAndUseGameplayBindings.cs b/osu.Game.Tests/Visual/Navigation/TestSceneChangeAndUseGameplayBindings.cs index bfcefdbbfe..6165c9d9b1 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneChangeAndUseGameplayBindings.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneChangeAndUseGameplayBindings.cs @@ -50,8 +50,11 @@ namespace osu.Game.Tests.Visual.Navigation AddStep("close settings", () => Game.Settings.Hide()); AddStep("import beatmap", () => BeatmapImportHelper.LoadQuickOszIntoOsu(Game).WaitSafely()); + PushAndConfirm(() => new PlaySongSelect()); + AddUntilStep("wait for selection", () => !Game.Beatmap.IsDefault); + AddStep("enter gameplay", () => InputManager.Key(Key.Enter)); AddUntilStep("wait for gameplay", () => player?.IsBreakTime.Value == false); From c0ed30801686e2a25486fb2a80a9ec2534e2b522 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 25 Jan 2022 14:30:32 +0900 Subject: [PATCH 02/12] Use more correct method of deletion in `TestScenePlaySongSelect` --- osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index 630171a4d0..458c6130c7 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -64,13 +64,13 @@ namespace osu.Game.Tests.Visual.SongSelect { base.SetUpSteps(); - AddStep("delete all beatmaps", () => + AddStep("reset defaults", () => { Ruleset.Value = new OsuRuleset().RulesetInfo; - manager?.Delete(manager.GetAllUsableBeatmapSets()); - Beatmap.SetDefault(); }); + + AddStep("delete all beatmaps", () => manager?.Delete()); } [Test] From 5085eb68018d1ae069cce9e754d5e6af326d3a06 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 26 Jan 2022 03:37:11 +0900 Subject: [PATCH 03/12] Ensure gameplay starts by dismissing any notifications in `TestSceneChangeAndUseGameplayBindings` --- .../TestSceneChangeAndUseGameplayBindings.cs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneChangeAndUseGameplayBindings.cs b/osu.Game.Tests/Visual/Navigation/TestSceneChangeAndUseGameplayBindings.cs index 6165c9d9b1..347b4b6c54 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneChangeAndUseGameplayBindings.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneChangeAndUseGameplayBindings.cs @@ -57,6 +57,13 @@ namespace osu.Game.Tests.Visual.Navigation AddStep("enter gameplay", () => InputManager.Key(Key.Enter)); + AddUntilStep("wait for player", () => + { + // dismiss any notifications that may appear (ie. muted notification). + clickMouseInCentre(); + return player != null; + }); + AddUntilStep("wait for gameplay", () => player?.IsBreakTime.Value == false); AddStep("press 'z'", () => InputManager.Key(Key.Z)); @@ -66,6 +73,12 @@ namespace osu.Game.Tests.Visual.Navigation AddAssert("key counter did increase", () => keyCounter.CountPresses == 1); } + private void clickMouseInCentre() + { + InputManager.MoveMouseTo(Game.ScreenSpaceDrawQuad.Centre); + InputManager.Click(MouseButton.Left); + } + private KeyBindingsSubsection osuBindingSubsection => keyBindingPanel .ChildrenOfType() .FirstOrDefault(s => s.Ruleset.ShortName == "osu"); From 064468faada4ba6af341296f634978e774bcec03 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Tue, 25 Jan 2022 11:31:05 +0300 Subject: [PATCH 04/12] Refactor editor saving test scene for scalability --- .../Visual/Editing/TestSceneEditorSaving.cs | 152 ++++++++++-------- osu.Game/Beatmaps/BeatmapInfo.cs | 2 +- .../Tests/Visual/EditorSavingTestScene.cs | 67 ++++++++ 3 files changed, 154 insertions(+), 67 deletions(-) create mode 100644 osu.Game/Tests/Visual/EditorSavingTestScene.cs diff --git a/osu.Game.Tests/Visual/Editing/TestSceneEditorSaving.cs b/osu.Game.Tests/Visual/Editing/TestSceneEditorSaving.cs index 96f815621c..58daab1ce2 100644 --- a/osu.Game.Tests/Visual/Editing/TestSceneEditorSaving.cs +++ b/osu.Game.Tests/Visual/Editing/TestSceneEditorSaving.cs @@ -4,97 +4,117 @@ using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; -using osu.Framework.Input; using osu.Framework.Testing; +using osu.Framework.Utils; using osu.Game.Beatmaps.ControlPoints; -using osu.Game.Rulesets.Edit; using osu.Game.Screens.Edit; -using osu.Game.Screens.Edit.Setup; -using osu.Game.Screens.Menu; -using osu.Game.Screens.Select; +using osu.Game.Screens.Edit.Compose.Components.Timeline; using osuTK.Input; namespace osu.Game.Tests.Visual.Editing { - public class TestSceneEditorSaving : OsuGameTestScene + public class TestSceneEditorSaving : EditorSavingTestScene { - private Editor editor => Game.ChildrenOfType().FirstOrDefault(); - - private EditorBeatmap editorBeatmap => (EditorBeatmap)editor.Dependencies.Get(typeof(EditorBeatmap)); - - /// - /// Tests the general expected flow of creating a new beatmap, saving it, then loading it back from song select. - /// [Test] - public void TestNewBeatmapSaveThenLoad() + public void TestMetadata() { - AddStep("set default beatmap", () => Game.Beatmap.SetDefault()); - - PushAndConfirm(() => new EditorLoader()); - - AddUntilStep("wait for editor load", () => editor?.IsLoaded == true); - - AddUntilStep("wait for metadata screen load", () => editor.ChildrenOfType().FirstOrDefault()?.IsLoaded == true); - - // We intentionally switch away from the metadata screen, else there is a feedback loop with the textbox handling which causes metadata changes below to get overwritten. - - AddStep("Enter compose mode", () => InputManager.Key(Key.F1)); - AddUntilStep("Wait for compose mode load", () => editor.ChildrenOfType().FirstOrDefault()?.IsLoaded == true); - - AddStep("Set beat divisor", () => editor.Dependencies.Get().Value = 16); - AddStep("Set overall difficulty", () => editorBeatmap.Difficulty.OverallDifficulty = 7); AddStep("Set artist and title", () => { - editorBeatmap.BeatmapInfo.Metadata.Artist = "artist"; - editorBeatmap.BeatmapInfo.Metadata.Title = "title"; + EditorBeatmap.BeatmapInfo.Metadata.Artist = "artist"; + EditorBeatmap.BeatmapInfo.Metadata.Title = "title"; }); - AddStep("Set author", () => editorBeatmap.BeatmapInfo.Metadata.Author.Username = "author"); - AddStep("Set difficulty name", () => editorBeatmap.BeatmapInfo.DifficultyName = "difficulty"); + AddStep("Set author", () => EditorBeatmap.BeatmapInfo.Metadata.Author.Username = "author"); + AddStep("Set difficulty name", () => EditorBeatmap.BeatmapInfo.DifficultyName = "difficulty"); - AddStep("Add timing point", () => editorBeatmap.ControlPointInfo.Add(0, new TimingControlPoint())); + SaveEditor(); + AddAssert("Beatmap has correct metadata", () => EditorBeatmap.BeatmapInfo.Metadata.Artist == "artist" && EditorBeatmap.BeatmapInfo.Metadata.Title == "title"); + AddAssert("Beatmap has correct author", () => EditorBeatmap.BeatmapInfo.Metadata.Author.Username == "author"); + AddAssert("Beatmap has correct difficulty name", () => EditorBeatmap.BeatmapInfo.DifficultyName == "difficulty"); + AddAssert("Beatmap has correct .osu file path", () => EditorBeatmap.BeatmapInfo.Path == "artist - title (author) [difficulty].osu"); + + ReloadEditorToSameBeatmap(); + + AddAssert("Beatmap still has correct metadata", () => EditorBeatmap.BeatmapInfo.Metadata.Artist == "artist" && EditorBeatmap.BeatmapInfo.Metadata.Title == "title"); + AddAssert("Beatmap still has correct author", () => EditorBeatmap.BeatmapInfo.Metadata.Author.Username == "author"); + AddAssert("Beatmap still has correct difficulty name", () => EditorBeatmap.BeatmapInfo.DifficultyName == "difficulty"); + AddAssert("Beatmap still has correct .osu file path", () => EditorBeatmap.BeatmapInfo.Path == "artist - title (author) [difficulty].osu"); + } + + [Test] + public void TestConfiguration() + { + double originalTimelineZoom = 0; + double changedTimelineZoom = 0; + + AddStep("Set beat divisor", () => Editor.Dependencies.Get().Value = 16); + AddStep("Set timeline zoom", () => + { + originalTimelineZoom = EditorBeatmap.BeatmapInfo.TimelineZoom; + + var timeline = Editor.ChildrenOfType().Single(); + InputManager.MoveMouseTo(timeline); + InputManager.PressKey(Key.AltLeft); + InputManager.ScrollVerticalBy(15f); + InputManager.ReleaseKey(Key.AltLeft); + }); + + AddAssert("Ensure timeline zoom changed", () => + { + changedTimelineZoom = EditorBeatmap.BeatmapInfo.TimelineZoom; + return !Precision.AlmostEquals(changedTimelineZoom, originalTimelineZoom); + }); + + SaveEditor(); + + AddAssert("Beatmap has correct beat divisor", () => EditorBeatmap.BeatmapInfo.BeatDivisor == 16); + AddAssert("Beatmap has correct timeline zoom", () => EditorBeatmap.BeatmapInfo.TimelineZoom == changedTimelineZoom); + + ReloadEditorToSameBeatmap(); + + AddAssert("Beatmap still has correct beat divisor", () => EditorBeatmap.BeatmapInfo.BeatDivisor == 16); + AddAssert("Beatmap still has correct timeline zoom", () => EditorBeatmap.BeatmapInfo.TimelineZoom == changedTimelineZoom); + } + + [Test] + public void TestDifficulty() + { + AddStep("Set overall difficulty", () => EditorBeatmap.Difficulty.OverallDifficulty = 7); + + SaveEditor(); + + AddAssert("Beatmap has correct overall difficulty", () => EditorBeatmap.Difficulty.OverallDifficulty == 7); + + ReloadEditorToSameBeatmap(); + + AddAssert("Beatmap still has correct overall difficulty", () => EditorBeatmap.Difficulty.OverallDifficulty == 7); + } + + [Test] + public void TestHitObjectPlacement() + { + AddStep("Add timing point", () => EditorBeatmap.ControlPointInfo.Add(500, new TimingControlPoint())); AddStep("Change to placement mode", () => InputManager.Key(Key.Number2)); AddStep("Move to playfield", () => InputManager.MoveMouseTo(Game.ScreenSpaceDrawQuad.Centre)); AddStep("Place single hitcircle", () => InputManager.Click(MouseButton.Left)); - checkMutations(); + SaveEditor(); + + AddAssert("Beatmap has correct timing point", () => EditorBeatmap.ControlPointInfo.TimingPoints.Single().Time == 500); // After placement these must be non-default as defaults are read-only. AddAssert("Placed object has non-default control points", () => - editorBeatmap.HitObjects[0].SampleControlPoint != SampleControlPoint.DEFAULT && - editorBeatmap.HitObjects[0].DifficultyControlPoint != DifficultyControlPoint.DEFAULT); + EditorBeatmap.HitObjects[0].SampleControlPoint != SampleControlPoint.DEFAULT && + EditorBeatmap.HitObjects[0].DifficultyControlPoint != DifficultyControlPoint.DEFAULT); - AddStep("Save", () => InputManager.Keys(PlatformAction.Save)); + ReloadEditorToSameBeatmap(); - checkMutations(); - AddAssert("Beatmap has correct .osu file path", () => editorBeatmap.BeatmapInfo.Path == "artist - title (author) [difficulty].osu"); + AddAssert("Beatmap still has correct timing point", () => EditorBeatmap.ControlPointInfo.TimingPoints.Single().Time == 500); - AddStep("Exit", () => InputManager.Key(Key.Escape)); - - AddUntilStep("Wait for main menu", () => Game.ScreenStack.CurrentScreen is MainMenu); - - Screens.Select.SongSelect songSelect = null; - - PushAndConfirm(() => songSelect = new PlaySongSelect()); - AddUntilStep("wait for carousel load", () => songSelect.BeatmapSetsLoaded); - - AddUntilStep("Wait for beatmap selected", () => !Game.Beatmap.IsDefault); - AddStep("Open options", () => InputManager.Key(Key.F3)); - AddStep("Enter editor", () => InputManager.Key(Key.Number5)); - - AddUntilStep("Wait for editor load", () => editor != null); - - checkMutations(); - } - - private void checkMutations() - { - AddAssert("Beatmap contains single hitcircle", () => editorBeatmap.HitObjects.Count == 1); - AddAssert("Beatmap has correct beat divisor", () => editorBeatmap.BeatmapInfo.BeatDivisor == 16); - AddAssert("Beatmap has correct overall difficulty", () => editorBeatmap.Difficulty.OverallDifficulty == 7); - AddAssert("Beatmap has correct metadata", () => editorBeatmap.BeatmapInfo.Metadata.Artist == "artist" && editorBeatmap.BeatmapInfo.Metadata.Title == "title"); - AddAssert("Beatmap has correct author", () => editorBeatmap.BeatmapInfo.Metadata.Author.Username == "author"); - AddAssert("Beatmap has correct difficulty name", () => editorBeatmap.BeatmapInfo.DifficultyName == "difficulty"); + // After placement these must be non-default as defaults are read-only. + AddAssert("Placed object still has non-default control points", () => + EditorBeatmap.HitObjects[0].SampleControlPoint != SampleControlPoint.DEFAULT && + EditorBeatmap.HitObjects[0].DifficultyControlPoint != DifficultyControlPoint.DEFAULT); } } } diff --git a/osu.Game/Beatmaps/BeatmapInfo.cs b/osu.Game/Beatmaps/BeatmapInfo.cs index 96254295a6..4e1a34ddbf 100644 --- a/osu.Game/Beatmaps/BeatmapInfo.cs +++ b/osu.Game/Beatmaps/BeatmapInfo.cs @@ -111,7 +111,7 @@ namespace osu.Game.Beatmaps public int GridSize { get; set; } - public double TimelineZoom { get; set; } + public double TimelineZoom { get; set; } = 1.0; [Ignored] public CountdownType Countdown { get; set; } = CountdownType.Normal; diff --git a/osu.Game/Tests/Visual/EditorSavingTestScene.cs b/osu.Game/Tests/Visual/EditorSavingTestScene.cs new file mode 100644 index 0000000000..72b5d076a5 --- /dev/null +++ b/osu.Game/Tests/Visual/EditorSavingTestScene.cs @@ -0,0 +1,67 @@ +// 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 osu.Framework.Input; +using osu.Framework.Testing; +using osu.Game.Rulesets.Edit; +using osu.Game.Screens.Edit; +using osu.Game.Screens.Edit.Setup; +using osu.Game.Screens.Menu; +using osu.Game.Screens.Select; +using osuTK.Input; + +namespace osu.Game.Tests.Visual +{ + /// + /// Tests the general expected flow of creating a new beatmap, saving it, then loading it back from song select. + /// + public class EditorSavingTestScene : OsuGameTestScene + { + protected Editor Editor => Game.ChildrenOfType().FirstOrDefault(); + + protected EditorBeatmap EditorBeatmap => (EditorBeatmap)Editor.Dependencies.Get(typeof(EditorBeatmap)); + + [SetUpSteps] + public override void SetUpSteps() + { + base.SetUpSteps(); + + AddStep("set default beatmap", () => Game.Beatmap.SetDefault()); + + PushAndConfirm(() => new EditorLoader()); + + AddUntilStep("wait for editor load", () => Editor?.IsLoaded == true); + + AddUntilStep("wait for metadata screen load", () => Editor.ChildrenOfType().FirstOrDefault()?.IsLoaded == true); + + // We intentionally switch away from the metadata screen, else there is a feedback loop with the textbox handling which causes metadata changes below to get overwritten. + + AddStep("Enter compose mode", () => InputManager.Key(Key.F1)); + AddUntilStep("Wait for compose mode load", () => Editor.ChildrenOfType().FirstOrDefault()?.IsLoaded == true); + } + + protected void SaveEditor() + { + AddStep("Save", () => InputManager.Keys(PlatformAction.Save)); + } + + protected void ReloadEditorToSameBeatmap() + { + AddStep("Exit", () => InputManager.Key(Key.Escape)); + + AddUntilStep("Wait for main menu", () => Game.ScreenStack.CurrentScreen is MainMenu); + + SongSelect songSelect = null; + + PushAndConfirm(() => songSelect = new PlaySongSelect()); + AddUntilStep("wait for carousel load", () => songSelect.BeatmapSetsLoaded); + + AddUntilStep("Wait for beatmap selected", () => !Game.Beatmap.IsDefault); + AddStep("Open options", () => InputManager.Key(Key.F3)); + AddStep("Enter editor", () => InputManager.Key(Key.Number5)); + + AddUntilStep("Wait for editor load", () => Editor != null); + } + } +} From de0a7d8501c5c3853755c49cf289a6be666c77a9 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Tue, 25 Jan 2022 12:29:41 +0300 Subject: [PATCH 05/12] Migrate taiko editor saving test scene to `EditorSavingTestScene` --- .../Editor/TestSceneEditorSaving.cs | 91 ------------------- .../Editor/TestSceneTaikoEditorSaving.cs | 38 ++++++++ 2 files changed, 38 insertions(+), 91 deletions(-) delete mode 100644 osu.Game.Rulesets.Taiko.Tests/Editor/TestSceneEditorSaving.cs create mode 100644 osu.Game.Rulesets.Taiko.Tests/Editor/TestSceneTaikoEditorSaving.cs diff --git a/osu.Game.Rulesets.Taiko.Tests/Editor/TestSceneEditorSaving.cs b/osu.Game.Rulesets.Taiko.Tests/Editor/TestSceneEditorSaving.cs deleted file mode 100644 index 42ab84714a..0000000000 --- a/osu.Game.Rulesets.Taiko.Tests/Editor/TestSceneEditorSaving.cs +++ /dev/null @@ -1,91 +0,0 @@ -// 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.Input; -using osu.Framework.Testing; -using osu.Framework.Utils; -using osu.Game.Beatmaps; -using osu.Game.Rulesets.Edit; -using osu.Game.Rulesets.Taiko.Beatmaps; -using osu.Game.Screens.Edit; -using osu.Game.Screens.Edit.Setup; -using osu.Game.Screens.Menu; -using osu.Game.Screens.Select; -using osu.Game.Tests.Visual; -using osuTK.Input; - -namespace osu.Game.Rulesets.Taiko.Tests.Editor -{ - public class TestSceneEditorSaving : OsuGameTestScene - { - private Screens.Edit.Editor editor => Game.ChildrenOfType().FirstOrDefault(); - - private EditorBeatmap editorBeatmap => (EditorBeatmap)editor.Dependencies.Get(typeof(EditorBeatmap)); - - /// - /// Tests the general expected flow of creating a new beatmap, saving it, then loading it back from song select. - /// Emphasis is placed on , since taiko has special handling for it to keep compatibility with stable. - /// - [Test] - public void TestNewBeatmapSaveThenLoad() - { - AddStep("set default beatmap", () => Game.Beatmap.SetDefault()); - AddStep("set taiko ruleset", () => Ruleset.Value = new TaikoRuleset().RulesetInfo); - - PushAndConfirm(() => new EditorLoader()); - - AddUntilStep("wait for editor load", () => editor?.IsLoaded == true); - - AddUntilStep("wait for metadata screen load", () => editor.ChildrenOfType().FirstOrDefault()?.IsLoaded == true); - - // We intentionally switch away from the metadata screen, else there is a feedback loop with the textbox handling which causes metadata changes below to get overwritten. - - AddStep("Enter compose mode", () => InputManager.Key(Key.F1)); - AddUntilStep("Wait for compose mode load", () => editor.ChildrenOfType().FirstOrDefault()?.IsLoaded == true); - - AddStep("Set slider multiplier", () => editorBeatmap.Difficulty.SliderMultiplier = 2); - AddStep("Set artist and title", () => - { - editorBeatmap.BeatmapInfo.Metadata.Artist = "artist"; - editorBeatmap.BeatmapInfo.Metadata.Title = "title"; - }); - AddStep("Set difficulty name", () => editorBeatmap.BeatmapInfo.DifficultyName = "difficulty"); - - checkMutations(); - - AddStep("Save", () => InputManager.Keys(PlatformAction.Save)); - - checkMutations(); - - AddStep("Exit", () => InputManager.Key(Key.Escape)); - - AddUntilStep("Wait for main menu", () => Game.ScreenStack.CurrentScreen is MainMenu); - - PushAndConfirm(() => new PlaySongSelect()); - - AddUntilStep("Wait for beatmap selected", () => !Game.Beatmap.IsDefault); - AddStep("Open options", () => InputManager.Key(Key.F3)); - AddStep("Enter editor", () => InputManager.Key(Key.Number5)); - - AddUntilStep("Wait for editor load", () => editor != null); - - checkMutations(); - } - - private void checkMutations() - { - AddAssert("Beatmap has correct slider multiplier", () => - { - // we can only assert value correctness on TaikoMultiplierAppliedDifficulty, because that is the final difficulty converted taiko beatmaps use. - // therefore, ensure that we have that difficulty type by calling .CopyFrom(), which is a no-op if the type is already correct. - var taikoDifficulty = new TaikoBeatmapConverter.TaikoMultiplierAppliedDifficulty(); - taikoDifficulty.CopyFrom(editorBeatmap.Difficulty); - return Precision.AlmostEquals(taikoDifficulty.SliderMultiplier, 2); - }); - AddAssert("Beatmap has correct metadata", () => editorBeatmap.BeatmapInfo.Metadata.Artist == "artist" && editorBeatmap.BeatmapInfo.Metadata.Title == "title"); - AddAssert("Beatmap has correct difficulty name", () => editorBeatmap.BeatmapInfo.DifficultyName == "difficulty"); - } - } -} diff --git a/osu.Game.Rulesets.Taiko.Tests/Editor/TestSceneTaikoEditorSaving.cs b/osu.Game.Rulesets.Taiko.Tests/Editor/TestSceneTaikoEditorSaving.cs new file mode 100644 index 0000000000..33c2ba532e --- /dev/null +++ b/osu.Game.Rulesets.Taiko.Tests/Editor/TestSceneTaikoEditorSaving.cs @@ -0,0 +1,38 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using NUnit.Framework; +using osu.Framework.Utils; +using osu.Game.Rulesets.Taiko.Beatmaps; +using osu.Game.Tests.Visual; + +namespace osu.Game.Rulesets.Taiko.Tests.Editor +{ + public class TestSceneTaikoEditorSaving : EditorSavingTestScene + { + protected override Ruleset CreateRuleset() => new TaikoRuleset(); + + [Test] + public void TestTaikoSliderMultiplier() + { + AddStep("Set slider multiplier", () => EditorBeatmap.Difficulty.SliderMultiplier = 2); + + SaveEditor(); + + AddAssert("Beatmap has correct slider multiplier", assertTaikoSliderMulitplier); + + ReloadEditorToSameBeatmap(); + + AddAssert("Beatmap still has correct slider multiplier", assertTaikoSliderMulitplier); + + bool assertTaikoSliderMulitplier() + { + // we can only assert value correctness on TaikoMultiplierAppliedDifficulty, because that is the final difficulty converted taiko beatmaps use. + // therefore, ensure that we have that difficulty type by calling .CopyFrom(), which is a no-op if the type is already correct. + var taikoDifficulty = new TaikoBeatmapConverter.TaikoMultiplierAppliedDifficulty(); + taikoDifficulty.CopyFrom(EditorBeatmap.Difficulty); + return Precision.AlmostEquals(taikoDifficulty.SliderMultiplier, 2); + } + } + } +} From 3491b77c8cc95ce752a25434e00f130227dad182 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 26 Jan 2022 14:25:55 +0900 Subject: [PATCH 06/12] Fix `ScoreInfo.RealmUser` not getting deep cloned correctly I'm still not at all happy with the play-to-results flow (with multiple clones), but this will have to do for now. --- osu.Game.Tests/NonVisual/ScoreInfoTest.cs | 4 ++++ osu.Game/Scoring/ScoreInfo.cs | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/osu.Game.Tests/NonVisual/ScoreInfoTest.cs b/osu.Game.Tests/NonVisual/ScoreInfoTest.cs index e0acc6d8db..836c978ba2 100644 --- a/osu.Game.Tests/NonVisual/ScoreInfoTest.cs +++ b/osu.Game.Tests/NonVisual/ScoreInfoTest.cs @@ -26,12 +26,16 @@ namespace osu.Game.Tests.NonVisual score.Statistics[HitResult.Good]++; score.Rank = ScoreRank.X; + score.RealmUser.Username = "test"; Assert.That(scoreCopy.Statistics[HitResult.Good], Is.EqualTo(10)); Assert.That(score.Statistics[HitResult.Good], Is.EqualTo(11)); Assert.That(scoreCopy.Rank, Is.EqualTo(ScoreRank.B)); Assert.That(score.Rank, Is.EqualTo(ScoreRank.X)); + + Assert.That(scoreCopy.RealmUser.Username, Is.EqualTo("test")); + Assert.That(score.Rank, Is.Empty); } [Test] diff --git a/osu.Game/Scoring/ScoreInfo.cs b/osu.Game/Scoring/ScoreInfo.cs index a28e16450f..4de1d580dc 100644 --- a/osu.Game/Scoring/ScoreInfo.cs +++ b/osu.Game/Scoring/ScoreInfo.cs @@ -133,6 +133,11 @@ namespace osu.Game.Scoring var clone = (ScoreInfo)this.Detach().MemberwiseClone(); clone.Statistics = new Dictionary(clone.Statistics); + clone.RealmUser = new RealmUser + { + OnlineID = RealmUser.OnlineID, + Username = RealmUser.Username, + }; return clone; } From d0a2818847d85fe68e99a3cc5feb11ca4602e130 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 26 Jan 2022 15:14:43 +0900 Subject: [PATCH 07/12] Fix incorrect testing --- osu.Game.Tests/NonVisual/ScoreInfoTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/NonVisual/ScoreInfoTest.cs b/osu.Game.Tests/NonVisual/ScoreInfoTest.cs index 836c978ba2..41b08a9e98 100644 --- a/osu.Game.Tests/NonVisual/ScoreInfoTest.cs +++ b/osu.Game.Tests/NonVisual/ScoreInfoTest.cs @@ -34,8 +34,8 @@ namespace osu.Game.Tests.NonVisual Assert.That(scoreCopy.Rank, Is.EqualTo(ScoreRank.B)); Assert.That(score.Rank, Is.EqualTo(ScoreRank.X)); - Assert.That(scoreCopy.RealmUser.Username, Is.EqualTo("test")); - Assert.That(score.Rank, Is.Empty); + Assert.That(scoreCopy.RealmUser.Username, Is.Empty); + Assert.That(score.RealmUser.Username, Is.EqualTo("test")); } [Test] From 267a7bd21fa709fc941d3620fe2c57c48e5bbe4a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 26 Jan 2022 15:14:49 +0900 Subject: [PATCH 08/12] Give `RealmUser.Username` a better default value --- osu.Game/Models/RealmUser.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Models/RealmUser.cs b/osu.Game/Models/RealmUser.cs index ff35528827..5fccff597c 100644 --- a/osu.Game/Models/RealmUser.cs +++ b/osu.Game/Models/RealmUser.cs @@ -11,7 +11,7 @@ namespace osu.Game.Models { public int OnlineID { get; set; } = 1; - public string Username { get; set; } + public string Username { get; set; } = string.Empty; public bool IsBot => false; From d1a22562621a2768056d3604400108f0b20c5441 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 26 Jan 2022 17:48:11 +0900 Subject: [PATCH 09/12] Refactor `SkinSection` to avoid unnecessary realm queries --- .../Overlays/Settings/Sections/SkinSection.cs | 83 ++++++++++--------- 1 file changed, 42 insertions(+), 41 deletions(-) diff --git a/osu.Game/Overlays/Settings/Sections/SkinSection.cs b/osu.Game/Overlays/Settings/Sections/SkinSection.cs index 0846c023c1..441a439596 100644 --- a/osu.Game/Overlays/Settings/Sections/SkinSection.cs +++ b/osu.Game/Overlays/Settings/Sections/SkinSection.cs @@ -18,6 +18,7 @@ using osu.Game.Graphics.UserInterface; using osu.Game.Localisation; using osu.Game.Skinning; using osu.Game.Skinning.Editor; +using Realms; namespace osu.Game.Overlays.Settings.Sections { @@ -51,12 +52,6 @@ namespace osu.Game.Overlays.Settings.Sections private IDisposable realmSubscription; - private IQueryable queryRealmSkins() => - realm.Realm.All() - .Where(s => !s.DeletePending) - .OrderByDescending(s => s.Protected) // protected skins should be at the top. - .ThenBy(s => s.Name, StringComparer.OrdinalIgnoreCase); - [BackgroundDependencyLoader(permitNulls: true)] private void load(OsuConfigManager config, [CanBeNull] SkinEditorOverlay skinEditor) { @@ -83,37 +78,54 @@ namespace osu.Game.Overlays.Settings.Sections skinDropdown.Current = dropdownBindable; - realmSubscription = realm.RegisterForNotifications(r => queryRealmSkins(), (sender, changes, error) => - { - // The first fire of this is a bit redundant due to the call below, - // but this is safest in case the subscription is restored after a context recycle. - updateItems(); - }); - - updateItems(); + realmSubscription = realm.RegisterForNotifications(r => realm.Realm.All() + .Where(s => !s.DeletePending) + .OrderByDescending(s => s.Protected) // protected skins should be at the top. + .ThenBy(s => s.Name, StringComparer.OrdinalIgnoreCase), skinsChanged); configBindable.BindValueChanged(id => Scheduler.AddOnce(updateSelectedSkinFromConfig)); - updateSelectedSkinFromConfig(); - dropdownBindable.BindValueChanged(skin => + dropdownBindable.BindValueChanged(dropdownSelectionChanged); + } + + private void dropdownSelectionChanged(ValueChangedEvent> skin) + { + // Only handle cases where it's clear the user has intent to change skins. + if (skin.OldValue == null) return; + + if (skin.NewValue.Equals(random_skin_info)) { - if (skin.NewValue.Equals(random_skin_info)) + var skinBefore = skins.CurrentSkinInfo.Value; + + skins.SelectRandomSkin(); + + if (skinBefore == skins.CurrentSkinInfo.Value) { - var skinBefore = skins.CurrentSkinInfo.Value; - - skins.SelectRandomSkin(); - - if (skinBefore == skins.CurrentSkinInfo.Value) - { - // the random selection didn't change the skin, so we should manually update the dropdown to match. - dropdownBindable.Value = skins.CurrentSkinInfo.Value; - } - - return; + // the random selection didn't change the skin, so we should manually update the dropdown to match. + dropdownBindable.Value = skins.CurrentSkinInfo.Value; } - configBindable.Value = skin.NewValue.ID.ToString(); - }); + return; + } + + configBindable.Value = skin.NewValue.ID.ToString(); + } + + private void skinsChanged(IRealmCollection skins, ChangeSet changes, Exception error) + { + // This can only mean that realm is recycling, else we would see the protected skins. + // Because we are using `Live<>` in this class, we don't need to worry about this scenario too much. + if (!skins.Any()) + return; + + int protectedCount = skins.Count(s => s.Protected); + + skinItems = skins.ToLive(realm); + skinItems.Insert(protectedCount, random_skin_info); + + skinDropdown.Items = skinItems; + + updateSelectedSkinFromConfig(); } private void updateSelectedSkinFromConfig() @@ -126,17 +138,6 @@ namespace osu.Game.Overlays.Settings.Sections dropdownBindable.Value = skin ?? skinDropdown.Items.First(); } - private void updateItems() - { - int protectedCount = queryRealmSkins().Count(s => s.Protected); - - skinItems = queryRealmSkins().ToLive(realm); - - skinItems.Insert(protectedCount, random_skin_info); - - skinDropdown.Items = skinItems; - } - protected override void Dispose(bool isDisposing) { base.Dispose(isDisposing); From f2d48d088d81773f1d93f298b8e7d142565b7e80 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 26 Jan 2022 17:57:03 +0900 Subject: [PATCH 10/12] Fix realm migration failures with presence of databased EF rulesets that don't exist on disk --- osu.Game/Rulesets/EFRulesetInfo.cs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/osu.Game/Rulesets/EFRulesetInfo.cs b/osu.Game/Rulesets/EFRulesetInfo.cs index 473b7c657e..9559cfa00c 100644 --- a/osu.Game/Rulesets/EFRulesetInfo.cs +++ b/osu.Game/Rulesets/EFRulesetInfo.cs @@ -28,21 +28,15 @@ namespace osu.Game.Rulesets public Ruleset CreateInstance() { if (!Available) - throw new RulesetLoadException(@"Ruleset not available"); + return null; var type = Type.GetType(InstantiationInfo); if (type == null) - throw new RulesetLoadException(@"Type lookup failure"); + return null; var ruleset = Activator.CreateInstance(type) as Ruleset; - if (ruleset == null) - throw new RulesetLoadException(@"Instantiation failure"); - - // overwrite the pre-populated RulesetInfo with a potentially database attached copy. - // ruleset.RulesetInfo = this; - return ruleset; } From 3aa681005bd5bf676b0cdf912aa0567a8120f077 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 26 Jan 2022 18:04:53 +0900 Subject: [PATCH 11/12] Skip importing scores which have no matching realm ruleset There's no real way to recover these unless we want to start importing rulesets into realm. And that seems counter productive. This can only happen if users don't have the dll present any more, and it was removed far before realm was tracking rulesets (else it would have an `Available=0` entry in realm to match). --- osu.Game/Database/EFToRealmMigrator.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/osu.Game/Database/EFToRealmMigrator.cs b/osu.Game/Database/EFToRealmMigrator.cs index adf91e4a41..33f7c25b28 100644 --- a/osu.Game/Database/EFToRealmMigrator.cs +++ b/osu.Game/Database/EFToRealmMigrator.cs @@ -286,6 +286,7 @@ namespace osu.Game.Database var transaction = r.BeginWrite(); int written = 0; + int missing = 0; try { @@ -300,6 +301,13 @@ namespace osu.Game.Database var beatmap = r.All().First(b => b.Hash == score.BeatmapInfo.Hash); var ruleset = r.Find(score.Ruleset.ShortName); + + if (ruleset == null) + { + log($"Skipping {++missing} scores with missing ruleset"); + continue; + } + var user = new RealmUser { OnlineID = score.User.OnlineID, From 45636ce04b224ca516b798c253f2289c357aaa5c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 26 Jan 2022 18:25:27 +0900 Subject: [PATCH 12/12] Remove collection `ToLive` helper method to avoid confusion --- osu.Game/Database/RealmObjectExtensions.cs | 6 ------ .../Overlays/Settings/Sections/SkinSection.cs | 18 +++++++++++------- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/osu.Game/Database/RealmObjectExtensions.cs b/osu.Game/Database/RealmObjectExtensions.cs index dba8633f53..7a0ca2c85a 100644 --- a/osu.Game/Database/RealmObjectExtensions.cs +++ b/osu.Game/Database/RealmObjectExtensions.cs @@ -216,12 +216,6 @@ namespace osu.Game.Database return new RealmLiveUnmanaged(realmObject); } - public static List> ToLive(this IEnumerable realmList, RealmAccess realm) - where T : RealmObject, IHasGuidPrimaryKey - { - return realmList.Select(l => new RealmLive(l, realm)).Cast>().ToList(); - } - public static Live ToLive(this T realmObject, RealmAccess realm) where T : RealmObject, IHasGuidPrimaryKey { diff --git a/osu.Game/Overlays/Settings/Sections/SkinSection.cs b/osu.Game/Overlays/Settings/Sections/SkinSection.cs index 441a439596..1dfe49945f 100644 --- a/osu.Game/Overlays/Settings/Sections/SkinSection.cs +++ b/osu.Game/Overlays/Settings/Sections/SkinSection.cs @@ -42,7 +42,7 @@ namespace osu.Game.Overlays.Settings.Sections Name = "", }.ToLiveUnmanaged(); - private List> skinItems; + private readonly List> dropdownItems = new List>(); [Resolved] private SkinManager skins { get; set; } @@ -111,19 +111,23 @@ namespace osu.Game.Overlays.Settings.Sections configBindable.Value = skin.NewValue.ID.ToString(); } - private void skinsChanged(IRealmCollection skins, ChangeSet changes, Exception error) + private void skinsChanged(IRealmCollection sender, ChangeSet changes, Exception error) { // This can only mean that realm is recycling, else we would see the protected skins. // Because we are using `Live<>` in this class, we don't need to worry about this scenario too much. - if (!skins.Any()) + if (!sender.Any()) return; - int protectedCount = skins.Count(s => s.Protected); + int protectedCount = sender.Count(s => s.Protected); - skinItems = skins.ToLive(realm); - skinItems.Insert(protectedCount, random_skin_info); + // For simplicity repopulate the full list. + // In the future we should change this to properly handle ChangeSet events. + dropdownItems.Clear(); + foreach (var skin in sender) + dropdownItems.Add(skin.ToLive(realm)); + dropdownItems.Insert(protectedCount, random_skin_info); - skinDropdown.Items = skinItems; + skinDropdown.Items = dropdownItems; updateSelectedSkinFromConfig(); }