From e59a00ac6eb1f30cb368392aedb299177fd8a168 Mon Sep 17 00:00:00 2001 From: David Zhao Date: Tue, 28 May 2019 14:04:33 +0900 Subject: [PATCH 01/60] Remove excessive selection updating --- .../SongSelect/TestScenePlaySongSelect.cs | 28 ++++++++++++++++++- osu.Game/Screens/Select/SongSelect.cs | 8 +++++- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index 7e962dbc06..85811e3a0a 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -209,7 +209,33 @@ namespace osu.Game.Tests.Visual.SongSelect AddAssert("start not requested", () => !startRequested); } - private void importForRuleset(int id) => AddStep($"import test map for ruleset {id}", () => manager.Import(createTestBeatmapSet(getImportId(), rulesets.AvailableRulesets.Where(r => r.ID == id).ToArray()))); + [Test] + public void TestAddNewBeatmap() + { + const int test_count = 10; + int beatmapChangedCount = 0; + createSongSelect(); + AddStep("Setup counter", () => + { + beatmapChangedCount = 0; + songSelect.Carousel.BeatmapSetsChanged += () => beatmapChangedCount++; + }); + AddRepeatStep($"Create beatmaps {test_count} times", () => + { + manager.Import(createTestBeatmapSet(getImportId(), rulesets.AvailableRulesets.Where(r => r.ID == 0).ToArray())); + + Scheduler.AddDelayed(() => + { + // Wait for debounce + songSelect.Carousel.SelectNextRandom(); + }, 400); + }, test_count); + + AddAssert($"Beatmap changed {test_count} times", () => beatmapChangedCount == test_count); + } + + private void importForRuleset(int id) => AddStep($"import test map for ruleset {id}", + () => manager.Import(createTestBeatmapSet(getImportId(), rulesets.AvailableRulesets.Where(r => r.ID == id).ToArray()))); private static int importId; private int getImportId() => ++importId; diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index fed1f7a944..f30618ce3f 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -597,11 +597,17 @@ namespace osu.Game.Screens.Select { bindBindables(); + // As a selection was already obtained, do not attempt to update the selected beatmap. + if (Carousel.SelectedBeatmapSet != null) + return; + + // Attempt to select the current beatmap on the carousel, if it is valid to be selected. if (!Beatmap.IsDefault && Beatmap.Value.BeatmapSetInfo?.DeletePending == false && Beatmap.Value.BeatmapSetInfo?.Protected == false && Carousel.SelectBeatmap(Beatmap.Value.BeatmapInfo, false)) return; - if (Carousel.SelectedBeatmapSet == null && !Carousel.SelectNextRandom()) + // If the current active beatmap could not be selected, select a new random beatmap. + if (!Carousel.SelectNextRandom()) { // in the case random selection failed, we want to trigger selectionChanged // to show the dummy beatmap (we have nothing else to display). From 436760de967e59400822e25356b502294bd487e3 Mon Sep 17 00:00:00 2001 From: David Zhao Date: Tue, 28 May 2019 14:34:52 +0900 Subject: [PATCH 02/60] Change test name --- osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index 85811e3a0a..8e3fe57d92 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -210,7 +210,7 @@ namespace osu.Game.Tests.Visual.SongSelect } [Test] - public void TestAddNewBeatmap() + public void TestAddNewBeatmapWhileSelectingRandom() { const int test_count = 10; int beatmapChangedCount = 0; From 1a871af5520113372c38d70740337dc794fe2b30 Mon Sep 17 00:00:00 2001 From: David Zhao Date: Tue, 28 May 2019 19:15:29 +0900 Subject: [PATCH 03/60] Fix hide selection, add test --- .../Visual/SongSelect/TestScenePlaySongSelect.cs | 12 ++++++++++++ osu.Game/Screens/Select/BeatmapCarousel.cs | 5 ++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index 8e3fe57d92..c33528c3bf 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -234,6 +234,18 @@ namespace osu.Game.Tests.Visual.SongSelect AddAssert($"Beatmap changed {test_count} times", () => beatmapChangedCount == test_count); } + [Test] + public void TestHideSetSelectsCorrectBeatmap() + { + int? previousID = null; + createSongSelect(); + importForRuleset(0); + AddStep("Move to last difficulty", () => songSelect.Carousel.SelectBeatmap(songSelect.Carousel.BeatmapSets.First().Beatmaps.Last())); + AddStep("Store current ID", () => previousID = songSelect.Carousel.SelectedBeatmap.ID); + AddStep("Hide first beatmap", () => manager.Hide(songSelect.Carousel.SelectedBeatmapSet.Beatmaps.First())); + AddAssert("Selected beatmap has not changed", () => songSelect.Carousel.SelectedBeatmap.ID == previousID); + } + private void importForRuleset(int id) => AddStep($"import test map for ruleset {id}", () => manager.Import(createTestBeatmapSet(getImportId(), rulesets.AvailableRulesets.Where(r => r.ID == id).ToArray()))); diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 63ad3b6ab2..0b3d0c448b 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -158,6 +158,9 @@ namespace osu.Game.Screens.Select var newSet = createCarouselSet(beatmapSet); + // Since we're about to remove the selected beatmap, store its ID so we can go back if needed. + var previouslySelectedID = selectedBeatmap?.Beatmap.ID; + if (existingSet != null) root.RemoveChild(existingSet); @@ -173,7 +176,7 @@ namespace osu.Game.Screens.Select //check if we can/need to maintain our current selection. if (hadSelection) - select((CarouselItem)newSet.Beatmaps.FirstOrDefault(b => b.Beatmap.ID == selectedBeatmap?.Beatmap.ID) ?? newSet); + select((CarouselItem)newSet.Beatmaps.FirstOrDefault(b => b.Beatmap.ID == previouslySelectedID) ?? newSet); itemsCache.Invalidate(); Schedule(() => BeatmapSetsChanged?.Invoke()); From 4f091417189baf51bf504cf401a914fe8d568234 Mon Sep 17 00:00:00 2001 From: David Zhao Date: Wed, 29 May 2019 12:22:34 +0900 Subject: [PATCH 04/60] remove extra bool --- osu.Game/Screens/Select/BeatmapCarousel.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 0b3d0c448b..6e3bec106f 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -152,15 +152,15 @@ namespace osu.Game.Screens.Select { Schedule(() => { + int? previouslySelectedID = null; CarouselBeatmapSet existingSet = beatmapSets.FirstOrDefault(b => b.BeatmapSet.ID == beatmapSet.ID); - bool hadSelection = existingSet?.State?.Value == CarouselItemState.Selected; + // Since we're about to remove the selected beatmap, store its ID so we can go back if needed. + if (existingSet?.State?.Value == CarouselItemState.Selected) + previouslySelectedID = selectedBeatmap?.Beatmap.ID; var newSet = createCarouselSet(beatmapSet); - // Since we're about to remove the selected beatmap, store its ID so we can go back if needed. - var previouslySelectedID = selectedBeatmap?.Beatmap.ID; - if (existingSet != null) root.RemoveChild(existingSet); @@ -175,7 +175,7 @@ namespace osu.Game.Screens.Select applyActiveCriteria(false, false); //check if we can/need to maintain our current selection. - if (hadSelection) + if (previouslySelectedID != null) select((CarouselItem)newSet.Beatmaps.FirstOrDefault(b => b.Beatmap.ID == previouslySelectedID) ?? newSet); itemsCache.Invalidate(); From 7e9f5a0939d41a262a9d19da778b54e140032346 Mon Sep 17 00:00:00 2001 From: HoLLy Date: Wed, 29 May 2019 11:22:51 +0200 Subject: [PATCH 05/60] Add Skills to DifficultyAttributes --- .../Difficulty/CatchDifficultyCalculator.cs | 5 +++-- .../Difficulty/ManiaDifficultyCalculator.cs | 3 ++- osu.Game.Rulesets.Osu/Difficulty/OsuDifficultyCalculator.cs | 5 +++-- .../Difficulty/TaikoDifficultyCalculator.cs | 3 ++- osu.Game/Rulesets/Difficulty/DifficultyAttributes.cs | 5 ++++- 5 files changed, 14 insertions(+), 7 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Difficulty/CatchDifficultyCalculator.cs b/osu.Game.Rulesets.Catch/Difficulty/CatchDifficultyCalculator.cs index d6a1ed632b..44e1a8e5cc 100644 --- a/osu.Game.Rulesets.Catch/Difficulty/CatchDifficultyCalculator.cs +++ b/osu.Game.Rulesets.Catch/Difficulty/CatchDifficultyCalculator.cs @@ -31,7 +31,7 @@ namespace osu.Game.Rulesets.Catch.Difficulty protected override DifficultyAttributes CreateDifficultyAttributes(IBeatmap beatmap, Mod[] mods, Skill[] skills, double clockRate) { if (beatmap.HitObjects.Count == 0) - return new CatchDifficultyAttributes { Mods = mods }; + return new CatchDifficultyAttributes { Mods = mods, Skills = skills }; // this is the same as osu!, so there's potential to share the implementation... maybe double preempt = BeatmapDifficulty.DifficultyRange(beatmap.BeatmapInfo.BaseDifficulty.ApproachRate, 1800, 1200, 450) / clockRate; @@ -41,7 +41,8 @@ namespace osu.Game.Rulesets.Catch.Difficulty StarRating = Math.Sqrt(skills[0].DifficultyValue()) * star_scaling_factor, Mods = mods, ApproachRate = preempt > 1200.0 ? -(preempt - 1800.0) / 120.0 : -(preempt - 1200.0) / 150.0 + 5.0, - MaxCombo = beatmap.HitObjects.Count(h => h is Fruit) + beatmap.HitObjects.OfType().SelectMany(j => j.NestedHitObjects).Count(h => !(h is TinyDroplet)) + MaxCombo = beatmap.HitObjects.Count(h => h is Fruit) + beatmap.HitObjects.OfType().SelectMany(j => j.NestedHitObjects).Count(h => !(h is TinyDroplet)), + Skills = skills }; } diff --git a/osu.Game.Rulesets.Mania/Difficulty/ManiaDifficultyCalculator.cs b/osu.Game.Rulesets.Mania/Difficulty/ManiaDifficultyCalculator.cs index 59fed1031f..4a9c22d339 100644 --- a/osu.Game.Rulesets.Mania/Difficulty/ManiaDifficultyCalculator.cs +++ b/osu.Game.Rulesets.Mania/Difficulty/ManiaDifficultyCalculator.cs @@ -30,7 +30,7 @@ namespace osu.Game.Rulesets.Mania.Difficulty protected override DifficultyAttributes CreateDifficultyAttributes(IBeatmap beatmap, Mod[] mods, Skill[] skills, double clockRate) { if (beatmap.HitObjects.Count == 0) - return new ManiaDifficultyAttributes { Mods = mods }; + return new ManiaDifficultyAttributes { Mods = mods, Skills = skills }; return new ManiaDifficultyAttributes { @@ -38,6 +38,7 @@ namespace osu.Game.Rulesets.Mania.Difficulty Mods = mods, // Todo: This int cast is temporary to achieve 1:1 results with osu!stable, and should be removed in the future GreatHitWindow = (int)(beatmap.HitObjects.First().HitWindows.Great / 2) / clockRate, + Skills = skills }; } diff --git a/osu.Game.Rulesets.Osu/Difficulty/OsuDifficultyCalculator.cs b/osu.Game.Rulesets.Osu/Difficulty/OsuDifficultyCalculator.cs index e2a1542574..c197933233 100644 --- a/osu.Game.Rulesets.Osu/Difficulty/OsuDifficultyCalculator.cs +++ b/osu.Game.Rulesets.Osu/Difficulty/OsuDifficultyCalculator.cs @@ -28,7 +28,7 @@ namespace osu.Game.Rulesets.Osu.Difficulty protected override DifficultyAttributes CreateDifficultyAttributes(IBeatmap beatmap, Mod[] mods, Skill[] skills, double clockRate) { if (beatmap.HitObjects.Count == 0) - return new OsuDifficultyAttributes { Mods = mods }; + return new OsuDifficultyAttributes { Mods = mods, Skills = skills }; double aimRating = Math.Sqrt(skills[0].DifficultyValue()) * difficulty_multiplier; double speedRating = Math.Sqrt(skills[1].DifficultyValue()) * difficulty_multiplier; @@ -50,7 +50,8 @@ namespace osu.Game.Rulesets.Osu.Difficulty SpeedStrain = speedRating, ApproachRate = preempt > 1200 ? (1800 - preempt) / 120 : (1200 - preempt) / 150 + 5, OverallDifficulty = (80 - hitWindowGreat) / 6, - MaxCombo = maxCombo + MaxCombo = maxCombo, + Skills = skills }; } diff --git a/osu.Game.Rulesets.Taiko/Difficulty/TaikoDifficultyCalculator.cs b/osu.Game.Rulesets.Taiko/Difficulty/TaikoDifficultyCalculator.cs index 685ad9949b..c8f3e18911 100644 --- a/osu.Game.Rulesets.Taiko/Difficulty/TaikoDifficultyCalculator.cs +++ b/osu.Game.Rulesets.Taiko/Difficulty/TaikoDifficultyCalculator.cs @@ -27,7 +27,7 @@ namespace osu.Game.Rulesets.Taiko.Difficulty protected override DifficultyAttributes CreateDifficultyAttributes(IBeatmap beatmap, Mod[] mods, Skill[] skills, double clockRate) { if (beatmap.HitObjects.Count == 0) - return new TaikoDifficultyAttributes { Mods = mods }; + return new TaikoDifficultyAttributes { Mods = mods, Skills = skills }; return new TaikoDifficultyAttributes { @@ -36,6 +36,7 @@ namespace osu.Game.Rulesets.Taiko.Difficulty // Todo: This int cast is temporary to achieve 1:1 results with osu!stable, and should be removed in the future GreatHitWindow = (int)(beatmap.HitObjects.First().HitWindows.Great / 2) / clockRate, MaxCombo = beatmap.HitObjects.Count(h => h is Hit), + Skills = skills }; } diff --git a/osu.Game/Rulesets/Difficulty/DifficultyAttributes.cs b/osu.Game/Rulesets/Difficulty/DifficultyAttributes.cs index d808ee528e..b4b4bb9cd1 100644 --- a/osu.Game/Rulesets/Difficulty/DifficultyAttributes.cs +++ b/osu.Game/Rulesets/Difficulty/DifficultyAttributes.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 osu.Game.Rulesets.Difficulty.Skills; using osu.Game.Rulesets.Mods; namespace osu.Game.Rulesets.Difficulty @@ -8,6 +9,7 @@ namespace osu.Game.Rulesets.Difficulty public class DifficultyAttributes { public Mod[] Mods; + public Skill[] Skills; public double StarRating; @@ -15,9 +17,10 @@ namespace osu.Game.Rulesets.Difficulty { } - public DifficultyAttributes(Mod[] mods, double starRating) + public DifficultyAttributes(Mod[] mods, Skill[] skills, double starRating) { Mods = mods; + Skills = skills; StarRating = starRating; } } From 2a295545a79b89c18447e0aee599eb5a6c27a04e Mon Sep 17 00:00:00 2001 From: HoLLy Date: Wed, 29 May 2019 11:25:25 +0200 Subject: [PATCH 06/60] Don't mutate strainPeaks --- osu.Game/Rulesets/Difficulty/Skills/Skill.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Game/Rulesets/Difficulty/Skills/Skill.cs b/osu.Game/Rulesets/Difficulty/Skills/Skill.cs index e8020ed185..227f2f4018 100644 --- a/osu.Game/Rulesets/Difficulty/Skills/Skill.cs +++ b/osu.Game/Rulesets/Difficulty/Skills/Skill.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Linq; using osu.Game.Rulesets.Difficulty.Preprocessing; using osu.Game.Rulesets.Difficulty.Utils; @@ -17,7 +18,7 @@ namespace osu.Game.Rulesets.Difficulty.Skills /// /// The peak strain for each section of the beatmap. /// - public IList StrainPeaks => strainPeaks; + public IReadOnlyList StrainPeaks => strainPeaks; /// /// Strain values are multiplied by this number for the given skill. Used to balance the value of different skills between each other. @@ -84,13 +85,12 @@ namespace osu.Game.Rulesets.Difficulty.Skills /// public double DifficultyValue() { - strainPeaks.Sort((a, b) => b.CompareTo(a)); // Sort from highest to lowest strain. - double difficulty = 0; double weight = 1; // Difficulty is the weighted sum of the highest strains from every section. - foreach (double strain in strainPeaks) + // We're sorting from highest to lowest strain. + foreach (double strain in strainPeaks.OrderByDescending(d => d)) { difficulty += strain * weight; weight *= DecayWeight; From f326264a85abacef478d45cf71fc28867b3554e2 Mon Sep 17 00:00:00 2001 From: Ganendra Afrasya Date: Sat, 8 Jun 2019 00:42:57 +0700 Subject: [PATCH 07/60] Adding increase first object grow mod visibility setting --- osu.Game/Configuration/OsuConfigManager.cs | 3 +++ osu.Game/Overlays/Settings/Sections/Gameplay/ModsSettings.cs | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/osu.Game/Configuration/OsuConfigManager.cs b/osu.Game/Configuration/OsuConfigManager.cs index 795f0b43f7..b5a099aa3b 100644 --- a/osu.Game/Configuration/OsuConfigManager.cs +++ b/osu.Game/Configuration/OsuConfigManager.cs @@ -85,6 +85,8 @@ namespace osu.Game.Configuration Set(OsuSetting.IncreaseFirstObjectVisibility, true); + Set(OsuSetting.IncreaseFirstObjectGrowVisibility, true); + // Update Set(OsuSetting.ReleaseStream, ReleaseStream.Lazer); @@ -158,6 +160,7 @@ namespace osu.Game.Configuration BeatmapSkins, BeatmapHitsounds, IncreaseFirstObjectVisibility, + IncreaseFirstObjectGrowVisibility, ScoreDisplayMode, ExternalLinkWarning, Scaling, diff --git a/osu.Game/Overlays/Settings/Sections/Gameplay/ModsSettings.cs b/osu.Game/Overlays/Settings/Sections/Gameplay/ModsSettings.cs index 2cf14f5aff..538b2b2761 100644 --- a/osu.Game/Overlays/Settings/Sections/Gameplay/ModsSettings.cs +++ b/osu.Game/Overlays/Settings/Sections/Gameplay/ModsSettings.cs @@ -20,6 +20,11 @@ namespace osu.Game.Overlays.Settings.Sections.Gameplay LabelText = "Increase visibility of first object with \"Hidden\" mod", Bindable = config.GetBindable(OsuSetting.IncreaseFirstObjectVisibility) }, + new SettingsCheckbox + { + LabelText = "Increase visibility of first object with \"Grow\" mod", + Bindable = config.GetBindable(OsuSetting.IncreaseFirstObjectGrowVisibility) + }, }; } } From 8ac64b5c16c0d8fe05f680a2226bc1dc37e15a6f Mon Sep 17 00:00:00 2001 From: Ganendra Afrasya Date: Sat, 8 Jun 2019 01:46:05 +0700 Subject: [PATCH 08/60] Make first object not applying custom state --- osu.Game.Rulesets.Osu/Mods/OsuModGrow.cs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModGrow.cs b/osu.Game.Rulesets.Osu/Mods/OsuModGrow.cs index a2da2bbf53..a5df36e9ff 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModGrow.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModGrow.cs @@ -2,8 +2,11 @@ // See the LICENCE file in the repository root for full licence text. using System.Collections.Generic; +using System.Linq; +using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Sprites; +using osu.Game.Configuration; using osu.Game.Rulesets.Mods; using osu.Game.Rulesets.Objects.Drawables; using osu.Game.Rulesets.Osu.Objects; @@ -11,7 +14,7 @@ using osu.Game.Rulesets.Osu.Objects.Drawables; namespace osu.Game.Rulesets.Osu.Mods { - internal class OsuModGrow : Mod, IApplicableToDrawableHitObjects + internal class OsuModGrow : Mod, IReadFromConfig, IApplicableToDrawableHitObjects { public override string Name => "Grow"; @@ -25,9 +28,16 @@ namespace osu.Game.Rulesets.Osu.Mods public override double ScoreMultiplier => 1; + protected Bindable IncreaseFirstObjectGrowVisibility = new Bindable(); + + public void ReadFromConfig(OsuConfigManager config) + { + IncreaseFirstObjectGrowVisibility = config.GetBindable(OsuSetting.IncreaseFirstObjectGrowVisibility); + } + public void ApplyToDrawableHitObjects(IEnumerable drawables) { - foreach (var drawable in drawables) + foreach (var drawable in drawables.Skip(IncreaseFirstObjectGrowVisibility.Value ? 1 : 0)) { switch (drawable) { From afc3a089536d72fe7f0b357866caa31dafb3c8fc Mon Sep 17 00:00:00 2001 From: Ganendra Afrasya Date: Sun, 9 Jun 2019 13:11:40 +0700 Subject: [PATCH 09/60] Use existing setting instead Now it read IncreaseFirstObjectVisibility bindable instead --- osu.Game.Rulesets.Osu/Mods/OsuModGrow.cs | 6 +++--- osu.Game/Configuration/OsuConfigManager.cs | 3 --- .../Overlays/Settings/Sections/Gameplay/ModsSettings.cs | 5 ----- 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModGrow.cs b/osu.Game.Rulesets.Osu/Mods/OsuModGrow.cs index a5df36e9ff..3d64bb4ce8 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModGrow.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModGrow.cs @@ -28,16 +28,16 @@ namespace osu.Game.Rulesets.Osu.Mods public override double ScoreMultiplier => 1; - protected Bindable IncreaseFirstObjectGrowVisibility = new Bindable(); + protected Bindable IncreaseFirstObjectVisibility = new Bindable(); public void ReadFromConfig(OsuConfigManager config) { - IncreaseFirstObjectGrowVisibility = config.GetBindable(OsuSetting.IncreaseFirstObjectGrowVisibility); + IncreaseFirstObjectVisibility = config.GetBindable(OsuSetting.IncreaseFirstObjectVisibility); } public void ApplyToDrawableHitObjects(IEnumerable drawables) { - foreach (var drawable in drawables.Skip(IncreaseFirstObjectGrowVisibility.Value ? 1 : 0)) + foreach (var drawable in drawables.Skip(IncreaseFirstObjectVisibility.Value ? 1 : 0)) { switch (drawable) { diff --git a/osu.Game/Configuration/OsuConfigManager.cs b/osu.Game/Configuration/OsuConfigManager.cs index b5a099aa3b..795f0b43f7 100644 --- a/osu.Game/Configuration/OsuConfigManager.cs +++ b/osu.Game/Configuration/OsuConfigManager.cs @@ -85,8 +85,6 @@ namespace osu.Game.Configuration Set(OsuSetting.IncreaseFirstObjectVisibility, true); - Set(OsuSetting.IncreaseFirstObjectGrowVisibility, true); - // Update Set(OsuSetting.ReleaseStream, ReleaseStream.Lazer); @@ -160,7 +158,6 @@ namespace osu.Game.Configuration BeatmapSkins, BeatmapHitsounds, IncreaseFirstObjectVisibility, - IncreaseFirstObjectGrowVisibility, ScoreDisplayMode, ExternalLinkWarning, Scaling, diff --git a/osu.Game/Overlays/Settings/Sections/Gameplay/ModsSettings.cs b/osu.Game/Overlays/Settings/Sections/Gameplay/ModsSettings.cs index 538b2b2761..2cf14f5aff 100644 --- a/osu.Game/Overlays/Settings/Sections/Gameplay/ModsSettings.cs +++ b/osu.Game/Overlays/Settings/Sections/Gameplay/ModsSettings.cs @@ -20,11 +20,6 @@ namespace osu.Game.Overlays.Settings.Sections.Gameplay LabelText = "Increase visibility of first object with \"Hidden\" mod", Bindable = config.GetBindable(OsuSetting.IncreaseFirstObjectVisibility) }, - new SettingsCheckbox - { - LabelText = "Increase visibility of first object with \"Grow\" mod", - Bindable = config.GetBindable(OsuSetting.IncreaseFirstObjectGrowVisibility) - }, }; } } From 49193a2703edb7c5d35b04d0c206031e2a2708f9 Mon Sep 17 00:00:00 2001 From: Ganendra Afrasya Date: Sun, 9 Jun 2019 13:12:41 +0700 Subject: [PATCH 10/60] Rename the setting label --- osu.Game/Overlays/Settings/Sections/Gameplay/ModsSettings.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Overlays/Settings/Sections/Gameplay/ModsSettings.cs b/osu.Game/Overlays/Settings/Sections/Gameplay/ModsSettings.cs index 2cf14f5aff..2c6b2663c6 100644 --- a/osu.Game/Overlays/Settings/Sections/Gameplay/ModsSettings.cs +++ b/osu.Game/Overlays/Settings/Sections/Gameplay/ModsSettings.cs @@ -17,7 +17,7 @@ namespace osu.Game.Overlays.Settings.Sections.Gameplay { new SettingsCheckbox { - LabelText = "Increase visibility of first object with \"Hidden\" mod", + LabelText = "Increase visibility of first object when visual impairment mods are enabled", Bindable = config.GetBindable(OsuSetting.IncreaseFirstObjectVisibility) }, }; From 8cdcf251b5aca691d3af18b459cbb12f5c1704b7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 9 Jun 2019 16:30:04 +0900 Subject: [PATCH 11/60] Make local bindable private --- osu.Game.Rulesets.Osu/Mods/OsuModGrow.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModGrow.cs b/osu.Game.Rulesets.Osu/Mods/OsuModGrow.cs index 3d64bb4ce8..8072dc09c1 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModGrow.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModGrow.cs @@ -28,16 +28,16 @@ namespace osu.Game.Rulesets.Osu.Mods public override double ScoreMultiplier => 1; - protected Bindable IncreaseFirstObjectVisibility = new Bindable(); + private Bindable increaseFirstObjectVisibility = new Bindable(); public void ReadFromConfig(OsuConfigManager config) { - IncreaseFirstObjectVisibility = config.GetBindable(OsuSetting.IncreaseFirstObjectVisibility); + increaseFirstObjectVisibility = config.GetBindable(OsuSetting.IncreaseFirstObjectVisibility); } public void ApplyToDrawableHitObjects(IEnumerable drawables) { - foreach (var drawable in drawables.Skip(IncreaseFirstObjectVisibility.Value ? 1 : 0)) + foreach (var drawable in drawables.Skip(increaseFirstObjectVisibility.Value ? 1 : 0)) { switch (drawable) { From d500f3605ef8c71f6a40256b86fe6a5a618d0558 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 9 Jun 2019 16:47:48 +0900 Subject: [PATCH 12/60] Fix checkboxes with long labels overlapping nub --- .../Graphics/UserInterface/OsuCheckbox.cs | 22 ++++++++++--------- .../Overlays/Settings/SettingsCheckbox.cs | 1 - 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/osu.Game/Graphics/UserInterface/OsuCheckbox.cs b/osu.Game/Graphics/UserInterface/OsuCheckbox.cs index cd1147e3d3..dd126d8518 100644 --- a/osu.Game/Graphics/UserInterface/OsuCheckbox.cs +++ b/osu.Game/Graphics/UserInterface/OsuCheckbox.cs @@ -6,10 +6,9 @@ using osu.Framework.Audio; using osu.Framework.Audio.Sample; using osu.Framework.Bindables; using osu.Framework.Graphics; -using osu.Framework.Graphics.Sprites; using osu.Framework.Graphics.UserInterface; using osu.Framework.Input.Events; -using osu.Game.Graphics.Sprites; +using osu.Game.Graphics.Containers; using osuTK.Graphics; namespace osu.Game.Graphics.UserInterface @@ -33,7 +32,6 @@ namespace osu.Game.Graphics.UserInterface public string LabelText { - get => labelSpriteText?.Text; set { if (labelSpriteText != null) @@ -53,7 +51,7 @@ namespace osu.Game.Graphics.UserInterface protected readonly Nub Nub; - private readonly SpriteText labelSpriteText; + private readonly OsuTextFlowContainer labelSpriteText; private SampleChannel sampleChecked; private SampleChannel sampleUnchecked; @@ -62,24 +60,28 @@ namespace osu.Game.Graphics.UserInterface AutoSizeAxes = Axes.Y; RelativeSizeAxes = Axes.X; + const float nub_padding = 5; + Children = new Drawable[] { - labelSpriteText = new OsuSpriteText(), + labelSpriteText = new OsuTextFlowContainer + { + AutoSizeAxes = Axes.Y, + RelativeSizeAxes = Axes.X, + Padding = new MarginPadding { Right = Nub.EXPANDED_SIZE + nub_padding } + }, Nub = new Nub { Anchor = Anchor.CentreRight, Origin = Anchor.CentreRight, - Margin = new MarginPadding { Right = 5 }, + Margin = new MarginPadding { Right = nub_padding }, }, new HoverClickSounds() }; Nub.Current.BindTo(Current); - Current.DisabledChanged += disabled => - { - labelSpriteText.Alpha = Nub.Alpha = disabled ? 0.3f : 1; - }; + Current.DisabledChanged += disabled => { labelSpriteText.Alpha = Nub.Alpha = disabled ? 0.3f : 1; }; } protected override void LoadComplete() diff --git a/osu.Game/Overlays/Settings/SettingsCheckbox.cs b/osu.Game/Overlays/Settings/SettingsCheckbox.cs index 46c23c3bbf..a1501d8015 100644 --- a/osu.Game/Overlays/Settings/SettingsCheckbox.cs +++ b/osu.Game/Overlays/Settings/SettingsCheckbox.cs @@ -14,7 +14,6 @@ namespace osu.Game.Overlays.Settings public override string LabelText { - get => checkbox.LabelText; set => checkbox.LabelText = value; } } From cd89633dee31f80d9181a00467b00159ae20cf76 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 9 Jun 2019 17:07:23 +0900 Subject: [PATCH 13/60] Rename variable to match --- osu.Game/Graphics/UserInterface/OsuCheckbox.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/osu.Game/Graphics/UserInterface/OsuCheckbox.cs b/osu.Game/Graphics/UserInterface/OsuCheckbox.cs index dd126d8518..5d41075725 100644 --- a/osu.Game/Graphics/UserInterface/OsuCheckbox.cs +++ b/osu.Game/Graphics/UserInterface/OsuCheckbox.cs @@ -34,24 +34,24 @@ namespace osu.Game.Graphics.UserInterface { set { - if (labelSpriteText != null) - labelSpriteText.Text = value; + if (labelText != null) + labelText.Text = value; } } public MarginPadding LabelPadding { - get => labelSpriteText?.Padding ?? new MarginPadding(); + get => labelText?.Padding ?? new MarginPadding(); set { - if (labelSpriteText != null) - labelSpriteText.Padding = value; + if (labelText != null) + labelText.Padding = value; } } protected readonly Nub Nub; - private readonly OsuTextFlowContainer labelSpriteText; + private readonly OsuTextFlowContainer labelText; private SampleChannel sampleChecked; private SampleChannel sampleUnchecked; @@ -64,7 +64,7 @@ namespace osu.Game.Graphics.UserInterface Children = new Drawable[] { - labelSpriteText = new OsuTextFlowContainer + labelText = new OsuTextFlowContainer { AutoSizeAxes = Axes.Y, RelativeSizeAxes = Axes.X, @@ -81,7 +81,7 @@ namespace osu.Game.Graphics.UserInterface Nub.Current.BindTo(Current); - Current.DisabledChanged += disabled => { labelSpriteText.Alpha = Nub.Alpha = disabled ? 0.3f : 1; }; + Current.DisabledChanged += disabled => { labelText.Alpha = Nub.Alpha = disabled ? 0.3f : 1; }; } protected override void LoadComplete() From d964f6ba9e0e166255fdedb1726472ef2e1bbd24 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 10 Jun 2019 00:56:35 +0900 Subject: [PATCH 14/60] Tween track frequency on pause --- osu.Game/Screens/Play/GameplayClockContainer.cs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/Play/GameplayClockContainer.cs b/osu.Game/Screens/Play/GameplayClockContainer.cs index c151e598f7..4b35e90f68 100644 --- a/osu.Game/Screens/Play/GameplayClockContainer.cs +++ b/osu.Game/Screens/Play/GameplayClockContainer.cs @@ -69,6 +69,7 @@ namespace osu.Game.Screens.Play RelativeSizeAxes = Axes.Both; sourceClock = (IAdjustableClock)beatmap.Track ?? new StopwatchClock(); + (sourceClock as IAdjustableAudioComponent)?.AddAdjustment(AdjustableProperty.Frequency, pauseFreqAdjust); adjustableClock = new DecoupleableInterpolatingFramedClock { IsCoupled = false }; @@ -85,8 +86,16 @@ namespace osu.Game.Screens.Play GameplayClock.IsPaused.BindTo(IsPaused); } + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + (sourceClock as IAdjustableAudioComponent)?.RemoveAdjustment(AdjustableProperty.Frequency, pauseFreqAdjust); + } + private double totalOffset => userOffsetClock.Offset + platformOffsetClock.Offset; + private readonly BindableDouble pauseFreqAdjust = new BindableDouble(1); + [BackgroundDependencyLoader] private void load(OsuConfigManager config) { @@ -122,6 +131,8 @@ namespace osu.Game.Screens.Play Seek(GameplayClock.CurrentTime); adjustableClock.Start(); IsPaused.Value = false; + + this.TransformBindableTo(pauseFreqAdjust, 1, 200, Easing.In); } /// @@ -143,7 +154,8 @@ namespace osu.Game.Screens.Play public void Stop() { - adjustableClock.Stop(); + this.TransformBindableTo(pauseFreqAdjust, 0, 200, Easing.Out).OnComplete(_ => { adjustableClock.Stop(); }); + IsPaused.Value = true; } From 59b624d4ba4a8cf89b068e5abcf4cec2a38a02b9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 10 Jun 2019 01:08:39 +0900 Subject: [PATCH 15/60] Fix test regression --- osu.Game.Tests/Visual/Gameplay/TestScenePause.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs b/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs index 12e91df77c..0f5ee66f50 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs @@ -189,7 +189,7 @@ namespace osu.Game.Tests.Visual.Gameplay AddAssert("pause overlay " + (isShown ? "shown" : "hidden"), () => Player.PauseOverlayVisible == isShown); private void confirmClockRunning(bool isRunning) => - AddAssert("clock " + (isRunning ? "running" : "stopped"), () => Player.GameplayClockContainer.GameplayClock.IsRunning == isRunning); + AddUntilStep("clock " + (isRunning ? "running" : "stopped"), () => Player.GameplayClockContainer.GameplayClock.IsRunning == isRunning); protected override bool AllowFail => true; From 5c2ea0b1a719348b5a1ae0bd251d3aecb45fa250 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 10 Jun 2019 01:14:46 +0900 Subject: [PATCH 16/60] Move dispose to end of file --- osu.Game/Screens/Play/GameplayClockContainer.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/osu.Game/Screens/Play/GameplayClockContainer.cs b/osu.Game/Screens/Play/GameplayClockContainer.cs index 4b35e90f68..6a03271b86 100644 --- a/osu.Game/Screens/Play/GameplayClockContainer.cs +++ b/osu.Game/Screens/Play/GameplayClockContainer.cs @@ -86,12 +86,6 @@ namespace osu.Game.Screens.Play GameplayClock.IsPaused.BindTo(IsPaused); } - protected override void Dispose(bool isDisposing) - { - base.Dispose(isDisposing); - (sourceClock as IAdjustableAudioComponent)?.RemoveAdjustment(AdjustableProperty.Frequency, pauseFreqAdjust); - } - private double totalOffset => userOffsetClock.Offset + platformOffsetClock.Offset; private readonly BindableDouble pauseFreqAdjust = new BindableDouble(1); @@ -154,7 +148,7 @@ namespace osu.Game.Screens.Play public void Stop() { - this.TransformBindableTo(pauseFreqAdjust, 0, 200, Easing.Out).OnComplete(_ => { adjustableClock.Stop(); }); + this.TransformBindableTo(pauseFreqAdjust, 0, 200, Easing.Out).OnComplete(_ => adjustableClock.Stop()); IsPaused.Value = true; } @@ -187,5 +181,11 @@ namespace osu.Game.Screens.Play foreach (var mod in mods.OfType()) mod.ApplyToClock(sourceClock); } + + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + (sourceClock as IAdjustableAudioComponent)?.RemoveAdjustment(AdjustableProperty.Frequency, pauseFreqAdjust); + } } } From f090e292c974c2147f7f34b3cc79a5461e7df817 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 28 May 2019 18:59:21 +0900 Subject: [PATCH 17/60] Move ArchiveModelManager import process to async flow --- .../Beatmaps/IO/ImportBeatmapTest.cs | 50 +++--- osu.Game.Tests/Scores/IO/ImportScoreTest.cs | 30 ++-- .../TestSceneBackgroundScreenBeatmap.cs | 2 +- ...tSceneUpdateableBeatmapBackgroundSprite.cs | 2 +- osu.Game.Tests/osu.Game.Tests.csproj | 5 + osu.Game/Beatmaps/BeatmapManager.cs | 163 +++++++++++++----- osu.Game/Database/ArchiveModelManager.cs | 158 +++++++++-------- osu.Game/Database/ICanAcceptFiles.cs | 4 +- osu.Game/IPC/ArchiveImportIPCChannel.cs | 2 +- osu.Game/OsuGameBase.cs | 5 +- .../Notifications/ProgressNotification.cs | 7 + osu.Game/Screens/Menu/Intro.cs | 2 +- osu.Game/Screens/Play/Player.cs | 2 +- osu.Game/Skinning/SkinManager.cs | 6 +- 14 files changed, 273 insertions(+), 165 deletions(-) diff --git a/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs b/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs index f020c2a805..4c9260f640 100644 --- a/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs +++ b/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs @@ -21,14 +21,14 @@ namespace osu.Game.Tests.Beatmaps.IO public class ImportBeatmapTest { [Test] - public void TestImportWhenClosed() + public async Task TestImportWhenClosed() { //unfortunately for the time being we need to reference osu.Framework.Desktop for a game host here. using (HeadlessGameHost host = new CleanRunHeadlessGameHost("TestImportWhenClosed")) { try { - LoadOszIntoOsu(loadOsu(host)); + await LoadOszIntoOsu(loadOsu(host)); } finally { @@ -38,7 +38,7 @@ namespace osu.Game.Tests.Beatmaps.IO } [Test] - public void TestImportThenDelete() + public async Task TestImportThenDelete() { //unfortunately for the time being we need to reference osu.Framework.Desktop for a game host here. using (HeadlessGameHost host = new CleanRunHeadlessGameHost("TestImportThenDelete")) @@ -47,7 +47,7 @@ namespace osu.Game.Tests.Beatmaps.IO { var osu = loadOsu(host); - var imported = LoadOszIntoOsu(osu); + var imported = await LoadOszIntoOsu(osu); deleteBeatmapSet(imported, osu); } @@ -59,7 +59,7 @@ namespace osu.Game.Tests.Beatmaps.IO } [Test] - public void TestImportThenImport() + public async Task TestImportThenImport() { //unfortunately for the time being we need to reference osu.Framework.Desktop for a game host here. using (HeadlessGameHost host = new CleanRunHeadlessGameHost("TestImportThenImport")) @@ -68,8 +68,8 @@ namespace osu.Game.Tests.Beatmaps.IO { var osu = loadOsu(host); - var imported = LoadOszIntoOsu(osu); - var importedSecondTime = LoadOszIntoOsu(osu); + var imported = await LoadOszIntoOsu(osu); + var importedSecondTime = await LoadOszIntoOsu(osu); // check the newly "imported" beatmap is actually just the restored previous import. since it matches hash. Assert.IsTrue(imported.ID == importedSecondTime.ID); @@ -88,7 +88,7 @@ namespace osu.Game.Tests.Beatmaps.IO } [Test] - public void TestRollbackOnFailure() + public async Task TestRollbackOnFailure() { //unfortunately for the time being we need to reference osu.Framework.Desktop for a game host here. using (HeadlessGameHost host = new CleanRunHeadlessGameHost("TestRollbackOnFailure")) @@ -104,7 +104,7 @@ namespace osu.Game.Tests.Beatmaps.IO manager.ItemAdded += (_, __) => fireCount++; manager.ItemRemoved += _ => fireCount++; - var imported = LoadOszIntoOsu(osu); + var imported = await LoadOszIntoOsu(osu); Assert.AreEqual(0, fireCount -= 1); @@ -132,7 +132,7 @@ namespace osu.Game.Tests.Beatmaps.IO Assert.AreEqual(12, manager.QueryBeatmaps(_ => true).ToList().Count); // this will trigger purging of the existing beatmap (online set id match) but should rollback due to broken osu. - manager.Import(breakTemp); + await manager.Import(breakTemp); // no events should be fired in the case of a rollback. Assert.AreEqual(0, fireCount); @@ -149,7 +149,7 @@ namespace osu.Game.Tests.Beatmaps.IO } [Test] - public void TestImportThenImportDifferentHash() + public async Task TestImportThenImportDifferentHash() { //unfortunately for the time being we need to reference osu.Framework.Desktop for a game host here. using (HeadlessGameHost host = new CleanRunHeadlessGameHost("TestImportThenImportDifferentHash")) @@ -159,12 +159,12 @@ namespace osu.Game.Tests.Beatmaps.IO var osu = loadOsu(host); var manager = osu.Dependencies.Get(); - var imported = LoadOszIntoOsu(osu); + var imported = await LoadOszIntoOsu(osu); imported.Hash += "-changed"; manager.Update(imported); - var importedSecondTime = LoadOszIntoOsu(osu); + var importedSecondTime = await LoadOszIntoOsu(osu); Assert.IsTrue(imported.ID != importedSecondTime.ID); Assert.IsTrue(imported.Beatmaps.First().ID < importedSecondTime.Beatmaps.First().ID); @@ -181,7 +181,7 @@ namespace osu.Game.Tests.Beatmaps.IO } [Test] - public void TestImportThenDeleteThenImport() + public async Task TestImportThenDeleteThenImport() { //unfortunately for the time being we need to reference osu.Framework.Desktop for a game host here. using (HeadlessGameHost host = new CleanRunHeadlessGameHost("TestImportThenDeleteThenImport")) @@ -190,11 +190,11 @@ namespace osu.Game.Tests.Beatmaps.IO { var osu = loadOsu(host); - var imported = LoadOszIntoOsu(osu); + var imported = await LoadOszIntoOsu(osu); deleteBeatmapSet(imported, osu); - var importedSecondTime = LoadOszIntoOsu(osu); + var importedSecondTime = await LoadOszIntoOsu(osu); // check the newly "imported" beatmap is actually just the restored previous import. since it matches hash. Assert.IsTrue(imported.ID == importedSecondTime.ID); @@ -209,7 +209,7 @@ namespace osu.Game.Tests.Beatmaps.IO [TestCase(true)] [TestCase(false)] - public void TestImportThenDeleteThenImportWithOnlineIDMismatch(bool set) + public async Task TestImportThenDeleteThenImportWithOnlineIDMismatch(bool set) { //unfortunately for the time being we need to reference osu.Framework.Desktop for a game host here. using (HeadlessGameHost host = new CleanRunHeadlessGameHost($"TestImportThenDeleteThenImport-{set}")) @@ -218,7 +218,7 @@ namespace osu.Game.Tests.Beatmaps.IO { var osu = loadOsu(host); - var imported = LoadOszIntoOsu(osu); + var imported = await LoadOszIntoOsu(osu); if (set) imported.OnlineBeatmapSetID = 1234; @@ -229,7 +229,7 @@ namespace osu.Game.Tests.Beatmaps.IO deleteBeatmapSet(imported, osu); - var importedSecondTime = LoadOszIntoOsu(osu); + var importedSecondTime = await LoadOszIntoOsu(osu); // check the newly "imported" beatmap has been reimported due to mismatch (even though hashes matched) Assert.IsTrue(imported.ID != importedSecondTime.ID); @@ -243,7 +243,7 @@ namespace osu.Game.Tests.Beatmaps.IO } [Test] - public void TestImportWithDuplicateBeatmapIDs() + public async Task TestImportWithDuplicateBeatmapIDs() { //unfortunately for the time being we need to reference osu.Framework.Desktop for a game host here. using (HeadlessGameHost host = new CleanRunHeadlessGameHost("TestImportWithDuplicateBeatmapID")) @@ -284,7 +284,7 @@ namespace osu.Game.Tests.Beatmaps.IO var manager = osu.Dependencies.Get(); - var imported = manager.Import(toImport); + var imported = await manager.Import(toImport); Assert.NotNull(imported); Assert.AreEqual(null, imported.Beatmaps[0].OnlineBeatmapID); @@ -330,7 +330,7 @@ namespace osu.Game.Tests.Beatmaps.IO } [Test] - public void TestImportWhenFileOpen() + public async Task TestImportWhenFileOpen() { using (HeadlessGameHost host = new CleanRunHeadlessGameHost("TestImportWhenFileOpen")) { @@ -339,7 +339,7 @@ namespace osu.Game.Tests.Beatmaps.IO var osu = loadOsu(host); var temp = TestResources.GetTestBeatmapForImport(); using (File.OpenRead(temp)) - osu.Dependencies.Get().Import(temp); + await osu.Dependencies.Get().Import(temp); ensureLoaded(osu); File.Delete(temp); Assert.IsFalse(File.Exists(temp), "We likely held a read lock on the file when we shouldn't"); @@ -351,13 +351,13 @@ namespace osu.Game.Tests.Beatmaps.IO } } - public static BeatmapSetInfo LoadOszIntoOsu(OsuGameBase osu, string path = null) + public static async Task LoadOszIntoOsu(OsuGameBase osu, string path = null) { var temp = path ?? TestResources.GetTestBeatmapForImport(); var manager = osu.Dependencies.Get(); - manager.Import(temp); + await manager.Import(temp); var imported = manager.GetAllUsableBeatmapSets(); diff --git a/osu.Game.Tests/Scores/IO/ImportScoreTest.cs b/osu.Game.Tests/Scores/IO/ImportScoreTest.cs index e39f18c3cd..4babb07213 100644 --- a/osu.Game.Tests/Scores/IO/ImportScoreTest.cs +++ b/osu.Game.Tests/Scores/IO/ImportScoreTest.cs @@ -23,13 +23,13 @@ namespace osu.Game.Tests.Scores.IO public class ImportScoreTest { [Test] - public void TestBasicImport() + public async Task TestBasicImport() { using (HeadlessGameHost host = new CleanRunHeadlessGameHost("TestBasicImport")) { try { - var osu = loadOsu(host); + var osu = await loadOsu(host); var toImport = new ScoreInfo { @@ -43,7 +43,7 @@ namespace osu.Game.Tests.Scores.IO OnlineScoreID = 12345, }; - var imported = loadIntoOsu(osu, toImport); + var imported = await loadIntoOsu(osu, toImport); Assert.AreEqual(toImport.Rank, imported.Rank); Assert.AreEqual(toImport.TotalScore, imported.TotalScore); @@ -62,20 +62,20 @@ namespace osu.Game.Tests.Scores.IO } [Test] - public void TestImportMods() + public async Task TestImportMods() { using (HeadlessGameHost host = new CleanRunHeadlessGameHost("TestImportMods")) { try { - var osu = loadOsu(host); + var osu = await loadOsu(host); var toImport = new ScoreInfo { Mods = new Mod[] { new OsuModHardRock(), new OsuModDoubleTime() }, }; - var imported = loadIntoOsu(osu, toImport); + var imported = await loadIntoOsu(osu, toImport); Assert.IsTrue(imported.Mods.Any(m => m is OsuModHardRock)); Assert.IsTrue(imported.Mods.Any(m => m is OsuModDoubleTime)); @@ -88,13 +88,13 @@ namespace osu.Game.Tests.Scores.IO } [Test] - public void TestImportStatistics() + public async Task TestImportStatistics() { using (HeadlessGameHost host = new CleanRunHeadlessGameHost("TestImportStatistics")) { try { - var osu = loadOsu(host); + var osu = await loadOsu(host); var toImport = new ScoreInfo { @@ -105,7 +105,7 @@ namespace osu.Game.Tests.Scores.IO } }; - var imported = loadIntoOsu(osu, toImport); + var imported = await loadIntoOsu(osu, toImport); Assert.AreEqual(toImport.Statistics[HitResult.Perfect], imported.Statistics[HitResult.Perfect]); Assert.AreEqual(toImport.Statistics[HitResult.Miss], imported.Statistics[HitResult.Miss]); @@ -117,7 +117,7 @@ namespace osu.Game.Tests.Scores.IO } } - private ScoreInfo loadIntoOsu(OsuGameBase osu, ScoreInfo score) + private async Task loadIntoOsu(OsuGameBase osu, ScoreInfo score) { var beatmapManager = osu.Dependencies.Get(); @@ -125,20 +125,24 @@ namespace osu.Game.Tests.Scores.IO score.Ruleset = new OsuRuleset().RulesetInfo; var scoreManager = osu.Dependencies.Get(); - scoreManager.Import(score); + await scoreManager.Import(score); return scoreManager.GetAllUsableScores().First(); } - private OsuGameBase loadOsu(GameHost host) + private async Task loadOsu(GameHost host) { var osu = new OsuGameBase(); + +#pragma warning disable 4014 Task.Run(() => host.Run(osu)); +#pragma warning restore 4014 + waitForOrAssert(() => osu.IsLoaded, @"osu! failed to start in a reasonable amount of time"); var beatmapFile = TestResources.GetTestBeatmapForImport(); var beatmapManager = osu.Dependencies.Get(); - beatmapManager.Import(beatmapFile); + await beatmapManager.Import(beatmapFile); return osu; } diff --git a/osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenBeatmap.cs b/osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenBeatmap.cs index 7104a420a3..8b941e4633 100644 --- a/osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenBeatmap.cs +++ b/osu.Game.Tests/Visual/Background/TestSceneBackgroundScreenBeatmap.cs @@ -72,7 +72,7 @@ namespace osu.Game.Tests.Visual.Background Dependencies.Cache(manager = new BeatmapManager(LocalStorage, factory, rulesets, null, audio, host, Beatmap.Default)); Dependencies.Cache(new OsuConfigManager(LocalStorage)); - manager.Import(TestResources.GetTestBeatmapForImport()); + manager.Import(TestResources.GetTestBeatmapForImport()).Wait(); Beatmap.SetDefault(); } diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneUpdateableBeatmapBackgroundSprite.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneUpdateableBeatmapBackgroundSprite.cs index f59458ef8d..c361598354 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneUpdateableBeatmapBackgroundSprite.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneUpdateableBeatmapBackgroundSprite.cs @@ -32,7 +32,7 @@ namespace osu.Game.Tests.Visual.UserInterface this.api = api; this.rulesets = rulesets; - testBeatmap = ImportBeatmapTest.LoadOszIntoOsu(osu); + testBeatmap = ImportBeatmapTest.LoadOszIntoOsu(osu).Result; } [Test] diff --git a/osu.Game.Tests/osu.Game.Tests.csproj b/osu.Game.Tests/osu.Game.Tests.csproj index 11d70ee7be..403ae16885 100644 --- a/osu.Game.Tests/osu.Game.Tests.csproj +++ b/osu.Game.Tests/osu.Game.Tests.csproj @@ -18,4 +18,9 @@ + + + ..\..\..\..\..\usr\local\share\dotnet\sdk\NuGetFallbackFolder\microsoft.win32.registry\4.5.0\ref\netstandard2.0\Microsoft.Win32.Registry.dll + + \ No newline at end of file diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index b6fe7f88fa..f5ef52a93b 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -2,10 +2,12 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.IO; using System.Linq; using System.Linq.Expressions; +using System.Threading; using System.Threading.Tasks; using Microsoft.EntityFrameworkCore; using osu.Framework.Audio; @@ -14,6 +16,7 @@ using osu.Framework.Extensions; using osu.Framework.Graphics.Textures; using osu.Framework.Logging; using osu.Framework.Platform; +using osu.Framework.Threading; using osu.Game.Beatmaps.Formats; using osu.Game.Database; using osu.Game.IO.Archives; @@ -72,6 +75,8 @@ namespace osu.Game.Beatmaps private readonly List currentDownloads = new List(); + private readonly BeatmapUpdateQueue updateQueue; + public BeatmapManager(Storage storage, IDatabaseContextFactory contextFactory, RulesetStore rulesets, IAPIProvider api, AudioManager audioManager, GameHost host = null, WorkingBeatmap defaultBeatmap = null) : base(storage, contextFactory, new BeatmapStore(contextFactory), host) @@ -86,9 +91,11 @@ namespace osu.Game.Beatmaps beatmaps = (BeatmapStore)ModelStore; beatmaps.BeatmapHidden += b => BeatmapHidden?.Invoke(b); beatmaps.BeatmapRestored += b => BeatmapRestored?.Invoke(b); + + updateQueue = new BeatmapUpdateQueue(api); } - protected override void Populate(BeatmapSetInfo beatmapSet, ArchiveReader archive) + protected override async Task Populate(BeatmapSetInfo beatmapSet, ArchiveReader archive, CancellationToken cancellationToken = default) { if (archive != null) beatmapSet.Beatmaps = createBeatmapDifficulties(archive); @@ -104,8 +111,7 @@ namespace osu.Game.Beatmaps validateOnlineIds(beatmapSet); - foreach (BeatmapInfo b in beatmapSet.Beatmaps) - fetchAndPopulateOnlineValues(b); + await Task.WhenAll(beatmapSet.Beatmaps.Select(b => updateQueue.Enqueue(new UpdateItem(b, cancellationToken)).Task).ToArray()); } protected override void PreImport(BeatmapSetInfo beatmapSet) @@ -181,10 +187,10 @@ namespace osu.Game.Beatmaps request.Success += filename => { - Task.Factory.StartNew(() => + Task.Factory.StartNew(async () => { // This gets scheduled back to the update thread, but we want the import to run in the background. - Import(downloadNotification, filename); + await Import(downloadNotification, filename); currentDownloads.Remove(request); }, TaskCreationOptions.LongRunning); }; @@ -381,47 +387,6 @@ namespace osu.Game.Beatmaps return beatmapInfos; } - /// - /// Query the API to populate missing values like OnlineBeatmapID / OnlineBeatmapSetID or (Rank-)Status. - /// - /// The beatmap to populate. - /// Whether to re-query if the provided beatmap already has populated values. - /// True if population was successful. - private bool fetchAndPopulateOnlineValues(BeatmapInfo beatmap, bool force = false) - { - if (api?.State != APIState.Online) - return false; - - if (!force && beatmap.OnlineBeatmapID != null && beatmap.BeatmapSet.OnlineBeatmapSetID != null - && beatmap.Status != BeatmapSetOnlineStatus.None && beatmap.BeatmapSet.Status != BeatmapSetOnlineStatus.None) - return true; - - Logger.Log("Attempting online lookup for the missing values...", LoggingTarget.Database); - - try - { - var req = new GetBeatmapRequest(beatmap); - - req.Perform(api); - - var res = req.Result; - - Logger.Log($"Successfully mapped to {res.OnlineBeatmapSetID} / {res.OnlineBeatmapID}.", LoggingTarget.Database); - - beatmap.Status = res.Status; - beatmap.BeatmapSet.Status = res.BeatmapSet.Status; - beatmap.BeatmapSet.OnlineBeatmapSetID = res.OnlineBeatmapSetID; - beatmap.OnlineBeatmapID = res.OnlineBeatmapID; - - return true; - } - catch (Exception e) - { - Logger.Log($"Failed ({e})", LoggingTarget.Database); - return false; - } - } - /// /// A dummy WorkingBeatmap for the purpose of retrieving a beatmap for star difficulty calculation. /// @@ -455,5 +420,111 @@ namespace osu.Game.Beatmaps public override bool IsImportant => false; } } + + private class BeatmapUpdateQueue + { + private readonly IAPIProvider api; + private readonly Queue queue = new Queue(); + + private int activeThreads; + + public BeatmapUpdateQueue(IAPIProvider api) + { + this.api = api; + } + + public UpdateItem Enqueue(UpdateItem item) + { + lock (queue) + { + queue.Enqueue(item); + + if (activeThreads >= 16) + return item; + + new Thread(runWork) { IsBackground = true }.Start(); + activeThreads++; + } + + return item; + } + + private void runWork() + { + while (true) + { + UpdateItem toProcess; + + lock (queue) + { + if (queue.Count == 0) + break; + + toProcess = queue.Dequeue(); + } + + toProcess.PerformUpdate(api); + } + + lock (queue) + activeThreads--; + } + } + + private class UpdateItem + { + public Task Task => tcs.Task; + + private readonly BeatmapInfo beatmap; + private readonly CancellationToken cancellationToken; + + private readonly TaskCompletionSource tcs = new TaskCompletionSource(); + + public UpdateItem(BeatmapInfo beatmap, CancellationToken cancellationToken) + { + this.beatmap = beatmap; + this.cancellationToken = cancellationToken; + } + + public void PerformUpdate(IAPIProvider api) + { + if (cancellationToken.IsCancellationRequested) + { + tcs.SetCanceled(); + return; + } + + if (api?.State != APIState.Online) + { + tcs.SetResult(false); + return; + } + + Logger.Log("Attempting online lookup for the missing values...", LoggingTarget.Database); + + var req = new GetBeatmapRequest(beatmap); + + req.Success += res => + { + Logger.Log($"Successfully mapped to {res.OnlineBeatmapSetID} / {res.OnlineBeatmapID}.", LoggingTarget.Database); + + beatmap.Status = res.Status; + beatmap.BeatmapSet.Status = res.BeatmapSet.Status; + beatmap.BeatmapSet.OnlineBeatmapSetID = res.OnlineBeatmapSetID; + beatmap.OnlineBeatmapID = res.OnlineBeatmapID; + + tcs.SetResult(true); + }; + + req.Failure += e => + { + Logger.Log($"Failed ({e})", LoggingTarget.Database); + + tcs.SetResult(false); + }; + + req.Perform(api); + } + } } } diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index 54dbae9ddc..afc614772e 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -5,14 +5,17 @@ using System; using System.Collections.Generic; using System.IO; using System.Linq; +using System.Threading; using System.Threading.Tasks; using JetBrains.Annotations; using Microsoft.EntityFrameworkCore; using osu.Framework; using osu.Framework.Extensions; +using osu.Framework.Extensions.TypeExtensions; using osu.Framework.IO.File; using osu.Framework.Logging; using osu.Framework.Platform; +using osu.Framework.Threading; using osu.Game.IO; using osu.Game.IO.Archives; using osu.Game.IPC; @@ -109,8 +112,11 @@ namespace osu.Game.Database a.Invoke(); } + private readonly ThreadedTaskScheduler importScheduler; + protected ArchiveModelManager(Storage storage, IDatabaseContextFactory contextFactory, MutableDatabaseBackedStoreWithFileIncludes modelStore, IIpcHost importHost = null) { + importScheduler = new ThreadedTaskScheduler(16, $"{GetType().ReadableName()}.Import"); ContextFactory = contextFactory; ModelStore = modelStore; @@ -130,92 +136,84 @@ namespace osu.Game.Database /// This will post notifications tracking progress. /// /// One or more archive locations on disk. - public void Import(params string[] paths) + public async Task Import(params string[] paths) { var notification = new ProgressNotification { State = ProgressNotificationState.Active }; PostNotification?.Invoke(notification); - Import(notification, paths); + + await Import(notification, paths); } - protected void Import(ProgressNotification notification, params string[] paths) + protected async Task Import(ProgressNotification notification, params string[] paths) { notification.Progress = 0; notification.Text = "Import is initialising..."; var term = $"{typeof(TModel).Name.Replace("Info", "").ToLower()}"; - List imported = new List(); + var tasks = new List(); int current = 0; foreach (string path in paths) { - if (notification.State == ProgressNotificationState.Cancelled) - // user requested abort - return; - - try + tasks.Add(Import(path, notification.CancellationToken).ContinueWith(t => { - var text = "Importing "; - - if (path.Length > 1) - text += $"{++current} of {paths.Length} {term}s.."; - else - text += $"{term}.."; - - // only show the filename if it isn't a temporary one (as those look ugly). - if (!path.Contains(Path.GetTempPath())) - text += $"\n{Path.GetFileName(path)}"; - - notification.Text = text; - - imported.Add(Import(path)); - - notification.Progress = (float)current / paths.Length; - } - catch (Exception e) - { - e = e.InnerException ?? e; - Logger.Error(e, $@"Could not import ({Path.GetFileName(path)})"); - } - } - - if (imported.Count == 0) - { - notification.Text = "Import failed!"; - notification.State = ProgressNotificationState.Cancelled; - } - else - { - notification.CompletionText = imported.Count == 1 - ? $"Imported {imported.First()}!" - : $"Imported {current} {term}s!"; - - if (imported.Count > 0 && PresentImport != null) - { - notification.CompletionText += " Click to view."; - notification.CompletionClickAction = () => + lock (notification) { - PresentImport?.Invoke(imported); - return true; - }; - } + current++; - notification.State = ProgressNotificationState.Completed; + notification.Text = $"Imported {current} of {paths.Length} {term}s"; + notification.Progress = (float)current / paths.Length; + } + + if (t.Exception != null) + { + var e = t.Exception.InnerException ?? t.Exception; + Logger.Error(e, $@"Could not import ({Path.GetFileName(path)})"); + } + })); } + + await Task.WhenAll(tasks); + + // if (imported.Count == 0) + // { + // notification.Text = "Import failed!"; + // notification.State = ProgressNotificationState.Cancelled; + // } + // else + // { + // notification.CompletionText = imported.Count == 1 + // ? $"Imported {imported.First()}!" + // : $"Imported {current} {term}s!"; + // + // if (imported.Count > 0 && PresentImport != null) + // { + // notification.CompletionText += " Click to view."; + // notification.CompletionClickAction = () => + // { + // PresentImport?.Invoke(imported); + // return true; + // }; + // } + // + // notification.State = ProgressNotificationState.Completed; + // } } /// /// Import one from the filesystem and delete the file on success. /// /// The archive location on disk. + /// An optional cancellation token. /// The imported model, if successful. - public TModel Import(string path) + public async Task Import(string path, CancellationToken cancellationToken = default) { TModel import; using (ArchiveReader reader = getReaderFrom(path)) - import = Import(reader); + import = await Import(reader, cancellationToken); // We may or may not want to delete the file depending on where it is stored. // e.g. reconstructing/repairing database with items from default storage. @@ -243,7 +241,8 @@ namespace osu.Game.Database /// Import an item from an . /// /// The archive to be imported. - public TModel Import(ArchiveReader archive) + /// An optional cancellation token. + public async Task Import(ArchiveReader archive, CancellationToken cancellationToken = default) { try { @@ -253,7 +252,7 @@ namespace osu.Game.Database model.Hash = computeHash(archive); - return Import(model, archive); + return await Import(model, archive, cancellationToken); } catch (Exception e) { @@ -288,7 +287,8 @@ namespace osu.Game.Database /// /// The model to be imported. /// An optional archive to use for model population. - public TModel Import(TModel item, ArchiveReader archive = null) + /// An optional cancellation token. + public async Task Import(TModel item, ArchiveReader archive = null, CancellationToken cancellationToken = default) => await Task.Factory.StartNew(async () => { delayEvents(); @@ -296,17 +296,31 @@ namespace osu.Game.Database { Logger.Log($"Importing {item}...", LoggingTarget.Database); + if (archive != null) + item.Files = createFileInfos(archive, Files); + + var localItem = item; + + try + { + await Populate(item, archive, cancellationToken); + } + catch (TaskCanceledException) + { + return item = null; + } + finally + { + if (!Delete(localItem)) + Files.Dereference(localItem.Files.Select(f => f.FileInfo).ToArray()); + } + using (var write = ContextFactory.GetForWrite()) // used to share a context for full import. keep in mind this will block all writes. { try { if (!write.IsTransactionLeader) throw new InvalidOperationException($"Ensure there is no parent transaction so errors can correctly be handled by {this}"); - if (archive != null) - item.Files = createFileInfos(archive, Files); - - Populate(item, archive); - var existing = CheckForExisting(item); if (existing != null) @@ -332,6 +346,9 @@ namespace osu.Game.Database } catch (Exception e) { + if (!Delete(item)) + Files.Dereference(item.Files.Select(f => f.FileInfo).ToArray()); + write.Errors.Add(e); throw; } @@ -351,7 +368,7 @@ namespace osu.Game.Database } return item; - } + }, CancellationToken.None, TaskCreationOptions.None, importScheduler).Unwrap(); /// /// Perform an update of the specified item. @@ -516,24 +533,24 @@ namespace osu.Game.Database /// /// This is a temporary method and will likely be replaced by a full-fledged (and more correctly placed) migration process in the future. /// - public Task ImportFromStableAsync() + public async Task ImportFromStableAsync() { var stable = GetStableStorage?.Invoke(); if (stable == null) { Logger.Log("No osu!stable installation available!", LoggingTarget.Information, LogLevel.Error); - return Task.CompletedTask; + return; } if (!stable.ExistsDirectory(ImportFromStablePath)) { // This handles situations like when the user does not have a Skins folder Logger.Log($"No {ImportFromStablePath} folder available in osu!stable installation", LoggingTarget.Information, LogLevel.Error); - return Task.CompletedTask; + return; } - return Task.Factory.StartNew(() => Import(stable.GetDirectories(ImportFromStablePath).Select(f => stable.GetFullPath(f)).ToArray()), TaskCreationOptions.LongRunning); + await Task.Run(async () => await Import(stable.GetDirectories(ImportFromStablePath).Select(f => stable.GetFullPath(f)).ToArray())); } #endregion @@ -552,9 +569,8 @@ namespace osu.Game.Database /// /// The model to populate. /// The archive to use as a reference for population. May be null. - protected virtual void Populate(TModel model, [CanBeNull] ArchiveReader archive) - { - } + /// An optional cancellation token. + protected virtual async Task Populate(TModel model, [CanBeNull] ArchiveReader archive, CancellationToken cancellationToken = default) => await Task.CompletedTask; /// /// Perform any final actions before the import to database executes. diff --git a/osu.Game/Database/ICanAcceptFiles.cs b/osu.Game/Database/ICanAcceptFiles.cs index f55d0c389e..b9f882468d 100644 --- a/osu.Game/Database/ICanAcceptFiles.cs +++ b/osu.Game/Database/ICanAcceptFiles.cs @@ -1,6 +1,8 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System.Threading.Tasks; + namespace osu.Game.Database { /// @@ -12,7 +14,7 @@ namespace osu.Game.Database /// Import the specified paths. /// /// The files which should be imported. - void Import(params string[] paths); + Task Import(params string[] paths); /// /// An array of accepted file extensions (in the standard format of ".abc"). diff --git a/osu.Game/IPC/ArchiveImportIPCChannel.cs b/osu.Game/IPC/ArchiveImportIPCChannel.cs index fc747cd446..484db932f8 100644 --- a/osu.Game/IPC/ArchiveImportIPCChannel.cs +++ b/osu.Game/IPC/ArchiveImportIPCChannel.cs @@ -38,7 +38,7 @@ namespace osu.Game.IPC } if (importer.HandledExtensions.Contains(Path.GetExtension(path)?.ToLowerInvariant())) - importer.Import(path); + await importer.Import(path); } } diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs index f9128687d6..d6b8caaf5b 100644 --- a/osu.Game/OsuGameBase.cs +++ b/osu.Game/OsuGameBase.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; using System.Reflection; +using System.Threading.Tasks; using osu.Framework.Allocation; using osu.Framework.Audio; using osu.Framework.Bindables; @@ -268,13 +269,13 @@ namespace osu.Game private readonly List fileImporters = new List(); - public void Import(params string[] paths) + public async Task Import(params string[] paths) { var extension = Path.GetExtension(paths.First())?.ToLowerInvariant(); foreach (var importer in fileImporters) if (importer.HandledExtensions.Contains(extension)) - importer.Import(paths); + await importer.Import(paths); } public string[] HandledExtensions => fileImporters.SelectMany(i => i.HandledExtensions).ToArray(); diff --git a/osu.Game/Overlays/Notifications/ProgressNotification.cs b/osu.Game/Overlays/Notifications/ProgressNotification.cs index 857a0bda9e..c8e081d29f 100644 --- a/osu.Game/Overlays/Notifications/ProgressNotification.cs +++ b/osu.Game/Overlays/Notifications/ProgressNotification.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Threading; using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; @@ -36,6 +37,10 @@ namespace osu.Game.Overlays.Notifications State = state; } + private readonly CancellationTokenSource cancellationTokenSource = new CancellationTokenSource(); + + public CancellationToken CancellationToken => cancellationTokenSource.Token; + public virtual ProgressNotificationState State { get => state; @@ -62,6 +67,8 @@ namespace osu.Game.Overlays.Notifications break; case ProgressNotificationState.Cancelled: + cancellationTokenSource.Cancel(); + Light.Colour = colourCancelled; Light.Pulsate = false; progressBar.Active = false; diff --git a/osu.Game/Screens/Menu/Intro.cs b/osu.Game/Screens/Menu/Intro.cs index 98a2fe8f13..cf5d247482 100644 --- a/osu.Game/Screens/Menu/Intro.cs +++ b/osu.Game/Screens/Menu/Intro.cs @@ -66,7 +66,7 @@ namespace osu.Game.Screens.Menu if (setInfo == null) { // we need to import the default menu background beatmap - setInfo = beatmaps.Import(new ZipArchiveReader(game.Resources.GetStream(@"Tracks/circles.osz"), "circles.osz")); + setInfo = beatmaps.Import(new ZipArchiveReader(game.Resources.GetStream(@"Tracks/circles.osz"), "circles.osz")).Result; setInfo.Protected = true; beatmaps.Update(setInfo); diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index d8389fa6d9..949f9e5ff2 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -279,7 +279,7 @@ namespace osu.Game.Screens.Play var score = CreateScore(); if (DrawableRuleset.ReplayScore == null) - scoreManager.Import(score); + scoreManager.Import(score).Wait(); this.Push(CreateResults(score)); diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index 3a4d44f608..73cc47ea47 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -5,6 +5,8 @@ using System; using System.Collections.Generic; using System.Linq; using System.Linq.Expressions; +using System.Threading; +using System.Threading.Tasks; using Microsoft.EntityFrameworkCore; using osu.Framework.Audio; using osu.Framework.Audio.Sample; @@ -71,9 +73,9 @@ namespace osu.Game.Skinning protected override SkinInfo CreateModel(ArchiveReader archive) => new SkinInfo { Name = archive.Name }; - protected override void Populate(SkinInfo model, ArchiveReader archive) + protected override async Task Populate(SkinInfo model, ArchiveReader archive, CancellationToken cancellationToken = default) { - base.Populate(model, archive); + await base.Populate(model, archive, cancellationToken); Skin reference = getSkin(model); From 600503ec8ebdb6233beab64b0faa53f02b91af4e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 10 Jun 2019 12:46:21 +0900 Subject: [PATCH 18/60] Use Task.Run/Wait to avoid warnings --- osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs | 4 ++-- osu.Game/Screens/Select/SongSelect.cs | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index ebee358730..c2e8078bd6 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -210,7 +210,7 @@ namespace osu.Game.Tests.Visual.SongSelect AddAssert("start not requested", () => !startRequested); } - private void importForRuleset(int id) => AddStep($"import test map for ruleset {id}", () => manager.Import(createTestBeatmapSet(getImportId(), rulesets.AvailableRulesets.Where(r => r.ID == id).ToArray()))); + private void importForRuleset(int id) => AddStep($"import test map for ruleset {id}", () => manager.Import(createTestBeatmapSet(getImportId(), rulesets.AvailableRulesets.Where(r => r.ID == id).ToArray())).Wait()); private static int importId; private int getImportId() => ++importId; @@ -232,7 +232,7 @@ namespace osu.Game.Tests.Visual.SongSelect var usableRulesets = rulesets.AvailableRulesets.Where(r => r.ID != 2).ToArray(); for (int i = 0; i < 100; i += 10) - manager.Import(createTestBeatmapSet(i, usableRulesets)); + manager.Import(createTestBeatmapSet(i, usableRulesets)).Wait(); }); } diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index 6d5be607f4..196d655851 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -32,6 +32,7 @@ using osuTK.Input; using System; using System.Collections.Generic; using System.Linq; +using System.Threading.Tasks; using osu.Framework.Graphics.Sprites; namespace osu.Game.Screens.Select @@ -256,8 +257,8 @@ namespace osu.Game.Screens.Select if (!beatmaps.GetAllUsableBeatmapSets().Any() && beatmaps.StableInstallationAvailable) dialogOverlay.Push(new ImportFromStablePopup(() => { - beatmaps.ImportFromStableAsync(); - skins.ImportFromStableAsync(); + Task.Run(beatmaps.ImportFromStableAsync); + Task.Run(skins.ImportFromStableAsync); })); }); } From b4d2d0bd0b245cba425d9e5d981e3831d9269931 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 10 Jun 2019 13:19:58 +0900 Subject: [PATCH 19/60] Simplify and combine concurrency of ArchiveModelManager --- osu.Game/Beatmaps/BeatmapManager.cs | 77 ++----------------- osu.Game/Database/ArchiveModelManager.cs | 96 ++++++++++++------------ 2 files changed, 57 insertions(+), 116 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index f5ef52a93b..9e7b6d7971 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -2,7 +2,6 @@ // See the LICENCE file in the repository root for full licence text. using System; -using System.Collections.Concurrent; using System.Collections.Generic; using System.IO; using System.Linq; @@ -111,7 +110,7 @@ namespace osu.Game.Beatmaps validateOnlineIds(beatmapSet); - await Task.WhenAll(beatmapSet.Beatmaps.Select(b => updateQueue.Enqueue(new UpdateItem(b, cancellationToken)).Task).ToArray()); + await Task.WhenAll(beatmapSet.Beatmaps.Select(b => updateQueue.Perform(b, cancellationToken)).ToArray()); } protected override void PreImport(BeatmapSetInfo beatmapSet) @@ -424,81 +423,24 @@ namespace osu.Game.Beatmaps private class BeatmapUpdateQueue { private readonly IAPIProvider api; - private readonly Queue queue = new Queue(); - private int activeThreads; + private readonly ThreadedTaskScheduler updateScheduler = new ThreadedTaskScheduler(4); public BeatmapUpdateQueue(IAPIProvider api) { this.api = api; } - public UpdateItem Enqueue(UpdateItem item) + public Task Perform(BeatmapInfo beatmap, CancellationToken cancellationToken) + => Task.Factory.StartNew(() => perform(beatmap, cancellationToken), cancellationToken, TaskCreationOptions.HideScheduler, updateScheduler); + + private void perform(BeatmapInfo beatmap, CancellationToken cancellation) { - lock (queue) - { - queue.Enqueue(item); - - if (activeThreads >= 16) - return item; - - new Thread(runWork) { IsBackground = true }.Start(); - activeThreads++; - } - - return item; - } - - private void runWork() - { - while (true) - { - UpdateItem toProcess; - - lock (queue) - { - if (queue.Count == 0) - break; - - toProcess = queue.Dequeue(); - } - - toProcess.PerformUpdate(api); - } - - lock (queue) - activeThreads--; - } - } - - private class UpdateItem - { - public Task Task => tcs.Task; - - private readonly BeatmapInfo beatmap; - private readonly CancellationToken cancellationToken; - - private readonly TaskCompletionSource tcs = new TaskCompletionSource(); - - public UpdateItem(BeatmapInfo beatmap, CancellationToken cancellationToken) - { - this.beatmap = beatmap; - this.cancellationToken = cancellationToken; - } - - public void PerformUpdate(IAPIProvider api) - { - if (cancellationToken.IsCancellationRequested) - { - tcs.SetCanceled(); + if (cancellation.IsCancellationRequested) return; - } if (api?.State != APIState.Online) - { - tcs.SetResult(false); return; - } Logger.Log("Attempting online lookup for the missing values...", LoggingTarget.Database); @@ -512,17 +454,14 @@ namespace osu.Game.Beatmaps beatmap.BeatmapSet.Status = res.BeatmapSet.Status; beatmap.BeatmapSet.OnlineBeatmapSetID = res.OnlineBeatmapSetID; beatmap.OnlineBeatmapID = res.OnlineBeatmapID; - - tcs.SetResult(true); }; req.Failure += e => { Logger.Log($"Failed ({e})", LoggingTarget.Database); - - tcs.SetResult(false); }; + // intentionally blocking to limit web request concurrency req.Perform(api); } } diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index afc614772e..e30e1cd3ee 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -1,4 +1,4 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. using System; @@ -11,7 +11,6 @@ using JetBrains.Annotations; using Microsoft.EntityFrameworkCore; using osu.Framework; using osu.Framework.Extensions; -using osu.Framework.Extensions.TypeExtensions; using osu.Framework.IO.File; using osu.Framework.Logging; using osu.Framework.Platform; @@ -32,7 +31,7 @@ namespace osu.Game.Database /// /// The model type. /// The associated file join type. - public abstract class ArchiveModelManager : ICanAcceptFiles + public abstract class ArchiveModelManager : ArchiveModelManager, ICanAcceptFiles where TModel : class, IHasFiles, IHasPrimaryKey, ISoftDelete where TFileModel : INamedFileInfo, new() { @@ -112,11 +111,8 @@ namespace osu.Game.Database a.Invoke(); } - private readonly ThreadedTaskScheduler importScheduler; - protected ArchiveModelManager(Storage storage, IDatabaseContextFactory contextFactory, MutableDatabaseBackedStoreWithFileIncludes modelStore, IIpcHost importHost = null) { - importScheduler = new ThreadedTaskScheduler(16, $"{GetType().ReadableName()}.Import"); ContextFactory = contextFactory; ModelStore = modelStore; @@ -152,55 +148,55 @@ namespace osu.Game.Database var term = $"{typeof(TModel).Name.Replace("Info", "").ToLower()}"; - var tasks = new List(); - int current = 0; - foreach (string path in paths) + var imported = new List(); + + await Task.WhenAll(paths.Select(path => Import(path, notification.CancellationToken).ContinueWith(t => { - tasks.Add(Import(path, notification.CancellationToken).ContinueWith(t => + lock (notification) { - lock (notification) - { - current++; + current++; - notification.Text = $"Imported {current} of {paths.Length} {term}s"; - notification.Progress = (float)current / paths.Length; - } + notification.Text = $"Imported {current} of {paths.Length} {term}s"; + notification.Progress = (float)current / paths.Length; + } - if (t.Exception != null) - { - var e = t.Exception.InnerException ?? t.Exception; - Logger.Error(e, $@"Could not import ({Path.GetFileName(path)})"); - } - })); + if (t.Exception == null) + { + lock (imported) + imported.Add(t.Result); + } + else + { + var e = t.Exception.InnerException ?? t.Exception; + Logger.Error(e, $@"Could not import ({Path.GetFileName(path)})"); + } + }))); + + if (imported.Count == 0) + { + notification.Text = "Import failed!"; + notification.State = ProgressNotificationState.Cancelled; } + else + { + notification.CompletionText = imported.Count == 1 + ? $"Imported {imported.First()}!" + : $"Imported {current} {term}s!"; - await Task.WhenAll(tasks); + if (imported.Count > 0 && PresentImport != null) + { + notification.CompletionText += " Click to view."; + notification.CompletionClickAction = () => + { + PresentImport?.Invoke(imported); + return true; + }; + } - // if (imported.Count == 0) - // { - // notification.Text = "Import failed!"; - // notification.State = ProgressNotificationState.Cancelled; - // } - // else - // { - // notification.CompletionText = imported.Count == 1 - // ? $"Imported {imported.First()}!" - // : $"Imported {current} {term}s!"; - // - // if (imported.Count > 0 && PresentImport != null) - // { - // notification.CompletionText += " Click to view."; - // notification.CompletionClickAction = () => - // { - // PresentImport?.Invoke(imported); - // return true; - // }; - // } - // - // notification.State = ProgressNotificationState.Completed; - // } + notification.State = ProgressNotificationState.Completed; + } } /// @@ -368,7 +364,7 @@ namespace osu.Game.Database } return item; - }, CancellationToken.None, TaskCreationOptions.None, importScheduler).Unwrap(); + }, CancellationToken.None, TaskCreationOptions.HideScheduler, IMPORT_SCHEDULER).Unwrap(); /// /// Perform an update of the specified item. @@ -615,4 +611,10 @@ namespace osu.Game.Database throw new InvalidFormatException($"{path} is not a valid archive"); } } + + public abstract class ArchiveModelManager + { + // allow sharing static across all generic types + protected static readonly ThreadedTaskScheduler IMPORT_SCHEDULER = new ThreadedTaskScheduler(1); + } } From 2d1a54e63489b36cfb4fa5261abd0ed95c092051 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 10 Jun 2019 13:37:20 +0900 Subject: [PATCH 20/60] Properly implement cancellation --- osu.Game/Database/ArchiveModelManager.cs | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index e30e1cd3ee..1d576ff82f 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -154,6 +154,9 @@ namespace osu.Game.Database await Task.WhenAll(paths.Select(path => Import(path, notification.CancellationToken).ContinueWith(t => { + if (notification.CancellationToken.IsCancellationRequested) + return; + lock (notification) { current++; @@ -172,7 +175,7 @@ namespace osu.Game.Database var e = t.Exception.InnerException ?? t.Exception; Logger.Error(e, $@"Could not import ({Path.GetFileName(path)})"); } - }))); + }, TaskContinuationOptions.NotOnCanceled))); if (imported.Count == 0) { @@ -207,6 +210,8 @@ namespace osu.Game.Database /// The imported model, if successful. public async Task Import(string path, CancellationToken cancellationToken = default) { + cancellationToken.ThrowIfCancellationRequested(); + TModel import; using (ArchiveReader reader = getReaderFrom(path)) import = await Import(reader, cancellationToken); @@ -240,6 +245,8 @@ namespace osu.Game.Database /// An optional cancellation token. public async Task Import(ArchiveReader archive, CancellationToken cancellationToken = default) { + cancellationToken.ThrowIfCancellationRequested(); + try { var model = CreateModel(archive); @@ -250,6 +257,10 @@ namespace osu.Game.Database return await Import(model, archive, cancellationToken); } + catch (TaskCanceledException) + { + throw; + } catch (Exception e) { Logger.Error(e, $"Model creation of {archive.Name} failed.", LoggingTarget.Database); @@ -286,6 +297,8 @@ namespace osu.Game.Database /// An optional cancellation token. public async Task Import(TModel item, ArchiveReader archive = null, CancellationToken cancellationToken = default) => await Task.Factory.StartNew(async () => { + cancellationToken.ThrowIfCancellationRequested(); + delayEvents(); try @@ -301,10 +314,6 @@ namespace osu.Game.Database { await Populate(item, archive, cancellationToken); } - catch (TaskCanceledException) - { - return item = null; - } finally { if (!Delete(localItem)) @@ -364,7 +373,7 @@ namespace osu.Game.Database } return item; - }, CancellationToken.None, TaskCreationOptions.HideScheduler, IMPORT_SCHEDULER).Unwrap(); + }, cancellationToken, TaskCreationOptions.HideScheduler, IMPORT_SCHEDULER).Unwrap(); /// /// Perform an update of the specified item. From e12b03e275c40de2b3629b92da24da45545be619 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 10 Jun 2019 13:46:31 +0900 Subject: [PATCH 21/60] Remove unnecessary reference --- osu.Game.Tests/osu.Game.Tests.csproj | 5 ----- 1 file changed, 5 deletions(-) diff --git a/osu.Game.Tests/osu.Game.Tests.csproj b/osu.Game.Tests/osu.Game.Tests.csproj index 403ae16885..11d70ee7be 100644 --- a/osu.Game.Tests/osu.Game.Tests.csproj +++ b/osu.Game.Tests/osu.Game.Tests.csproj @@ -18,9 +18,4 @@ - - - ..\..\..\..\..\usr\local\share\dotnet\sdk\NuGetFallbackFolder\microsoft.win32.registry\4.5.0\ref\netstandard2.0\Microsoft.Win32.Registry.dll - - \ No newline at end of file From b79fdfc12f2d65da2f0fd80b68fa58aa2aa11882 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 10 Jun 2019 13:48:58 +0900 Subject: [PATCH 22/60] Fix one more instance of improperly handled cancellation --- osu.Game/Beatmaps/BeatmapManager.cs | 3 +-- osu.Game/Database/ArchiveModelManager.cs | 4 ++++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 9e7b6d7971..cfc6a0be28 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -436,8 +436,7 @@ namespace osu.Game.Beatmaps private void perform(BeatmapInfo beatmap, CancellationToken cancellation) { - if (cancellation.IsCancellationRequested) - return; + cancellation.ThrowIfCancellationRequested(); if (api?.State != APIState.Online) return; diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index 1d576ff82f..df96d9984a 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -361,6 +361,10 @@ namespace osu.Game.Database Logger.Log($"Import of {item} successfully completed!", LoggingTarget.Database); } + catch (TaskCanceledException) + { + throw; + } catch (Exception e) { Logger.Error(e, $"Import of {item} failed and has been rolled back.", LoggingTarget.Database); From e4bad93b6668868079dad7227b868b43736e6336 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 10 Jun 2019 13:52:09 +0900 Subject: [PATCH 23/60] Use variable for web request concurrency for clarity --- osu.Game/Beatmaps/BeatmapManager.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index cfc6a0be28..435edcf722 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -424,7 +424,9 @@ namespace osu.Game.Beatmaps { private readonly IAPIProvider api; - private readonly ThreadedTaskScheduler updateScheduler = new ThreadedTaskScheduler(4); + private const int update_queue_request_concurrency = 4; + + private readonly ThreadedTaskScheduler updateScheduler = new ThreadedTaskScheduler(update_queue_request_concurrency); public BeatmapUpdateQueue(IAPIProvider api) { @@ -455,10 +457,7 @@ namespace osu.Game.Beatmaps beatmap.OnlineBeatmapID = res.OnlineBeatmapID; }; - req.Failure += e => - { - Logger.Log($"Failed ({e})", LoggingTarget.Database); - }; + req.Failure += e => { Logger.Log($"Failed ({e})", LoggingTarget.Database); }; // intentionally blocking to limit web request concurrency req.Perform(api); From e19f4935c3cbdd6d037d093d6b41e2f7cb1bf719 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 10 Jun 2019 14:13:36 +0900 Subject: [PATCH 24/60] Fix incorrect undo logic on exception --- osu.Game/Database/ArchiveModelManager.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index df96d9984a..83c51e2abf 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -305,8 +305,7 @@ namespace osu.Game.Database { Logger.Log($"Importing {item}...", LoggingTarget.Database); - if (archive != null) - item.Files = createFileInfos(archive, Files); + item.Files = archive != null ? createFileInfos(archive, Files) : new List(); var localItem = item; @@ -314,7 +313,7 @@ namespace osu.Game.Database { await Populate(item, archive, cancellationToken); } - finally + catch (Exception) { if (!Delete(localItem)) Files.Dereference(localItem.Files.Select(f => f.FileInfo).ToArray()); From f31b19e0d7b2837bfaa1c26fa381d6ab1df43e41 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 10 Jun 2019 16:02:49 +0900 Subject: [PATCH 25/60] Don't unwrap exception manually --- osu.Game/Database/ArchiveModelManager.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index 83c51e2abf..4c4878edee 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -172,8 +172,7 @@ namespace osu.Game.Database } else { - var e = t.Exception.InnerException ?? t.Exception; - Logger.Error(e, $@"Could not import ({Path.GetFileName(path)})"); + Logger.Error(t.Exception, $@"Could not import ({Path.GetFileName(path)})"); } }, TaskContinuationOptions.NotOnCanceled))); From 9bdc8b47bb1e1c8ab8e57b48d0d8604d94ae5d3e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 10 Jun 2019 16:13:51 +0900 Subject: [PATCH 26/60] Remove unnecessary async-await pair --- osu.Game/Database/ArchiveModelManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index 4c4878edee..fb7f0ffe5d 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -577,7 +577,7 @@ namespace osu.Game.Database /// The model to populate. /// The archive to use as a reference for population. May be null. /// An optional cancellation token. - protected virtual async Task Populate(TModel model, [CanBeNull] ArchiveReader archive, CancellationToken cancellationToken = default) => await Task.CompletedTask; + protected virtual Task Populate(TModel model, [CanBeNull] ArchiveReader archive, CancellationToken cancellationToken = default) => Task.CompletedTask; /// /// Perform any final actions before the import to database executes. From fae32b390171738feed00336df54db7046c9ecd8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 10 Jun 2019 16:14:11 +0900 Subject: [PATCH 27/60] Return shorter class name in error messages --- osu.Game/Database/ArchiveModelManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index fb7f0ffe5d..e2a07a860f 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -331,7 +331,7 @@ namespace osu.Game.Database if (CanUndelete(existing, item)) { Undelete(existing); - Logger.Log($"Found existing {typeof(TModel)} for {item} (ID {existing.ID}). Skipping import.", LoggingTarget.Database); + Logger.Log($"Found existing {nameof(TModel)} for {item} (ID {existing.ID}). Skipping import.", LoggingTarget.Database); handleEvent(() => ItemAdded?.Invoke(existing, true)); return existing; } From 02b376d962d364f7b2525e8478dd4c4e3dd7cf04 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 10 Jun 2019 16:14:42 +0900 Subject: [PATCH 28/60] Fix rollback logic not necessrily cleaning up file store --- osu.Game/Database/ArchiveModelManager.cs | 63 +++++++++++------------- 1 file changed, 29 insertions(+), 34 deletions(-) diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index e2a07a860f..45460dd11c 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -154,25 +154,22 @@ namespace osu.Game.Database await Task.WhenAll(paths.Select(path => Import(path, notification.CancellationToken).ContinueWith(t => { - if (notification.CancellationToken.IsCancellationRequested) - return; + notification.CancellationToken.ThrowIfCancellationRequested(); - lock (notification) + lock (imported) { - current++; + Interlocked.Increment(ref current); - notification.Text = $"Imported {current} of {paths.Length} {term}s"; - notification.Progress = (float)current / paths.Length; - } - - if (t.Exception == null) - { - lock (imported) + if (t.Exception == null) + { imported.Add(t.Result); - } - else - { - Logger.Error(t.Exception, $@"Could not import ({Path.GetFileName(path)})"); + notification.Text = $"Imported {current} of {paths.Length} {term}s"; + notification.Progress = (float)current / paths.Length; + } + else + { + Logger.Error(t.Exception, $@"Could not import ({Path.GetFileName(path)})"); + } } }, TaskContinuationOptions.NotOnCanceled))); @@ -300,23 +297,23 @@ namespace osu.Game.Database delayEvents(); + void rollback() + { + if (!Delete(item)) + { + // We may have not yet added the model to the underlying table, but should still clean up files. + Logger.Log($"Dereferencing files for incomplete import of {item}.", LoggingTarget.Database); + Files.Dereference(item.Files.Select(f => f.FileInfo).ToArray()); + } + } + try { Logger.Log($"Importing {item}...", LoggingTarget.Database); item.Files = archive != null ? createFileInfos(archive, Files) : new List(); - var localItem = item; - - try - { - await Populate(item, archive, cancellationToken); - } - catch (Exception) - { - if (!Delete(localItem)) - Files.Dereference(localItem.Files.Select(f => f.FileInfo).ToArray()); - } + await Populate(item, archive, cancellationToken); using (var write = ContextFactory.GetForWrite()) // used to share a context for full import. keep in mind this will block all writes. { @@ -333,6 +330,8 @@ namespace osu.Game.Database Undelete(existing); Logger.Log($"Found existing {nameof(TModel)} for {item} (ID {existing.ID}). Skipping import.", LoggingTarget.Database); handleEvent(() => ItemAdded?.Invoke(existing, true)); + + rollback(); return existing; } else @@ -349,9 +348,6 @@ namespace osu.Game.Database } catch (Exception e) { - if (!Delete(item)) - Files.Dereference(item.Files.Select(f => f.FileInfo).ToArray()); - write.Errors.Add(e); throw; } @@ -359,13 +355,12 @@ namespace osu.Game.Database Logger.Log($"Import of {item} successfully completed!", LoggingTarget.Database); } - catch (TaskCanceledException) - { - throw; - } catch (Exception e) { - Logger.Error(e, $"Import of {item} failed and has been rolled back.", LoggingTarget.Database); + if (!(e is TaskCanceledException)) + Logger.Error(e, $"Import of {item} failed and has been rolled back.", LoggingTarget.Database); + + rollback(); item = null; } finally From 5b75060b94999e3a50a665a4e40cee3994162fa2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 10 Jun 2019 16:45:45 +0900 Subject: [PATCH 29/60] Add test for rollback logic correctly dereferencing files --- osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs | 13 +++++++++++++ osu.Game/IO/FileStore.cs | 10 ++++++++++ 2 files changed, 23 insertions(+) diff --git a/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs b/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs index 4c9260f640..3f4f40781c 100644 --- a/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs +++ b/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs @@ -12,6 +12,7 @@ using osu.Framework.Platform; using osu.Game.IPC; using osu.Framework.Allocation; using osu.Game.Beatmaps; +using osu.Game.IO; using osu.Game.Tests.Resources; using SharpCompress.Archives.Zip; @@ -97,6 +98,7 @@ namespace osu.Game.Tests.Beatmaps.IO { var osu = loadOsu(host); var manager = osu.Dependencies.Get(); + var files = osu.Dependencies.Get(); int fireCount = 0; @@ -113,6 +115,12 @@ namespace osu.Game.Tests.Beatmaps.IO Assert.AreEqual(0, fireCount -= 2); + Assert.AreEqual(1, manager.GetAllUsableBeatmapSets().Count); + Assert.AreEqual(1, manager.QueryBeatmapSets(_ => true).ToList().Count); + Assert.AreEqual(12, manager.QueryBeatmaps(_ => true).ToList().Count); + + Assert.AreEqual(18, files.QueryFiles(_ => true).Count()); + var breakTemp = TestResources.GetTestBeatmapForImport(); MemoryStream brokenOsu = new MemoryStream(new byte[] { 1, 3, 3, 7 }); @@ -131,6 +139,8 @@ namespace osu.Game.Tests.Beatmaps.IO Assert.AreEqual(1, manager.QueryBeatmapSets(_ => true).ToList().Count); Assert.AreEqual(12, manager.QueryBeatmaps(_ => true).ToList().Count); + Assert.AreEqual(18, files.QueryFiles(_ => true).Count()); + // this will trigger purging of the existing beatmap (online set id match) but should rollback due to broken osu. await manager.Import(breakTemp); @@ -140,6 +150,9 @@ namespace osu.Game.Tests.Beatmaps.IO Assert.AreEqual(1, manager.GetAllUsableBeatmapSets().Count); Assert.AreEqual(1, manager.QueryBeatmapSets(_ => true).ToList().Count); Assert.AreEqual(12, manager.QueryBeatmaps(_ => true).ToList().Count); + + Assert.AreEqual(18, files.QueryFiles(_ => true).Count()); + Assert.AreEqual(18, files.QueryFiles(f => f.ReferenceCount == 1).Count()); } finally { diff --git a/osu.Game/IO/FileStore.cs b/osu.Game/IO/FileStore.cs index 458f8964f9..370d6786f5 100644 --- a/osu.Game/IO/FileStore.cs +++ b/osu.Game/IO/FileStore.cs @@ -2,8 +2,11 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Collections.Generic; using System.IO; using System.Linq; +using System.Linq.Expressions; +using Microsoft.EntityFrameworkCore; using osu.Framework.Extensions; using osu.Framework.IO.Stores; using osu.Framework.Logging; @@ -27,6 +30,13 @@ namespace osu.Game.IO Store = new StorageBackedResourceStore(Storage); } + /// + /// Perform a lookup query on available s. + /// + /// The query. + /// Results from the provided query. + public IEnumerable QueryFiles(Expression> query) => ContextFactory.Get().Set().AsNoTracking().Where(f => f.ReferenceCount > 0).Where(query); + public FileInfo Add(Stream data, bool reference = true) { using (var usage = ContextFactory.GetForWrite()) From 559413f766fef4fef2f8c16e59e61a39f100d785 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 10 Jun 2019 17:12:25 +0900 Subject: [PATCH 30/60] Avoid using ContinueWith in already async context --- osu.Game/Database/ArchiveModelManager.cs | 29 ++++++++++++++++-------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index 45460dd11c..d09aa37d7e 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -152,26 +152,35 @@ namespace osu.Game.Database var imported = new List(); - await Task.WhenAll(paths.Select(path => Import(path, notification.CancellationToken).ContinueWith(t => + await Task.WhenAll(paths.Select(async path => { notification.CancellationToken.ThrowIfCancellationRequested(); - lock (imported) + try { - Interlocked.Increment(ref current); + var model = await Import(path, notification.CancellationToken); - if (t.Exception == null) + lock (imported) { - imported.Add(t.Result); + imported.Add(model); + notification.Text = $"Imported {current} of {paths.Length} {term}s"; notification.Progress = (float)current / paths.Length; } - else - { - Logger.Error(t.Exception, $@"Could not import ({Path.GetFileName(path)})"); - } } - }, TaskContinuationOptions.NotOnCanceled))); + catch (TaskCanceledException) + { + throw; + } + catch (Exception e) + { + Logger.Error(e, $@"Could not import ({Path.GetFileName(path)})"); + } + finally + { + Interlocked.Increment(ref current); + } + })); if (imported.Count == 0) { From c8bd92659bab551a36e0c40dc1b604deca0e4fdb Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 10 Jun 2019 17:12:37 +0900 Subject: [PATCH 31/60] Clean up exception and null handling in Import process --- osu.Game/Database/ArchiveModelManager.cs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index d09aa37d7e..fa8301bb2e 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -252,15 +252,15 @@ namespace osu.Game.Database { cancellationToken.ThrowIfCancellationRequested(); + TModel model; + try { - var model = CreateModel(archive); + model = CreateModel(archive); if (model == null) return null; model.Hash = computeHash(archive); - - return await Import(model, archive, cancellationToken); } catch (TaskCanceledException) { @@ -271,6 +271,8 @@ namespace osu.Game.Database Logger.Error(e, $"Model creation of {archive.Name} failed.", LoggingTarget.Database); return null; } + + return await Import(model, archive, cancellationToken); } /// @@ -340,7 +342,9 @@ namespace osu.Game.Database Logger.Log($"Found existing {nameof(TModel)} for {item} (ID {existing.ID}). Skipping import.", LoggingTarget.Database); handleEvent(() => ItemAdded?.Invoke(existing, true)); + // existing item will be used; rollback new import and exit early. rollback(); + flushEvents(true); return existing; } else @@ -370,14 +374,11 @@ namespace osu.Game.Database Logger.Error(e, $"Import of {item} failed and has been rolled back.", LoggingTarget.Database); rollback(); - item = null; - } - finally - { - // we only want to flush events after we've confirmed the write context didn't have any errors. - flushEvents(item != null); + flushEvents(false); + throw; } + flushEvents(true); return item; }, cancellationToken, TaskCreationOptions.HideScheduler, IMPORT_SCHEDULER).Unwrap(); From dcdb806120c6b581af4be341b60bd84cba5ea9c3 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 10 Jun 2019 17:26:56 +0900 Subject: [PATCH 32/60] Catch newly thrown exception in test --- osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs b/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs index 3f4f40781c..7f08674a95 100644 --- a/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs +++ b/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs @@ -135,14 +135,14 @@ namespace osu.Game.Tests.Beatmaps.IO zip.SaveTo(outStream, SharpCompress.Common.CompressionType.Deflate); } - Assert.AreEqual(1, manager.GetAllUsableBeatmapSets().Count); - Assert.AreEqual(1, manager.QueryBeatmapSets(_ => true).ToList().Count); - Assert.AreEqual(12, manager.QueryBeatmaps(_ => true).ToList().Count); - - Assert.AreEqual(18, files.QueryFiles(_ => true).Count()); - // this will trigger purging of the existing beatmap (online set id match) but should rollback due to broken osu. - await manager.Import(breakTemp); + try + { + await manager.Import(breakTemp); + } + catch + { + } // no events should be fired in the case of a rollback. Assert.AreEqual(0, fireCount); From 28b2a516e34354eb0f2d958c2f55da91dd8d4f78 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 10 Jun 2019 18:13:33 +0900 Subject: [PATCH 33/60] Ensure exception is only thrown once on rollback --- .../Beatmaps/IO/ImportBeatmapTest.cs | 24 +++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs b/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs index 7f08674a95..f3680e3c1e 100644 --- a/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs +++ b/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs @@ -11,6 +11,7 @@ using NUnit.Framework; using osu.Framework.Platform; using osu.Game.IPC; using osu.Framework.Allocation; +using osu.Framework.Logging; using osu.Game.Beatmaps; using osu.Game.IO; using osu.Game.Tests.Resources; @@ -96,24 +97,31 @@ namespace osu.Game.Tests.Beatmaps.IO { try { + int itemAddRemoveFireCount = 0; + int loggedExceptionCount = 0; + + Logger.NewEntry += l => + { + if (l.Target == LoggingTarget.Database && l.Exception != null) + Interlocked.Increment(ref loggedExceptionCount); + }; + var osu = loadOsu(host); var manager = osu.Dependencies.Get(); var files = osu.Dependencies.Get(); - int fireCount = 0; - // ReSharper disable once AccessToModifiedClosure - manager.ItemAdded += (_, __) => fireCount++; - manager.ItemRemoved += _ => fireCount++; + manager.ItemAdded += (_, __) => Interlocked.Increment(ref itemAddRemoveFireCount); + manager.ItemRemoved += _ => Interlocked.Increment(ref itemAddRemoveFireCount); var imported = await LoadOszIntoOsu(osu); - Assert.AreEqual(0, fireCount -= 1); + Assert.AreEqual(0, itemAddRemoveFireCount -= 1); imported.Hash += "-changed"; manager.Update(imported); - Assert.AreEqual(0, fireCount -= 2); + Assert.AreEqual(0, itemAddRemoveFireCount -= 2); Assert.AreEqual(1, manager.GetAllUsableBeatmapSets().Count); Assert.AreEqual(1, manager.QueryBeatmapSets(_ => true).ToList().Count); @@ -145,7 +153,7 @@ namespace osu.Game.Tests.Beatmaps.IO } // no events should be fired in the case of a rollback. - Assert.AreEqual(0, fireCount); + Assert.AreEqual(0, itemAddRemoveFireCount); Assert.AreEqual(1, manager.GetAllUsableBeatmapSets().Count); Assert.AreEqual(1, manager.QueryBeatmapSets(_ => true).ToList().Count); @@ -153,6 +161,8 @@ namespace osu.Game.Tests.Beatmaps.IO Assert.AreEqual(18, files.QueryFiles(_ => true).Count()); Assert.AreEqual(18, files.QueryFiles(f => f.ReferenceCount == 1).Count()); + + Assert.AreEqual(1, loggedExceptionCount); } finally { From 1aa865c3fb154d0ce88321f38cbef309c7799c22 Mon Sep 17 00:00:00 2001 From: David Zhao Date: Mon, 10 Jun 2019 18:34:24 +0900 Subject: [PATCH 34/60] split out method for reuse --- .../Visual/SongSelect/TestScenePlaySongSelect.cs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index 8636890081..d6b5e0175f 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -138,7 +138,7 @@ namespace osu.Game.Tests.Visual.SongSelect { createSongSelect(); changeRuleset(2); - importForRuleset(0); + addRulesetImportStep(0); AddUntilStep("no selection", () => songSelect.Carousel.SelectedBeatmap == null); } @@ -147,8 +147,8 @@ namespace osu.Game.Tests.Visual.SongSelect { createSongSelect(); changeRuleset(2); - importForRuleset(2); - importForRuleset(1); + addRulesetImportStep(2); + addRulesetImportStep(1); AddUntilStep("has selection", () => songSelect.Carousel.SelectedBeatmap.RulesetID == 2); changeRuleset(1); @@ -223,7 +223,7 @@ namespace osu.Game.Tests.Visual.SongSelect }); AddRepeatStep($"Create beatmaps {test_count} times", () => { - manager.Import(createTestBeatmapSet(getImportId(), rulesets.AvailableRulesets.Where(r => r.ID == 0).ToArray())); + importForRuleset(0); Scheduler.AddDelayed(() => { @@ -240,15 +240,16 @@ namespace osu.Game.Tests.Visual.SongSelect { int? previousID = null; createSongSelect(); - importForRuleset(0); + addRulesetImportStep(0); AddStep("Move to last difficulty", () => songSelect.Carousel.SelectBeatmap(songSelect.Carousel.BeatmapSets.First().Beatmaps.Last())); AddStep("Store current ID", () => previousID = songSelect.Carousel.SelectedBeatmap.ID); AddStep("Hide first beatmap", () => manager.Hide(songSelect.Carousel.SelectedBeatmapSet.Beatmaps.First())); AddAssert("Selected beatmap has not changed", () => songSelect.Carousel.SelectedBeatmap.ID == previousID); } - private void importForRuleset(int id) => AddStep($"import test map for ruleset {id}", - () => manager.Import(createTestBeatmapSet(getImportId(), rulesets.AvailableRulesets.Where(r => r.ID == id).ToArray()))); + private void addRulesetImportStep(int id) => AddStep($"import test map for ruleset {id}", () => importForRuleset(id)); + + private void importForRuleset(int id) => manager.Import(createTestBeatmapSet(getImportId(), rulesets.AvailableRulesets.Where(r => r.ID == id).ToArray())); private static int importId; private int getImportId() => ++importId; From 12aa264657061f456405a65c0eaaf781f3461304 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 10 Jun 2019 18:35:23 +0900 Subject: [PATCH 35/60] Consolidate tests and check for file reference counts --- .../Beatmaps/IO/ImportBeatmapTest.cs | 51 ++++++++++++------- 1 file changed, 32 insertions(+), 19 deletions(-) diff --git a/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs b/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs index f3680e3c1e..5fc05a4b2f 100644 --- a/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs +++ b/osu.Game.Tests/Beatmaps/IO/ImportBeatmapTest.cs @@ -77,10 +77,8 @@ namespace osu.Game.Tests.Beatmaps.IO Assert.IsTrue(imported.ID == importedSecondTime.ID); Assert.IsTrue(imported.Beatmaps.First().ID == importedSecondTime.Beatmaps.First().ID); - var manager = osu.Dependencies.Get(); - - Assert.AreEqual(1, manager.GetAllUsableBeatmapSets().Count); - Assert.AreEqual(1, manager.QueryBeatmapSets(_ => true).ToList().Count); + checkBeatmapSetCount(osu, 1); + checkSingleReferencedFileCount(osu, 18); } finally { @@ -108,7 +106,6 @@ namespace osu.Game.Tests.Beatmaps.IO var osu = loadOsu(host); var manager = osu.Dependencies.Get(); - var files = osu.Dependencies.Get(); // ReSharper disable once AccessToModifiedClosure manager.ItemAdded += (_, __) => Interlocked.Increment(ref itemAddRemoveFireCount); @@ -123,11 +120,9 @@ namespace osu.Game.Tests.Beatmaps.IO Assert.AreEqual(0, itemAddRemoveFireCount -= 2); - Assert.AreEqual(1, manager.GetAllUsableBeatmapSets().Count); - Assert.AreEqual(1, manager.QueryBeatmapSets(_ => true).ToList().Count); - Assert.AreEqual(12, manager.QueryBeatmaps(_ => true).ToList().Count); - - Assert.AreEqual(18, files.QueryFiles(_ => true).Count()); + checkBeatmapSetCount(osu, 1); + checkBeatmapCount(osu, 12); + checkSingleReferencedFileCount(osu, 18); var breakTemp = TestResources.GetTestBeatmapForImport(); @@ -155,12 +150,10 @@ namespace osu.Game.Tests.Beatmaps.IO // no events should be fired in the case of a rollback. Assert.AreEqual(0, itemAddRemoveFireCount); - Assert.AreEqual(1, manager.GetAllUsableBeatmapSets().Count); - Assert.AreEqual(1, manager.QueryBeatmapSets(_ => true).ToList().Count); - Assert.AreEqual(12, manager.QueryBeatmaps(_ => true).ToList().Count); + checkBeatmapSetCount(osu, 1); + checkBeatmapCount(osu, 12); - Assert.AreEqual(18, files.QueryFiles(_ => true).Count()); - Assert.AreEqual(18, files.QueryFiles(f => f.ReferenceCount == 1).Count()); + checkSingleReferencedFileCount(osu, 18); Assert.AreEqual(1, loggedExceptionCount); } @@ -193,8 +186,7 @@ namespace osu.Game.Tests.Beatmaps.IO Assert.IsTrue(imported.Beatmaps.First().ID < importedSecondTime.Beatmaps.First().ID); // only one beatmap will exist as the online set ID matched, causing purging of the first import. - Assert.AreEqual(1, manager.GetAllUsableBeatmapSets().Count); - Assert.AreEqual(1, manager.QueryBeatmapSets(_ => true).ToList().Count); + checkBeatmapSetCount(osu, 1); } finally { @@ -396,11 +388,32 @@ namespace osu.Game.Tests.Beatmaps.IO var manager = osu.Dependencies.Get(); manager.Delete(imported); - Assert.IsTrue(manager.GetAllUsableBeatmapSets().Count == 0); - Assert.AreEqual(1, manager.QueryBeatmapSets(_ => true).ToList().Count); + checkBeatmapSetCount(osu, 0); + checkBeatmapSetCount(osu, 1, true); + checkSingleReferencedFileCount(osu, 0); + Assert.IsTrue(manager.QueryBeatmapSets(_ => true).First().DeletePending); } + private void checkBeatmapSetCount(OsuGameBase osu, int expected, bool includeDeletePending = false) + { + var manager = osu.Dependencies.Get(); + + Assert.AreEqual(expected, includeDeletePending + ? manager.QueryBeatmapSets(_ => true).ToList().Count + : manager.GetAllUsableBeatmapSets().Count); + } + + private void checkBeatmapCount(OsuGameBase osu, int expected) + { + Assert.AreEqual(expected, osu.Dependencies.Get().QueryBeatmaps(_ => true).ToList().Count); + } + + private void checkSingleReferencedFileCount(OsuGameBase osu, int expected) + { + Assert.AreEqual(expected, osu.Dependencies.Get().QueryFiles(f => f.ReferenceCount == 1).Count()); + } + private OsuGameBase loadOsu(GameHost host) { var osu = new OsuGameBase(); From f7a699e4a2e0031224661370ba840d7d9327df3f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 10 Jun 2019 18:38:03 +0900 Subject: [PATCH 36/60] Better documentation for import scheduler singleton --- osu.Game/Database/ArchiveModelManager.cs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index fa8301bb2e..afc2690106 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -630,7 +630,15 @@ namespace osu.Game.Database public abstract class ArchiveModelManager { - // allow sharing static across all generic types - protected static readonly ThreadedTaskScheduler IMPORT_SCHEDULER = new ThreadedTaskScheduler(1); + private const int import_queue_request_concurrency = 1; + + /// + /// A singleton scheduler shared by all . + /// + /// + /// This scheduler generally performs IO and CPU intensive work so concurrency is limited harshly. + /// It is mainly being used as a queue mechanism for large imports. + /// + protected static readonly ThreadedTaskScheduler IMPORT_SCHEDULER = new ThreadedTaskScheduler(import_queue_request_concurrency); } } From 41e3e2222bc7d9c75e44a1340144c2d52e1f7acb Mon Sep 17 00:00:00 2001 From: David Zhao Date: Mon, 10 Jun 2019 18:40:49 +0900 Subject: [PATCH 37/60] fix test --- osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index d6b5e0175f..bc15bc6b6d 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -219,7 +219,7 @@ namespace osu.Game.Tests.Visual.SongSelect AddStep("Setup counter", () => { beatmapChangedCount = 0; - songSelect.Carousel.BeatmapSetsChanged += () => beatmapChangedCount++; + songSelect.Carousel.SelectionChanged += _ => beatmapChangedCount++; }); AddRepeatStep($"Create beatmaps {test_count} times", () => { From 6cda2cdb8225000f1a7de5e8c842a8d3f0a02aaf Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 10 Jun 2019 18:41:56 +0900 Subject: [PATCH 38/60] Fix exception output to use humanised model name --- osu.Game/Database/ArchiveModelManager.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index afc2690106..93e924fab5 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -146,8 +146,6 @@ namespace osu.Game.Database notification.Progress = 0; notification.Text = "Import is initialising..."; - var term = $"{typeof(TModel).Name.Replace("Info", "").ToLower()}"; - int current = 0; var imported = new List(); @@ -164,7 +162,7 @@ namespace osu.Game.Database { imported.Add(model); - notification.Text = $"Imported {current} of {paths.Length} {term}s"; + notification.Text = $"Imported {current} of {paths.Length} {humanisedModelName}s"; notification.Progress = (float)current / paths.Length; } } @@ -191,7 +189,7 @@ namespace osu.Game.Database { notification.CompletionText = imported.Count == 1 ? $"Imported {imported.First()}!" - : $"Imported {current} {term}s!"; + : $"Imported {current} {humanisedModelName}s!"; if (imported.Count > 0 && PresentImport != null) { @@ -339,7 +337,7 @@ namespace osu.Game.Database if (CanUndelete(existing, item)) { Undelete(existing); - Logger.Log($"Found existing {nameof(TModel)} for {item} (ID {existing.ID}). Skipping import.", LoggingTarget.Database); + Logger.Log($"Found existing {humanisedModelName} for {item} (ID {existing.ID}). Skipping import.", LoggingTarget.Database); handleEvent(() => ItemAdded?.Invoke(existing, true)); // existing item will be used; rollback new import and exit early. @@ -610,6 +608,8 @@ namespace osu.Game.Database private DbSet queryModel() => ContextFactory.Get().Set(); + private string humanisedModelName => $"{typeof(TModel).Name.Replace("Info", "").ToLower()}"; + /// /// Creates an from a valid storage path. /// From 54497fb1e78dd5447111cd256067f3959c1a61dd Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 10 Jun 2019 19:33:23 +0900 Subject: [PATCH 39/60] Fix prefixing spaces in BeatmapInfo's ToString when metadata is not populated yet --- osu.Game/Beatmaps/BeatmapInfo.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Beatmaps/BeatmapInfo.cs b/osu.Game/Beatmaps/BeatmapInfo.cs index 52238c26fe..3c082bb71e 100644 --- a/osu.Game/Beatmaps/BeatmapInfo.cs +++ b/osu.Game/Beatmaps/BeatmapInfo.cs @@ -119,7 +119,7 @@ namespace osu.Game.Beatmaps /// public List Scores { get; set; } - public override string ToString() => $"{Metadata} [{Version}]"; + public override string ToString() => $"{Metadata} [{Version}]".Trim(); public bool Equals(BeatmapInfo other) { From 29945f27c5f22bcbb36ed693d813fd34707053ee Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 10 Jun 2019 19:33:55 +0900 Subject: [PATCH 40/60] Fix imported count incrementing on failures --- osu.Game/Database/ArchiveModelManager.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index 93e924fab5..4e9478dc10 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -162,6 +162,7 @@ namespace osu.Game.Database { imported.Add(model); + Interlocked.Increment(ref current); notification.Text = $"Imported {current} of {paths.Length} {humanisedModelName}s"; notification.Progress = (float)current / paths.Length; } @@ -174,10 +175,6 @@ namespace osu.Game.Database { Logger.Error(e, $@"Could not import ({Path.GetFileName(path)})"); } - finally - { - Interlocked.Increment(ref current); - } })); if (imported.Count == 0) From 6ca2fcebfce6467398d12fc69911ffb3f1c03eff Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 10 Jun 2019 19:34:32 +0900 Subject: [PATCH 41/60] Centalise and prefix all ArchiveModelManager database logging --- osu.Game/Beatmaps/BeatmapManager.cs | 28 ++++++++++-------- osu.Game/Database/ArchiveModelManager.cs | 36 +++++++++++++++--------- 2 files changed, 39 insertions(+), 25 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 435edcf722..47411c69ec 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -110,7 +110,7 @@ namespace osu.Game.Beatmaps validateOnlineIds(beatmapSet); - await Task.WhenAll(beatmapSet.Beatmaps.Select(b => updateQueue.Perform(b, cancellationToken)).ToArray()); + await updateQueue.Perform(beatmapSet, cancellationToken); } protected override void PreImport(BeatmapSetInfo beatmapSet) @@ -127,7 +127,7 @@ namespace osu.Game.Beatmaps { Delete(existingOnlineId); beatmaps.PurgeDeletable(s => s.ID == existingOnlineId.ID); - Logger.Log($"Found existing beatmap set with same OnlineBeatmapSetID ({beatmapSet.OnlineBeatmapSetID}). It has been purged.", LoggingTarget.Database); + LogForModel(beatmapSet, $"Found existing beatmap set with same OnlineBeatmapSetID ({beatmapSet.OnlineBeatmapSetID}). It has been purged."); } } } @@ -433,23 +433,29 @@ namespace osu.Game.Beatmaps this.api = api; } - public Task Perform(BeatmapInfo beatmap, CancellationToken cancellationToken) - => Task.Factory.StartNew(() => perform(beatmap, cancellationToken), cancellationToken, TaskCreationOptions.HideScheduler, updateScheduler); - - private void perform(BeatmapInfo beatmap, CancellationToken cancellation) + public async Task Perform(BeatmapSetInfo beatmapSet, CancellationToken cancellationToken) { - cancellation.ThrowIfCancellationRequested(); - if (api?.State != APIState.Online) return; - Logger.Log("Attempting online lookup for the missing values...", LoggingTarget.Database); + LogForModel(beatmapSet, "Performing online lookups..."); + await Task.WhenAll(beatmapSet.Beatmaps.Select(b => Perform(beatmapSet, b, cancellationToken)).ToArray()); + } + + // todo: expose this when we need to do individual difficulty lookups. + protected Task Perform(BeatmapSetInfo beatmapSet, BeatmapInfo beatmap, CancellationToken cancellationToken) + => Task.Factory.StartNew(() => perform(beatmapSet, beatmap), cancellationToken, TaskCreationOptions.HideScheduler, updateScheduler); + + private void perform(BeatmapSetInfo set, BeatmapInfo beatmap) + { + if (api?.State != APIState.Online) + return; var req = new GetBeatmapRequest(beatmap); req.Success += res => { - Logger.Log($"Successfully mapped to {res.OnlineBeatmapSetID} / {res.OnlineBeatmapID}.", LoggingTarget.Database); + LogForModel(set, $"Online retrieval mapped {beatmap} to {res.OnlineBeatmapSetID} / {res.OnlineBeatmapID}."); beatmap.Status = res.Status; beatmap.BeatmapSet.Status = res.BeatmapSet.Status; @@ -457,7 +463,7 @@ namespace osu.Game.Beatmaps beatmap.OnlineBeatmapID = res.OnlineBeatmapID; }; - req.Failure += e => { Logger.Log($"Failed ({e})", LoggingTarget.Database); }; + req.Failure += e => { LogForModel(set, $"Online retrieval failed for {beatmap}", e); }; // intentionally blocking to limit web request concurrency req.Perform(api); diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index 4e9478dc10..844ae622a5 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -173,7 +173,7 @@ namespace osu.Game.Database } catch (Exception e) { - Logger.Error(e, $@"Could not import ({Path.GetFileName(path)})"); + Logger.Error(e, $@"Could not import ({Path.GetFileName(path)})", LoggingTarget.Database); } })); @@ -227,7 +227,7 @@ namespace osu.Game.Database } catch (Exception e) { - Logger.Error(e, $@"Could not delete original file after import ({Path.GetFileName(path)})"); + LogForModel(import, $@"Could not delete original file after import ({Path.GetFileName(path)})", e); } return import; @@ -247,7 +247,7 @@ namespace osu.Game.Database { cancellationToken.ThrowIfCancellationRequested(); - TModel model; + TModel model = null; try { @@ -263,7 +263,7 @@ namespace osu.Game.Database } catch (Exception e) { - Logger.Error(e, $"Model creation of {archive.Name} failed.", LoggingTarget.Database); + LogForModel(model, $"Model creation of {archive.Name} failed.", e); return null; } @@ -277,6 +277,16 @@ namespace osu.Game.Database /// protected abstract string[] HashableFileTypes { get; } + protected static void LogForModel(TModel model, string message, Exception e = null) + { + string prefix = $"[{(model?.Hash ?? "?????").Substring(0, 5)}]"; + + if (e != null) + Logger.Error(e, $"{prefix} {message}", LoggingTarget.Database); + else + Logger.Log($"{prefix} {message}", LoggingTarget.Database); + } + /// /// Create a SHA-2 hash from the provided archive based on file content of all files matching . /// @@ -308,14 +318,14 @@ namespace osu.Game.Database if (!Delete(item)) { // We may have not yet added the model to the underlying table, but should still clean up files. - Logger.Log($"Dereferencing files for incomplete import of {item}.", LoggingTarget.Database); + LogForModel(item, "Dereferencing files for incomplete import."); Files.Dereference(item.Files.Select(f => f.FileInfo).ToArray()); } } try { - Logger.Log($"Importing {item}...", LoggingTarget.Database); + LogForModel(item, "Beginning import..."); item.Files = archive != null ? createFileInfos(archive, Files) : new List(); @@ -334,7 +344,7 @@ namespace osu.Game.Database if (CanUndelete(existing, item)) { Undelete(existing); - Logger.Log($"Found existing {humanisedModelName} for {item} (ID {existing.ID}). Skipping import.", LoggingTarget.Database); + LogForModel(item, $"Found existing {humanisedModelName} for {item} (ID {existing.ID}) – skipping import."); handleEvent(() => ItemAdded?.Invoke(existing, true)); // existing item will be used; rollback new import and exit early. @@ -342,11 +352,9 @@ namespace osu.Game.Database flushEvents(true); return existing; } - else - { - Delete(existing); - ModelStore.PurgeDeletable(s => s.ID == existing.ID); - } + + Delete(existing); + ModelStore.PurgeDeletable(s => s.ID == existing.ID); } PreImport(item); @@ -361,12 +369,12 @@ namespace osu.Game.Database } } - Logger.Log($"Import of {item} successfully completed!", LoggingTarget.Database); + LogForModel(item, "Import successfully completed!"); } catch (Exception e) { if (!(e is TaskCanceledException)) - Logger.Error(e, $"Import of {item} failed and has been rolled back.", LoggingTarget.Database); + LogForModel(item, "Database import or population failed and has been rolled back.", e); rollback(); flushEvents(false); From 2b768bef96e28f438116f9458ce61130e76c17a5 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Mon, 10 Jun 2019 20:29:01 +0900 Subject: [PATCH 42/60] Make CursorTrail use VertexBatch --- .../UI/Cursor/CursorTrail.cs | 76 ++++++------------- 1 file changed, 22 insertions(+), 54 deletions(-) diff --git a/osu.Game.Rulesets.Osu/UI/Cursor/CursorTrail.cs b/osu.Game.Rulesets.Osu/UI/Cursor/CursorTrail.cs index 2276b9f9f4..b986076593 100644 --- a/osu.Game.Rulesets.Osu/UI/Cursor/CursorTrail.cs +++ b/osu.Game.Rulesets.Osu/UI/Cursor/CursorTrail.cs @@ -6,7 +6,7 @@ using System.Diagnostics; using System.Runtime.InteropServices; using osu.Framework.Allocation; using osu.Framework.Graphics; -using osu.Framework.Graphics.OpenGL.Buffers; +using osu.Framework.Graphics.Batches; using osu.Framework.Graphics.OpenGL.Vertices; using osu.Framework.Graphics.Primitives; using osu.Framework.Graphics.Shaders; @@ -57,7 +57,6 @@ namespace osu.Game.Rulesets.Osu.UI.Cursor // InvalidationID 1 forces an update of each part of the cursor trail the first time ApplyState is run on the draw node // This is to prevent garbage data from being sent to the vertex shader, resulting in visual issues on some platforms parts[i].InvalidationID = 1; - parts[i].WasUpdated = true; } } @@ -149,7 +148,6 @@ namespace osu.Game.Rulesets.Osu.UI.Cursor public Vector2 Position; public float Time; public long InvalidationID; - public bool WasUpdated; } private class TrailDrawNode : DrawNode @@ -164,16 +162,13 @@ namespace osu.Game.Rulesets.Osu.UI.Cursor private readonly TrailPart[] parts = new TrailPart[max_sprites]; private Vector2 size; - private readonly VertexBuffer vertexBuffer = new QuadVertexBuffer(max_sprites, BufferUsageHint.DynamicDraw); + private readonly VertexBatch vertexBatch = new QuadBatch(max_sprites, 1); public TrailDrawNode(CursorTrail source) : base(source) { for (int i = 0; i < max_sprites; i++) - { parts[i].InvalidationID = 0; - parts[i].WasUpdated = false; - } } public override void ApplyState() @@ -194,56 +189,29 @@ namespace osu.Game.Rulesets.Osu.UI.Cursor public override void Draw(Action vertexAction) { - shader.GetUniform("g_FadeClock").UpdateValue(ref time); - - int updateStart = -1, updateEnd = 0; - - for (int i = 0; i < parts.Length; ++i) - { - if (parts[i].WasUpdated) - { - if (updateStart == -1) - updateStart = i; - updateEnd = i + 1; - - int start = i * 4; - int end = start; - - Vector2 pos = parts[i].Position; - float localTime = parts[i].Time; - - DrawQuad( - texture, - new Quad(pos.X - size.X / 2, pos.Y - size.Y / 2, size.X, size.Y), - DrawColourInfo.Colour, - null, - v => vertexBuffer.Vertices[end++] = new TexturedTrailVertex - { - Position = v.Position, - TexturePosition = v.TexturePosition, - Time = localTime + 1, - Colour = v.Colour, - }); - - parts[i].WasUpdated = false; - } - else if (updateStart != -1) - { - vertexBuffer.UpdateRange(updateStart * 4, updateEnd * 4); - updateStart = -1; - } - } - - // Update all remaining vertices that have been changed. - if (updateStart != -1) - vertexBuffer.UpdateRange(updateStart * 4, updateEnd * 4); - base.Draw(vertexAction); shader.Bind(); + shader.GetUniform("g_FadeClock").UpdateValue(ref time); - texture.TextureGL.Bind(); - vertexBuffer.Draw(); + for (int i = 0; i < parts.Length; ++i) + { + Vector2 pos = parts[i].Position; + float localTime = parts[i].Time; + + DrawQuad( + texture, + new Quad(pos.X - size.X / 2, pos.Y - size.Y / 2, size.X, size.Y), + DrawColourInfo.Colour, + null, + v => vertexBatch.Add(new TexturedTrailVertex + { + Position = v.Position, + TexturePosition = v.TexturePosition, + Time = localTime + 1, + Colour = v.Colour, + })); + } shader.Unbind(); } @@ -252,7 +220,7 @@ namespace osu.Game.Rulesets.Osu.UI.Cursor { base.Dispose(isDisposing); - vertexBuffer.Dispose(); + vertexBatch.Dispose(); } } From e87123342cab2004c7baea09310a1ddf83b6b547 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 10 Jun 2019 23:50:44 +0900 Subject: [PATCH 43/60] Load results pages asynchronously Reduces performance burden when first displaying pages. Closes #4977. --- osu.Game/Screens/Ranking/Results.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Ranking/Results.cs b/osu.Game/Screens/Ranking/Results.cs index bebeaee00a..370c856d1d 100644 --- a/osu.Game/Screens/Ranking/Results.cs +++ b/osu.Game/Screens/Ranking/Results.cs @@ -275,7 +275,7 @@ namespace osu.Game.Screens.Ranking currentPage = page.NewValue?.CreatePage(); if (currentPage != null) - circleInner.Add(currentPage); + LoadComponentAsync(currentPage, circleInner.Add); }, true); } From 71e15fe0f112aa6fb693c54536bb486ad97eedef Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 11 Jun 2019 01:18:49 +0900 Subject: [PATCH 44/60] Fix incorrect xmldoc in OsuAnimatedButton --- osu.Game/Graphics/UserInterface/OsuAnimatedButton.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Graphics/UserInterface/OsuAnimatedButton.cs b/osu.Game/Graphics/UserInterface/OsuAnimatedButton.cs index 236b72766f..1a8fea4ff9 100644 --- a/osu.Game/Graphics/UserInterface/OsuAnimatedButton.cs +++ b/osu.Game/Graphics/UserInterface/OsuAnimatedButton.cs @@ -19,14 +19,14 @@ namespace osu.Game.Graphics.UserInterface public class OsuAnimatedButton : OsuClickableContainer { /// - /// The colour that should be flashed when the is clicked. + /// The colour that should be flashed when the is clicked. /// protected Color4 FlashColour = Color4.White.Opacity(0.3f); private Color4 hoverColour = Color4.White.Opacity(0.1f); /// - /// The background colour of the while it is hovered. + /// The background colour of the while it is hovered. /// protected Color4 HoverColour { From d4ba67747b37f4db9a0a482a5fef54470a6bcbfc Mon Sep 17 00:00:00 2001 From: David Zhao Date: Mon, 10 Jun 2019 18:58:03 +0900 Subject: [PATCH 45/60] fix test count --- .idea/.idea.osu/.idea/.name | 1 + .../Visual/SongSelect/TestScenePlaySongSelect.cs | 13 ++++++++++--- 2 files changed, 11 insertions(+), 3 deletions(-) create mode 100644 .idea/.idea.osu/.idea/.name diff --git a/.idea/.idea.osu/.idea/.name b/.idea/.idea.osu/.idea/.name new file mode 100644 index 0000000000..21cb4db60e --- /dev/null +++ b/.idea/.idea.osu/.idea/.name @@ -0,0 +1 @@ +osu \ No newline at end of file diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index bc15bc6b6d..fa73a14252 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -11,6 +11,7 @@ using osu.Framework.Allocation; using osu.Framework.Audio; using osu.Framework.Bindables; using osu.Framework.Extensions; +using osu.Framework.Logging; using osu.Framework.MathUtils; using osu.Framework.Platform; using osu.Framework.Screens; @@ -215,11 +216,13 @@ namespace osu.Game.Tests.Visual.SongSelect { const int test_count = 10; int beatmapChangedCount = 0; + int debounceCount = 0; createSongSelect(); - AddStep("Setup counter", () => + AddStep("Setup counters", () => { beatmapChangedCount = 0; - songSelect.Carousel.SelectionChanged += _ => beatmapChangedCount++; + debounceCount = 0; + songSelect.Carousel.SelectionChanged += _ => beatmapChangedCount += 1; }); AddRepeatStep($"Create beatmaps {test_count} times", () => { @@ -229,10 +232,14 @@ namespace osu.Game.Tests.Visual.SongSelect { // Wait for debounce songSelect.Carousel.SelectNextRandom(); + ++debounceCount; }, 400); }, test_count); - AddAssert($"Beatmap changed {test_count} times", () => beatmapChangedCount == test_count); + AddUntilStep("Debounce limit reached", () => debounceCount == test_count); + + // The selected beatmap should have changed an additional 2 times since both initially loading songselect and the first import also triggers selectionChanged + AddAssert($"Beatmap changed {test_count + 2} times", () => beatmapChangedCount == test_count + 2); } [Test] From 609a82bc948dc14f8e604aa932d71b470e8657a7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 11 Jun 2019 14:28:52 +0900 Subject: [PATCH 46/60] Update VisibilityContainer usage in line with framework --- osu.Desktop/OsuGameDesktop.cs | 9 ++---- .../TestSceneResumeOverlay.cs | 4 +-- osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs | 2 +- .../Visual/Gameplay/TestSceneFailAnimation.cs | 2 +- .../Gameplay/TestSceneGameplayMenuOverlay.cs | 6 ++-- .../Visual/Gameplay/TestScenePause.cs | 4 +-- .../Visual/Gameplay/TestSceneStoryboard.cs | 2 +- .../Visual/Menus/TestSceneToolbar.cs | 2 +- .../TestSceneMatchSettingsOverlay.cs | 2 +- .../Online/TestSceneAccountCreationOverlay.cs | 2 +- .../Visual/Online/TestSceneChatDisplay.cs | 2 +- .../Visual/Settings/TestSceneSettings.cs | 2 +- .../SongSelect/TestSceneBeatmapInfoWedge.cs | 9 +++--- .../Visual/UserInterface/TestSceneCursors.cs | 4 +-- .../UserInterface/TestSceneMusicController.cs | 4 +-- .../TestSceneNotificationOverlay.cs | 2 +- .../UserInterface/TestScenePopupDialog.cs | 2 +- .../Containers/OsuFocusedOverlayContainer.cs | 12 ++++---- osu.Game/Graphics/Containers/WaveContainer.cs | 4 +-- .../Graphics/Cursor/MenuCursorContainer.cs | 2 +- .../UserInterface/BreadcrumbControl.cs | 4 +++ .../UserInterface/ProcessingOverlay.cs | 2 +- osu.Game/OsuGame.cs | 28 +++++++++---------- osu.Game/Overlays/AccountCreationOverlay.cs | 2 +- osu.Game/Overlays/BeatmapSetOverlay.cs | 2 +- osu.Game/Overlays/ChangelogOverlay.cs | 8 +++--- osu.Game/Overlays/ChatOverlay.cs | 24 ++++++++-------- osu.Game/Overlays/DialogOverlay.cs | 8 +++--- osu.Game/Overlays/DirectOverlay.cs | 2 +- osu.Game/Overlays/Mods/ModSelectOverlay.cs | 4 +-- osu.Game/Overlays/Music/PlaylistOverlay.cs | 2 +- osu.Game/Overlays/MusicController.cs | 4 +-- osu.Game/Overlays/NotificationOverlay.cs | 8 +++--- .../Sections/General/LoginSettings.cs | 2 +- osu.Game/Overlays/SettingsOverlay.cs | 11 ++++---- osu.Game/Overlays/Toolbar/Toolbar.cs | 4 +-- .../Toolbar/ToolbarOverlayToggleButton.cs | 20 ++++++------- osu.Game/Overlays/VolumeOverlay.cs | 6 ++-- .../Rulesets/UI/GameplayCursorContainer.cs | 2 +- osu.Game/Screens/Menu/ButtonArea.cs | 8 ++++-- osu.Game/Screens/Play/GameplayMenuOverlay.cs | 2 +- .../Screens/Play/HUD/PlayerSettingsOverlay.cs | 2 +- osu.Game/Screens/Play/Player.cs | 2 +- osu.Game/Screens/Play/SkipOverlay.cs | 14 ++++++---- osu.Game/Screens/Play/SongProgress.cs | 2 +- osu.Game/Screens/Select/BeatmapDetails.cs | 4 +-- osu.Game/Screens/Select/BeatmapInfoWedge.cs | 2 +- osu.Game/Screens/Select/SongSelect.cs | 4 +-- osu.Game/osu.Game.csproj | 4 ++- osu.sln | 12 ++++++++ 50 files changed, 150 insertions(+), 127 deletions(-) diff --git a/osu.Desktop/OsuGameDesktop.cs b/osu.Desktop/OsuGameDesktop.cs index 00cabbadf7..975b7f9f5a 100644 --- a/osu.Desktop/OsuGameDesktop.cs +++ b/osu.Desktop/OsuGameDesktop.cs @@ -7,7 +7,6 @@ using System.Linq; using System.Reflection; using System.Threading.Tasks; using osu.Desktop.Overlays; -using osu.Framework.Graphics.Containers; using osu.Framework.Platform; using osu.Game; using osuTK.Input; @@ -56,7 +55,7 @@ namespace osu.Desktop LoadComponentAsync(versionManager = new VersionManager { Depth = int.MinValue }, v => { Add(v); - v.State = Visibility.Visible; + v.Show(); }); if (RuntimeInfo.OS == RuntimeInfo.Platform.Windows) @@ -74,13 +73,11 @@ namespace osu.Desktop { case Intro _: case MainMenu _: - if (versionManager != null) - versionManager.State = Visibility.Visible; + versionManager?.Show(); break; default: - if (versionManager != null) - versionManager.State = Visibility.Hidden; + versionManager?.Hide(); break; } } diff --git a/osu.Game.Rulesets.Osu.Tests/TestSceneResumeOverlay.cs b/osu.Game.Rulesets.Osu.Tests/TestSceneResumeOverlay.cs index 12a3a8d27e..8e73d6152f 100644 --- a/osu.Game.Rulesets.Osu.Tests/TestSceneResumeOverlay.cs +++ b/osu.Game.Rulesets.Osu.Tests/TestSceneResumeOverlay.cs @@ -46,11 +46,11 @@ namespace osu.Game.Rulesets.Osu.Tests AddStep("move mouse away", () => InputManager.MoveMouseTo(ScreenSpaceDrawQuad.TopLeft)); AddStep("click", () => osuInputManager.GameClick()); - AddAssert("not dismissed", () => !resumeFired && resume.State == Visibility.Visible); + AddAssert("not dismissed", () => !resumeFired && resume.State.Value == Visibility.Visible); AddStep("move mouse back", () => InputManager.MoveMouseTo(ScreenSpaceDrawQuad.Centre)); AddStep("click", () => osuInputManager.GameClick()); - AddAssert("dismissed", () => resumeFired && resume.State == Visibility.Hidden); + AddAssert("dismissed", () => resumeFired && resume.State.Value == Visibility.Hidden); } private class ManualOsuInputManager : OsuInputManager diff --git a/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs b/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs index 0d4e7edb7b..9e5df0d6b1 100644 --- a/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs +++ b/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs @@ -22,7 +22,7 @@ namespace osu.Game.Rulesets.Osu.UI private GameplayCursorContainer localCursorContainer; - public override CursorContainer LocalCursor => State == Visibility.Visible ? localCursorContainer : null; + public override CursorContainer LocalCursor => State.Value == Visibility.Visible ? localCursorContainer : null; protected override string Message => "Click the orange cursor to resume"; diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneFailAnimation.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneFailAnimation.cs index 4878587dcd..f06f72615b 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneFailAnimation.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneFailAnimation.cs @@ -28,7 +28,7 @@ namespace osu.Game.Tests.Visual.Gameplay protected override void AddCheckSteps() { AddUntilStep("wait for fail", () => Player.HasFailed); - AddUntilStep("wait for fail overlay", () => ((FailPlayer)Player).FailOverlay.State == Visibility.Visible); + AddUntilStep("wait for fail overlay", () => ((FailPlayer)Player).FailOverlay.State.Value == Visibility.Visible); } private class FailPlayer : TestPlayer diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneGameplayMenuOverlay.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneGameplayMenuOverlay.cs index ba9c583b08..4727140d99 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneGameplayMenuOverlay.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneGameplayMenuOverlay.cs @@ -97,7 +97,7 @@ namespace osu.Game.Tests.Visual.Gameplay AddStep("Show overlay", () => pauseOverlay.Show()); AddStep("Press select", () => press(GlobalAction.Select)); - AddAssert("Overlay still open", () => pauseOverlay.State == Visibility.Visible); + AddAssert("Overlay still open", () => pauseOverlay.State.Value == Visibility.Visible); AddStep("Hide overlay", () => pauseOverlay.Hide()); } @@ -237,7 +237,7 @@ namespace osu.Game.Tests.Visual.Gameplay }); AddAssert("Action was triggered", () => triggered); - AddAssert("Overlay is closed", () => pauseOverlay.State == Visibility.Hidden); + AddAssert("Overlay is closed", () => pauseOverlay.State.Value == Visibility.Hidden); } /// @@ -272,7 +272,7 @@ namespace osu.Game.Tests.Visual.Gameplay return triggered; }); - AddAssert("Overlay is closed", () => pauseOverlay.State == Visibility.Hidden); + AddAssert("Overlay is closed", () => pauseOverlay.State.Value == Visibility.Hidden); } private void press(Key key) diff --git a/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs b/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs index 12e91df77c..0de0fcfc38 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestScenePause.cs @@ -203,9 +203,9 @@ namespace osu.Game.Tests.Visual.Gameplay public new HUDOverlay HUDOverlay => base.HUDOverlay; - public bool FailOverlayVisible => FailOverlay.State == Visibility.Visible; + public bool FailOverlayVisible => FailOverlay.State.Value == Visibility.Visible; - public bool PauseOverlayVisible => PauseOverlay.State == Visibility.Visible; + public bool PauseOverlayVisible => PauseOverlay.State.Value == Visibility.Visible; public override void OnEntering(IScreen last) { diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneStoryboard.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneStoryboard.cs index 213cdf5e48..ead7a4b7fc 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneStoryboard.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneStoryboard.cs @@ -46,7 +46,7 @@ namespace osu.Game.Tests.Visual.Gameplay { Origin = Anchor.TopRight, Anchor = Anchor.TopRight, - State = Visibility.Visible, + State = { Value = Visibility.Visible }, }); AddStep("Restart", restart); diff --git a/osu.Game.Tests/Visual/Menus/TestSceneToolbar.cs b/osu.Game.Tests/Visual/Menus/TestSceneToolbar.cs index 0c789d8cb7..0df6605cdd 100644 --- a/osu.Game.Tests/Visual/Menus/TestSceneToolbar.cs +++ b/osu.Game.Tests/Visual/Menus/TestSceneToolbar.cs @@ -23,7 +23,7 @@ namespace osu.Game.Tests.Visual.Menus public TestSceneToolbar() { - var toolbar = new Toolbar { State = Visibility.Visible }; + var toolbar = new Toolbar { State = { Value = Visibility.Visible } }; ToolbarNotificationButton notificationButton = null; AddStep("create toolbar", () => diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMatchSettingsOverlay.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMatchSettingsOverlay.cs index 8091e93471..8d842fc865 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMatchSettingsOverlay.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMatchSettingsOverlay.cs @@ -36,7 +36,7 @@ namespace osu.Game.Tests.Visual.Multiplayer settings = new TestRoomSettings { RelativeSizeAxes = Axes.Both, - State = Visibility.Visible + State = { Value = Visibility.Visible } }; Child = settings; diff --git a/osu.Game.Tests/Visual/Online/TestSceneAccountCreationOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneAccountCreationOverlay.cs index a7e725ec3f..35449f5687 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneAccountCreationOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneAccountCreationOverlay.cs @@ -49,7 +49,7 @@ namespace osu.Game.Tests.Visual.Online api.Logout(); api.LocalUser.BindValueChanged(user => { userPanelArea.Child = new UserPanel(user.NewValue) { Width = 200 }; }, true); - AddStep("show", () => accountCreation.State = Visibility.Visible); + AddStep("show", () => accountCreation.Show()); AddStep("logout", () => api.Logout()); } } diff --git a/osu.Game.Tests/Visual/Online/TestSceneChatDisplay.cs b/osu.Game.Tests/Visual/Online/TestSceneChatDisplay.cs index 634176e65f..2789feef3d 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneChatDisplay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneChatDisplay.cs @@ -38,7 +38,7 @@ namespace osu.Game.Tests.Visual.Online Children = new Drawable[] { channelManager, - new ChatOverlay { State = Visibility.Visible } + new ChatOverlay { State = { Value = Visibility.Visible } } }; } } diff --git a/osu.Game.Tests/Visual/Settings/TestSceneSettings.cs b/osu.Game.Tests/Visual/Settings/TestSceneSettings.cs index 964754f8d0..f97ce8c69e 100644 --- a/osu.Game.Tests/Visual/Settings/TestSceneSettings.cs +++ b/osu.Game.Tests/Visual/Settings/TestSceneSettings.cs @@ -18,7 +18,7 @@ namespace osu.Game.Tests.Visual.Settings { settings = new SettingsOverlay { - State = Visibility.Visible + State = { Value = Visibility.Visible } }; Add(dialogOverlay = new DialogOverlay { diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapInfoWedge.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapInfoWedge.cs index 9969795ecf..932e114580 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapInfoWedge.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapInfoWedge.cs @@ -1,4 +1,4 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. using System.Collections.Generic; @@ -7,7 +7,6 @@ using JetBrains.Annotations; using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Graphics; -using osu.Framework.Graphics.Containers; using osu.Game.Beatmaps; using osu.Game.Graphics.Sprites; using osu.Game.Rulesets; @@ -48,7 +47,7 @@ namespace osu.Game.Tests.Visual.SongSelect AddStep("show", () => { - infoWedge.State = Visibility.Visible; + infoWedge.Show(); infoWedge.Beatmap = Beatmap.Value; }); @@ -57,11 +56,11 @@ namespace osu.Game.Tests.Visual.SongSelect AddWaitStep("wait for select", 3); - AddStep("hide", () => { infoWedge.State = Visibility.Hidden; }); + AddStep("hide", () => { infoWedge.Hide(); }); AddWaitStep("wait for hide", 3); - AddStep("show", () => { infoWedge.State = Visibility.Visible; }); + AddStep("show", () => { infoWedge.Show(); }); foreach (var rulesetInfo in rulesets.AvailableRulesets) { diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneCursors.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneCursors.cs index 8fe31b7ad6..e7dbbc8bc4 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneCursors.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneCursors.cs @@ -176,7 +176,7 @@ namespace osu.Game.Tests.Visual.UserInterface /// Checks if a cursor is visible. /// /// The cursor to check. - private bool checkVisible(CursorContainer cursorContainer) => cursorContainer.State == Visibility.Visible; + private bool checkVisible(CursorContainer cursorContainer) => cursorContainer.State.Value == Visibility.Visible; /// /// Checks if a cursor is at the current inputmanager screen position. @@ -218,7 +218,7 @@ namespace osu.Game.Tests.Visual.UserInterface }, Cursor = new TestCursorContainer { - State = providesUserCursor ? Visibility.Hidden : Visibility.Visible, + State = { Value = providesUserCursor ? Visibility.Hidden : Visibility.Visible }, } }; } diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneMusicController.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneMusicController.cs index a62fd6467b..2f2a40925f 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneMusicController.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneMusicController.cs @@ -23,8 +23,8 @@ namespace osu.Game.Tests.Visual.UserInterface }; Add(mc); - AddToggleStep(@"toggle visibility", state => mc.State = state ? Visibility.Visible : Visibility.Hidden); - AddStep(@"show", () => mc.State = Visibility.Visible); + AddToggleStep(@"toggle visibility", state => mc.State.Value = state ? Visibility.Visible : Visibility.Hidden); + AddStep(@"show", () => mc.Show()); AddToggleStep(@"toggle beatmap lock", state => Beatmap.Disabled = state); } } diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneNotificationOverlay.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneNotificationOverlay.cs index 71033fcd2f..6b7427cef5 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneNotificationOverlay.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneNotificationOverlay.cs @@ -45,7 +45,7 @@ namespace osu.Game.Tests.Visual.UserInterface Content.Add(displayedCount); - void setState(Visibility state) => AddStep(state.ToString(), () => manager.State = state); + void setState(Visibility state) => AddStep(state.ToString(), () => manager.State.Value = state); void checkProgressingCount(int expected) => AddAssert($"progressing count is {expected}", () => progressingNotifications.Count == expected); manager.UnreadCount.ValueChanged += count => { displayedCount.Text = $"displayed count: {count.NewValue}"; }; diff --git a/osu.Game.Tests/Visual/UserInterface/TestScenePopupDialog.cs b/osu.Game.Tests/Visual/UserInterface/TestScenePopupDialog.cs index 24140125e0..9ddd8f4038 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestScenePopupDialog.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestScenePopupDialog.cs @@ -16,7 +16,7 @@ namespace osu.Game.Tests.Visual.UserInterface var popup = new PopupDialog { RelativeSizeAxes = Axes.Both, - State = Framework.Graphics.Containers.Visibility.Visible, + State = { Value = Framework.Graphics.Containers.Visibility.Visible }, Icon = FontAwesome.Solid.AssistiveListeningSystems, HeaderText = @"This is a test popup", BodyText = "I can say lots of stuff and even wrap my words!", diff --git a/osu.Game/Graphics/Containers/OsuFocusedOverlayContainer.cs b/osu.Game/Graphics/Containers/OsuFocusedOverlayContainer.cs index 8b34459710..f6db3102f2 100644 --- a/osu.Game/Graphics/Containers/OsuFocusedOverlayContainer.cs +++ b/osu.Game/Graphics/Containers/OsuFocusedOverlayContainer.cs @@ -54,7 +54,7 @@ namespace osu.Game.Graphics.Containers samplePopIn = audio.Samples.Get(@"UI/overlay-pop-in"); samplePopOut = audio.Samples.Get(@"UI/overlay-pop-out"); - StateChanged += onStateChanged; + State.ValueChanged += onStateChanged; } /// @@ -70,7 +70,7 @@ namespace osu.Game.Graphics.Containers { if (!base.ReceivePositionalInputAt(e.ScreenSpaceMousePosition)) { - State = Visibility.Hidden; + Hide(); return true; } @@ -82,7 +82,7 @@ namespace osu.Game.Graphics.Containers switch (action) { case GlobalAction.Back: - State = Visibility.Hidden; + Hide(); return true; case GlobalAction.Select: @@ -94,9 +94,9 @@ namespace osu.Game.Graphics.Containers public bool OnReleased(GlobalAction action) => false; - private void onStateChanged(Visibility visibility) + private void onStateChanged(ValueChangedEvent state) { - switch (visibility) + switch (state.NewValue) { case Visibility.Visible: if (OverlayActivationMode.Value != OverlayActivation.Disabled) @@ -105,7 +105,7 @@ namespace osu.Game.Graphics.Containers if (BlockScreenWideMouse && DimMainContent) osuGame?.AddBlockingOverlay(this); } else - State = Visibility.Hidden; + Hide(); break; diff --git a/osu.Game/Graphics/Containers/WaveContainer.cs b/osu.Game/Graphics/Containers/WaveContainer.cs index 464682a0ad..f87909ab17 100644 --- a/osu.Game/Graphics/Containers/WaveContainer.cs +++ b/osu.Game/Graphics/Containers/WaveContainer.cs @@ -103,7 +103,7 @@ namespace osu.Game.Graphics.Containers protected override void PopIn() { foreach (var w in wavesContainer.Children) - w.State = Visibility.Visible; + w.Show(); this.FadeIn(100, Easing.OutQuint); contentContainer.MoveToY(0, APPEAR_DURATION, Easing.OutQuint); @@ -117,7 +117,7 @@ namespace osu.Game.Graphics.Containers contentContainer.MoveToY(DrawHeight * 2f, DISAPPEAR_DURATION, Easing.In); foreach (var w in wavesContainer.Children) - w.State = Visibility.Hidden; + w.Hide(); this.FadeOut(DISAPPEAR_DURATION, Easing.InQuint); } diff --git a/osu.Game/Graphics/Cursor/MenuCursorContainer.cs b/osu.Game/Graphics/Cursor/MenuCursorContainer.cs index 92e5ba6195..b7ea1ba56a 100644 --- a/osu.Game/Graphics/Cursor/MenuCursorContainer.cs +++ b/osu.Game/Graphics/Cursor/MenuCursorContainer.cs @@ -29,7 +29,7 @@ namespace osu.Game.Graphics.Cursor { AddRangeInternal(new Drawable[] { - Cursor = new MenuCursor { State = Visibility.Hidden }, + Cursor = new MenuCursor { State = { Value = Visibility.Hidden } }, content = new Container { RelativeSizeAxes = Axes.Both } }); } diff --git a/osu.Game/Graphics/UserInterface/BreadcrumbControl.cs b/osu.Game/Graphics/UserInterface/BreadcrumbControl.cs index f5e57e5f27..d1e55fee24 100644 --- a/osu.Game/Graphics/UserInterface/BreadcrumbControl.cs +++ b/osu.Game/Graphics/UserInterface/BreadcrumbControl.cs @@ -82,6 +82,10 @@ namespace osu.Game.Graphics.UserInterface } } + public override void Hide() => State = Visibility.Hidden; + + public override void Show() => State = Visibility.Visible; + public BreadcrumbTabItem(T value) : base(value) { diff --git a/osu.Game/Graphics/UserInterface/ProcessingOverlay.cs b/osu.Game/Graphics/UserInterface/ProcessingOverlay.cs index 8b50f4a97a..d75e78e2d9 100644 --- a/osu.Game/Graphics/UserInterface/ProcessingOverlay.cs +++ b/osu.Game/Graphics/UserInterface/ProcessingOverlay.cs @@ -34,7 +34,7 @@ namespace osu.Game.Graphics.UserInterface RelativeSizeAxes = Axes.Both, Alpha = 0.9f, }, - new LoadingAnimation { State = Visibility.Visible } + new LoadingAnimation { State = { Value = Visibility.Visible } } }; } diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index ba9abcdefc..d5fbcdfee3 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -132,12 +132,12 @@ namespace osu.Game public void CloseAllOverlays(bool hideToolbarElements = true) { foreach (var overlay in overlays) - overlay.State = Visibility.Hidden; + overlay.Hide(); if (hideToolbarElements) { foreach (var overlay in toolbarElements) - overlay.State = Visibility.Hidden; + overlay.Hide(); } } @@ -461,7 +461,7 @@ namespace osu.Game loadComponentSingleFile(new DialogOverlay(), topMostOverlayContent.Add, true); loadComponentSingleFile(externalLinkOpener = new ExternalLinkOpener(), topMostOverlayContent.Add); - chatOverlay.StateChanged += state => channelManager.HighPollRate.Value = state == Visibility.Visible; + chatOverlay.State.ValueChanged += state => channelManager.HighPollRate.Value = state.NewValue == Visibility.Visible; Add(externalLinkOpener = new ExternalLinkOpener()); @@ -470,9 +470,9 @@ namespace osu.Game foreach (var overlay in singleDisplaySideOverlays) { - overlay.StateChanged += state => + overlay.State.ValueChanged += state => { - if (state == Visibility.Hidden) return; + if (state.NewValue == Visibility.Hidden) return; singleDisplaySideOverlays.Where(o => o != overlay).ForEach(o => o.Hide()); }; @@ -484,9 +484,9 @@ namespace osu.Game foreach (var overlay in informationalOverlays) { - overlay.StateChanged += state => + overlay.State.ValueChanged += state => { - if (state == Visibility.Hidden) return; + if (state.NewValue == Visibility.Hidden) return; informationalOverlays.Where(o => o != overlay).ForEach(o => o.Hide()); }; @@ -498,12 +498,12 @@ namespace osu.Game foreach (var overlay in singleDisplayOverlays) { - overlay.StateChanged += state => + overlay.State.ValueChanged += state => { // informational overlays should be dismissed on a show or hide of a full overlay. informationalOverlays.ForEach(o => o.Hide()); - if (state == Visibility.Hidden) return; + if (state.NewValue == Visibility.Hidden) return; singleDisplayOverlays.Where(o => o != overlay).ForEach(o => o.Hide()); }; @@ -518,16 +518,16 @@ namespace osu.Game { float offset = 0; - if (settings.State == Visibility.Visible) + if (settings.State.Value == Visibility.Visible) offset += ToolbarButton.WIDTH / 2; - if (notifications.State == Visibility.Visible) + if (notifications.State.Value == Visibility.Visible) offset -= ToolbarButton.WIDTH / 2; screenContainer.MoveToX(offset, SettingsPanel.TRANSITION_LENGTH, Easing.OutQuint); } - settings.StateChanged += _ => updateScreenOffset(); - notifications.StateChanged += _ => updateScreenOffset(); + settings.State.ValueChanged += _ => updateScreenOffset(); + notifications.State.ValueChanged += _ => updateScreenOffset(); } public class GameIdleTracker : IdleTracker @@ -768,7 +768,7 @@ namespace osu.Game if (newOsuScreen.HideOverlaysOnEnter) CloseAllOverlays(); else - Toolbar.State = Visibility.Visible; + Toolbar.Show(); } } diff --git a/osu.Game/Overlays/AccountCreationOverlay.cs b/osu.Game/Overlays/AccountCreationOverlay.cs index 52d2917677..89d8cbde11 100644 --- a/osu.Game/Overlays/AccountCreationOverlay.cs +++ b/osu.Game/Overlays/AccountCreationOverlay.cs @@ -109,7 +109,7 @@ namespace osu.Game.Overlays break; case APIState.Online: - State = Visibility.Hidden; + Hide(); break; } } diff --git a/osu.Game/Overlays/BeatmapSetOverlay.cs b/osu.Game/Overlays/BeatmapSetOverlay.cs index 3ed398d31a..e0852a890c 100644 --- a/osu.Game/Overlays/BeatmapSetOverlay.cs +++ b/osu.Game/Overlays/BeatmapSetOverlay.cs @@ -92,7 +92,7 @@ namespace osu.Game.Overlays protected override bool OnClick(ClickEvent e) { - State = Visibility.Hidden; + Hide(); return true; } diff --git a/osu.Game/Overlays/ChangelogOverlay.cs b/osu.Game/Overlays/ChangelogOverlay.cs index 4a6d53b480..67f195580e 100644 --- a/osu.Game/Overlays/ChangelogOverlay.cs +++ b/osu.Game/Overlays/ChangelogOverlay.cs @@ -92,7 +92,7 @@ namespace osu.Game.Overlays public void ShowListing() { Current.Value = null; - State = Visibility.Visible; + Show(); } /// @@ -106,7 +106,7 @@ namespace osu.Game.Overlays if (build == null) throw new ArgumentNullException(nameof(build)); Current.Value = build; - State = Visibility.Visible; + Show(); } public void ShowBuild([NotNull] string updateStream, [NotNull] string version) @@ -123,7 +123,7 @@ namespace osu.Game.Overlays ShowBuild(build); }); - State = Visibility.Visible; + Show(); } public override bool OnPressed(GlobalAction action) @@ -133,7 +133,7 @@ namespace osu.Game.Overlays case GlobalAction.Back: if (Current.Value == null) { - State = Visibility.Hidden; + Hide(); } else { diff --git a/osu.Game/Overlays/ChatOverlay.cs b/osu.Game/Overlays/ChatOverlay.cs index eb95fabe02..0c1cca3d49 100644 --- a/osu.Game/Overlays/ChatOverlay.cs +++ b/osu.Game/Overlays/ChatOverlay.cs @@ -56,7 +56,7 @@ namespace osu.Game.Overlays private readonly Container channelSelectionContainer; private readonly ChannelSelectionOverlay channelSelectionOverlay; - public override bool Contains(Vector2 screenSpacePos) => chatContainer.ReceivePositionalInputAt(screenSpacePos) || channelSelectionOverlay.State == Visibility.Visible && channelSelectionOverlay.ReceivePositionalInputAt(screenSpacePos); + public override bool Contains(Vector2 screenSpacePos) => chatContainer.ReceivePositionalInputAt(screenSpacePos) || channelSelectionOverlay.State.Value == Visibility.Visible && channelSelectionOverlay.ReceivePositionalInputAt(screenSpacePos); public ChatOverlay() { @@ -130,7 +130,7 @@ namespace osu.Game.Overlays RelativeSizeAxes = Axes.Both, Height = 1, PlaceholderText = "type your message", - Exit = () => State = Visibility.Hidden, + Exit = Hide, OnCommit = postMessage, ReleaseFocusOnCommit = false, HoldFocus = true, @@ -163,19 +163,19 @@ namespace osu.Game.Overlays }; channelTabControl.Current.ValueChanged += current => channelManager.CurrentChannel.Value = current.NewValue; - channelTabControl.ChannelSelectorActive.ValueChanged += active => channelSelectionOverlay.State = active.NewValue ? Visibility.Visible : Visibility.Hidden; - channelSelectionOverlay.StateChanged += state => + channelTabControl.ChannelSelectorActive.ValueChanged += active => channelSelectionOverlay.State.Value = active.NewValue ? Visibility.Visible : Visibility.Hidden; + channelSelectionOverlay.State.ValueChanged += state => { - if (state == Visibility.Hidden && channelManager.CurrentChannel.Value == null) + if (state.NewValue == Visibility.Hidden && channelManager.CurrentChannel.Value == null) { - channelSelectionOverlay.State = Visibility.Visible; - State = Visibility.Hidden; + channelSelectionOverlay.Show(); + Hide(); return; } - channelTabControl.ChannelSelectorActive.Value = state == Visibility.Visible; + channelTabControl.ChannelSelectorActive.Value = state.NewValue == Visibility.Visible; - if (state == Visibility.Visible) + if (state.NewValue == Visibility.Visible) { textbox.HoldFocus = false; if (1f - ChatHeight.Value < channel_selection_min_height) @@ -195,7 +195,7 @@ namespace osu.Game.Overlays { textbox.Current.Disabled = true; currentChannelContainer.Clear(false); - channelSelectionOverlay.State = Visibility.Visible; + channelSelectionOverlay.Show(); return; } @@ -253,7 +253,7 @@ namespace osu.Game.Overlays double targetChatHeight = startDragChatHeight - (e.MousePosition.Y - e.MouseDownPosition.Y) / Parent.DrawSize.Y; // If the channel selection screen is shown, mind its minimum height - if (channelSelectionOverlay.State == Visibility.Visible && targetChatHeight > 1f - channel_selection_min_height) + if (channelSelectionOverlay.State.Value == Visibility.Visible && targetChatHeight > 1f - channel_selection_min_height) targetChatHeight = 1f - channel_selection_min_height; ChatHeight.Value = targetChatHeight; @@ -325,7 +325,7 @@ namespace osu.Game.Overlays this.MoveToY(Height, transition_length, Easing.InSine); this.FadeOut(transition_length, Easing.InSine); - channelSelectionOverlay.State = Visibility.Hidden; + channelSelectionOverlay.Hide(); textbox.HoldFocus = false; base.PopOut(); diff --git a/osu.Game/Overlays/DialogOverlay.cs b/osu.Game/Overlays/DialogOverlay.cs index 2cc1c20a10..aaae7bcf5c 100644 --- a/osu.Game/Overlays/DialogOverlay.cs +++ b/osu.Game/Overlays/DialogOverlay.cs @@ -37,8 +37,8 @@ namespace osu.Game.Overlays dialogContainer.Add(currentDialog); currentDialog.Show(); - currentDialog.StateChanged += state => onDialogOnStateChanged(dialog, state); - State = Visibility.Visible; + currentDialog.State.ValueChanged += state => onDialogOnStateChanged(dialog, state.NewValue); + Show(); } protected override bool PlaySamplesOnStateChange => false; @@ -53,7 +53,7 @@ namespace osu.Game.Overlays dialog.Delay(PopupDialog.EXIT_DURATION).Expire(); if (dialog == currentDialog) - State = Visibility.Hidden; + Hide(); } protected override void PopIn() @@ -66,7 +66,7 @@ namespace osu.Game.Overlays { base.PopOut(); - if (currentDialog?.State == Visibility.Visible) + if (currentDialog?.State.Value == Visibility.Visible) { currentDialog.Hide(); return; diff --git a/osu.Game/Overlays/DirectOverlay.cs b/osu.Game/Overlays/DirectOverlay.cs index 975bf4e3ca..7dcf76e41f 100644 --- a/osu.Game/Overlays/DirectOverlay.cs +++ b/osu.Game/Overlays/DirectOverlay.cs @@ -252,7 +252,7 @@ namespace osu.Game.Overlays if (!IsLoaded) return; - if (State == Visibility.Hidden) + if (State.Value == Visibility.Hidden) return; if (API == null) diff --git a/osu.Game/Overlays/Mods/ModSelectOverlay.cs b/osu.Game/Overlays/Mods/ModSelectOverlay.cs index dec58f4c9e..8e5c9588ce 100644 --- a/osu.Game/Overlays/Mods/ModSelectOverlay.cs +++ b/osu.Game/Overlays/Mods/ModSelectOverlay.cs @@ -413,12 +413,12 @@ namespace osu.Game.Overlays.Mods { if (selectedMod != null) { - if (State == Visibility.Visible) sampleOn?.Play(); + if (State.Value == Visibility.Visible) sampleOn?.Play(); DeselectTypes(selectedMod.IncompatibleMods, true); } else { - if (State == Visibility.Visible) sampleOff?.Play(); + if (State.Value == Visibility.Visible) sampleOff?.Play(); } refreshSelectedMods(); diff --git a/osu.Game/Overlays/Music/PlaylistOverlay.cs b/osu.Game/Overlays/Music/PlaylistOverlay.cs index 4431288a1a..ec3d708645 100644 --- a/osu.Game/Overlays/Music/PlaylistOverlay.cs +++ b/osu.Game/Overlays/Music/PlaylistOverlay.cs @@ -71,7 +71,7 @@ namespace osu.Game.Overlays.Music { RelativeSizeAxes = Axes.X, AutoSizeAxes = Axes.Y, - ExitRequested = () => State = Visibility.Hidden, + ExitRequested = Hide, FilterChanged = search => list.Filter(search), Padding = new MarginPadding(10), }, diff --git a/osu.Game/Overlays/MusicController.cs b/osu.Game/Overlays/MusicController.cs index d7b915efe3..85524e992c 100644 --- a/osu.Game/Overlays/MusicController.cs +++ b/osu.Game/Overlays/MusicController.cs @@ -200,7 +200,7 @@ namespace osu.Game.Overlays beatmaps.ItemAdded += handleBeatmapAdded; beatmaps.ItemRemoved += handleBeatmapRemoved; - playlist.StateChanged += s => playlistButton.FadeColour(s == Visibility.Visible ? colours.Yellow : Color4.White, 200, Easing.OutQuint); + playlist.State.ValueChanged += s => playlistButton.FadeColour(s.NewValue == Visibility.Visible ? colours.Yellow : Color4.White, 200, Easing.OutQuint); } private ScheduledDelegate seekDelegate; @@ -449,7 +449,7 @@ namespace osu.Game.Overlays // This is here mostly as a performance fix. // If the playlist is not hidden it will update children even when the music controller is hidden (due to AlwaysPresent). - playlist.State = Visibility.Hidden; + playlist.Hide(); this.FadeOut(transition_length, Easing.OutQuint); dragContainer.ScaleTo(0.9f, transition_length, Easing.OutQuint); diff --git a/osu.Game/Overlays/NotificationOverlay.cs b/osu.Game/Overlays/NotificationOverlay.cs index 8f75d3ebf0..2e4c504645 100644 --- a/osu.Game/Overlays/NotificationOverlay.cs +++ b/osu.Game/Overlays/NotificationOverlay.cs @@ -81,13 +81,13 @@ namespace osu.Game.Overlays private void updateProcessingMode() { - bool enabled = OverlayActivationMode.Value == OverlayActivation.All || State == Visibility.Visible; + bool enabled = OverlayActivationMode.Value == OverlayActivation.All || State.Value == Visibility.Visible; notificationsEnabler?.Cancel(); if (enabled) // we want a slight delay before toggling notifications on to avoid the user becoming overwhelmed. - notificationsEnabler = Scheduler.AddDelayed(() => processingPosts = true, State == Visibility.Visible ? 0 : 1000); + notificationsEnabler = Scheduler.AddDelayed(() => processingPosts = true, State.Value == Visibility.Visible ? 0 : 1000); else processingPosts = false; } @@ -96,7 +96,7 @@ namespace osu.Game.Overlays { base.LoadComplete(); - StateChanged += _ => updateProcessingMode(); + State.ValueChanged += _ => updateProcessingMode(); OverlayActivationMode.BindValueChanged(_ => updateProcessingMode(), true); } @@ -128,7 +128,7 @@ namespace osu.Game.Overlays section?.Add(notification, notification.DisplayOnTop ? -runningDepth : runningDepth); if (notification.IsImportant) - State = Visibility.Visible; + Show(); updateCounts(); }); diff --git a/osu.Game/Overlays/Settings/Sections/General/LoginSettings.cs b/osu.Game/Overlays/Settings/Sections/General/LoginSettings.cs index 2f56ace24d..36d6a22165 100644 --- a/osu.Game/Overlays/Settings/Sections/General/LoginSettings.cs +++ b/osu.Game/Overlays/Settings/Sections/General/LoginSettings.cs @@ -97,7 +97,7 @@ namespace osu.Game.Overlays.Settings.Sections.General { new LoadingAnimation { - State = Visibility.Visible, + State = { Value = Visibility.Visible }, Anchor = Anchor.TopCentre, Origin = Anchor.TopCentre, }, diff --git a/osu.Game/Overlays/SettingsOverlay.cs b/osu.Game/Overlays/SettingsOverlay.cs index 6e3eaae0a1..bb84de5d3a 100644 --- a/osu.Game/Overlays/SettingsOverlay.cs +++ b/osu.Game/Overlays/SettingsOverlay.cs @@ -9,6 +9,7 @@ using osu.Game.Overlays.Settings.Sections; using osuTK.Graphics; using System.Collections.Generic; using System.Linq; +using osu.Framework.Bindables; namespace osu.Game.Overlays { @@ -37,23 +38,23 @@ namespace osu.Game.Overlays { } - public override bool AcceptsFocus => subPanels.All(s => s.State != Visibility.Visible); + public override bool AcceptsFocus => subPanels.All(s => s.State.Value != Visibility.Visible); private T createSubPanel(T subPanel) where T : SettingsSubPanel { subPanel.Depth = 1; subPanel.Anchor = Anchor.TopRight; - subPanel.StateChanged += subPanelStateChanged; + subPanel.State.ValueChanged += subPanelStateChanged; subPanels.Add(subPanel); return subPanel; } - private void subPanelStateChanged(Visibility visibility) + private void subPanelStateChanged(ValueChangedEvent state) { - switch (visibility) + switch (state.NewValue) { case Visibility.Visible: Background.FadeTo(0.9f, 300, Easing.OutQuint); @@ -73,7 +74,7 @@ namespace osu.Game.Overlays } } - protected override float ExpandedPosition => subPanels.Any(s => s.State == Visibility.Visible) ? -WIDTH : base.ExpandedPosition; + protected override float ExpandedPosition => subPanels.Any(s => s.State.Value == Visibility.Visible) ? -WIDTH : base.ExpandedPosition; [BackgroundDependencyLoader] private void load() diff --git a/osu.Game/Overlays/Toolbar/Toolbar.cs b/osu.Game/Overlays/Toolbar/Toolbar.cs index 3c8b96fe8a..982fb26b6b 100644 --- a/osu.Game/Overlays/Toolbar/Toolbar.cs +++ b/osu.Game/Overlays/Toolbar/Toolbar.cs @@ -84,10 +84,10 @@ namespace osu.Game.Overlays.Toolbar } }; - StateChanged += visibility => + State.ValueChanged += visibility => { if (overlayActivationMode.Value == OverlayActivation.Disabled) - State = Visibility.Hidden; + Hide(); }; if (osuGame != null) diff --git a/osu.Game/Overlays/Toolbar/ToolbarOverlayToggleButton.cs b/osu.Game/Overlays/Toolbar/ToolbarOverlayToggleButton.cs index b2ae273e31..b286cbfb1d 100644 --- a/osu.Game/Overlays/Toolbar/ToolbarOverlayToggleButton.cs +++ b/osu.Game/Overlays/Toolbar/ToolbarOverlayToggleButton.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 osu.Framework.Bindables; using osu.Framework.Extensions.Color4Extensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; @@ -15,6 +16,8 @@ namespace osu.Game.Overlays.Toolbar private OverlayContainer stateContainer; + private readonly Bindable overlayState = new Bindable(); + public OverlayContainer StateContainer { get => stateContainer; @@ -22,10 +25,12 @@ namespace osu.Game.Overlays.Toolbar { stateContainer = value; + overlayState.UnbindBindings(); + if (stateContainer != null) { Action = stateContainer.ToggleVisibility; - stateContainer.StateChanged += stateChanged; + overlayState.BindTo(stateContainer.State); } } } @@ -40,18 +45,13 @@ namespace osu.Game.Overlays.Toolbar Depth = 2, Alpha = 0, }); + + overlayState.ValueChanged += stateChanged; } - protected override void Dispose(bool isDisposing) + private void stateChanged(ValueChangedEvent state) { - base.Dispose(isDisposing); - if (stateContainer != null) - stateContainer.StateChanged -= stateChanged; - } - - private void stateChanged(Visibility state) - { - switch (state) + switch (state.NewValue) { case Visibility.Hidden: stateBackground.FadeOut(200); diff --git a/osu.Game/Overlays/VolumeOverlay.cs b/osu.Game/Overlays/VolumeOverlay.cs index 34b15d958d..02e0f59f26 100644 --- a/osu.Game/Overlays/VolumeOverlay.cs +++ b/osu.Game/Overlays/VolumeOverlay.cs @@ -100,14 +100,14 @@ namespace osu.Game.Overlays switch (action) { case GlobalAction.DecreaseVolume: - if (State == Visibility.Hidden) + if (State.Value == Visibility.Hidden) Show(); else volumeMeterMaster.Decrease(amount, isPrecise); return true; case GlobalAction.IncreaseVolume: - if (State == Visibility.Hidden) + if (State.Value == Visibility.Hidden) Show(); else volumeMeterMaster.Increase(amount, isPrecise); @@ -126,7 +126,7 @@ namespace osu.Game.Overlays public override void Show() { - if (State == Visibility.Visible) + if (State.Value == Visibility.Visible) schedulePopOut(); base.Show(); diff --git a/osu.Game/Rulesets/UI/GameplayCursorContainer.cs b/osu.Game/Rulesets/UI/GameplayCursorContainer.cs index 41edfa0b68..ae5f9c6111 100644 --- a/osu.Game/Rulesets/UI/GameplayCursorContainer.cs +++ b/osu.Game/Rulesets/UI/GameplayCursorContainer.cs @@ -19,7 +19,7 @@ namespace osu.Game.Rulesets.UI protected override void Update() { base.Update(); - LastFrameState = State; + LastFrameState = State.Value; } } } diff --git a/osu.Game/Screens/Menu/ButtonArea.cs b/osu.Game/Screens/Menu/ButtonArea.cs index c7650a08fa..d59996a4eb 100644 --- a/osu.Game/Screens/Menu/ButtonArea.cs +++ b/osu.Game/Screens/Menu/ButtonArea.cs @@ -56,12 +56,12 @@ namespace osu.Game.Screens.Menu case ButtonSystemState.Exit: case ButtonSystemState.Initial: case ButtonSystemState.EnteringMode: - State = Visibility.Hidden; + Hide(); break; case ButtonSystemState.TopLevel: case ButtonSystemState.Play: - State = Visibility.Visible; + Show(); break; } @@ -82,6 +82,10 @@ namespace osu.Game.Screens.Menu } } + public override void Hide() => State = Visibility.Hidden; + + public override void Show() => State = Visibility.Visible; + public event Action StateChanged; private class ButtonAreaBackground : Box, IStateful diff --git a/osu.Game/Screens/Play/GameplayMenuOverlay.cs b/osu.Game/Screens/Play/GameplayMenuOverlay.cs index 456fb4faf9..c7e762714c 100644 --- a/osu.Game/Screens/Play/GameplayMenuOverlay.cs +++ b/osu.Game/Screens/Play/GameplayMenuOverlay.cs @@ -59,7 +59,7 @@ namespace osu.Game.Screens.Play { RelativeSizeAxes = Axes.Both; - StateChanged += s => selectionIndex = -1; + State.ValueChanged += s => selectionIndex = -1; } [BackgroundDependencyLoader] diff --git a/osu.Game/Screens/Play/HUD/PlayerSettingsOverlay.cs b/osu.Game/Screens/Play/HUD/PlayerSettingsOverlay.cs index e99f6d836e..b2c3952f38 100644 --- a/osu.Game/Screens/Play/HUD/PlayerSettingsOverlay.cs +++ b/osu.Game/Screens/Play/HUD/PlayerSettingsOverlay.cs @@ -46,7 +46,7 @@ namespace osu.Game.Screens.Play.HUD } }; - State = Visibility.Visible; + Show(); } protected override void PopIn() => this.FadeIn(fade_duration); diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index d8389fa6d9..35ef7b3200 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -358,7 +358,7 @@ namespace osu.Game.Screens.Play // There is a chance that we could be in a paused state as the ruleset's internal clock (see FrameStabilityContainer) // could process an extra frame after the GameplayClock is stopped. // In such cases we want the fail state to precede a user triggered pause. - if (PauseOverlay.State == Visibility.Visible) + if (PauseOverlay.State.Value == Visibility.Visible) PauseOverlay.Hide(); failAnimation.Start(); diff --git a/osu.Game/Screens/Play/SkipOverlay.cs b/osu.Game/Screens/Play/SkipOverlay.cs index 4ecc15f22b..38dd179f25 100644 --- a/osu.Game/Screens/Play/SkipOverlay.cs +++ b/osu.Game/Screens/Play/SkipOverlay.cs @@ -46,7 +46,7 @@ namespace osu.Game.Screens.Play { this.startTime = startTime; - State = Visibility.Visible; + Show(); RelativePositionAxes = Axes.Both; RelativeSizeAxes = Axes.X; @@ -136,7 +136,7 @@ namespace osu.Game.Screens.Play protected override bool OnMouseMove(MouseMoveEvent e) { if (!e.HasAnyButtonPressed) - fadeContainer.State = Visibility.Visible; + fadeContainer.Show(); return base.OnMouseMove(e); } @@ -181,7 +181,7 @@ namespace osu.Game.Screens.Play if (!IsHovered && !IsDragged) using (BeginDelayedSequence(1000)) - scheduledHide = Schedule(() => State = Visibility.Hidden); + scheduledHide = Schedule(Hide); break; case Visibility.Hidden: @@ -196,7 +196,7 @@ namespace osu.Game.Screens.Play protected override void LoadComplete() { base.LoadComplete(); - State = Visibility.Visible; + Show(); } protected override bool OnMouseDown(MouseDownEvent e) @@ -207,9 +207,13 @@ namespace osu.Game.Screens.Play protected override bool OnMouseUp(MouseUpEvent e) { - State = Visibility.Visible; + Show(); return base.OnMouseUp(e); } + + public override void Hide() => State = Visibility.Hidden; + + public override void Show() => State = Visibility.Visible; } private class Button : OsuClickableContainer diff --git a/osu.Game/Screens/Play/SongProgress.cs b/osu.Game/Screens/Play/SongProgress.cs index d478454f00..6642efdf8b 100644 --- a/osu.Game/Screens/Play/SongProgress.cs +++ b/osu.Game/Screens/Play/SongProgress.cs @@ -106,7 +106,7 @@ namespace osu.Game.Screens.Play protected override void LoadComplete() { - State = Visibility.Visible; + Show(); replayLoaded.ValueChanged += loaded => AllowSeeking = loaded.NewValue; replayLoaded.TriggerChange(); diff --git a/osu.Game/Screens/Select/BeatmapDetails.cs b/osu.Game/Screens/Select/BeatmapDetails.cs index a78ab97960..378b1b1dc6 100644 --- a/osu.Game/Screens/Select/BeatmapDetails.cs +++ b/osu.Game/Screens/Select/BeatmapDetails.cs @@ -360,13 +360,13 @@ namespace osu.Game.Screens.Select protected override void PopIn() { this.FadeIn(transition_duration, Easing.OutQuint); - loading.State = Visibility.Visible; + loading.Show(); } protected override void PopOut() { this.FadeOut(transition_duration, Easing.OutQuint); - loading.State = Visibility.Hidden; + loading.Hide(); } } } diff --git a/osu.Game/Screens/Select/BeatmapInfoWedge.cs b/osu.Game/Screens/Select/BeatmapInfoWedge.cs index 1508de2730..fa9ffd0706 100644 --- a/osu.Game/Screens/Select/BeatmapInfoWedge.cs +++ b/osu.Game/Screens/Select/BeatmapInfoWedge.cs @@ -100,7 +100,7 @@ namespace osu.Game.Screens.Select { void removeOldInfo() { - State = beatmap == null ? Visibility.Hidden : Visibility.Visible; + State.Value = beatmap == null ? Visibility.Hidden : Visibility.Visible; Info?.FadeOut(250); Info?.Expire(); diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index 6d5be607f4..5390d24892 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -278,7 +278,7 @@ namespace osu.Game.Screens.Select protected virtual void ExitFromBack() { - if (ModSelect.State == Visibility.Visible) + if (ModSelect.State.Value == Visibility.Visible) { ModSelect.Hide(); return; @@ -520,7 +520,7 @@ namespace osu.Game.Screens.Select if (base.OnExiting(next)) return true; - beatmapInfoWedge.State = Visibility.Hidden; + beatmapInfoWedge.Hide(); this.FadeOut(100); diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index eeb1f2bee3..fb27caca11 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -15,10 +15,12 @@ - + + + diff --git a/osu.sln b/osu.sln index 3c38309d86..a106ab2800 100644 --- a/osu.sln +++ b/osu.sln @@ -25,6 +25,10 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "osu.Game.Rulesets.Taiko.Tes EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "osu.Game.Rulesets.Osu.Tests", "osu.Game.Rulesets.Osu.Tests\osu.Game.Rulesets.Osu.Tests.csproj", "{DECCCC75-67AD-4C3D-BB84-FD0E01323511}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "osu.Framework", "..\osu-framework\osu.Framework\osu.Framework.csproj", "{C7D2DA3C-97BF-403C-9F75-115C8A64DAC1}" +EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "osu.Framework.NativeLibs", "..\osu-framework\osu.Framework.NativeLibs\osu.Framework.NativeLibs.csproj", "{35AD7F4C-81DC-4060-BFD1-C376DC4C4801}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -79,6 +83,14 @@ Global {DECCCC75-67AD-4C3D-BB84-FD0E01323511}.Debug|Any CPU.Build.0 = Debug|Any CPU {DECCCC75-67AD-4C3D-BB84-FD0E01323511}.Release|Any CPU.ActiveCfg = Release|Any CPU {DECCCC75-67AD-4C3D-BB84-FD0E01323511}.Release|Any CPU.Build.0 = Release|Any CPU + {C7D2DA3C-97BF-403C-9F75-115C8A64DAC1}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {C7D2DA3C-97BF-403C-9F75-115C8A64DAC1}.Debug|Any CPU.Build.0 = Debug|Any CPU + {C7D2DA3C-97BF-403C-9F75-115C8A64DAC1}.Release|Any CPU.ActiveCfg = Release|Any CPU + {C7D2DA3C-97BF-403C-9F75-115C8A64DAC1}.Release|Any CPU.Build.0 = Release|Any CPU + {35AD7F4C-81DC-4060-BFD1-C376DC4C4801}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {35AD7F4C-81DC-4060-BFD1-C376DC4C4801}.Debug|Any CPU.Build.0 = Debug|Any CPU + {35AD7F4C-81DC-4060-BFD1-C376DC4C4801}.Release|Any CPU.ActiveCfg = Release|Any CPU + {35AD7F4C-81DC-4060-BFD1-C376DC4C4801}.Release|Any CPU.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE From 8de62b608e76e268fea5d03551c0ba6a9fc10949 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 11 Jun 2019 15:22:27 +0900 Subject: [PATCH 47/60] Allow FullscreenOverlay to surface to front on subsequent Show requests --- osu.Game/Overlays/FullscreenOverlay.cs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/osu.Game/Overlays/FullscreenOverlay.cs b/osu.Game/Overlays/FullscreenOverlay.cs index 9706f75087..0911ee84de 100644 --- a/osu.Game/Overlays/FullscreenOverlay.cs +++ b/osu.Game/Overlays/FullscreenOverlay.cs @@ -4,6 +4,7 @@ using osu.Framework.Allocation; using osu.Framework.Extensions.Color4Extensions; using osu.Framework.Graphics; +using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Effects; using osu.Game.Graphics; using osu.Game.Graphics.Containers; @@ -40,6 +41,19 @@ namespace osu.Game.Overlays }; } + public override void Show() + { + if (State.Value == Visibility.Visible) + { + // re-trigger the state changed so we can potentially surface to front + State.TriggerChange(); + } + else + { + base.Show(); + } + } + protected override void PopIn() { base.PopIn(); From 620c2311ac887eebf82c8c31583661bfe0aba517 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 11 Jun 2019 15:39:12 +0900 Subject: [PATCH 48/60] Add test --- .../Online/TestSceneFullscreenOverlay.cs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneFullscreenOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneFullscreenOverlay.cs index 6dc3428bff..fe8437be17 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneFullscreenOverlay.cs +++ b/osu.Game.Tests/Visual/Online/TestSceneFullscreenOverlay.cs @@ -18,8 +18,24 @@ namespace osu.Game.Tests.Visual.Online { base.LoadComplete(); + int fireCount = 0; + Add(overlay = new TestFullscreenOverlay()); - AddStep(@"toggle", overlay.ToggleVisibility); + + overlay.State.ValueChanged += _ => fireCount++; + + AddStep(@"show", overlay.Show); + + AddAssert("fire count 1", () => fireCount == 1); + + AddStep(@"show again", overlay.Show); + + // this logic is specific to FullscreenOverlay + AddAssert("fire count 2", () => fireCount == 2); + + AddStep(@"hide", overlay.Hide); + + AddAssert("fire count 3", () => fireCount == 3); } private class TestFullscreenOverlay : FullscreenOverlay From 975bb3db8a2aba107858543eb12b419f363cad44 Mon Sep 17 00:00:00 2001 From: David Zhao Date: Tue, 11 Jun 2019 15:51:14 +0900 Subject: [PATCH 49/60] cleanup --- .idea/.idea.osu/.idea/.name | 1 - osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) delete mode 100644 .idea/.idea.osu/.idea/.name diff --git a/.idea/.idea.osu/.idea/.name b/.idea/.idea.osu/.idea/.name deleted file mode 100644 index 21cb4db60e..0000000000 --- a/.idea/.idea.osu/.idea/.name +++ /dev/null @@ -1 +0,0 @@ -osu \ No newline at end of file diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index fa73a14252..198da2c5b1 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -222,7 +222,7 @@ namespace osu.Game.Tests.Visual.SongSelect { beatmapChangedCount = 0; debounceCount = 0; - songSelect.Carousel.SelectionChanged += _ => beatmapChangedCount += 1; + songSelect.Carousel.SelectionChanged += _ => beatmapChangedCount++; }); AddRepeatStep($"Create beatmaps {test_count} times", () => { From a53ade07a5ea39046b5b90d240435664010021a3 Mon Sep 17 00:00:00 2001 From: David Zhao Date: Tue, 11 Jun 2019 15:51:57 +0900 Subject: [PATCH 50/60] remove unused using --- osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs index 198da2c5b1..2664c7a42c 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestScenePlaySongSelect.cs @@ -11,7 +11,6 @@ using osu.Framework.Allocation; using osu.Framework.Audio; using osu.Framework.Bindables; using osu.Framework.Extensions; -using osu.Framework.Logging; using osu.Framework.MathUtils; using osu.Framework.Platform; using osu.Framework.Screens; From 07e17518e93cc81d64e9215a9b1465a0e8859be4 Mon Sep 17 00:00:00 2001 From: Arphox Date: Tue, 11 Jun 2019 10:28:16 +0200 Subject: [PATCH 51/60] Fix all "Maintainability" CodeFactor issues --- osu.Game.Rulesets.Catch/UI/CatcherArea.cs | 4 ++-- osu.Game.Rulesets.Osu/Beatmaps/OsuBeatmapProcessor.cs | 2 +- osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs | 4 ++-- osu.Game.Tests/Visual/UserInterface/TestSceneCursors.cs | 2 +- osu.Game/Graphics/Containers/HoldToConfirmContainer.cs | 2 +- osu.Game/Online/API/APIAccess.cs | 2 +- osu.Game/Online/Chat/MessageFormatter.cs | 2 +- osu.Game/Overlays/ChatOverlay.cs | 2 +- osu.Game/Rulesets/Edit/SelectionBlueprint.cs | 2 +- osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs | 2 +- osu.Game/Scoring/Legacy/LegacyScoreParser.cs | 8 ++++---- 11 files changed, 16 insertions(+), 16 deletions(-) diff --git a/osu.Game.Rulesets.Catch/UI/CatcherArea.cs b/osu.Game.Rulesets.Catch/UI/CatcherArea.cs index e7c7fd77df..90052d9b11 100644 --- a/osu.Game.Rulesets.Catch/UI/CatcherArea.cs +++ b/osu.Game.Rulesets.Catch/UI/CatcherArea.cs @@ -379,8 +379,8 @@ namespace osu.Game.Rulesets.Catch.UI X = (float)MathHelper.Clamp(X + direction * Clock.ElapsedFrameTime * speed, 0, 1); // Correct overshooting. - if (hyperDashDirection > 0 && hyperDashTargetPosition < X || - hyperDashDirection < 0 && hyperDashTargetPosition > X) + if ((hyperDashDirection > 0 && hyperDashTargetPosition < X) || + (hyperDashDirection < 0 && hyperDashTargetPosition > X)) { X = hyperDashTargetPosition; SetHyperDashState(); diff --git a/osu.Game.Rulesets.Osu/Beatmaps/OsuBeatmapProcessor.cs b/osu.Game.Rulesets.Osu/Beatmaps/OsuBeatmapProcessor.cs index b2beda18f4..7bb1f42802 100644 --- a/osu.Game.Rulesets.Osu/Beatmaps/OsuBeatmapProcessor.cs +++ b/osu.Game.Rulesets.Osu/Beatmaps/OsuBeatmapProcessor.cs @@ -69,7 +69,7 @@ namespace osu.Game.Rulesets.Osu.Beatmaps break; if (Vector2Extensions.Distance(stackBaseObject.Position, objectN.Position) < stack_distance - || stackBaseObject is Slider && Vector2Extensions.Distance(stackBaseObject.EndPosition, objectN.Position) < stack_distance) + || (stackBaseObject is Slider && Vector2Extensions.Distance(stackBaseObject.EndPosition, objectN.Position) < stack_distance)) { stackBaseIndex = n; diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs b/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs index ec23570f54..bc5d02258f 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs @@ -37,11 +37,11 @@ namespace osu.Game.Rulesets.Osu.Mods if (time < osuHit.HitObject.StartTime - relax_leniency) continue; - if (osuHit.HitObject is IHasEndTime hasEnd && time > hasEnd.EndTime || osuHit.IsHit) + if ((osuHit.HitObject is IHasEndTime hasEnd && time > hasEnd.EndTime) || osuHit.IsHit) continue; requiresHit |= osuHit is DrawableHitCircle && osuHit.IsHovered && osuHit.HitObject.HitWindows.CanBeHit(relativetime); - requiresHold |= osuHit is DrawableSlider slider && (slider.Ball.IsHovered || osuHit.IsHovered) || osuHit is DrawableSpinner; + requiresHold |= (osuHit is DrawableSlider slider && (slider.Ball.IsHovered || osuHit.IsHovered)) || osuHit is DrawableSpinner; } if (requiresHit) diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneCursors.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneCursors.cs index 8fe31b7ad6..f59cb75a46 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneCursors.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneCursors.cs @@ -192,7 +192,7 @@ namespace osu.Game.Tests.Visual.UserInterface public CursorContainer Cursor { get; } public bool ProvidingUserCursor { get; } - public override bool ReceivePositionalInputAt(Vector2 screenSpacePos) => base.ReceivePositionalInputAt(screenSpacePos) || SmoothTransition && !ProvidingUserCursor; + public override bool ReceivePositionalInputAt(Vector2 screenSpacePos) => base.ReceivePositionalInputAt(screenSpacePos) || (SmoothTransition && !ProvidingUserCursor); private readonly Box background; diff --git a/osu.Game/Graphics/Containers/HoldToConfirmContainer.cs b/osu.Game/Graphics/Containers/HoldToConfirmContainer.cs index a5b5b7af42..cda5e150de 100644 --- a/osu.Game/Graphics/Containers/HoldToConfirmContainer.cs +++ b/osu.Game/Graphics/Containers/HoldToConfirmContainer.cs @@ -27,7 +27,7 @@ namespace osu.Game.Graphics.Containers protected void BeginConfirm() { - if (confirming || !AllowMultipleFires && fired) return; + if (confirming || (!AllowMultipleFires && fired)) return; confirming = true; diff --git a/osu.Game/Online/API/APIAccess.cs b/osu.Game/Online/API/APIAccess.cs index 594bc1e3ca..343d6a67b7 100644 --- a/osu.Game/Online/API/APIAccess.cs +++ b/osu.Game/Online/API/APIAccess.cs @@ -37,7 +37,7 @@ namespace osu.Game.Online.API public Bindable LocalUser { get; } = new Bindable(createGuestUser()); - protected bool HasLogin => authentication.Token.Value != null || !string.IsNullOrEmpty(ProvidedUsername) && !string.IsNullOrEmpty(password); + protected bool HasLogin => authentication.Token.Value != null || (!string.IsNullOrEmpty(ProvidedUsername) && !string.IsNullOrEmpty(password)); private readonly CancellationTokenSource cancellationToken = new CancellationTokenSource(); diff --git a/osu.Game/Online/Chat/MessageFormatter.cs b/osu.Game/Online/Chat/MessageFormatter.cs index e1fc65da6c..4aaffdd161 100644 --- a/osu.Game/Online/Chat/MessageFormatter.cs +++ b/osu.Game/Online/Chat/MessageFormatter.cs @@ -69,7 +69,7 @@ namespace osu.Game.Online.Chat if (displayText.Length == 0 || linkText.Length == 0) continue; // Check for encapsulated links - if (result.Links.Find(l => l.Index <= index && l.Index + l.Length >= index + m.Length || index <= l.Index && index + m.Length >= l.Index + l.Length) == null) + if (result.Links.Find(l => (l.Index <= index && l.Index + l.Length >= index + m.Length) || (index <= l.Index && index + m.Length >= l.Index + l.Length)) == null) { result.Text = result.Text.Remove(index, m.Length).Insert(index, displayText); diff --git a/osu.Game/Overlays/ChatOverlay.cs b/osu.Game/Overlays/ChatOverlay.cs index eb95fabe02..978848d9fc 100644 --- a/osu.Game/Overlays/ChatOverlay.cs +++ b/osu.Game/Overlays/ChatOverlay.cs @@ -56,7 +56,7 @@ namespace osu.Game.Overlays private readonly Container channelSelectionContainer; private readonly ChannelSelectionOverlay channelSelectionOverlay; - public override bool Contains(Vector2 screenSpacePos) => chatContainer.ReceivePositionalInputAt(screenSpacePos) || channelSelectionOverlay.State == Visibility.Visible && channelSelectionOverlay.ReceivePositionalInputAt(screenSpacePos); + public override bool Contains(Vector2 screenSpacePos) => chatContainer.ReceivePositionalInputAt(screenSpacePos) || (channelSelectionOverlay.State == Visibility.Visible && channelSelectionOverlay.ReceivePositionalInputAt(screenSpacePos)); public ChatOverlay() { diff --git a/osu.Game/Rulesets/Edit/SelectionBlueprint.cs b/osu.Game/Rulesets/Edit/SelectionBlueprint.cs index e94604554c..0f77b8d584 100644 --- a/osu.Game/Rulesets/Edit/SelectionBlueprint.cs +++ b/osu.Game/Rulesets/Edit/SelectionBlueprint.cs @@ -45,7 +45,7 @@ namespace osu.Game.Rulesets.Edit /// public readonly DrawableHitObject HitObject; - protected override bool ShouldBeAlive => HitObject.IsAlive && HitObject.IsPresent || State == SelectionState.Selected; + protected override bool ShouldBeAlive => (HitObject.IsAlive && HitObject.IsPresent) || State == SelectionState.Selected; public override bool HandlePositionalInput => ShouldBeAlive; public override bool RemoveWhenNotAlive => false; diff --git a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs index e91100608b..ec7e6dc303 100644 --- a/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs +++ b/osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs @@ -85,7 +85,7 @@ namespace osu.Game.Rulesets.Objects.Drawables public override bool RemoveCompletedTransforms => false; protected override bool RequiresChildrenUpdate => true; - public override bool IsPresent => base.IsPresent || State.Value == ArmedState.Idle && Clock?.CurrentTime >= LifetimeStart; + public override bool IsPresent => base.IsPresent || (State.Value == ArmedState.Idle && Clock?.CurrentTime >= LifetimeStart); public readonly Bindable State = new Bindable(); diff --git a/osu.Game/Scoring/Legacy/LegacyScoreParser.cs b/osu.Game/Scoring/Legacy/LegacyScoreParser.cs index d2c9ce81c3..0fdbd56c92 100644 --- a/osu.Game/Scoring/Legacy/LegacyScoreParser.cs +++ b/osu.Game/Scoring/Legacy/LegacyScoreParser.cs @@ -136,9 +136,9 @@ namespace osu.Game.Scoring.Legacy score.Rank = score.Mods.Any(m => m is ModHidden || m is ModFlashlight) ? ScoreRank.XH : ScoreRank.X; else if (ratio300 > 0.9 && ratio50 <= 0.01 && countMiss == 0) score.Rank = score.Mods.Any(m => m is ModHidden || m is ModFlashlight) ? ScoreRank.SH : ScoreRank.S; - else if (ratio300 > 0.8 && countMiss == 0 || ratio300 > 0.9) + else if ((ratio300 > 0.8 && countMiss == 0) || ratio300 > 0.9) score.Rank = ScoreRank.A; - else if (ratio300 > 0.7 && countMiss == 0 || ratio300 > 0.8) + else if ((ratio300 > 0.7 && countMiss == 0) || ratio300 > 0.8) score.Rank = ScoreRank.B; else if (ratio300 > 0.6) score.Rank = ScoreRank.C; @@ -159,9 +159,9 @@ namespace osu.Game.Scoring.Legacy score.Rank = score.Mods.Any(m => m is ModHidden || m is ModFlashlight) ? ScoreRank.XH : ScoreRank.X; else if (ratio300 > 0.9 && ratio50 <= 0.01 && countMiss == 0) score.Rank = score.Mods.Any(m => m is ModHidden || m is ModFlashlight) ? ScoreRank.SH : ScoreRank.S; - else if (ratio300 > 0.8 && countMiss == 0 || ratio300 > 0.9) + else if ((ratio300 > 0.8 && countMiss == 0) || ratio300 > 0.9) score.Rank = ScoreRank.A; - else if (ratio300 > 0.7 && countMiss == 0 || ratio300 > 0.8) + else if ((ratio300 > 0.7 && countMiss == 0) || ratio300 > 0.8) score.Rank = ScoreRank.B; else if (ratio300 > 0.6) score.Rank = ScoreRank.C; From e5417416a23a0ef743a53589ee42e5362f0f6937 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Tue, 11 Jun 2019 18:24:50 +0900 Subject: [PATCH 52/60] Remove braces --- osu.Game/Graphics/UserInterface/OsuCheckbox.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Graphics/UserInterface/OsuCheckbox.cs b/osu.Game/Graphics/UserInterface/OsuCheckbox.cs index 5d41075725..5ead5987a1 100644 --- a/osu.Game/Graphics/UserInterface/OsuCheckbox.cs +++ b/osu.Game/Graphics/UserInterface/OsuCheckbox.cs @@ -81,7 +81,7 @@ namespace osu.Game.Graphics.UserInterface Nub.Current.BindTo(Current); - Current.DisabledChanged += disabled => { labelText.Alpha = Nub.Alpha = disabled ? 0.3f : 1; }; + Current.DisabledChanged += disabled => labelText.Alpha = Nub.Alpha = disabled ? 0.3f : 1; } protected override void LoadComplete() From 261badc770330dd1e086bb30880bb73f6bb90bb9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 11 Jun 2019 19:24:54 +0900 Subject: [PATCH 53/60] Update framework --- osu.Game/osu.Game.csproj | 2 +- osu.iOS.props | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index eeb1f2bee3..140d0c7f0e 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -15,7 +15,7 @@ - + diff --git a/osu.iOS.props b/osu.iOS.props index 3a5090d968..39be738a5c 100644 --- a/osu.iOS.props +++ b/osu.iOS.props @@ -105,8 +105,8 @@ - - + + From 6a34d5575bd69b28393ab7dfb8f00448d03553ed Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 11 Jun 2019 20:44:11 +0900 Subject: [PATCH 54/60] Bump framework version --- osu.Game/osu.Game.csproj | 2 +- osu.iOS.props | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj index 140d0c7f0e..75a464d0b8 100644 --- a/osu.Game/osu.Game.csproj +++ b/osu.Game/osu.Game.csproj @@ -15,7 +15,7 @@ - + diff --git a/osu.iOS.props b/osu.iOS.props index 39be738a5c..2c25498b89 100644 --- a/osu.iOS.props +++ b/osu.iOS.props @@ -105,8 +105,8 @@ - - + + From 27054a744ed316fd623b33f4a8cfeeec9c787dc8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 12 Jun 2019 00:35:13 +0900 Subject: [PATCH 55/60] Fill in thread pool names --- osu.Game/Beatmaps/BeatmapManager.cs | 2 +- osu.Game/Database/ArchiveModelManager.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 47411c69ec..67d3382e01 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -426,7 +426,7 @@ namespace osu.Game.Beatmaps private const int update_queue_request_concurrency = 4; - private readonly ThreadedTaskScheduler updateScheduler = new ThreadedTaskScheduler(update_queue_request_concurrency); + private readonly ThreadedTaskScheduler updateScheduler = new ThreadedTaskScheduler(update_queue_request_concurrency, nameof(BeatmapUpdateQueue)); public BeatmapUpdateQueue(IAPIProvider api) { diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index 844ae622a5..d764601b32 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -644,6 +644,6 @@ namespace osu.Game.Database /// This scheduler generally performs IO and CPU intensive work so concurrency is limited harshly. /// It is mainly being used as a queue mechanism for large imports. /// - protected static readonly ThreadedTaskScheduler IMPORT_SCHEDULER = new ThreadedTaskScheduler(import_queue_request_concurrency); + protected static readonly ThreadedTaskScheduler IMPORT_SCHEDULER = new ThreadedTaskScheduler(import_queue_request_concurrency, nameof(ArchiveModelManager)); } } From 94e65a324456172971e1b387f039ec1fd7343c0d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 12 Jun 2019 15:16:59 +0900 Subject: [PATCH 56/60] Fix settings checkboxes not being searchable --- osu.Game/Overlays/Settings/SettingsCheckbox.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/osu.Game/Overlays/Settings/SettingsCheckbox.cs b/osu.Game/Overlays/Settings/SettingsCheckbox.cs index a1501d8015..a554159fd7 100644 --- a/osu.Game/Overlays/Settings/SettingsCheckbox.cs +++ b/osu.Game/Overlays/Settings/SettingsCheckbox.cs @@ -10,11 +10,14 @@ namespace osu.Game.Overlays.Settings { private OsuCheckbox checkbox; + private string labelText; + protected override Drawable CreateControl() => checkbox = new OsuCheckbox(); public override string LabelText { - set => checkbox.LabelText = value; + get => labelText; + set => checkbox.LabelText = labelText = value; } } } From 13234fb4a4b5e9a26a3480c3ac3c960fd7f56c60 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Wed, 12 Jun 2019 16:07:35 +0900 Subject: [PATCH 57/60] Adjust comments a bit --- osu.Game/Screens/Select/BeatmapCarousel.cs | 2 +- osu.Game/Screens/Select/SongSelect.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/Select/BeatmapCarousel.cs b/osu.Game/Screens/Select/BeatmapCarousel.cs index 6e3bec106f..cf21c78c7f 100644 --- a/osu.Game/Screens/Select/BeatmapCarousel.cs +++ b/osu.Game/Screens/Select/BeatmapCarousel.cs @@ -155,7 +155,7 @@ namespace osu.Game.Screens.Select int? previouslySelectedID = null; CarouselBeatmapSet existingSet = beatmapSets.FirstOrDefault(b => b.BeatmapSet.ID == beatmapSet.ID); - // Since we're about to remove the selected beatmap, store its ID so we can go back if needed. + // If the selected beatmap is about to be removed, store its ID so it can be re-selected if required if (existingSet?.State?.Value == CarouselItemState.Selected) previouslySelectedID = selectedBeatmap?.Beatmap.ID; diff --git a/osu.Game/Screens/Select/SongSelect.cs b/osu.Game/Screens/Select/SongSelect.cs index dcfe1de8f2..d0645dbab6 100644 --- a/osu.Game/Screens/Select/SongSelect.cs +++ b/osu.Game/Screens/Select/SongSelect.cs @@ -594,7 +594,7 @@ namespace osu.Game.Screens.Select { bindBindables(); - // As a selection was already obtained, do not attempt to update the selected beatmap. + // If a selection was already obtained, do not attempt to update the selected beatmap. if (Carousel.SelectedBeatmapSet != null) return; From c4f54d94bc5e8aa9d52b7e17c0dd3508693cc6bc Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Wed, 12 Jun 2019 17:00:27 +0900 Subject: [PATCH 58/60] Rename methods --- osu.Game/Beatmaps/BeatmapManager.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 67d3382e01..5204bda353 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -110,7 +110,7 @@ namespace osu.Game.Beatmaps validateOnlineIds(beatmapSet); - await updateQueue.Perform(beatmapSet, cancellationToken); + await updateQueue.UpdateAsync(beatmapSet, cancellationToken); } protected override void PreImport(BeatmapSetInfo beatmapSet) @@ -433,20 +433,20 @@ namespace osu.Game.Beatmaps this.api = api; } - public async Task Perform(BeatmapSetInfo beatmapSet, CancellationToken cancellationToken) + public async Task UpdateAsync(BeatmapSetInfo beatmapSet, CancellationToken cancellationToken) { if (api?.State != APIState.Online) return; LogForModel(beatmapSet, "Performing online lookups..."); - await Task.WhenAll(beatmapSet.Beatmaps.Select(b => Perform(beatmapSet, b, cancellationToken)).ToArray()); + await Task.WhenAll(beatmapSet.Beatmaps.Select(b => UpdateAsync(beatmapSet, b, cancellationToken)).ToArray()); } // todo: expose this when we need to do individual difficulty lookups. - protected Task Perform(BeatmapSetInfo beatmapSet, BeatmapInfo beatmap, CancellationToken cancellationToken) - => Task.Factory.StartNew(() => perform(beatmapSet, beatmap), cancellationToken, TaskCreationOptions.HideScheduler, updateScheduler); + protected Task UpdateAsync(BeatmapSetInfo beatmapSet, BeatmapInfo beatmap, CancellationToken cancellationToken) + => Task.Factory.StartNew(() => update(beatmapSet, beatmap), cancellationToken, TaskCreationOptions.HideScheduler, updateScheduler); - private void perform(BeatmapSetInfo set, BeatmapInfo beatmap) + private void update(BeatmapSetInfo set, BeatmapInfo beatmap) { if (api?.State != APIState.Online) return; From fd7dc9504e6c91b9502c5e389d28677be49b81d5 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Wed, 12 Jun 2019 17:08:50 +0900 Subject: [PATCH 59/60] Remove async when not required --- osu.Game/Beatmaps/BeatmapManager.cs | 10 +++++----- osu.Game/Database/ArchiveModelManager.cs | 18 +++++++++--------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 5204bda353..d90657bff5 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -94,7 +94,7 @@ namespace osu.Game.Beatmaps updateQueue = new BeatmapUpdateQueue(api); } - protected override async Task Populate(BeatmapSetInfo beatmapSet, ArchiveReader archive, CancellationToken cancellationToken = default) + protected override Task Populate(BeatmapSetInfo beatmapSet, ArchiveReader archive, CancellationToken cancellationToken = default) { if (archive != null) beatmapSet.Beatmaps = createBeatmapDifficulties(archive); @@ -110,7 +110,7 @@ namespace osu.Game.Beatmaps validateOnlineIds(beatmapSet); - await updateQueue.UpdateAsync(beatmapSet, cancellationToken); + return updateQueue.UpdateAsync(beatmapSet, cancellationToken); } protected override void PreImport(BeatmapSetInfo beatmapSet) @@ -433,13 +433,13 @@ namespace osu.Game.Beatmaps this.api = api; } - public async Task UpdateAsync(BeatmapSetInfo beatmapSet, CancellationToken cancellationToken) + public Task UpdateAsync(BeatmapSetInfo beatmapSet, CancellationToken cancellationToken) { if (api?.State != APIState.Online) - return; + return Task.CompletedTask; LogForModel(beatmapSet, "Performing online lookups..."); - await Task.WhenAll(beatmapSet.Beatmaps.Select(b => UpdateAsync(beatmapSet, b, cancellationToken)).ToArray()); + return Task.WhenAll(beatmapSet.Beatmaps.Select(b => UpdateAsync(beatmapSet, b, cancellationToken)).ToArray()); } // todo: expose this when we need to do individual difficulty lookups. diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index d764601b32..0cf7816250 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -1,4 +1,4 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. using System; @@ -132,13 +132,13 @@ namespace osu.Game.Database /// This will post notifications tracking progress. /// /// One or more archive locations on disk. - public async Task Import(params string[] paths) + public Task Import(params string[] paths) { var notification = new ProgressNotification { State = ProgressNotificationState.Active }; PostNotification?.Invoke(notification); - await Import(notification, paths); + return Import(notification, paths); } protected async Task Import(ProgressNotification notification, params string[] paths) @@ -243,7 +243,7 @@ namespace osu.Game.Database /// /// The archive to be imported. /// An optional cancellation token. - public async Task Import(ArchiveReader archive, CancellationToken cancellationToken = default) + public Task Import(ArchiveReader archive, CancellationToken cancellationToken = default) { cancellationToken.ThrowIfCancellationRequested(); @@ -267,7 +267,7 @@ namespace osu.Game.Database return null; } - return await Import(model, archive, cancellationToken); + return Import(model, archive, cancellationToken); } /// @@ -548,24 +548,24 @@ namespace osu.Game.Database /// /// This is a temporary method and will likely be replaced by a full-fledged (and more correctly placed) migration process in the future. /// - public async Task ImportFromStableAsync() + public Task ImportFromStableAsync() { var stable = GetStableStorage?.Invoke(); if (stable == null) { Logger.Log("No osu!stable installation available!", LoggingTarget.Information, LogLevel.Error); - return; + return Task.CompletedTask; } if (!stable.ExistsDirectory(ImportFromStablePath)) { // This handles situations like when the user does not have a Skins folder Logger.Log($"No {ImportFromStablePath} folder available in osu!stable installation", LoggingTarget.Information, LogLevel.Error); - return; + return Task.CompletedTask; } - await Task.Run(async () => await Import(stable.GetDirectories(ImportFromStablePath).Select(f => stable.GetFullPath(f)).ToArray())); + return Task.Run(async () => await Import(stable.GetDirectories(ImportFromStablePath).Select(f => stable.GetFullPath(f)).ToArray())); } #endregion From 2a67944889c302fe69a87838f70d835c63b50740 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Wed, 12 Jun 2019 17:10:55 +0900 Subject: [PATCH 60/60] Remove interlocked within a lock --- osu.Game/Database/ArchiveModelManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index 0cf7816250..1c17adf7b7 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -161,8 +161,8 @@ namespace osu.Game.Database lock (imported) { imported.Add(model); + current++; - Interlocked.Increment(ref current); notification.Text = $"Imported {current} of {paths.Length} {humanisedModelName}s"; notification.Progress = (float)current / paths.Length; }