From d6968ca09cf0d8137034768266da2ee5a6b7ff05 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 20 Jul 2017 11:01:50 +0900 Subject: [PATCH 1/7] Remove FullyLoaded logic Always parse storyboards for now. Let's not optimise this until it is necessary. It was leading to weird threading problems due to the load call in Player's async load method. --- osu.Desktop.VisualTests/Beatmaps/TestWorkingBeatmap.cs | 2 +- osu.Game/Beatmaps/WorkingBeatmap.cs | 8 +------- osu.Game/Database/BeatmapDatabase.cs | 2 +- osu.Game/Database/DatabaseWorkingBeatmap.cs | 6 +++--- osu.Game/Screens/Play/Player.cs | 6 +----- 5 files changed, 7 insertions(+), 17 deletions(-) diff --git a/osu.Desktop.VisualTests/Beatmaps/TestWorkingBeatmap.cs b/osu.Desktop.VisualTests/Beatmaps/TestWorkingBeatmap.cs index 580e062aaf..b45574b761 100644 --- a/osu.Desktop.VisualTests/Beatmaps/TestWorkingBeatmap.cs +++ b/osu.Desktop.VisualTests/Beatmaps/TestWorkingBeatmap.cs @@ -10,7 +10,7 @@ namespace osu.Desktop.VisualTests.Beatmaps public class TestWorkingBeatmap : WorkingBeatmap { public TestWorkingBeatmap(Beatmap beatmap) - : base(beatmap.BeatmapInfo, true) + : base(beatmap.BeatmapInfo) { this.beatmap = beatmap; } diff --git a/osu.Game/Beatmaps/WorkingBeatmap.cs b/osu.Game/Beatmaps/WorkingBeatmap.cs index 0362d06c66..4dd624c14e 100644 --- a/osu.Game/Beatmaps/WorkingBeatmap.cs +++ b/osu.Game/Beatmaps/WorkingBeatmap.cs @@ -22,17 +22,11 @@ namespace osu.Game.Beatmaps public readonly Bindable> Mods = new Bindable>(new Mod[] { }); - /// - /// Denotes whether extras like storyboards have been loaded for this . - /// - public bool FullyLoaded { get; protected set; } - - protected WorkingBeatmap(BeatmapInfo beatmapInfo, bool fullyLoaded = false) + protected WorkingBeatmap(BeatmapInfo beatmapInfo) { BeatmapInfo = beatmapInfo; BeatmapSetInfo = beatmapInfo.BeatmapSet; Metadata = beatmapInfo.Metadata ?? BeatmapSetInfo.Metadata; - FullyLoaded = fullyLoaded; Mods.ValueChanged += mods => applyRateAdjustments(); } diff --git a/osu.Game/Database/BeatmapDatabase.cs b/osu.Game/Database/BeatmapDatabase.cs index b31b71a728..84a7096da4 100644 --- a/osu.Game/Database/BeatmapDatabase.cs +++ b/osu.Game/Database/BeatmapDatabase.cs @@ -291,7 +291,7 @@ namespace osu.Game.Database if (beatmapInfo.Metadata == null) beatmapInfo.Metadata = beatmapInfo.BeatmapSet.Metadata; - WorkingBeatmap working = new DatabaseWorkingBeatmap(this, beatmapInfo, withStoryboard); + WorkingBeatmap working = new DatabaseWorkingBeatmap(this, beatmapInfo); previous?.TransferTo(working); diff --git a/osu.Game/Database/DatabaseWorkingBeatmap.cs b/osu.Game/Database/DatabaseWorkingBeatmap.cs index 01a8509c3e..25944faa42 100644 --- a/osu.Game/Database/DatabaseWorkingBeatmap.cs +++ b/osu.Game/Database/DatabaseWorkingBeatmap.cs @@ -14,8 +14,8 @@ namespace osu.Game.Database { private readonly BeatmapDatabase database; - public DatabaseWorkingBeatmap(BeatmapDatabase database, BeatmapInfo beatmapInfo, bool fullyLoaded = false) - : base(beatmapInfo, fullyLoaded) + public DatabaseWorkingBeatmap(BeatmapDatabase database, BeatmapInfo beatmapInfo) + : base(beatmapInfo) { this.database = database; } @@ -37,7 +37,7 @@ namespace osu.Game.Database beatmap = decoder.Decode(stream); } - if (beatmap == null || !FullyLoaded || BeatmapSetInfo.StoryboardFile == null) + if (beatmap == null || BeatmapSetInfo.StoryboardFile == null) return beatmap; using (var stream = new StreamReader(reader.GetStream(BeatmapSetInfo.StoryboardFile))) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 40f24a77c9..1711ee027a 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -70,7 +70,7 @@ namespace osu.Game.Screens.Play private bool loadedSuccessfully => HitRenderer?.Objects.Any() == true; [BackgroundDependencyLoader(permitNulls: true)] - private void load(AudioManager audio, BeatmapDatabase beatmaps, OsuConfigManager config, OsuGame osu) + private void load(AudioManager audio, OsuConfigManager config, OsuGame osu) { dimLevel = config.GetBindable(OsuSetting.DimLevel); mouseWheelDisabled = config.GetBindable(OsuSetting.MouseDisableWheel); @@ -81,10 +81,6 @@ namespace osu.Game.Screens.Play try { - if (!Beatmap.Value.FullyLoaded) - // we need to ensure extras like storyboards are loaded. - Beatmap.Value = beatmaps.GetWorkingBeatmap(Beatmap.Value.BeatmapInfo, withStoryboard: true); - if (Beatmap.Value.Beatmap == null) throw new InvalidOperationException("Beatmap was not loaded"); From 15eb6954da6d41f0f451a9528af8e4be887f966e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 20 Jul 2017 11:50:31 +0900 Subject: [PATCH 2/7] Fix hitting down and enter at song select causing a hard-crash Carousel was not aware of the disabled beatmap change state. Also it was being set too late (in an async load) so wasn't useful. It's now pre-emptively set in PlaySongSelect before loading Player. --- osu.Game/Screens/Select/BeatmapCarousel.cs | 4 ++++ osu.Game/Screens/Select/PlaySongSelect.cs | 1 + osu.Game/Screens/Select/SongSelect.cs | 5 ++++- 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index e135fefc6d..90b9808da5 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -25,6 +25,8 @@ namespace osu.Game.Screens.Select { public BeatmapInfo SelectedBeatmap => selectedPanel?.Beatmap; + public override bool HandleInput => AllowSelection; + public Action BeatmapsChanged; public IEnumerable Beatmaps @@ -228,6 +230,8 @@ namespace osu.Game.Screens.Select private ScheduledDelegate filterTask; + public bool AllowSelection = true; + public void Filter(FilterCriteria newCriteria = null, bool debounce = true) { if (newCriteria != null) diff --git a/osu.Game/Screens/Select/PlaySongSelect.cs b/osu.Game/Screens/Select/PlaySongSelect.cs index 51f570e901..e393caf931 100644 --- a/osu.Game/Screens/Select/PlaySongSelect.cs +++ b/osu.Game/Screens/Select/PlaySongSelect.cs @@ -105,6 +105,7 @@ namespace osu.Game.Screens.Select if (player != null) return; Beatmap.Value.Track.Looping = false; + Beatmap.Disabled = true; LoadComponentAsync(player = new PlayerLoader(new Player()), l => Push(player)); } diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index ea012f45e4..0d0b15d91e 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -110,7 +110,7 @@ namespace osu.Game.Screens.Select Origin = Anchor.CentreRight, SelectionChanged = carouselSelectionChanged, BeatmapsChanged = carouselBeatmapsLoaded, - StartRequested = carouselRaisedStart + StartRequested = carouselRaisedStart, }); Add(FilterControl = new FilterControl { @@ -185,6 +185,9 @@ namespace osu.Game.Screens.Select carousel.Beatmaps = database.GetAllWithChildren(b => !b.DeletePending); Beatmap.ValueChanged += beatmap_ValueChanged; + + Beatmap.DisabledChanged += disabled => carousel.AllowSelection = !disabled; + carousel.AllowSelection = !Beatmap.Disabled; } private void carouselBeatmapsLoaded() From c13098118478568718f64d72df1b32a76d213392 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 20 Jul 2017 14:05:42 +0900 Subject: [PATCH 3/7] Fix WorkingBeatmap being loaded twice when using MusicController at SongSelect --- osu.Game/Screens/Select/SongSelect.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index ea012f45e4..4f6f1a5c76 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -221,11 +221,16 @@ namespace osu.Game.Screens.Select { Action performLoad = delegate { - bool preview = beatmap?.BeatmapSetInfoID != Beatmap.Value.BeatmapInfo.BeatmapSetInfoID; + // We may be arriving here due to another component changing the bindable Beatmap. + // In these cases, the other component has already loaded the beatmap, so we don't need to do so again. + if (!beatmap.Equals(Beatmap.Value.BeatmapInfo)) + { + bool preview = beatmap?.BeatmapSetInfoID != Beatmap.Value.BeatmapInfo.BeatmapSetInfoID; - Beatmap.Value = database.GetWorkingBeatmap(beatmap, Beatmap); + Beatmap.Value = database.GetWorkingBeatmap(beatmap, Beatmap); + ensurePlayingSelected(preview); + } - ensurePlayingSelected(preview); changeBackground(Beatmap.Value); }; From a59557f03926159ba379e8a77df19cdf892a9894 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 20 Jul 2017 15:12:20 +0900 Subject: [PATCH 4/7] Fix selection not being reset correct when changing between rulesets Carousels filtered to results with no maps visible were not being handled correctly in a few different ways. This covers all those scenarios. --- osu.Game/Screens/Select/BeatmapCarousel.cs | 2 +- osu.Game/Screens/Select/SongSelect.cs | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index e135fefc6d..d2984ec2ae 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -133,7 +133,7 @@ namespace osu.Game.Screens.Select public void SelectNext(int direction = 1, bool skipDifficulties = true) { - if (groups.Count == 0) + if (groups.Count == 0 || groups.All(g => g.State == BeatmapGroupState.Hidden)) { selectedGroup = null; selectedPanel = null; diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index ea012f45e4..15bdc08eea 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -229,6 +229,13 @@ namespace osu.Game.Screens.Select changeBackground(Beatmap.Value); }; + selectionChangedDebounce?.Cancel(); + + if (beatmap?.Equals(beatmapNoDebounce) == true) + return; + + beatmapNoDebounce = beatmap; + if (beatmap == null) { if (!Beatmap.IsDefault) @@ -236,18 +243,11 @@ namespace osu.Game.Screens.Select } else { - selectionChangedDebounce?.Cancel(); - - if (beatmap.Equals(beatmapNoDebounce)) - return; - if (beatmap.BeatmapSetInfoID == beatmapNoDebounce?.BeatmapSetInfoID) sampleChangeDifficulty.Play(); else sampleChangeBeatmap.Play(); - beatmapNoDebounce = beatmap; - if (beatmap == Beatmap.Value.BeatmapInfo) performLoad(); else From 61c665f239b3ca57364954ae7e2213205838371a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 20 Jul 2017 15:16:07 +0900 Subject: [PATCH 5/7] Add required null check --- osu.Game/Screens/Select/SongSelect.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index 4f6f1a5c76..e3e588c000 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -223,7 +223,7 @@ namespace osu.Game.Screens.Select { // We may be arriving here due to another component changing the bindable Beatmap. // In these cases, the other component has already loaded the beatmap, so we don't need to do so again. - if (!beatmap.Equals(Beatmap.Value.BeatmapInfo)) + if (beatmap?.Equals(Beatmap.Value.BeatmapInfo) != true) { bool preview = beatmap?.BeatmapSetInfoID != Beatmap.Value.BeatmapInfo.BeatmapSetInfoID; From 4f102561827c17d98d7fc4a27a40e401f2a217cc Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 20 Jul 2017 15:34:44 +0900 Subject: [PATCH 6/7] Remove unnecessary count check --- osu.Game/Screens/Select/BeatmapCarousel.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index d2984ec2ae..b5855fe0e0 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -133,7 +133,7 @@ namespace osu.Game.Screens.Select public void SelectNext(int direction = 1, bool skipDifficulties = true) { - if (groups.Count == 0 || groups.All(g => g.State == BeatmapGroupState.Hidden)) + if (groups.All(g => g.State == BeatmapGroupState.Hidden)) { selectedGroup = null; selectedPanel = null; From 6e0b7b81f8de1ae95b6d78ddde7afc665c232bb6 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 20 Jul 2017 19:10:20 +0900 Subject: [PATCH 7/7] Switch to correct ruleset when changing beatmap This is only really noticeable when using the MusicController to change tracks while at song select. --- osu.Game/Screens/Select/SongSelect.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index 15bdc08eea..4ecbd91b11 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -243,6 +243,8 @@ namespace osu.Game.Screens.Select } else { + ruleset.Value = beatmap.Ruleset; + if (beatmap.BeatmapSetInfoID == beatmapNoDebounce?.BeatmapSetInfoID) sampleChangeDifficulty.Play(); else