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

Merge pull request #22248 from peppy/fix-carousel-sort-change-after-play

Browse Source 
Fix beatmap sorting at song select sometimes changing after setting a score
This commit is contained in:
Bartłomiej Dach 2023-01-18 17:57:08 +01:00 committed by GitHub
commit c7249383ba
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 108 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -580,10 +580,9 @@ namespace osu.Game.Tests.Visual.SongSelect
/// Ensures stability is maintained on different sort modes for items with equal properties.
/// </summary>
[Test]
public void TestSortingStability()
public void TestSortingStabilityDateAdded()
{
var sets = new List<BeatmapSetInfo>();
int idOffset = 0;
AddStep("Populuate beatmap sets", () =>
{
@ -593,38 +592,34 @@ namespace osu.Game.Tests.Visual.SongSelect
{
var set = TestResources.CreateTestBeatmapSetInfo();
set.DateAdded = DateTimeOffset.FromUnixTimeSeconds(i);
// only need to set the first as they are a shared reference.
var beatmap = set.Beatmaps.First();
beatmap.Metadata.Artist = $"artist {i / 2}";
beatmap.Metadata.Title = $"title {9 - i}";
beatmap.Metadata.Artist = "a";
beatmap.Metadata.Title = "b";
sets.Add(set);
}
idOffset = sets.First().OnlineID;
});
loadBeatmaps(sets);
AddStep("Sort by artist", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Artist }, false));
AddAssert("Items remain in original order", () => carousel.BeatmapSets.Select((set, index) => set.OnlineID == idOffset + index).All(b => b));
AddStep("Sort by title", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Title }, false));
AddAssert("Items are in reverse order", () => carousel.BeatmapSets.Select((set, index) => set.OnlineID == idOffset + sets.Count - index - 1).All(b => b));
AddAssert("Items remain in descending added order", () => carousel.BeatmapSets.Select(s => s.DateAdded), () => Is.Ordered.Descending);
AddStep("Sort by artist", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Artist }, false));
AddAssert("Items reset to original order", () => carousel.BeatmapSets.Select((set, index) => set.OnlineID == idOffset + index).All(b => b));
AddAssert("Items remain in descending added order", () => carousel.BeatmapSets.Select(s => s.DateAdded), () => Is.Ordered.Descending);
}
/// <summary>
/// Ensures stability is maintained on different sort modes while a new item is added to the carousel.
/// </summary>
[Test]
public void TestSortingStabilityWithNewItems()
public void TestSortingStabilityWithRemovedAndReaddedItem()
{
List<BeatmapSetInfo> sets = new List<BeatmapSetInfo>();
int idOffset = 0;
AddStep("Populuate beatmap sets", () =>
{
@ -640,16 +635,68 @@ namespace osu.Game.Tests.Visual.SongSelect
beatmap.Metadata.Artist = "same artist";
beatmap.Metadata.Title = "same title";
// testing the case where DateAdded happens to equal (quite rare).
set.DateAdded = DateTimeOffset.UnixEpoch;
sets.Add(set);
}
idOffset = sets.First().OnlineID;
});
Guid[] originalOrder = null!;
loadBeatmaps(sets);
AddStep("Sort by artist", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Artist }, false));
assertOriginalOrderMaintained();
AddAssert("Items in descending added order", () => carousel.BeatmapSets.Select(s => s.DateAdded), () => Is.Ordered.Descending);
AddStep("Save order", () => originalOrder = carousel.BeatmapSets.Select(s => s.ID).ToArray());
AddStep("Remove item", () => carousel.RemoveBeatmapSet(sets[1]));
AddStep("Re-add item", () => carousel.UpdateBeatmapSet(sets[1]));
AddAssert("Order didn't change", () => carousel.BeatmapSets.Select(s => s.ID), () => Is.EqualTo(originalOrder));
AddStep("Sort by title", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Title }, false));
AddAssert("Order didn't change", () => carousel.BeatmapSets.Select(s => s.ID), () => Is.EqualTo(originalOrder));
}
/// <summary>
/// Ensures stability is maintained on different sort modes while a new item is added to the carousel.
/// </summary>
[Test]
public void TestSortingStabilityWithNewItems()
{
List<BeatmapSetInfo> sets = new List<BeatmapSetInfo>();
AddStep("Populuate beatmap sets", () =>
{
sets.Clear();
for (int i = 0; i < 3; i++)
{
var set = TestResources.CreateTestBeatmapSetInfo(3);
// only need to set the first as they are a shared reference.
var beatmap = set.Beatmaps.First();
beatmap.Metadata.Artist = "same artist";
beatmap.Metadata.Title = "same title";
// testing the case where DateAdded happens to equal (quite rare).
set.DateAdded = DateTimeOffset.UnixEpoch;
sets.Add(set);
}
});
Guid[] originalOrder = null!;
loadBeatmaps(sets);
AddStep("Sort by artist", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Artist }, false));
AddAssert("Items in descending added order", () => carousel.BeatmapSets.Select(s => s.DateAdded), () => Is.Ordered.Descending);
AddStep("Save order", () => originalOrder = carousel.BeatmapSets.Select(s => s.ID).ToArray());
AddStep("Add new item", () =>
{
@ -661,19 +708,18 @@ namespace osu.Game.Tests.Visual.SongSelect
beatmap.Metadata.Artist = "same artist";
beatmap.Metadata.Title = "same title";
set.DateAdded = DateTimeOffset.FromUnixTimeSeconds(1);
carousel.UpdateBeatmapSet(set);
// add set to expected ordering
originalOrder = originalOrder.Prepend(set.ID).ToArray();
});
assertOriginalOrderMaintained();
AddAssert("Order didn't change", () => carousel.BeatmapSets.Select(s => s.ID), () => Is.EqualTo(originalOrder));
AddStep("Sort by title", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Title }, false));
assertOriginalOrderMaintained();
void assertOriginalOrderMaintained()
{
AddAssert("Items remain in original order",
() => carousel.BeatmapSets.Select(s => s.OnlineID), () => Is.EqualTo(carousel.BeatmapSets.Select((set, index) => idOffset + index)));
}
AddAssert("Order didn't change", () => carousel.BeatmapSets.Select(s => s.ID), () => Is.EqualTo(originalOrder));
}
[Test]

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

@ -61,50 +61,75 @@ namespace osu.Game.Screens.Select.Carousel
if (!(other is CarouselBeatmapSet otherSet))
return base.CompareTo(criteria, other);
int comparison = 0;
switch (criteria.Sort)
{
default:
case SortMode.Artist:
return string.Compare(BeatmapSet.Metadata.Artist, otherSet.BeatmapSet.Metadata.Artist, StringComparison.OrdinalIgnoreCase);
comparison = string.Compare(BeatmapSet.Metadata.Artist, otherSet.BeatmapSet.Metadata.Artist, StringComparison.OrdinalIgnoreCase);
break;
case SortMode.Title:
return string.Compare(BeatmapSet.Metadata.Title, otherSet.BeatmapSet.Metadata.Title, StringComparison.OrdinalIgnoreCase);
comparison = string.Compare(BeatmapSet.Metadata.Title, otherSet.BeatmapSet.Metadata.Title, StringComparison.OrdinalIgnoreCase);
break;
case SortMode.Author:
return string.Compare(BeatmapSet.Metadata.Author.Username, otherSet.BeatmapSet.Metadata.Author.Username, StringComparison.OrdinalIgnoreCase);
comparison = string.Compare(BeatmapSet.Metadata.Author.Username, otherSet.BeatmapSet.Metadata.Author.Username, StringComparison.OrdinalIgnoreCase);
break;
case SortMode.Source:
return string.Compare(BeatmapSet.Metadata.Source, otherSet.BeatmapSet.Metadata.Source, StringComparison.OrdinalIgnoreCase);
comparison = string.Compare(BeatmapSet.Metadata.Source, otherSet.BeatmapSet.Metadata.Source, StringComparison.OrdinalIgnoreCase);
break;
case SortMode.DateAdded:
return otherSet.BeatmapSet.DateAdded.CompareTo(BeatmapSet.DateAdded);
comparison = otherSet.BeatmapSet.DateAdded.CompareTo(BeatmapSet.DateAdded);
break;
case SortMode.DateRanked:
// Beatmaps which have no ranked date should already be filtered away in this mode.
if (BeatmapSet.DateRanked == null || otherSet.BeatmapSet.DateRanked == null)
return 0;
break;
return otherSet.BeatmapSet.DateRanked.Value.CompareTo(BeatmapSet.DateRanked.Value);
comparison = otherSet.BeatmapSet.DateRanked.Value.CompareTo(BeatmapSet.DateRanked.Value);
break;
case SortMode.LastPlayed:
return -compareUsingAggregateMax(otherSet, b => (b.LastPlayed ?? DateTimeOffset.MinValue).ToUnixTimeSeconds());
comparison = -compareUsingAggregateMax(otherSet, b => (b.LastPlayed ?? DateTimeOffset.MinValue).ToUnixTimeSeconds());
break;
case SortMode.BPM:
return compareUsingAggregateMax(otherSet, b => b.BPM);
comparison = compareUsingAggregateMax(otherSet, b => b.BPM);
break;
case SortMode.Length:
return compareUsingAggregateMax(otherSet, b => b.Length);
comparison = compareUsingAggregateMax(otherSet, b => b.Length);
break;
case SortMode.Difficulty:
return compareUsingAggregateMax(otherSet, b => b.StarRating);
comparison = compareUsingAggregateMax(otherSet, b => b.StarRating);
break;
case SortMode.DateSubmitted:
// Beatmaps which have no submitted date should already be filtered away in this mode.
if (BeatmapSet.DateSubmitted == null || otherSet.BeatmapSet.DateSubmitted == null)
return 0;
break;
return otherSet.BeatmapSet.DateSubmitted.Value.CompareTo(BeatmapSet.DateSubmitted.Value);
comparison = otherSet.BeatmapSet.DateSubmitted.Value.CompareTo(BeatmapSet.DateSubmitted.Value);
break;
}
if (comparison != 0) return comparison;
// If the initial sort could not differentiate, attempt to use DateAdded to order sets in a stable fashion.
// The directionality of this matches the current SortMode.DateAdded, but we may want to reconsider if that becomes a user decision (ie. asc / desc).
comparison = otherSet.BeatmapSet.DateAdded.CompareTo(BeatmapSet.DateAdded);
if (comparison != 0) return comparison;
// If DateAdded fails to break the tie, fallback to our internal GUID for stability.
// This basically means it's a stable random sort.
return otherSet.BeatmapSet.ID.CompareTo(BeatmapSet.ID);
}
/// <summary>