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

Remove OnlineID sort consideration

Browse Source
This commit is contained in:
Dean Herbert 2023-01-18 19:13:26 +09:00
parent 6f11fbdeed
commit 3b27774561
2 changed files with 20 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -613,50 +613,6 @@ namespace osu.Game.Tests.Visual.SongSelect
AddAssert("Items remain in descending added order", () => carousel.BeatmapSets.Select(s => s.DateAdded), () => Is.Ordered.Descending); 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 for items with equal properties.
/// </summary>
[Test]
public void TestSortingStabilityOnlineID()
{
var sets = new List<BeatmapSetInfo>();
int idOffset = 0;
AddStep("Populuate beatmap sets", () =>
{
sets.Clear();
for (int i = 0; i < 10; i++)
{
var set = TestResources.CreateTestBeatmapSetInfo();
// 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}";
// testing the case where DateAdded happens to equal (quite rare).
set.DateAdded = DateTimeOffset.UnixEpoch;
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));
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));
}
/// <summary> /// <summary>
/// Ensures stability is maintained on different sort modes while a new item is added to the carousel. /// Ensures stability is maintained on different sort modes while a new item is added to the carousel.
/// </summary> /// </summary>
@ -664,7 +620,6 @@ namespace osu.Game.Tests.Visual.SongSelect
public void TestSortingStabilityWithRemovedAndReaddedItem() public void TestSortingStabilityWithRemovedAndReaddedItem()
{ {
List<BeatmapSetInfo> sets = new List<BeatmapSetInfo>(); List<BeatmapSetInfo> sets = new List<BeatmapSetInfo>();
int idOffset = 0;
AddStep("Populuate beatmap sets", () => AddStep("Populuate beatmap sets", () =>
{ {
@ -685,28 +640,24 @@ namespace osu.Game.Tests.Visual.SongSelect
sets.Add(set); sets.Add(set);
} }
idOffset = sets.First().OnlineID;
}); });
Guid[] originalOrder = null!;
loadBeatmaps(sets); loadBeatmaps(sets);
AddStep("Sort by artist", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Artist }, false)); 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("Remove item", () => carousel.RemoveBeatmapSet(sets[1]));
AddStep("Re-add item", () => carousel.UpdateBeatmapSet(sets[1])); AddStep("Re-add item", () => carousel.UpdateBeatmapSet(sets[1]));
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)); AddStep("Sort by title", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Title }, false));
assertOriginalOrderMaintained(); AddAssert("Order didn't change", () => carousel.BeatmapSets.Select(s => s.ID), () => Is.EqualTo(originalOrder));
void assertOriginalOrderMaintained()
{
AddAssert("Items remain in original order",
() => carousel.BeatmapSets.Select(s => s.OnlineID), () => Is.EqualTo(carousel.BeatmapSets.Select((set, index) => idOffset + index)));
}
} }
/// <summary> /// <summary>
@ -716,7 +667,6 @@ namespace osu.Game.Tests.Visual.SongSelect
public void TestSortingStabilityWithNewItems() public void TestSortingStabilityWithNewItems()
{ {
List<BeatmapSetInfo> sets = new List<BeatmapSetInfo>(); List<BeatmapSetInfo> sets = new List<BeatmapSetInfo>();
int idOffset = 0;
AddStep("Populuate beatmap sets", () => AddStep("Populuate beatmap sets", () =>
{ {
@ -737,14 +687,16 @@ namespace osu.Game.Tests.Visual.SongSelect
sets.Add(set); sets.Add(set);
} }
idOffset = sets.First().OnlineID;
}); });
Guid[] originalOrder = null!;
loadBeatmaps(sets); loadBeatmaps(sets);
AddStep("Sort by artist", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Artist }, false)); 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("Add new item", () => AddStep("Add new item", () =>
{ {
@ -756,22 +708,18 @@ namespace osu.Game.Tests.Visual.SongSelect
beatmap.Metadata.Artist = "same artist"; beatmap.Metadata.Artist = "same artist";
beatmap.Metadata.Title = "same title"; beatmap.Metadata.Title = "same title";
// testing the case where DateAdded happens to equal (quite rare). set.DateAdded = DateTimeOffset.FromUnixTimeSeconds(1);
set.DateAdded = DateTimeOffset.UnixEpoch;
carousel.UpdateBeatmapSet(set); 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)); AddStep("Sort by title", () => carousel.Filter(new FilterCriteria { Sort = SortMode.Title }, false));
assertOriginalOrderMaintained(); AddAssert("Order didn't change", () => carousel.BeatmapSets.Select(s => s.ID), () => Is.EqualTo(originalOrder));
void assertOriginalOrderMaintained()
{
AddAssert("Items remain in original order",
() => carousel.BeatmapSets.Select(s => s.OnlineID), () => Is.EqualTo(carousel.BeatmapSets.Select((set, index) => idOffset + index)));
}
} }
[Test] [Test]

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

@ -121,14 +121,10 @@ namespace osu.Game.Screens.Select.Carousel
if (comparison != 0) return comparison; if (comparison != 0) return comparison;
// If the initial sort could not differentiate, attempt to use DateAdded and OnlineID to order sets in a stable fashion. // If the initial sort could not differentiate, attempt to use DateAdded to order sets in a stable fashion.
// This directionality is a touch arbitrary as while DateAdded puts newer beatmaps first, the OnlineID fallback puts lower IDs first. // The directionality of this matches the current SortMode.DateAdded, but we may want to reconsider if that becomes a user decision (ie. asc / desc).
// Can potentially be changed in the future if users actually notice / have preference, but keeping it this way matches historical tests.
comparison = otherSet.BeatmapSet.DateAdded.CompareTo(BeatmapSet.DateAdded); comparison = otherSet.BeatmapSet.DateAdded.CompareTo(BeatmapSet.DateAdded);
if (comparison != 0) return comparison;
comparison = BeatmapSet.OnlineID.CompareTo(otherSet.BeatmapSet.OnlineID);
if (comparison != 0) return comparison; if (comparison != 0) return comparison;
// If no online ID is available, fallback to our internal GUID for stability. // If no online ID is available, fallback to our internal GUID for stability.