From 7464a486d980a4180aa86f3404a77e8234688bcd Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 4 Mar 2020 16:35:45 +0900 Subject: [PATCH 1/5] Fix DummyWorkingBeatmap's track completion attempting to change game-wide beatmap --- osu.Game/OsuGame.cs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index 5781a7fbc4..69d5a9a583 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -428,11 +428,17 @@ namespace osu.Game } } - private void currentTrackCompleted() => Schedule(() => + private void currentTrackCompleted() { - if (!Beatmap.Value.Track.Looping && !Beatmap.Disabled) - musicController.NextTrack(); - }); + if (Beatmap.Value is DummyWorkingBeatmap) + return; + + Schedule(() => + { + if (Beatmap.Value.Track.Looping && !Beatmap.Disabled) + musicController.NextTrack(); + }); + } #endregion From bac35b2a68f15a0abe079f94a17883e6c040ef68 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 4 Mar 2020 19:09:52 +0900 Subject: [PATCH 2/5] Handle beatmap track changing in a saner way --- osu.Game/OsuGame.cs | 39 ++++++++++++++++++--------------------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index 69d5a9a583..0be9e6cdaa 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -396,18 +396,27 @@ namespace osu.Game private void beatmapChanged(ValueChangedEvent beatmap) { - var nextBeatmap = beatmap.NewValue; - if (nextBeatmap?.Track != null) - nextBeatmap.Track.Completed += currentTrackCompleted; - - var oldBeatmap = beatmap.OldValue; - if (oldBeatmap?.Track != null) - oldBeatmap.Track.Completed -= currentTrackCompleted; + beatmap.OldValue?.CancelAsyncLoad(); updateModDefaults(); - oldBeatmap?.CancelAsyncLoad(); - nextBeatmap?.BeginAsyncLoad(); + var newBeatmap = beatmap.NewValue; + + if (newBeatmap != null) + { + newBeatmap.Track.Completed += () => Scheduler.AddOnce(() => trackCompleted(newBeatmap)); + newBeatmap.BeginAsyncLoad(); + } + + void trackCompleted(WorkingBeatmap b) + { + // the source of track completion is the audio thread, so the beatmap may have changed before a firing. + if (Beatmap.Value != b) + return; + + if (Beatmap.Value.Track.Looping && !Beatmap.Disabled) + musicController.NextTrack(); + } } private void modsChanged(ValueChangedEvent> mods) @@ -428,18 +437,6 @@ namespace osu.Game } } - private void currentTrackCompleted() - { - if (Beatmap.Value is DummyWorkingBeatmap) - return; - - Schedule(() => - { - if (Beatmap.Value.Track.Looping && !Beatmap.Disabled) - musicController.NextTrack(); - }); - } - #endregion private ScheduledDelegate performFromMainMenuTask; From 38d91ccd0d20dcd4f9f4e016f03da00da2ba8152 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 4 Mar 2020 19:07:34 +0900 Subject: [PATCH 3/5] Add comment regarding no-longer-required schedule --- osu.Game/OsuGameBase.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index a890331f05..67aa4a8d4d 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -211,6 +211,10 @@ namespace osu.Game Audio.Tracks.AddAdjustment(AdjustableProperty.Volume, new BindableDouble(0.8)); Beatmap = new NonNullableBindable(defaultBeatmap); + + // ScheduleAfterChildren is safety against something in the current frame accessing the previous beatmap's track + // and potentially causing a reload of it after just unloading. + // Note that the reason for this being added *has* been resolved, so it may be feasible to remover this if required. Beatmap.BindValueChanged(b => ScheduleAfterChildren(() => { // compare to last beatmap as sometimes the two may share a track representation (optimisation, see WorkingBeatmap.TransferTo) From 9307caa3bfb60e7636033fe78311549da1653487 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Thu, 5 Mar 2020 16:58:07 +0900 Subject: [PATCH 4/5] Fix typos --- osu.Game/OsuGame.cs | 2 +- osu.Game/OsuGameBase.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index 0be9e6cdaa..e54bbaabb2 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -410,7 +410,7 @@ namespace osu.Game void trackCompleted(WorkingBeatmap b) { - // the source of track completion is the audio thread, so the beatmap may have changed before a firing. + // the source of track completion is the audio thread, so the beatmap may have changed before firing. if (Beatmap.Value != b) return; diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index 67aa4a8d4d..1048b37348 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -214,7 +214,7 @@ namespace osu.Game // ScheduleAfterChildren is safety against something in the current frame accessing the previous beatmap's track // and potentially causing a reload of it after just unloading. - // Note that the reason for this being added *has* been resolved, so it may be feasible to remover this if required. + // Note that the reason for this being added *has* been resolved, so it may be feasible to removed this if required. Beatmap.BindValueChanged(b => ScheduleAfterChildren(() => { // compare to last beatmap as sometimes the two may share a track representation (optimisation, see WorkingBeatmap.TransferTo) From 0c1775b52281517e8f6f0518fc930b42b0f26aca Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Thu, 5 Mar 2020 17:12:14 +0900 Subject: [PATCH 5/5] Fix incorrect condition and add test --- .../Visual/Navigation/OsuGameTestScene.cs | 2 ++ .../Navigation/TestSceneScreenNavigation.cs | 16 ++++++++++++++++ osu.Game/OsuGame.cs | 10 +++++----- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/osu.Game.Tests/Visual/Navigation/OsuGameTestScene.cs b/osu.Game.Tests/Visual/Navigation/OsuGameTestScene.cs index 70d71d0952..e984806dc9 100644 --- a/osu.Game.Tests/Visual/Navigation/OsuGameTestScene.cs +++ b/osu.Game.Tests/Visual/Navigation/OsuGameTestScene.cs @@ -97,6 +97,8 @@ namespace osu.Game.Tests.Visual.Navigation public new SettingsPanel Settings => base.Settings; + public new MusicController MusicController => base.MusicController; + public new OsuConfigManager LocalConfig => base.LocalConfig; public new Bindable Beatmap => base.Beatmap; diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs index 8258cc9465..9d603ac471 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs @@ -114,6 +114,22 @@ namespace osu.Game.Tests.Visual.Navigation AddAssert("Options overlay was closed", () => Game.Settings.State.Value == Visibility.Hidden); } + [Test] + public void TestWaitForNextTrackInMenu() + { + bool trackCompleted = false; + + AddUntilStep("Wait for music controller", () => Game.MusicController.IsLoaded); + AddStep("Seek close to end", () => + { + Game.MusicController.SeekTo(Game.Beatmap.Value.Track.Length - 1000); + Game.Beatmap.Value.Track.Completed += () => trackCompleted = true; + }); + + AddUntilStep("Track was completed", () => trackCompleted); + AddUntilStep("Track was restarted", () => Game.Beatmap.Value.Track.IsRunning); + } + private void pushEscape() => AddStep("Press escape", () => pressAndRelease(Key.Escape)); diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index e54bbaabb2..19602d524e 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -414,8 +414,8 @@ namespace osu.Game if (Beatmap.Value != b) return; - if (Beatmap.Value.Track.Looping && !Beatmap.Disabled) - musicController.NextTrack(); + if (!Beatmap.Value.Track.Looping && !Beatmap.Disabled) + MusicController.NextTrack(); } } @@ -588,7 +588,7 @@ namespace osu.Game loadComponentSingleFile(new OnScreenDisplay(), Add, true); - loadComponentSingleFile(musicController = new MusicController(), Add, true); + loadComponentSingleFile(MusicController = new MusicController(), Add, true); loadComponentSingleFile(notifications = new NotificationOverlay { @@ -896,7 +896,7 @@ namespace osu.Game private ScalingContainer screenContainer; - private MusicController musicController; + protected MusicController MusicController { get; private set; } protected override bool OnExiting() { @@ -954,7 +954,7 @@ namespace osu.Game { OverlayActivationMode.Value = newOsuScreen.InitialOverlayActivationMode; - musicController.AllowRateAdjustments = newOsuScreen.AllowRateAdjustments; + MusicController.AllowRateAdjustments = newOsuScreen.AllowRateAdjustments; if (newOsuScreen.HideOverlaysOnEnter) CloseAllOverlays();