Merge pull request #17586 from Joehuu/move-difficulty-graph-toggle

Move difficulty graph toggle to individual skin settings

osu.Game/Configuration/OsuConfigManager.cs

@@ -275,6 +275,8 @@ namespace osu.Game.Configuration
         AlwaysPlayFirstComboBreak,
         FloatingComments,
         HUDVisibilityMode,
+
+        // This has been migrated to the component itself. can be removed 20221027.
         ShowProgressGraph,
         ShowHealthDisplayWhenCantFail,
         FadePlayfieldWhenHealthLow,

osu.Game/Localisation/GameplaySettingsStrings.cs

@@ -64,11 +64,6 @@ namespace osu.Game.Localisation
         /// </summary>
         public static LocalisableString HUDVisibilityMode => new TranslatableString(getKey(@"hud_visibility_mode"), @"HUD overlay visibility mode");
 
-        /// <summary>
-        /// "Show difficulty graph on progress bar"
-        /// </summary>
-        public static LocalisableString ShowDifficultyGraph => new TranslatableString(getKey(@"show_difficulty_graph"), @"Show difficulty graph on progress bar");
-
         /// <summary>
         /// "Show health display even when you can't fail"
         /// </summary>

osu.Game/Overlays/Settings/Sections/Gameplay/HUDSettings.cs

@@ -24,11 +24,6 @@ namespace osu.Game.Overlays.Settings.Sections.Gameplay
                     Current = config.GetBindable<HUDVisibilityMode>(OsuSetting.HUDVisibilityMode)
                 },
                 new SettingsCheckbox
-                {
-                    LabelText = GameplaySettingsStrings.ShowDifficultyGraph,
-                    Current = config.GetBindable<bool>(OsuSetting.ShowProgressGraph)
-                },
-                new SettingsCheckbox
                 {
                     ClassicDefault = false,
                     LabelText = GameplaySettingsStrings.ShowHealthDisplayWhenCantFail,

osu.Game/Screens/Play/SongProgress.cs

@@ -1,20 +1,20 @@
 // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
-using osuTK;
-using osu.Framework.Graphics;
-using osu.Framework.Graphics.Containers;
 using System;
 using System.Collections.Generic;
-using osu.Game.Graphics;
-using osu.Framework.Allocation;
 using System.Linq;
+using osu.Framework.Allocation;
 using osu.Framework.Bindables;
+using osu.Framework.Graphics;
+using osu.Framework.Graphics.Containers;
 using osu.Framework.Timing;
 using osu.Game.Configuration;
+using osu.Game.Graphics;
 using osu.Game.Rulesets.Objects;
 using osu.Game.Rulesets.UI;
 using osu.Game.Skinning;
+using osuTK;
 
 namespace osu.Game.Screens.Play
 {
@@ -42,7 +42,8 @@ namespace osu.Game.Screens.Play
         /// </summary>
         public readonly Bindable<bool> AllowSeeking = new Bindable<bool>();
 
-        public readonly Bindable<bool> ShowGraph = new Bindable<bool>();
+        [SettingSource("Show difficulty graph", "Whether a graph displaying difficulty throughout the beatmap should be shown")]
+        public Bindable<bool> ShowGraph { get; } = new BindableBool(true);
 
         public override bool HandleNonPositionalInput => AllowSeeking.Value;
         public override bool HandlePositionalInput => AllowSeeking.Value;
@@ -116,7 +117,7 @@ namespace osu.Game.Screens.Play
         }
 
         [BackgroundDependencyLoader(true)]
-        private void load(OsuColour colours, OsuConfigManager config)
+        private void load(OsuColour colours)
         {
             base.LoadComplete();
 
@@ -129,8 +130,6 @@ namespace osu.Game.Screens.Play
                 Objects = drawableRuleset.Objects;
             }
 
-            config.BindWith(OsuSetting.ShowProgressGraph, ShowGraph);
-
             graph.FillColour = bar.FillColour = colours.BlueLighter;
         }
 
@@ -140,6 +139,56 @@ namespace osu.Game.Screens.Play
 
             AllowSeeking.BindValueChanged(_ => updateBarVisibility(), true);
             ShowGraph.BindValueChanged(_ => updateGraphVisibility(), true);
+
+            migrateSettingFromConfig();
+        }
+
+        [Resolved]
+        private OsuConfigManager config { get; set; }
+
+        [Resolved]
+        private SkinManager skinManager { get; set; }
+
+        /// <summary>
+        /// This setting has been migrated to a per-component level.
+        /// Only take the value from the config if it is in a non-default state (then reset it to default so it only applies once).
+        ///
+        /// Can be removed 20221027.
+        /// </summary>
+        private void migrateSettingFromConfig()
+        {
+            Bindable<bool> configShowGraph = config.GetBindable<bool>(OsuSetting.ShowProgressGraph);
+
+            if (!configShowGraph.IsDefault)
+            {
+                ShowGraph.Value = configShowGraph.Value;
+
+                // This is pretty ugly, but the only way to make this stick...
+                if (skinManager != null)
+                {
+                    var skinnableTarget = this.FindClosestParent<ISkinnableTarget>();
+
+                    if (skinnableTarget != null)
+                    {
+                        // If the skin is not mutable, a mutable instance will be created, causing this migration logic to run again on the correct skin.
+                        // Therefore we want to avoid resetting the config value on this invocation.
+                        if (skinManager.EnsureMutableSkin())
+                            return;
+
+                        // If `EnsureMutableSkin` actually changed the skin, default layout may take a frame to apply.
+                        // See `SkinnableTargetComponentsContainer`'s use of ScheduleAfterChildren.
+                        ScheduleAfterChildren(() =>
+                        {
+                            var skin = skinManager.CurrentSkin.Value;
+                            skin.UpdateDrawableTarget(skinnableTarget);
+
+                            skinManager.Save(skin);
+                        });
+
+                        configShowGraph.SetDefault();
+                    }
+                }
+            }
         }
 
         protected override void PopIn()

osu.Game/Skinning/SkinManager.cs

@@ -143,12 +143,15 @@ namespace osu.Game.Skinning
         /// Ensure that the current skin is in a state it can accept user modifications.
         /// This will create a copy of any internal skin and being tracking in the database if not already.
         /// </summary>
-        public void EnsureMutableSkin()
+        /// <returns>
+        /// Whether a new skin was created to allow for mutation.
+        /// </returns>
+        public bool EnsureMutableSkin()
         {
-            CurrentSkinInfo.Value.PerformRead(s =>
+            return CurrentSkinInfo.Value.PerformRead(s =>
             {
                 if (!s.Protected)
-                    return;
+                    return false;
 
                 string[] existingSkinNames = realm.Run(r => r.All<SkinInfo>()
                                                              .Where(skin => !skin.DeletePending)
@@ -160,7 +163,7 @@ namespace osu.Game.Skinning
                 {
                     Creator = s.Creator,
                     InstantiationInfo = s.InstantiationInfo,
-                    Name = NamingUtils.GetNextBestName(existingSkinNames, $"{s.Name} (modified)")
+                    Name = NamingUtils.GetNextBestName(existingSkinNames, $@"{s.Name} (modified)")
                 };
 
                 var result = skinModelManager.Import(skinInfo);
@@ -171,7 +174,10 @@ namespace osu.Game.Skinning
                     // currently this only happens on save.
                     result.PerformRead(skin => Save(skin.CreateInstance(this)));
                     CurrentSkinInfo.Value = result;
+                    return true;
                 }
+
+                return false;
             });
         }