From 25e142965d925ef6936b6424175b3736041dbcfd Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 4 Sep 2020 15:01:32 +0900 Subject: [PATCH 1/7] Strongly type and expose default beatmap information icon implementations for other rulesets --- .../Beatmaps/CatchBeatmap.cs | 6 +-- .../Beatmaps/ManiaBeatmap.cs | 4 +- osu.Game.Rulesets.Osu/Beatmaps/OsuBeatmap.cs | 6 +-- .../Beatmaps/TaikoBeatmap.cs | 6 +-- osu.Game/Beatmaps/BeatmapStatistic.cs | 2 +- osu.Game/Beatmaps/BeatmapStatisticIcon.cs | 43 +++++++++++++++++++ osu.Game/Beatmaps/BeatmapStatisticSprite.cs | 25 ----------- osu.Game/Screens/Select/BeatmapInfoWedge.cs | 4 +- 8 files changed, 57 insertions(+), 39 deletions(-) create mode 100644 osu.Game/Beatmaps/BeatmapStatisticIcon.cs delete mode 100644 osu.Game/Beatmaps/BeatmapStatisticSprite.cs diff --git a/osu.Game.Rulesets.Catch/Beatmaps/CatchBeatmap.cs b/osu.Game.Rulesets.Catch/Beatmaps/CatchBeatmap.cs index 5dc19ce15b..f009c10a9c 100644 --- a/osu.Game.Rulesets.Catch/Beatmaps/CatchBeatmap.cs +++ b/osu.Game.Rulesets.Catch/Beatmaps/CatchBeatmap.cs @@ -22,19 +22,19 @@ namespace osu.Game.Rulesets.Catch.Beatmaps { Name = @"Fruit Count", Content = fruits.ToString(), - CreateIcon = () => new BeatmapStatisticSprite("circles"), + CreateIcon = () => new BeatmapStatisticIcon(BeatmapStatisticsIconType.Circles), }, new BeatmapStatistic { Name = @"Juice Stream Count", Content = juiceStreams.ToString(), - CreateIcon = () => new BeatmapStatisticSprite("sliders"), + CreateIcon = () => new BeatmapStatisticIcon(BeatmapStatisticsIconType.Sliders), }, new BeatmapStatistic { Name = @"Banana Shower Count", Content = bananaShowers.ToString(), - CreateIcon = () => new BeatmapStatisticSprite("spinners"), + CreateIcon = () => new BeatmapStatisticIcon(BeatmapStatisticsIconType.Spinners), } }; } diff --git a/osu.Game.Rulesets.Mania/Beatmaps/ManiaBeatmap.cs b/osu.Game.Rulesets.Mania/Beatmaps/ManiaBeatmap.cs index f6b460f269..d1d5adea75 100644 --- a/osu.Game.Rulesets.Mania/Beatmaps/ManiaBeatmap.cs +++ b/osu.Game.Rulesets.Mania/Beatmaps/ManiaBeatmap.cs @@ -40,13 +40,13 @@ namespace osu.Game.Rulesets.Mania.Beatmaps new BeatmapStatistic { Name = @"Note Count", - CreateIcon = () => new BeatmapStatisticSprite("circles"), + CreateIcon = () => new BeatmapStatisticIcon(BeatmapStatisticsIconType.Circles), Content = notes.ToString(), }, new BeatmapStatistic { Name = @"Hold Note Count", - CreateIcon = () => new BeatmapStatisticSprite("sliders"), + CreateIcon = () => new BeatmapStatisticIcon(BeatmapStatisticsIconType.Sliders), Content = holdnotes.ToString(), }, }; diff --git a/osu.Game.Rulesets.Osu/Beatmaps/OsuBeatmap.cs b/osu.Game.Rulesets.Osu/Beatmaps/OsuBeatmap.cs index 513a9254ec..2d3cc3c103 100644 --- a/osu.Game.Rulesets.Osu/Beatmaps/OsuBeatmap.cs +++ b/osu.Game.Rulesets.Osu/Beatmaps/OsuBeatmap.cs @@ -22,19 +22,19 @@ namespace osu.Game.Rulesets.Osu.Beatmaps { Name = @"Circle Count", Content = circles.ToString(), - CreateIcon = () => new BeatmapStatisticSprite("circles"), + CreateIcon = () => new BeatmapStatisticIcon(BeatmapStatisticsIconType.Circles), }, new BeatmapStatistic { Name = @"Slider Count", Content = sliders.ToString(), - CreateIcon = () => new BeatmapStatisticSprite("sliders"), + CreateIcon = () => new BeatmapStatisticIcon(BeatmapStatisticsIconType.Sliders), }, new BeatmapStatistic { Name = @"Spinner Count", Content = spinners.ToString(), - CreateIcon = () => new BeatmapStatisticSprite("spinners"), + CreateIcon = () => new BeatmapStatisticIcon(BeatmapStatisticsIconType.Spinners), } }; } diff --git a/osu.Game.Rulesets.Taiko/Beatmaps/TaikoBeatmap.cs b/osu.Game.Rulesets.Taiko/Beatmaps/TaikoBeatmap.cs index c0f8af4fff..16a0726c8c 100644 --- a/osu.Game.Rulesets.Taiko/Beatmaps/TaikoBeatmap.cs +++ b/osu.Game.Rulesets.Taiko/Beatmaps/TaikoBeatmap.cs @@ -21,19 +21,19 @@ namespace osu.Game.Rulesets.Taiko.Beatmaps new BeatmapStatistic { Name = @"Hit Count", - CreateIcon = () => new BeatmapStatisticSprite("circles"), + CreateIcon = () => new BeatmapStatisticIcon(BeatmapStatisticsIconType.Circles), Content = hits.ToString(), }, new BeatmapStatistic { Name = @"Drumroll Count", - CreateIcon = () => new BeatmapStatisticSprite("sliders"), + CreateIcon = () => new BeatmapStatisticIcon(BeatmapStatisticsIconType.Sliders), Content = drumrolls.ToString(), }, new BeatmapStatistic { Name = @"Swell Count", - CreateIcon = () => new BeatmapStatisticSprite("spinners"), + CreateIcon = () => new BeatmapStatisticIcon(BeatmapStatisticsIconType.Spinners), Content = swells.ToString(), } }; diff --git a/osu.Game/Beatmaps/BeatmapStatistic.cs b/osu.Game/Beatmaps/BeatmapStatistic.cs index 825bb08246..9d87a20d60 100644 --- a/osu.Game/Beatmaps/BeatmapStatistic.cs +++ b/osu.Game/Beatmaps/BeatmapStatistic.cs @@ -14,7 +14,7 @@ namespace osu.Game.Beatmaps public IconUsage Icon = FontAwesome.Regular.QuestionCircle; /// - /// A function to create the icon for display purposes. + /// A function to create the icon for display purposes. Use default icons available via whenever possible for conformity. /// public Func CreateIcon; diff --git a/osu.Game/Beatmaps/BeatmapStatisticIcon.cs b/osu.Game/Beatmaps/BeatmapStatisticIcon.cs new file mode 100644 index 0000000000..181fb540df --- /dev/null +++ b/osu.Game/Beatmaps/BeatmapStatisticIcon.cs @@ -0,0 +1,43 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using Humanizer; +using osu.Framework.Allocation; +using osu.Framework.Graphics.Sprites; +using osu.Framework.Graphics.Textures; + +namespace osu.Game.Beatmaps +{ + /// + /// A default implementation of an icon used to represent beatmap statistics. + /// + public class BeatmapStatisticIcon : Sprite + { + private readonly BeatmapStatisticsIconType iconType; + + public BeatmapStatisticIcon(BeatmapStatisticsIconType iconType) + { + this.iconType = iconType; + } + + [BackgroundDependencyLoader] + private void load(TextureStore textures) + { + Texture = textures.Get($"Icons/BeatmapDetails/{iconType.ToString().Kebaberize()}"); + } + } + + public enum BeatmapStatisticsIconType + { + Accuracy, + ApproachRate, + Bpm, + Circles, + HpDrain, + Length, + OverallDifficulty, + Size, + Sliders, + Spinners, + } +} diff --git a/osu.Game/Beatmaps/BeatmapStatisticSprite.cs b/osu.Game/Beatmaps/BeatmapStatisticSprite.cs deleted file mode 100644 index 1cb0bacf0f..0000000000 --- a/osu.Game/Beatmaps/BeatmapStatisticSprite.cs +++ /dev/null @@ -1,25 +0,0 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. -// See the LICENCE file in the repository root for full licence text. - -using osu.Framework.Allocation; -using osu.Framework.Graphics.Sprites; -using osu.Framework.Graphics.Textures; - -namespace osu.Game.Beatmaps -{ - public class BeatmapStatisticSprite : Sprite - { - private readonly string iconName; - - public BeatmapStatisticSprite(string iconName) - { - this.iconName = iconName; - } - - [BackgroundDependencyLoader] - private void load(TextureStore textures) - { - Texture = textures.Get($"Icons/BeatmapDetails/{iconName}"); - } - } -} diff --git a/osu.Game/Screens/Select/BeatmapInfoWedge.cs b/osu.Game/Screens/Select/BeatmapInfoWedge.cs index 400f3e3063..2a3eb8c67a 100644 --- a/osu.Game/Screens/Select/BeatmapInfoWedge.cs +++ b/osu.Game/Screens/Select/BeatmapInfoWedge.cs @@ -318,14 +318,14 @@ namespace osu.Game.Screens.Select labels.Add(new InfoLabel(new BeatmapStatistic { Name = "Length", - CreateIcon = () => new BeatmapStatisticSprite("length"), + CreateIcon = () => new BeatmapStatisticIcon(BeatmapStatisticsIconType.Length), Content = TimeSpan.FromMilliseconds(b.BeatmapInfo.Length).ToString(@"m\:ss"), })); labels.Add(new InfoLabel(new BeatmapStatistic { Name = "BPM", - CreateIcon = () => new BeatmapStatisticSprite("bpm"), + CreateIcon = () => new BeatmapStatisticIcon(BeatmapStatisticsIconType.Bpm), Content = getBPMRange(b), })); From a15653c77cf79c1a6fc8628979d30c2dbb95492e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 4 Sep 2020 16:15:57 +0900 Subject: [PATCH 2/7] Fix potential hard crash if ruleset settings fail to construct --- .../Overlays/Settings/Sections/GameplaySection.cs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/osu.Game/Overlays/Settings/Sections/GameplaySection.cs b/osu.Game/Overlays/Settings/Sections/GameplaySection.cs index aca507f20a..c09e3a227d 100644 --- a/osu.Game/Overlays/Settings/Sections/GameplaySection.cs +++ b/osu.Game/Overlays/Settings/Sections/GameplaySection.cs @@ -1,12 +1,14 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System; using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Game.Overlays.Settings.Sections.Gameplay; using osu.Game.Rulesets; using System.Linq; using osu.Framework.Graphics.Sprites; +using osu.Framework.Logging; namespace osu.Game.Overlays.Settings.Sections { @@ -35,8 +37,18 @@ namespace osu.Game.Overlays.Settings.Sections foreach (Ruleset ruleset in rulesets.AvailableRulesets.Select(info => info.CreateInstance())) { SettingsSubsection section = ruleset.CreateSettings(); + if (section != null) - Add(section); + { + try + { + Add(section); + } + catch (Exception e) + { + Logger.Error(e, $"Failed to load ruleset settings"); + } + } } } } From 644f3375ac2210428cf7dfbd8afb58fb9e0dd0aa Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 4 Sep 2020 16:28:19 +0900 Subject: [PATCH 3/7] Also catch exceptions in the construction call --- .../Settings/Sections/GameplaySection.cs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/osu.Game/Overlays/Settings/Sections/GameplaySection.cs b/osu.Game/Overlays/Settings/Sections/GameplaySection.cs index c09e3a227d..f76b8e085b 100644 --- a/osu.Game/Overlays/Settings/Sections/GameplaySection.cs +++ b/osu.Game/Overlays/Settings/Sections/GameplaySection.cs @@ -36,18 +36,16 @@ namespace osu.Game.Overlays.Settings.Sections { foreach (Ruleset ruleset in rulesets.AvailableRulesets.Select(info => info.CreateInstance())) { - SettingsSubsection section = ruleset.CreateSettings(); - - if (section != null) + try { - try - { + SettingsSubsection section = ruleset.CreateSettings(); + + if (section != null) Add(section); - } - catch (Exception e) - { - Logger.Error(e, $"Failed to load ruleset settings"); - } + } + catch (Exception e) + { + Logger.Error(e, $"Failed to load ruleset settings"); } } } From ab057e6c654886d961b5abd7b2bdd795a93ed28f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 4 Sep 2020 16:28:35 +0900 Subject: [PATCH 4/7] Remove unnecessary string interpolation --- osu.Game/Overlays/Settings/Sections/GameplaySection.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Overlays/Settings/Sections/GameplaySection.cs b/osu.Game/Overlays/Settings/Sections/GameplaySection.cs index f76b8e085b..e5cebd28e2 100644 --- a/osu.Game/Overlays/Settings/Sections/GameplaySection.cs +++ b/osu.Game/Overlays/Settings/Sections/GameplaySection.cs @@ -45,7 +45,7 @@ namespace osu.Game.Overlays.Settings.Sections } catch (Exception e) { - Logger.Error(e, $"Failed to load ruleset settings"); + Logger.Error(e, "Failed to load ruleset settings"); } } } From 65d541456ab8a408095c1b028930f72dfc72f902 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 4 Sep 2020 11:11:07 +0300 Subject: [PATCH 5/7] Slight rewording --- osu.Game/Overlays/MusicController.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Overlays/MusicController.cs b/osu.Game/Overlays/MusicController.cs index 119aad5226..74a438a124 100644 --- a/osu.Game/Overlays/MusicController.cs +++ b/osu.Game/Overlays/MusicController.cs @@ -279,7 +279,7 @@ namespace osu.Game.Overlays private void changeBeatmap(WorkingBeatmap newWorking) { - // The provided beatmap is same as current, no need to do any changes. + // If the provided beatmap is same as current, then there is no need to do any changes. if (newWorking == current) return; From 4236e5fe71ceec15ffd6881532d8fda26dbd8f38 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 4 Sep 2020 11:31:54 +0300 Subject: [PATCH 6/7] Replace useless "matching-code" comment with explanation of how it could happen Co-authored-by: Dean Herbert --- osu.Game/Overlays/MusicController.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game/Overlays/MusicController.cs b/osu.Game/Overlays/MusicController.cs index 74a438a124..edef4d8589 100644 --- a/osu.Game/Overlays/MusicController.cs +++ b/osu.Game/Overlays/MusicController.cs @@ -279,7 +279,8 @@ namespace osu.Game.Overlays private void changeBeatmap(WorkingBeatmap newWorking) { - // If the provided beatmap is same as current, then there is no need to do any changes. + // This method can potentially be triggered multiple times as it is eagerly fired in next() / prev() to ensure correct execution order + // (changeBeatmap must be called before consumers receive the bindable changed event, which is not the case when called from the bindable itself). if (newWorking == current) return; From 3239576a239922e8dad80f61bff40a1b42205618 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 4 Sep 2020 11:50:49 +0300 Subject: [PATCH 7/7] Minor rewording of new comment Co-authored-by: Dean Herbert --- osu.Game/Overlays/MusicController.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Overlays/MusicController.cs b/osu.Game/Overlays/MusicController.cs index edef4d8589..31bd80d6f3 100644 --- a/osu.Game/Overlays/MusicController.cs +++ b/osu.Game/Overlays/MusicController.cs @@ -280,7 +280,7 @@ namespace osu.Game.Overlays private void changeBeatmap(WorkingBeatmap newWorking) { // This method can potentially be triggered multiple times as it is eagerly fired in next() / prev() to ensure correct execution order - // (changeBeatmap must be called before consumers receive the bindable changed event, which is not the case when called from the bindable itself). + // (changeBeatmap must be called before consumers receive the bindable changed event, which is not the case when the local beatmap bindable is updated directly). if (newWorking == current) return;