From 7476cb3047b400489d39af6494c782de531313ed Mon Sep 17 00:00:00 2001 From: rednir Date: Mon, 18 Jan 2021 19:51:42 +0000 Subject: [PATCH 01/12] Sort SkinSection in alphabetical order --- .../Overlays/Settings/Sections/SkinSection.cs | 35 ++++++++++++++----- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/osu.Game/Overlays/Settings/Sections/SkinSection.cs b/osu.Game/Overlays/Settings/Sections/SkinSection.cs index 5898482e4a..0bfa0ba4f0 100644 --- a/osu.Game/Overlays/Settings/Sections/SkinSection.cs +++ b/osu.Game/Overlays/Settings/Sections/SkinSection.cs @@ -107,21 +107,20 @@ namespace osu.Game.Overlays.Settings.Sections private void updateItems() { skinItems = skins.GetAllUsableSkins(); - - // insert after lazer built-in skins - int firstNonDefault = skinItems.FindIndex(s => s.ID > 0); - if (firstNonDefault < 0) - firstNonDefault = skinItems.Count; - - skinItems.Insert(firstNonDefault, random_skin_info); - + skinItems = sortList(skinItems); + skinDropdown.Items = skinItems; } private void itemUpdated(ValueChangedEvent> weakItem) { if (weakItem.NewValue.TryGetTarget(out var item)) - Schedule(() => skinDropdown.Items = skinDropdown.Items.Where(i => !i.Equals(item)).Append(item).ToArray()); + { + List newDropdownItems = skinDropdown.Items.ToList(); + newDropdownItems.Add(item); + newDropdownItems = sortList(newDropdownItems); + Schedule(() => skinDropdown.Items = newDropdownItems.ToArray()); + } } private void itemRemoved(ValueChangedEvent> weakItem) @@ -130,6 +129,24 @@ namespace osu.Game.Overlays.Settings.Sections Schedule(() => skinDropdown.Items = skinDropdown.Items.Where(i => i.ID != item.ID).ToArray()); } + private List sortList(List skinsList) + { + skinsList.Sort((a, b) => String.Compare(a.Name, b.Name, StringComparison.Ordinal)); + for (int i = 0; i < skinsList.Count; i++) + { + // insert lazer built-in skins before user skins + if (skinsList[i].ID <= 0) { + var itemToMove = skinsList[i]; + skinsList.RemoveAt(i); + skinsList.Insert(0, itemToMove); + } + } + skinsList.RemoveAll(s => s.ID == SkinInfo.RANDOM_SKIN); + skinsList.Insert(0, random_skin_info); + + return skinsList; + } + private class SkinSettingsDropdown : SettingsDropdown { protected override OsuDropdown CreateDropdown() => new SkinDropdownControl(); From 0b65c0cd25a727fbfa121395e515fe9e8bd295b1 Mon Sep 17 00:00:00 2001 From: rednir Date: Mon, 18 Jan 2021 20:17:42 +0000 Subject: [PATCH 02/12] Remove whitespace --- osu.Game/Overlays/Settings/Sections/SkinSection.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game/Overlays/Settings/Sections/SkinSection.cs b/osu.Game/Overlays/Settings/Sections/SkinSection.cs index 8e78940ac2..20953b7a9b 100644 --- a/osu.Game/Overlays/Settings/Sections/SkinSection.cs +++ b/osu.Game/Overlays/Settings/Sections/SkinSection.cs @@ -113,7 +113,6 @@ namespace osu.Game.Overlays.Settings.Sections { skinItems = skins.GetAllUsableSkins(); skinItems = sortList(skinItems); - skinDropdown.Items = skinItems; } From f1894a8bacb83c8e35c62422447d2d7cd78a3aa9 Mon Sep 17 00:00:00 2001 From: rednir Date: Tue, 19 Jan 2021 12:17:56 +0000 Subject: [PATCH 03/12] fixed itemUpdated() --- osu.Game/Overlays/Settings/Sections/SkinSection.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Game/Overlays/Settings/Sections/SkinSection.cs b/osu.Game/Overlays/Settings/Sections/SkinSection.cs index 0bfa0ba4f0..ba92ed5b04 100644 --- a/osu.Game/Overlays/Settings/Sections/SkinSection.cs +++ b/osu.Game/Overlays/Settings/Sections/SkinSection.cs @@ -116,8 +116,7 @@ namespace osu.Game.Overlays.Settings.Sections { if (weakItem.NewValue.TryGetTarget(out var item)) { - List newDropdownItems = skinDropdown.Items.ToList(); - newDropdownItems.Add(item); + List newDropdownItems = skinDropdown.Items.Where(i => !i.Equals(item)).Append(item).ToList(); newDropdownItems = sortList(newDropdownItems); Schedule(() => skinDropdown.Items = newDropdownItems.ToArray()); } From 31e61326e1ea4d4f3b3e1e8450c8fdc385939d0d Mon Sep 17 00:00:00 2001 From: rednir Date: Tue, 19 Jan 2021 14:00:17 +0000 Subject: [PATCH 04/12] Only user skins are sorted --- .../Overlays/Settings/Sections/SkinSection.cs | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/osu.Game/Overlays/Settings/Sections/SkinSection.cs b/osu.Game/Overlays/Settings/Sections/SkinSection.cs index ba92ed5b04..1dcc5d824d 100644 --- a/osu.Game/Overlays/Settings/Sections/SkinSection.cs +++ b/osu.Game/Overlays/Settings/Sections/SkinSection.cs @@ -38,6 +38,8 @@ namespace osu.Game.Overlays.Settings.Sections private List skinItems; + private int firstNonDefault; + [Resolved] private SkinManager skins { get; set; } @@ -107,8 +109,10 @@ namespace osu.Game.Overlays.Settings.Sections private void updateItems() { skinItems = skins.GetAllUsableSkins(); + firstNonDefault = skinItems.FindIndex(s => s.ID > 0); + + skinItems.Insert(firstNonDefault, random_skin_info); skinItems = sortList(skinItems); - skinDropdown.Items = skinItems; } @@ -130,19 +134,12 @@ namespace osu.Game.Overlays.Settings.Sections private List sortList(List skinsList) { - skinsList.Sort((a, b) => String.Compare(a.Name, b.Name, StringComparison.Ordinal)); - for (int i = 0; i < skinsList.Count; i++) - { - // insert lazer built-in skins before user skins - if (skinsList[i].ID <= 0) { - var itemToMove = skinsList[i]; - skinsList.RemoveAt(i); - skinsList.Insert(0, itemToMove); - } - } - skinsList.RemoveAll(s => s.ID == SkinInfo.RANDOM_SKIN); - skinsList.Insert(0, random_skin_info); - + // Sort user skins seperate from built-in skins + List userSkinsList = skinsList.GetRange(firstNonDefault + 1, skinsList.Count - (firstNonDefault + 1)); + skinsList.RemoveRange(firstNonDefault + 1, skinsList.Count - (firstNonDefault + 1)); + userSkinsList.Sort((a, b) => String.Compare(a.Name, b.Name, StringComparison.OrdinalIgnoreCase)); + + skinsList.AddRange(userSkinsList); return skinsList; } From c5c5fdca454cd9f1a036eba686d33673d074c038 Mon Sep 17 00:00:00 2001 From: rednir Date: Tue, 19 Jan 2021 14:10:08 +0000 Subject: [PATCH 05/12] Remove another whitespace --- osu.Game/Overlays/Settings/Sections/SkinSection.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game/Overlays/Settings/Sections/SkinSection.cs b/osu.Game/Overlays/Settings/Sections/SkinSection.cs index b42091ce30..9ca8ec9051 100644 --- a/osu.Game/Overlays/Settings/Sections/SkinSection.cs +++ b/osu.Game/Overlays/Settings/Sections/SkinSection.cs @@ -143,7 +143,6 @@ namespace osu.Game.Overlays.Settings.Sections List userSkinsList = skinsList.GetRange(firstNonDefault + 1, skinsList.Count - (firstNonDefault + 1)); skinsList.RemoveRange(firstNonDefault + 1, skinsList.Count - (firstNonDefault + 1)); userSkinsList.Sort((a, b) => String.Compare(a.Name, b.Name, StringComparison.OrdinalIgnoreCase)); - skinsList.AddRange(userSkinsList); return skinsList; } From b265d2dab499f4cdf75fadbc1acc20acb2f21ad3 Mon Sep 17 00:00:00 2001 From: rednir Date: Tue, 19 Jan 2021 14:16:22 +0000 Subject: [PATCH 06/12] Remove another whitespace --- osu.Game/Overlays/Settings/Sections/SkinSection.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game/Overlays/Settings/Sections/SkinSection.cs b/osu.Game/Overlays/Settings/Sections/SkinSection.cs index b42091ce30..9ca8ec9051 100644 --- a/osu.Game/Overlays/Settings/Sections/SkinSection.cs +++ b/osu.Game/Overlays/Settings/Sections/SkinSection.cs @@ -143,7 +143,6 @@ namespace osu.Game.Overlays.Settings.Sections List userSkinsList = skinsList.GetRange(firstNonDefault + 1, skinsList.Count - (firstNonDefault + 1)); skinsList.RemoveRange(firstNonDefault + 1, skinsList.Count - (firstNonDefault + 1)); userSkinsList.Sort((a, b) => String.Compare(a.Name, b.Name, StringComparison.OrdinalIgnoreCase)); - skinsList.AddRange(userSkinsList); return skinsList; } From 14ff2af00ef868bd1ff65dd22f7eed242d7c2e8b Mon Sep 17 00:00:00 2001 From: rednir Date: Tue, 19 Jan 2021 15:37:59 +0000 Subject: [PATCH 07/12] Satisfy AppVeyor --- osu.Game/Overlays/Settings/Sections/SkinSection.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Overlays/Settings/Sections/SkinSection.cs b/osu.Game/Overlays/Settings/Sections/SkinSection.cs index 9ca8ec9051..7023702d0e 100644 --- a/osu.Game/Overlays/Settings/Sections/SkinSection.cs +++ b/osu.Game/Overlays/Settings/Sections/SkinSection.cs @@ -142,7 +142,7 @@ namespace osu.Game.Overlays.Settings.Sections // Sort user skins seperate from built-in skins List userSkinsList = skinsList.GetRange(firstNonDefault + 1, skinsList.Count - (firstNonDefault + 1)); skinsList.RemoveRange(firstNonDefault + 1, skinsList.Count - (firstNonDefault + 1)); - userSkinsList.Sort((a, b) => String.Compare(a.Name, b.Name, StringComparison.OrdinalIgnoreCase)); + userSkinsList.Sort((a, b) => System.String.Compare(a.Name, b.Name, StringComparison.OrdinalIgnoreCase)); skinsList.AddRange(userSkinsList); return skinsList; } From a880b8d21dae103d8e3340678384356b9218b4f2 Mon Sep 17 00:00:00 2001 From: rednir Date: Tue, 19 Jan 2021 16:11:16 +0000 Subject: [PATCH 08/12] Satisfy AppVeyor --- osu.Game/Overlays/Settings/Sections/SkinSection.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Overlays/Settings/Sections/SkinSection.cs b/osu.Game/Overlays/Settings/Sections/SkinSection.cs index 9ca8ec9051..c39f95d93a 100644 --- a/osu.Game/Overlays/Settings/Sections/SkinSection.cs +++ b/osu.Game/Overlays/Settings/Sections/SkinSection.cs @@ -142,7 +142,7 @@ namespace osu.Game.Overlays.Settings.Sections // Sort user skins seperate from built-in skins List userSkinsList = skinsList.GetRange(firstNonDefault + 1, skinsList.Count - (firstNonDefault + 1)); skinsList.RemoveRange(firstNonDefault + 1, skinsList.Count - (firstNonDefault + 1)); - userSkinsList.Sort((a, b) => String.Compare(a.Name, b.Name, StringComparison.OrdinalIgnoreCase)); + userSkinsList.Sort((a, b) => string.Compare(a.Name, b.Name, StringComparison.OrdinalIgnoreCase)); skinsList.AddRange(userSkinsList); return skinsList; } From 206a0b8bace7733f6ac6f12909adcaecb6472d08 Mon Sep 17 00:00:00 2001 From: rednir Date: Tue, 19 Jan 2021 16:55:50 +0000 Subject: [PATCH 09/12] Fix firstNonDefault staying as -1 --- osu.Game/Overlays/Settings/Sections/SkinSection.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/osu.Game/Overlays/Settings/Sections/SkinSection.cs b/osu.Game/Overlays/Settings/Sections/SkinSection.cs index c39f95d93a..a3e472a9f9 100644 --- a/osu.Game/Overlays/Settings/Sections/SkinSection.cs +++ b/osu.Game/Overlays/Settings/Sections/SkinSection.cs @@ -115,7 +115,8 @@ namespace osu.Game.Overlays.Settings.Sections { skinItems = skins.GetAllUsableSkins(); firstNonDefault = skinItems.FindIndex(s => s.ID > 0); - + if (firstNonDefault < 0) + firstNonDefault = skinItems.Count; skinItems.Insert(firstNonDefault, random_skin_info); skinItems = sortList(skinItems); skinDropdown.Items = skinItems; From b00c6a1d60723622101512b13168214ff61696aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 19 Jan 2021 18:29:55 +0100 Subject: [PATCH 10/12] Make first non-default skin index a property The previous code was very brittle - it was not always updating properly, and seems to have worked either by a carefully crafted set of circumstances, or just plain coincidence. Having this be a get-only property avoids potential error in the future caused by not updating the index properly, at the expense of an added linear lookup. --- .../Overlays/Settings/Sections/SkinSection.cs | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/osu.Game/Overlays/Settings/Sections/SkinSection.cs b/osu.Game/Overlays/Settings/Sections/SkinSection.cs index a3e472a9f9..bbb4c50f6b 100644 --- a/osu.Game/Overlays/Settings/Sections/SkinSection.cs +++ b/osu.Game/Overlays/Settings/Sections/SkinSection.cs @@ -38,7 +38,17 @@ namespace osu.Game.Overlays.Settings.Sections private List skinItems; - private int firstNonDefault; + private int firstNonDefaultSkinIndex + { + get + { + var index = skinItems.FindIndex(s => s.ID > 0); + if (index < 0) + index = skinItems.Count; + + return index; + } + } [Resolved] private SkinManager skins { get; set; } @@ -114,10 +124,7 @@ namespace osu.Game.Overlays.Settings.Sections private void updateItems() { skinItems = skins.GetAllUsableSkins(); - firstNonDefault = skinItems.FindIndex(s => s.ID > 0); - if (firstNonDefault < 0) - firstNonDefault = skinItems.Count; - skinItems.Insert(firstNonDefault, random_skin_info); + skinItems.Insert(firstNonDefaultSkinIndex, random_skin_info); skinItems = sortList(skinItems); skinDropdown.Items = skinItems; } @@ -141,8 +148,8 @@ namespace osu.Game.Overlays.Settings.Sections private List sortList(List skinsList) { // Sort user skins seperate from built-in skins - List userSkinsList = skinsList.GetRange(firstNonDefault + 1, skinsList.Count - (firstNonDefault + 1)); - skinsList.RemoveRange(firstNonDefault + 1, skinsList.Count - (firstNonDefault + 1)); + List userSkinsList = skinsList.GetRange(firstNonDefaultSkinIndex, skinsList.Count - firstNonDefaultSkinIndex); + skinsList.RemoveRange(firstNonDefaultSkinIndex, skinsList.Count - firstNonDefaultSkinIndex); userSkinsList.Sort((a, b) => string.Compare(a.Name, b.Name, StringComparison.OrdinalIgnoreCase)); skinsList.AddRange(userSkinsList); return skinsList; From 78e590d25da4b570447d8fb3b8462f56fa52f50c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 19 Jan 2021 18:36:42 +0100 Subject: [PATCH 11/12] Refactor skin sorting method * Rename to `sortUserSkins` to convey meaning better. * Sort in-place instead of slicing the list. * Change to `void` to avoid misleading users that the method returns a new list instance. * Fix typo in comment. --- .../Overlays/Settings/Sections/SkinSection.cs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/osu.Game/Overlays/Settings/Sections/SkinSection.cs b/osu.Game/Overlays/Settings/Sections/SkinSection.cs index bbb4c50f6b..75c0324408 100644 --- a/osu.Game/Overlays/Settings/Sections/SkinSection.cs +++ b/osu.Game/Overlays/Settings/Sections/SkinSection.cs @@ -125,7 +125,7 @@ namespace osu.Game.Overlays.Settings.Sections { skinItems = skins.GetAllUsableSkins(); skinItems.Insert(firstNonDefaultSkinIndex, random_skin_info); - skinItems = sortList(skinItems); + sortUserSkins(skinItems); skinDropdown.Items = skinItems; } @@ -134,7 +134,7 @@ namespace osu.Game.Overlays.Settings.Sections if (weakItem.NewValue.TryGetTarget(out var item)) { List newDropdownItems = skinDropdown.Items.Where(i => !i.Equals(item)).Append(item).ToList(); - newDropdownItems = sortList(newDropdownItems); + sortUserSkins(newDropdownItems); Schedule(() => skinDropdown.Items = newDropdownItems.ToArray()); } } @@ -145,14 +145,11 @@ namespace osu.Game.Overlays.Settings.Sections Schedule(() => skinDropdown.Items = skinDropdown.Items.Where(i => i.ID != item.ID).ToArray()); } - private List sortList(List skinsList) + private void sortUserSkins(List skinsList) { - // Sort user skins seperate from built-in skins - List userSkinsList = skinsList.GetRange(firstNonDefaultSkinIndex, skinsList.Count - firstNonDefaultSkinIndex); - skinsList.RemoveRange(firstNonDefaultSkinIndex, skinsList.Count - firstNonDefaultSkinIndex); - userSkinsList.Sort((a, b) => string.Compare(a.Name, b.Name, StringComparison.OrdinalIgnoreCase)); - skinsList.AddRange(userSkinsList); - return skinsList; + // Sort user skins separately from built-in skins + skinsList.Sort(firstNonDefaultSkinIndex, skinsList.Count - firstNonDefaultSkinIndex, + Comparer.Create((a, b) => string.Compare(a.Name, b.Name, StringComparison.OrdinalIgnoreCase))); } private class SkinSettingsDropdown : SettingsDropdown From 3b49b7461ef628031ba3440ea795cf5a42073a2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 19 Jan 2021 18:46:21 +0100 Subject: [PATCH 12/12] Schedule entire operation for safety Also removes a redundant list copy. --- osu.Game/Overlays/Settings/Sections/SkinSection.cs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/osu.Game/Overlays/Settings/Sections/SkinSection.cs b/osu.Game/Overlays/Settings/Sections/SkinSection.cs index 75c0324408..4cfd801caf 100644 --- a/osu.Game/Overlays/Settings/Sections/SkinSection.cs +++ b/osu.Game/Overlays/Settings/Sections/SkinSection.cs @@ -133,9 +133,12 @@ namespace osu.Game.Overlays.Settings.Sections { if (weakItem.NewValue.TryGetTarget(out var item)) { - List newDropdownItems = skinDropdown.Items.Where(i => !i.Equals(item)).Append(item).ToList(); - sortUserSkins(newDropdownItems); - Schedule(() => skinDropdown.Items = newDropdownItems.ToArray()); + Schedule(() => + { + List newDropdownItems = skinDropdown.Items.Where(i => !i.Equals(item)).Append(item).ToList(); + sortUserSkins(newDropdownItems); + skinDropdown.Items = newDropdownItems; + }); } }