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

Merge pull request #20273 from frenzibyte/duplicate-beatmap-listing-cards

Browse Source 
Fix beatmap listing potentially showing duplicate beatmap cards
This commit is contained in:
Dan Balasescu 2022-09-14 10:25:24 +09:00 committed by GitHub
commit ef6d60ffe9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 65 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

55
osu.Game.Tests/Visual/Online/TestSceneBeatmapListingOverlay.cs
View File

@ -35,6 +35,8 @@ namespace osu.Game.Tests.Visual.Online
private OsuConfigManager localConfig;
private bool returnCursorOnResponse;
[BackgroundDependencyLoader]
private void load()
{
@ -61,6 +63,7 @@ namespace osu.Game.Tests.Visual.Online
searchBeatmapSetsRequest.TriggerSuccess(new SearchBeatmapSetsResponse
{
BeatmapSets = setsForResponse,
Cursor = returnCursorOnResponse ? new Cursor() : null,
});
return true;
@ -106,7 +109,7 @@ namespace osu.Game.Tests.Visual.Online
{
AddAssert("is visible", () => overlay.State.Value == Visibility.Visible);
AddStep("show many results", () => fetchFor(Enumerable.Repeat(CreateAPIBeatmapSet(Ruleset.Value), 100).ToArray()));
AddStep("show many results", () => fetchFor(getManyBeatmaps(100).ToArray()));
AddUntilStep("placeholder hidden", () => !overlay.ChildrenOfType<BeatmapListingOverlay.NotFoundDrawable>().Any(d => d.IsPresent));
@ -127,10 +130,10 @@ namespace osu.Game.Tests.Visual.Online
{
AddAssert("is visible", () => overlay.State.Value == Visibility.Visible);
AddStep("show many results", () => fetchFor(Enumerable.Repeat(CreateAPIBeatmapSet(Ruleset.Value), 100).ToArray()));
AddStep("show many results", () => fetchFor(getManyBeatmaps(100).ToArray()));
assertAllCardsOfType<BeatmapCardNormal>(100);
AddStep("show more results", () => fetchFor(Enumerable.Repeat(CreateAPIBeatmapSet(Ruleset.Value), 30).ToArray()));
AddStep("show more results", () => fetchFor(getManyBeatmaps(30).ToArray()));
assertAllCardsOfType<BeatmapCardNormal>(30);
}
@ -139,7 +142,7 @@ namespace osu.Game.Tests.Visual.Online
{
AddAssert("is visible", () => overlay.State.Value == Visibility.Visible);
AddStep("show many results", () => fetchFor(Enumerable.Repeat(CreateAPIBeatmapSet(Ruleset.Value), 100).ToArray()));
AddStep("show many results", () => fetchFor(getManyBeatmaps(100).ToArray()));
assertAllCardsOfType<BeatmapCardNormal>(100);
setCardSize(BeatmapCardSize.Extra, viaConfig);
@ -161,7 +164,7 @@ namespace osu.Game.Tests.Visual.Online
AddStep("fetch for 0 beatmaps", () => fetchFor());
placeholderShown();
AddStep("show many results", () => fetchFor(Enumerable.Repeat(CreateAPIBeatmapSet(Ruleset.Value), 100).ToArray()));
AddStep("show many results", () => fetchFor(getManyBeatmaps(100).ToArray()));
AddUntilStep("wait for loaded", () => this.ChildrenOfType<BeatmapCard>().Count() == 100);
AddUntilStep("placeholder hidden", () => !overlay.ChildrenOfType<BeatmapListingOverlay.NotFoundDrawable>().Any(d => d.IsPresent));
@ -180,6 +183,32 @@ namespace osu.Game.Tests.Visual.Online
});
}
/// <summary>
/// During pagination, the first beatmap of the second page may be a duplicate of the last beatmap from the previous page.
/// This is currently the case with osu!web API due to ES relevance score's presence in the response cursor.
/// See: https://github.com/ppy/osu-web/issues/9270
/// </summary>
[Test]
public void TestDuplicatedBeatmapOnlyShowsOnce()
{
APIBeatmapSet beatmapSet = null;
AddStep("show many results", () =>
{
beatmapSet = CreateAPIBeatmapSet(Ruleset.Value);
beatmapSet.Title = "last beatmap of first page";
fetchFor(getManyBeatmaps(49).Append(beatmapSet).ToArray(), true);
});
AddUntilStep("wait for loaded", () => this.ChildrenOfType<BeatmapCard>().Count() == 50);
AddStep("set next page", () => setSearchResponse(getManyBeatmaps(49).Prepend(beatmapSet).ToArray(), false));
AddStep("scroll to end", () => overlay.ChildrenOfType<OverlayScrollContainer>().Single().ScrollToEnd());
AddUntilStep("wait for loaded", () => this.ChildrenOfType<BeatmapCard>().Count() == 99);
AddAssert("beatmap not duplicated", () => overlay.ChildrenOfType<BeatmapCard>().Count(c => c.BeatmapSet.Equals(beatmapSet)) == 1);
}
[Test]
public void TestUserWithoutSupporterUsesSupporterOnlyFiltersWithoutResults()
{
@ -336,15 +365,25 @@ namespace osu.Game.Tests.Visual.Online
private static int searchCount;
private void fetchFor(params APIBeatmapSet[] beatmaps)
private APIBeatmapSet[] getManyBeatmaps(int count) => Enumerable.Range(0, count).Select(_ => CreateAPIBeatmapSet(Ruleset.Value)).ToArray();
private void fetchFor(params APIBeatmapSet[] beatmaps) => fetchFor(beatmaps, false);
private void fetchFor(APIBeatmapSet[] beatmaps, bool hasNextPage)
{
setsForResponse.Clear();
setsForResponse.AddRange(beatmaps);
setSearchResponse(beatmaps, hasNextPage);
// trigger arbitrary change for fetching.
searchControl.Query.Value = $"search {searchCount++}";
}
private void setSearchResponse(APIBeatmapSet[] beatmaps, bool hasNextPage)
{
setsForResponse.Clear();
setsForResponse.AddRange(beatmaps);
returnCursorOnResponse = hasNextPage;
}
private void setRankAchievedFilter(ScoreRank[] ranks)
{
AddStep($"set Rank Achieved filter to [{string.Join(',', ranks)}]", () =>

13
osu.Game/Beatmaps/Drawables/Cards/BeatmapCard.cs
View File

@ -14,7 +14,7 @@ using osu.Game.Overlays;
namespace osu.Game.Beatmaps.Drawables.Cards
{
public abstract class BeatmapCard : OsuClickableContainer
public abstract class BeatmapCard : OsuClickableContainer, IEquatable<BeatmapCard>
{
public const float TRANSITION_DURATION = 400;
public const float CORNER_RADIUS = 10;
@ -96,5 +96,16 @@ namespace osu.Game.Beatmaps.Drawables.Cards
throw new ArgumentOutOfRangeException(nameof(size), size, @"Unsupported card size");
}
}
public bool Equals(BeatmapCard? other)
{
if (ReferenceEquals(null, other)) return false;
if (ReferenceEquals(this, other)) return true;
return BeatmapSet.Equals(other.BeatmapSet);
}
public override bool Equals(object obj) => obj is BeatmapCard other && Equals(other);
public override int GetHashCode() => BeatmapSet.GetHashCode();
}
}

7
osu.Game/Overlays/BeatmapListingOverlay.cs
View File

@ -176,6 +176,11 @@ namespace osu.Game.Overlays
}
else
{
// new results may contain beatmaps from a previous page,
// this is dodgy but matches web behaviour for now.
// see: https://github.com/ppy/osu-web/issues/9270
newCards = newCards.Except(foundContent);
panelLoadTask = LoadComponentsAsync(newCards, loaded =>
{
lastFetchDisplayedTime = Time.Current;
@ -185,7 +190,7 @@ namespace osu.Game.Overlays
}
}
private BeatmapCard[] createCardsFor(IEnumerable<APIBeatmapSet> beatmapSets) => beatmapSets.Select(set => BeatmapCard.Create(set, filterControl.CardSize.Value).With(c =>
private IEnumerable<BeatmapCard> createCardsFor(IEnumerable<APIBeatmapSet> beatmapSets) => beatmapSets.Select(set => BeatmapCard.Create(set, filterControl.CardSize.Value).With(c =>
{
c.Anchor = Anchor.TopCentre;
c.Origin = Anchor.TopCentre;