From 791ce218fcd4c8bff495f869d92d1d25a63d23bc Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 5 Sep 2024 18:55:11 +0900 Subject: [PATCH 1/2] Add test coverage of beatmap offset edge case failure --- .../Gameplay/TestSceneBeatmapOffsetControl.cs | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapOffsetControl.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapOffsetControl.cs index 3b88750013a8..c7f1eabab235 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapOffsetControl.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneBeatmapOffsetControl.cs @@ -5,6 +5,7 @@ using NUnit.Framework; using osu.Framework.Graphics; using osu.Framework.Testing; +using osu.Game.Beatmaps; using osu.Game.Overlays.Settings; using osu.Game.Rulesets.Mods; using osu.Game.Rulesets.Osu.Mods; @@ -136,6 +137,59 @@ public void TestCalibrationFromNonZero() AddAssert("No calibration button", () => !offsetControl.ChildrenOfType().Any()); } + [Test] + public void TestCalibrationFromNonZeroWithImmediateReferenceScore() + { + const double average_error = -4.5; + const double initial_offset = -2; + + AddStep("Set beatmap offset non-neutral", () => Realm.Write(r => + { + r.Add(new BeatmapInfo + { + ID = Beatmap.Value.BeatmapInfo.ID, + Ruleset = Beatmap.Value.BeatmapInfo.Ruleset, + UserSettings = + { + Offset = initial_offset, + } + }); + })); + + AddStep("Create control with preloaded reference score", () => + { + Child = new PlayerSettingsGroup("Some settings") + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Children = new Drawable[] + { + offsetControl = new BeatmapOffsetControl + { + ReferenceScore = + { + Value = new ScoreInfo + { + HitEvents = TestSceneHitEventTimingDistributionGraph.CreateDistributedHitEvents(average_error), + BeatmapInfo = Beatmap.Value.BeatmapInfo, + } + } + } + } + }; + }); + + AddUntilStep("Has calibration button", () => offsetControl.ChildrenOfType().Any()); + AddStep("Press button", () => offsetControl.ChildrenOfType().Single().TriggerClick()); + AddAssert("Offset is adjusted", () => offsetControl.Current.Value, () => Is.EqualTo(initial_offset - average_error)); + + AddUntilStep("Button is disabled", () => !offsetControl.ChildrenOfType().Single().Enabled.Value); + AddStep("Remove reference score", () => offsetControl.ReferenceScore.Value = null); + AddAssert("No calibration button", () => !offsetControl.ChildrenOfType().Any()); + + AddStep("Clean up beatmap", () => Realm.Write(r => r.RemoveAll())); + } + [Test] public void TestCalibrationNoChange() { From 37f61b26ea858b53e29aefc784b1563b8ed56c59 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 5 Sep 2024 18:45:49 +0900 Subject: [PATCH 2/2] Fix offset adjust control not correctly applying changes after song select quit This is an interesting scenario where we arrive at a fresh `BeatmapOffsetControl` but with a reference score (from the last play). Our best assumption here is that the beatmap's offset hasn't changed since the last play, so we want to use it for the `lastPlayBeatmapOffset`. But due to unfortunate order of execution, `Current.Value` was not yet initialised. --- osu.Game/Screens/Play/PlayerSettings/BeatmapOffsetControl.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Game/Screens/Play/PlayerSettings/BeatmapOffsetControl.cs b/osu.Game/Screens/Play/PlayerSettings/BeatmapOffsetControl.cs index 7668d3e63529..f312fb0ec5a9 100644 --- a/osu.Game/Screens/Play/PlayerSettings/BeatmapOffsetControl.cs +++ b/osu.Game/Screens/Play/PlayerSettings/BeatmapOffsetControl.cs @@ -104,8 +104,6 @@ protected override void LoadComplete() { base.LoadComplete(); - ReferenceScore.BindValueChanged(scoreChanged, true); - beatmapOffsetSubscription = realm.SubscribeToPropertyChanged( r => r.Find(beatmap.Value.BeatmapInfo.ID)?.UserSettings, settings => settings.Offset, @@ -124,6 +122,7 @@ protected override void LoadComplete() }); Current.BindValueChanged(currentChanged); + ReferenceScore.BindValueChanged(scoreChanged, true); } private void currentChanged(ValueChangedEvent offset)