Merge pull request #11918 from bdach/fix-setting-source-multiple-enumeration

Fix multiple enumerations when ordering setting sources

osu.Game.Tests/Mods/SettingsSourceAttributeTest.cs

@@ -0,0 +1,44 @@
+// 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 System.Linq;
+using NUnit.Framework;
+using osu.Framework.Bindables;
+using osu.Game.Configuration;
+
+namespace osu.Game.Tests.Mods
+{
+    [TestFixture]
+    public class SettingsSourceAttributeTest
+    {
+        [Test]
+        public void TestOrdering()
+        {
+            var objectWithSettings = new ClassWithSettings();
+
+            var orderedSettings = objectWithSettings.GetOrderedSettingsSourceProperties().ToArray();
+
+            Assert.That(orderedSettings, Has.Length.EqualTo(4));
+
+            Assert.That(orderedSettings[0].Item2.Name, Is.EqualTo(nameof(ClassWithSettings.FirstSetting)));
+            Assert.That(orderedSettings[1].Item2.Name, Is.EqualTo(nameof(ClassWithSettings.SecondSetting)));
+            Assert.That(orderedSettings[2].Item2.Name, Is.EqualTo(nameof(ClassWithSettings.ThirdSetting)));
+            Assert.That(orderedSettings[3].Item2.Name, Is.EqualTo(nameof(ClassWithSettings.UnorderedSetting)));
+        }
+
+        private class ClassWithSettings
+        {
+            [SettingSource("Unordered setting", "Should be last")]
+            public BindableFloat UnorderedSetting { get; set; } = new BindableFloat();
+
+            [SettingSource("Second setting", "Another description", 2)]
+            public BindableBool SecondSetting { get; set; } = new BindableBool();
+
+            [SettingSource("First setting", "A description", 1)]
+            public BindableDouble FirstSetting { get; set; } = new BindableDouble();
+
+            [SettingSource("Third setting", "Yet another description", 3)]
+            public BindableInt ThirdSetting { get; set; } = new BindableInt();
+        }
+    }
+}

osu.Game/Configuration/SettingSourceAttribute.cs

@@ -23,7 +23,7 @@ namespace osu.Game.Configuration
     /// </remarks>
     [MeansImplicitUse]
     [AttributeUsage(AttributeTargets.Property)]
-    public class SettingSourceAttribute : Attribute
+    public class SettingSourceAttribute : Attribute, IComparable<SettingSourceAttribute>
     {
         public LocalisableString Label { get; }
 
@@ -42,6 +42,21 @@ namespace osu.Game.Configuration
         {
             OrderPosition = orderPosition;
         }
+
+        public int CompareTo(SettingSourceAttribute other)
+        {
+            if (OrderPosition == other.OrderPosition)
+                return 0;
+
+            // unordered items come last (are greater than any ordered items).
+            if (OrderPosition == null)
+                return 1;
+            if (other.OrderPosition == null)
+                return -1;
+
+            // ordered items are sorted by the order value.
+            return OrderPosition.Value.CompareTo(other.OrderPosition);
+        }
     }
 
     public static class SettingSourceExtensions
@@ -137,14 +152,9 @@ namespace osu.Game.Configuration
             }
         }
 
-        public static IEnumerable<(SettingSourceAttribute, PropertyInfo)> GetOrderedSettingsSourceProperties(this object obj)
-        {
-            var original = obj.GetSettingsSourceProperties();
-
-            var orderedRelative = original.Where(attr => attr.Item1.OrderPosition != null).OrderBy(attr => attr.Item1.OrderPosition);
-            var unordered = original.Except(orderedRelative);
-
-            return orderedRelative.Concat(unordered);
-        }
+        public static ICollection<(SettingSourceAttribute, PropertyInfo)> GetOrderedSettingsSourceProperties(this object obj)
+            => obj.GetSettingsSourceProperties()
+                  .OrderBy(attr => attr.Item1)
+                  .ToArray();
     }
 }