Merge pull request #11413 from peppy/fix-mod-settings-transfer-issues

Fix difficulty adjust settings not being transferred correctly in multiplayer/playlists

osu.Game/Online/API/APIMod.cs

@@ -31,7 +31,12 @@ namespace osu.Game.Online.API
             Acronym = mod.Acronym;
 
             foreach (var (_, property) in mod.GetSettingsSourceProperties())
-                Settings.Add(property.Name.Underscore(), property.GetValue(mod));
+            {
+                var bindable = (IBindable)property.GetValue(mod);
+
+                if (!bindable.IsDefault)
+                    Settings.Add(property.Name.Underscore(), bindable);
+            }
         }
 
         public Mod ToMod(Ruleset ruleset)
@@ -46,7 +51,7 @@ namespace osu.Game.Online.API
                 if (!Settings.TryGetValue(property.Name.Underscore(), out object settingValue))
                     continue;
 
-                ((IBindable)property.GetValue(resultMod)).Parse(settingValue);
+                resultMod.CopyAdjustedSetting((IBindable)property.GetValue(resultMod), settingValue);
             }
 
             return resultMod;

osu.Game/Rulesets/Mods/Mod.cs

@@ -3,7 +3,6 @@
 
 using System;
 using System.Collections.Generic;
-using System.Diagnostics;
 using System.Linq;
 using System.Reflection;
 using Newtonsoft.Json;
@@ -84,12 +83,10 @@ namespace osu.Game.Rulesets.Mods
 
                 foreach ((SettingSourceAttribute attr, PropertyInfo property) in this.GetOrderedSettingsSourceProperties())
                 {
-                    object bindableObj = property.GetValue(this);
+                    var bindable = (IBindable)property.GetValue(this);
 
-                    if ((bindableObj as IHasDefaultValue)?.IsDefault == true)
+                    if (!bindable.IsDefault)
-                        continue;
+                        tooltipTexts.Add($"{attr.Label} {bindable}");
-
-                    tooltipTexts.Add($"{attr.Label} {bindableObj}");
                 }
 
                 return string.Join(", ", tooltipTexts.Where(s => !string.IsNullOrEmpty(s)));
@@ -136,19 +133,38 @@ namespace osu.Game.Rulesets.Mods
             // Copy bindable values across
             foreach (var (_, prop) in this.GetSettingsSourceProperties())
             {
-                var origBindable = prop.GetValue(this);
+                var origBindable = (IBindable)prop.GetValue(this);
-                var copyBindable = prop.GetValue(copy);
+                var copyBindable = (IBindable)prop.GetValue(copy);
 
-                // The bindables themselves are readonly, so the value must be transferred through the Bindable<T>.Value property.
+                // we only care about changes that have been made away from defaults.
-                var valueProperty = origBindable.GetType().GetProperty(nameof(Bindable<object>.Value), BindingFlags.Public | BindingFlags.Instance);
+                if (!origBindable.IsDefault)
-                Debug.Assert(valueProperty != null);
+                    copy.CopyAdjustedSetting(copyBindable, origBindable);
-
-                valueProperty.SetValue(copyBindable, valueProperty.GetValue(origBindable));
             }
 
             return copy;
         }
 
+        /// <summary>
+        /// When creating copies or clones of a Mod, this method will be called
+        /// to copy explicitly adjusted user settings from <paramref name="target"/>.
+        /// The base implementation will transfer the value via <see cref="Bindable{T}.Parse"/>
+        /// or by binding and unbinding (if <paramref name="source"/> is an <see cref="IBindable"/>)
+        /// and should be called unless replaced with custom logic.
+        /// </summary>
+        /// <param name="target">The target bindable to apply the adjustment to.</param>
+        /// <param name="source">The adjustment to apply.</param>
+        internal virtual void CopyAdjustedSetting(IBindable target, object source)
+        {
+            if (source is IBindable sourceBindable)
+            {
+                // copy including transfer of default values.
+                target.BindTo(sourceBindable);
+                target.UnbindFrom(sourceBindable);
+            }
+            else
+                target.Parse(source);
+        }
+
         public bool Equals(IMod other) => GetType() == other?.GetType();
     }
 }

osu.Game/Rulesets/Mods/ModDifficultyAdjust.cs

@@ -114,6 +114,12 @@ namespace osu.Game.Rulesets.Mods
             bindable.ValueChanged += _ => userChangedSettings[bindable] = !bindable.IsDefault;
         }
 
+        internal override void CopyAdjustedSetting(IBindable target, object source)
+        {
+            userChangedSettings[target] = true;
+            base.CopyAdjustedSetting(target, source);
+        }
+
         /// <summary>
         /// Apply all custom settings to the provided beatmap.
         /// </summary>