diff --git a/osu.Game.Benchmarks/BenchmarkStringComparison.cs b/osu.Game.Benchmarks/BenchmarkStringComparison.cs new file mode 100644 index 0000000000..d40b92db5f --- /dev/null +++ b/osu.Game.Benchmarks/BenchmarkStringComparison.cs @@ -0,0 +1,48 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System; +using System.Collections.Generic; +using BenchmarkDotNet.Attributes; +using osu.Game.Utils; + +namespace osu.Game.Benchmarks +{ + public class BenchmarkStringComparison + { + private string[] strings = null!; + + [GlobalSetup] + public void GlobalSetUp() + { + strings = new string[10000]; + + for (int i = 0; i < strings.Length; ++i) + strings[i] = Guid.NewGuid().ToString(); + + for (int i = 0; i < strings.Length; ++i) + { + if (i % 2 == 0) + strings[i] = strings[i].ToUpperInvariant(); + } + } + + [Benchmark] + public void OrdinalIgnoreCase() => compare(StringComparer.OrdinalIgnoreCase); + + [Benchmark] + public void OrdinalSortByCase() => compare(OrdinalSortByCaseStringComparer.DEFAULT); + + [Benchmark] + public void InvariantCulture() => compare(StringComparer.InvariantCulture); + + private void compare(IComparer comparer) + { + for (int i = 0; i < strings.Length; ++i) + { + for (int j = i + 1; j < strings.Length; ++j) + _ = comparer.Compare(strings[i], strings[j]); + } + } + } +} diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs index aa4c879468..de2ae3708f 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs @@ -629,7 +629,8 @@ namespace osu.Game.Tests.Visual.SongSelect { var sets = new List(); - const string zzz_string = "zzzzz"; + const string zzz_lowercase = "zzzzz"; + const string zzz_uppercase = "ZZZZZ"; AddStep("Populuate beatmap sets", () => { @@ -640,10 +641,16 @@ namespace osu.Game.Tests.Visual.SongSelect var set = TestResources.CreateTestBeatmapSetInfo(); if (i == 4) - set.Beatmaps.ForEach(b => b.Metadata.Artist = zzz_string); + set.Beatmaps.ForEach(b => b.Metadata.Artist = zzz_uppercase); + + if (i == 8) + set.Beatmaps.ForEach(b => b.Metadata.Artist = zzz_lowercase); + + if (i == 12) + set.Beatmaps.ForEach(b => b.Metadata.Author.Username = zzz_uppercase); if (i == 16) - set.Beatmaps.ForEach(b => b.Metadata.Author.Username = zzz_string); + set.Beatmaps.ForEach(b => b.Metadata.Author.Username = zzz_lowercase); sets.Add(set); } @@ -652,9 +659,11 @@ namespace osu.Game.Tests.Visual.SongSelect loadBeatmaps(sets); AddStep("Sort by author", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Author }, false)); - AddAssert($"Check {zzz_string} is at bottom", () => carousel.BeatmapSets.Last().Metadata.Author.Username == zzz_string); + AddAssert($"Check {zzz_uppercase} is last", () => carousel.BeatmapSets.Last().Metadata.Author.Username == zzz_uppercase); + AddAssert($"Check {zzz_lowercase} is second last", () => carousel.BeatmapSets.SkipLast(1).Last().Metadata.Author.Username == zzz_lowercase); AddStep("Sort by artist", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Artist }, false)); - AddAssert($"Check {zzz_string} is at bottom", () => carousel.BeatmapSets.Last().Metadata.Artist == zzz_string); + AddAssert($"Check {zzz_uppercase} is last", () => carousel.BeatmapSets.Last().Metadata.Artist == zzz_uppercase); + AddAssert($"Check {zzz_lowercase} is second last", () => carousel.BeatmapSets.SkipLast(1).Last().Metadata.Artist == zzz_lowercase); } /// diff --git a/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs b/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs index 6c41bc3805..43c9c621e8 100644 --- a/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs +++ b/osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs @@ -7,6 +7,7 @@ using System.Linq; using osu.Framework.Extensions.IEnumerableExtensions; using osu.Game.Beatmaps; using osu.Game.Screens.Select.Filter; +using osu.Game.Utils; namespace osu.Game.Screens.Select.Carousel { @@ -67,19 +68,19 @@ namespace osu.Game.Screens.Select.Carousel { default: case SortMode.Artist: - comparison = string.Compare(BeatmapSet.Metadata.Artist, otherSet.BeatmapSet.Metadata.Artist, StringComparison.Ordinal); + comparison = OrdinalSortByCaseStringComparer.DEFAULT.Compare(BeatmapSet.Metadata.Artist, otherSet.BeatmapSet.Metadata.Artist); break; case SortMode.Title: - comparison = string.Compare(BeatmapSet.Metadata.Title, otherSet.BeatmapSet.Metadata.Title, StringComparison.Ordinal); + comparison = OrdinalSortByCaseStringComparer.DEFAULT.Compare(BeatmapSet.Metadata.Title, otherSet.BeatmapSet.Metadata.Title); break; case SortMode.Author: - comparison = string.Compare(BeatmapSet.Metadata.Author.Username, otherSet.BeatmapSet.Metadata.Author.Username, StringComparison.Ordinal); + comparison = OrdinalSortByCaseStringComparer.DEFAULT.Compare(BeatmapSet.Metadata.Author.Username, otherSet.BeatmapSet.Metadata.Author.Username); break; case SortMode.Source: - comparison = string.Compare(BeatmapSet.Metadata.Source, otherSet.BeatmapSet.Metadata.Source, StringComparison.Ordinal); + comparison = OrdinalSortByCaseStringComparer.DEFAULT.Compare(BeatmapSet.Metadata.Source, otherSet.BeatmapSet.Metadata.Source); break; case SortMode.DateAdded: diff --git a/osu.Game/Utils/OrdinalSortByCaseStringComparer.cs b/osu.Game/Utils/OrdinalSortByCaseStringComparer.cs new file mode 100644 index 0000000000..6c1532eef5 --- /dev/null +++ b/osu.Game/Utils/OrdinalSortByCaseStringComparer.cs @@ -0,0 +1,49 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System; +using System.Collections.Generic; + +namespace osu.Game.Utils +{ + /// + /// This string comparer is something of a cross-over between and . + /// is used first, but is used as a tie-breaker. + /// + /// + /// This comparer's behaviour somewhat emulates , + /// but non-ordinal comparers - both culture-aware and culture-invariant - have huge performance overheads due to i18n factors (up to 5x slower). + /// + /// + /// Given the following strings to sort: [A, B, C, D, a, b, c, d, A] and a stable sorting algorithm: + /// + /// + /// would return [A, A, B, C, D, a, b, c, d]. + /// This is undesirable as letters are interleaved. + /// + /// + /// would return [A, a, A, B, b, C, c, D, d]. + /// Different letters are not interleaved, but because case is ignored, the As are left in arbitrary order. + /// + /// + /// + /// would return [a, A, A, b, B, c, C, d, D], which is the expected behaviour. + /// + /// + public class OrdinalSortByCaseStringComparer : IComparer + { + public static readonly OrdinalSortByCaseStringComparer DEFAULT = new OrdinalSortByCaseStringComparer(); + + private OrdinalSortByCaseStringComparer() + { + } + + public int Compare(string? a, string? b) + { + int result = StringComparer.OrdinalIgnoreCase.Compare(a, b); + if (result == 0) + result = -StringComparer.Ordinal.Compare(a, b); // negative to place lowercase letters before uppercase. + return result; + } + } +}