Improved readability and sounds

osu.Game.Tests/Mods/ModSettingsTest.cs

@@ -43,18 +43,18 @@ namespace osu.Game.Tests.Mods
 
             var modDouble = new TestNonMatchingSettingTypeModDouble { TestSetting = { Value = setting_change } };
             var modBool = new TestNonMatchingSettingTypeModBool { TestSetting = { Default = false, Value = true } };
-            var modInt = new TestNonMatchingSettingTypeModInt { TestSetting = { Value = (int)setting_change } };
+            var modInt = new TestNonMatchingSettingTypeModInt { TestSetting = { Value = (int)setting_change / 2 } };
 
-            modDouble.CopySharedSettings(modBool);
+            modDouble.CopyCommonSettings(modBool);
-            modDouble.CopySharedSettings(modInt);
+            modDouble.CopyCommonSettings(modInt);
-            modBool.CopySharedSettings(modDouble);
+            modBool.CopyCommonSettings(modDouble);
-            modBool.CopySharedSettings(modInt);
+            modBool.CopyCommonSettings(modInt);
-            modInt.CopySharedSettings(modDouble);
+            modInt.CopyCommonSettings(modDouble);
-            modInt.CopySharedSettings(modBool);
+            modInt.CopyCommonSettings(modBool);
 
             Assert.That(modDouble.TestSetting.Value, Is.EqualTo(setting_change));
             Assert.That(modBool.TestSetting.Value, Is.EqualTo(!modBool.TestSetting.Default));
-            Assert.That(modInt.TestSetting.Value, Is.EqualTo((int)setting_change));
+            Assert.That(modInt.TestSetting.Value, Is.EqualTo((int)setting_change / 2));
         }
 
         [Test]
@@ -63,23 +63,12 @@ namespace osu.Game.Tests.Mods
             var modBoolTrue = new TestNonMatchingSettingTypeModBool { TestSetting = { Default = true, Value = false } };
             var modBoolFalse = new TestNonMatchingSettingTypeModBool { TestSetting = { Default = false, Value = true } };
 
-            modBoolFalse.CopySharedSettings(modBoolTrue);
+            modBoolFalse.CopyCommonSettings(modBoolTrue);
 
             Assert.That(modBoolFalse.TestSetting.Default, Is.EqualTo(false));
             Assert.That(modBoolFalse.TestSetting.Value, Is.EqualTo(modBoolTrue.TestSetting.Value));
         }
 
-        [Test]
-        public void TestValueResetsToDefaultWhenCopied()
-        {
-            var modDouble = new TestNonMatchingSettingTypeModDouble();
-            var modBool = new TestNonMatchingSettingTypeModBool { TestSetting = { Default = false, Value = true } };
-
-            modBool.CopySharedSettings(modDouble);
-
-            Assert.That(modBool.TestSetting.Value, Is.EqualTo(modBool.TestSetting.Default));
-        }
-
         private class TestNonMatchingSettingTypeModDouble : TestNonMatchingSettingTypeMod
         {
             public override string Acronym => "NMD";

osu.Game/OsuGameBase.cs

@@ -58,7 +58,6 @@ using osu.Game.Rulesets.Mods;
 using osu.Game.Scoring;
 using osu.Game.Skinning;
 using osu.Game.Utils;
-using File = System.IO.File;
 using RuntimeInfo = osu.Framework.RuntimeInfo;
 
 namespace osu.Game
@@ -71,7 +70,7 @@ namespace osu.Game
     [Cached(typeof(OsuGameBase))]
     public partial class OsuGameBase : Framework.Game, ICanAcceptFiles, IBeatSyncProvider
     {
-        public static readonly string[] VIDEO_EXTENSIONS = { ".mp4", ".mov", ".avi", ".flv" };
+        public static readonly string[] VIDEO_EXTENSIONS = { ".mp4", ".mov", ".avi", ".flv", ".mpg", ".wmv", ".m4v" };
 
         public const string OSU_PROTOCOL = "osu://";
 
@@ -626,20 +625,30 @@ namespace osu.Game
                 return;
             }
 
+            var previouslySelectedCommonMods = new List<Mod>(SelectedMods.Value.Count);
+            var convertedCommonMods = new List<Mod>(SelectedMods.Value.Count);
+
+            foreach (var mod in SelectedMods.Value)
+            {
+                var convertedMod = instance.CreateModFromAcronym(mod.Acronym);
+
+                if (convertedMod == null)
+                    continue;
+
+                previouslySelectedCommonMods.Add(mod);
+
+                convertedMod.CopyCommonSettings(mod);
+                convertedCommonMods.Add(convertedMod);
+            }
+
+            if (!SelectedMods.Disabled)
+                // Select common mods to play the deselect samples for other mods
+                SelectedMods.Value = previouslySelectedCommonMods;
+
             AvailableMods.Value = dict;
 
             if (!SelectedMods.Disabled)
-            {
+                SelectedMods.Value = convertedCommonMods;
-                //converting mods from one ruleset to the other, while also keeping their shared settings unchanged
-                SelectedMods.Value = SelectedMods.Value.Select(oldMod =>
-                {
-                    Mod newMod = instance.CreateModFromAcronym(oldMod.Acronym);
-
-                    newMod?.CopySharedSettings(oldMod);
-
-                    return newMod;
-                }).Where(m => m != null).ToArray();
-            }
 
             void revertRulesetChange() => Ruleset.Value = r.OldValue?.Available == true ? r.OldValue : RulesetStore.AvailableRulesets.First();
         }

osu.Game/Rulesets/Mods/Mod.cs

@@ -115,21 +115,25 @@ namespace osu.Game.Rulesets.Mods
         [JsonIgnore]
         public virtual Type[] IncompatibleMods => Array.Empty<Type>();
 
-        private IReadOnlyList<IBindable>? settingsBacking;
+        private IReadOnlyDictionary<string, IBindable>? settingsBacking;
 
         /// <summary>
-        /// A list of the all <see cref="IBindable"/> settings within this mod.
+        /// All <see cref="IBindable"/> settings within this mod.
         /// </summary>
-        internal IReadOnlyList<IBindable> Settings =>
+        internal IEnumerable<IBindable> SettingsBindables => Settings.Values;
+
+        /// <summary>
+        /// Provides mapping of names to <see cref="IBindable"/>s of all settings within this mod.
+        /// </summary>
+        internal IReadOnlyDictionary<string, IBindable> Settings =>
             settingsBacking ??= this.GetSettingsSourceProperties()
-                                    .Select(p => p.Item2.GetValue(this))
+                                    .Select(p => p.Item2)
-                                    .Cast<IBindable>()
+                                    .ToDictionary(property => property.Name.ToSnakeCase(), property => (IBindable)property.GetValue(this)!);
-                                    .ToList();
 
         /// <summary>
         /// Whether all settings in this mod are set to their default state.
         /// </summary>
-        protected virtual bool UsesDefaultConfiguration => Settings.All(s => s.IsDefault);
+        protected virtual bool UsesDefaultConfiguration => SettingsBindables.All(s => s.IsDefault);
 
         /// <summary>
         /// Creates a copy of this <see cref="Mod"/> initialised to a default state.
@@ -160,60 +164,37 @@ namespace osu.Game.Rulesets.Mods
         }
 
         /// <summary>
-        /// Copies all mod setting values sharing same <see cref="MemberInfo.Name"/> from <paramref name="source"/> into this instance.
+        /// When converting mods from one ruleset to the other, this method makes sure
+        /// to also copy the values of all settings sharing same <see cref="MemberInfo.Name"/> between the two instances.
         /// </summary>
-        /// <param name="source">The mod to copy properties from.</param>
+        /// <remarks>Copied values are unchanged, even if they have different clamping ranges.</remarks>
-        internal void CopySharedSettings(Mod source)
+        /// <param name="source">The mod to extract settings from.</param>
+        public void CopyCommonSettings(Mod source)
         {
-            const string value = nameof(Bindable<int>.Value);
+            if (source.UsesDefaultConfiguration)
+                return;
 
-            Dictionary<string, object> sourceSettings = new Dictionary<string, object>();
+            foreach (var (name, targetSetting) in Settings)
-
-            foreach (var (_, sourceProperty) in source.GetSettingsSourceProperties())
             {
-                sourceSettings.Add(sourceProperty.Name.ToSnakeCase(), sourceProperty.GetValue(source)!);
+                if (!source.Settings.TryGetValue(name, out IBindable? sourceSetting))
+                    continue;
+
+                if (sourceSetting.IsDefault)
+                    continue;
+
+                if (getBindableGenericType(targetSetting) != getBindableGenericType(sourceSetting))
+                    continue;
+
+                // TODO: special case for handling number types
+
+                PropertyInfo property = targetSetting.GetType().GetProperty(nameof(Bindable<bool>.Value))!;
+                property.SetValue(targetSetting, property.GetValue(sourceSetting));
             }
 
-            foreach (var (_, targetProperty) in this.GetSettingsSourceProperties())
+            Type getBindableGenericType(IBindable setting) =>
-            {
+                setting.GetType().GetTypeInheritance()
-                object targetSetting = targetProperty.GetValue(this)!;
+                       .First(t => t.IsGenericType && t.GetGenericTypeDefinition() == typeof(Bindable<>))
-
+                       .GenericTypeArguments.Single();
-                if (!sourceSettings.TryGetValue(targetProperty.Name.ToSnakeCase(), out object? sourceSetting))
-                    continue;
-
-                if (((IBindable)sourceSetting).IsDefault)
-                {
-                    // reset to default value if the source is default
-                    targetSetting.GetType().GetMethod(nameof(Bindable<int>.SetDefault))!.Invoke(targetSetting, null);
-                    continue;
-                }
-
-                bool hasSameGenericArgument = getGenericBaseType(targetSetting, typeof(Bindable<>))!.GenericTypeArguments.Single() ==
-                                              getGenericBaseType(sourceSetting, typeof(Bindable<>))!.GenericTypeArguments.Single();
-
-                if (!hasSameGenericArgument)
-                    continue;
-
-                Type? targetBindableNumberType = getGenericBaseType(targetSetting, typeof(BindableNumber<>));
-                Type? sourceBindableNumberType = getGenericBaseType(sourceSetting, typeof(BindableNumber<>));
-
-                if (targetBindableNumberType == null || sourceBindableNumberType == null)
-                {
-                    setValue(targetSetting, value, getValue(sourceSetting, value));
-                    continue;
-                }
-
-                setValue(targetSetting, value, Convert.ChangeType(getValue(sourceSetting, value), targetBindableNumberType.GenericTypeArguments.Single()));
-            }
-
-            object? getValue(object setting, string name) =>
-                setting.GetType().GetProperty(name)!.GetValue(setting);
-
-            void setValue(object setting, string name, object? newValue) =>
-                setting.GetType().GetProperty(name)!.SetValue(setting, newValue);
-
-            Type? getGenericBaseType(object setting, Type genericType) =>
-                setting.GetType().GetTypeInheritance().FirstOrDefault(t => t.IsGenericType && t.GetGenericTypeDefinition() == genericType);
         }
 
         /// <summary>
@@ -250,7 +231,7 @@ namespace osu.Game.Rulesets.Mods
             if (ReferenceEquals(this, other)) return true;
 
             return GetType() == other.GetType() &&
-                   Settings.SequenceEqual(other.Settings, ModSettingsEqualityComparer.Default);
+                   SettingsBindables.SequenceEqual(other.SettingsBindables, ModSettingsEqualityComparer.Default);
         }
 
         public override int GetHashCode()
@@ -259,7 +240,7 @@ namespace osu.Game.Rulesets.Mods
 
             hashCode.Add(GetType());
 
-            foreach (var setting in Settings)
+            foreach (var setting in SettingsBindables)
                 hashCode.Add(setting.GetUnderlyingSettingValue());
 
             return hashCode.ToHashCode();