h1or4dme8/osu-lazer
1
0
Fork 0
mirror of https://github.com/ppy/osu.git synced 2025-01-12 21:52:55 +08:00
Code Issues Releases Wiki Activity

Merge pull request #27292 from bdach/carousel-sorting-again

Browse Source 
Fix beatmap carousel string carousel not matching expectations
This commit is contained in:
Dean Herbert 2024-02-21 21:09:28 +08:00 committed by GitHub
commit 67626e55cb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 116 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

48
osu.Game.Benchmarks/BenchmarkStringComparison.cs Normal file
View File

@ -0,0 +1,48 @@
// 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;
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<string> comparer)
{
for (int i = 0; i < strings.Length; ++i)
{
for (int j = i + 1; j < strings.Length; ++j)
_ = comparer.Compare(strings[i], strings[j]);
}
}
}
}

19
osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapCarousel.cs
View File

@ -629,7 +629,8 @@ namespace osu.Game.Tests.Visual.SongSelect
{
var sets = new List<BeatmapSetInfo>();
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);
}
/// <summary>

9
osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs
View File

@ -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:

49
osu.Game/Utils/OrdinalSortByCaseStringComparer.cs Normal file
View File

@ -0,0 +1,49 @@
// 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;
using System.Collections.Generic;
namespace osu.Game.Utils
{
/// <summary>
/// This string comparer is something of a cross-over between <see cref="StringComparer.Ordinal"/> and <see cref="StringComparer.OrdinalIgnoreCase"/>.
/// <see cref="StringComparer.OrdinalIgnoreCase"/> is used first, but <see cref="StringComparer.Ordinal"/> is used as a tie-breaker.
/// </summary>
/// <remarks>
/// This comparer's behaviour somewhat emulates <see cref="StringComparer.InvariantCulture"/>,
/// but non-ordinal comparers - both culture-aware and culture-invariant - have huge performance overheads due to i18n factors (up to 5x slower).
/// </remarks>
/// <example>
/// Given the following strings to sort: <c>[A, B, C, D, a, b, c, d, A]</c> and a stable sorting algorithm:
/// <list type="bullet">
/// <item>
/// <see cref="StringComparer.Ordinal"/> would return <c>[A, A, B, C, D, a, b, c, d]</c>.
/// This is undesirable as letters are interleaved.
/// </item>
/// <item>
/// <see cref="StringComparer.OrdinalIgnoreCase"/> would return <c>[A, a, A, B, b, C, c, D, d]</c>.
/// Different letters are not interleaved, but because case is ignored, the As are left in arbitrary order.
/// </item>
/// </list>
/// <item>
/// <see cref="OrdinalSortByCaseStringComparer"/> would return <c>[a, A, A, b, B, c, C, d, D]</c>, which is the expected behaviour.
/// </item>
/// </example>
public class OrdinalSortByCaseStringComparer : IComparer<string>
{
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;
}
}
}