h1or4dme8/osu-lazer
1
0
Fork 0
mirror of https://github.com/ppy/osu.git synced 2026-05-20 16:51:38 +08:00
Code Issues Releases Wiki Activity

Merge pull request #34808 from peppy/fix-difficulty-churn

Browse Source 
Fix beatmap carousel triggering full filters more often than it needs to
This commit is contained in:
Bartłomiej Dach
2025-08-28 14:26:25 +02:00
committed by GitHub
Unverified
parent f953d58922 7e109add96
commit 3eaa5314ac
4 changed files with 165 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselUpdateHandling.cs
+74 -7
View File
@@ -22,12 +22,16 @@ namespace osu.Game.Tests.Visual.SongSelectV2
{
private BeatmapSetInfo baseTestBeatmap = null!;
private const int initial_filter_count = 3;
[SetUpSteps]
public void SetUpSteps()
{
RemoveAllBeatmaps();
CreateCarousel();
WaitForFiltering();
AddBeatmaps(1, 3);
WaitForFiltering();
AddStep("generate and add test beatmap", () =>
{
baseTestBeatmap = TestResources.CreateTestBeatmapSetInfo(3);
@@ -42,8 +46,9 @@ namespace osu.Game.Tests.Visual.SongSelectV2
b.Metadata = metadata;
BeatmapSets.Add(baseTestBeatmap);
});
WaitForFiltering();
AddAssert("filter count correct", () => Carousel.FilterCount, () => Is.EqualTo(initial_filter_count));
}
[Test]
@@ -81,12 +86,18 @@ namespace osu.Game.Tests.Visual.SongSelectV2
AddUntilStep("is scrolled to end", () => Carousel.ChildrenOfType<UserTrackingScrollContainer>().Single().IsScrolledToEnd());
updateBeatmap(b => b.Metadata = new BeatmapMetadata
updateBeatmap(b =>
{
Artist = "updated test",
Title = $"beatmap {RNG.Next().ToString()}"
// hash will be updated when important metadata changes, such as title, difficulty, author etc.
b.Hash = "new hash";
b.Metadata = new BeatmapMetadata
{
Artist = "updated test",
Title = $"beatmap {RNG.Next().ToString()}"
};
});
assertDidFilter();
WaitForFiltering();
AddAssert("scroll is still at end", () => Carousel.ChildrenOfType<UserTrackingScrollContainer>().Single().IsScrolledToEnd());
@@ -113,8 +124,14 @@ namespace osu.Game.Tests.Visual.SongSelectV2
AddStep("find panel", () => panel = Carousel.ChildrenOfType<PanelBeatmapSet>().Single(p => p.ChildrenOfType<OsuSpriteText>().Any(t => t.Text.ToString() == "beatmap")));
updateBeatmap(b => b.Metadata = metadata);
updateBeatmap(b =>
{
b.Metadata = metadata;
// hash will be updated when important metadata changes, such as title, difficulty, author etc.
b.Hash = "new hash";
});
assertDidFilter();
WaitForFiltering();
AddAssert("drawables unchanged", () => Carousel.ChildrenOfType<Panel>(), () => Is.EqualTo(originalDrawables));
@@ -123,7 +140,41 @@ namespace osu.Game.Tests.Visual.SongSelectV2
}
[Test]
public void TestSelectionHeld()
public void TestOnlineStatusUpdated()
{
List<Panel> originalDrawables = new List<Panel>();
AddStep("store drawable references", () =>
{
originalDrawables.Clear();
originalDrawables.AddRange(Carousel.ChildrenOfType<Panel>().ToList());
});
updateBeatmap(b => b.Status = BeatmapOnlineStatus.Graveyard);
assertDidFilter();
WaitForFiltering();
AddAssert("drawables unchanged", () => Carousel.ChildrenOfType<Panel>(), () => Is.EqualTo(originalDrawables));
}
[Test]
public void TestNoUpdateTriggeredOnUserTagsChange()
{
var metadata = new BeatmapMetadata
{
Artist = "updated test",
Title = "new beatmap title",
UserTags = { "hi" }
};
updateBeatmap(b => b.Metadata = metadata);
assertDidNotFilter();
}
[TestCase(false)]
[TestCase(true)]
public void TestSelectionHeld(bool hashChanged)
{
SelectNextSet();
@@ -131,7 +182,17 @@ namespace osu.Game.Tests.Visual.SongSelectV2
AddAssert("selection is updateable beatmap", () => Carousel.CurrentSelection, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0]));
AddAssert("visible panel is updateable beatmap", () => GetSelectedPanel()?.Item?.Model, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0]));
updateBeatmap();
updateBeatmap(b =>
{
if (hashChanged)
b.Hash = "new hash";
});
if (hashChanged)
assertDidFilter();
else
assertDidNotFilter();
WaitForFiltering();
AddAssert("selection is updateable beatmap", () => Carousel.CurrentSelection, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0]));
@@ -148,6 +209,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2
AddAssert("visible panel is updateable beatmap", () => GetSelectedPanel()?.Item?.Model, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0]));
updateBeatmap(b => b.DifficultyName = "new name");
assertDidFilter();
WaitForFiltering();
AddAssert("selection is updateable beatmap", () => Carousel.CurrentSelection, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0]));
@@ -164,6 +226,7 @@ namespace osu.Game.Tests.Visual.SongSelectV2
AddAssert("visible panel is updateable beatmap", () => GetSelectedPanel()?.Item?.Model, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0]));
updateBeatmap(b => b.OnlineID = b.OnlineID + 1);
assertDidFilter();
WaitForFiltering();
AddAssert("selection is updateable beatmap", () => Carousel.CurrentSelection, () => Is.EqualTo(baseTestBeatmap.Beatmaps[0]));
@@ -339,6 +402,10 @@ namespace osu.Game.Tests.Visual.SongSelectV2
AddAssert("Order didn't change", () => Carousel.PostFilterBeatmaps.Select(b => b.ID), () => Is.EqualTo(originalOrder));
}
private void assertDidFilter() => AddAssert("did filter", () => Carousel.FilterCount, () => Is.EqualTo(initial_filter_count + 1));
private void assertDidNotFilter() => AddAssert("did not filter", () => Carousel.FilterCount, () => Is.EqualTo(initial_filter_count));
private void updateBeatmap(Action<BeatmapInfo>? updateBeatmap = null, Action<BeatmapSetInfo>? updateSet = null)
{
AddStep("update beatmap with different reference", () =>
osu.Game.Tests/Visual/SongSelectV2/TestSceneSongSelect.cs
+24
View File
@@ -23,7 +23,9 @@ using osu.Game.Screens.Ranking;
using osu.Game.Screens.Select;
using osu.Game.Screens.Select.Leaderboards;
using osu.Game.Screens.SelectV2;
using osu.Game.Tests.Resources;
using osuTK.Input;
using BeatmapCarousel = osu.Game.Screens.SelectV2.BeatmapCarousel;
using FooterButtonMods = osu.Game.Screens.SelectV2.FooterButtonMods;
using FooterButtonOptions = osu.Game.Screens.SelectV2.FooterButtonOptions;
using FooterButtonRandom = osu.Game.Screens.SelectV2.FooterButtonRandom;
@@ -302,6 +304,28 @@ namespace osu.Game.Tests.Visual.SongSelectV2
});
}
/// <summary>
/// Last played and rank achieved may have changed, so we want to make sure filtering runs on resume to song select.
/// </summary>
[Test]
public void TestFilteringRunsAfterReturningFromGameplay()
{
AddStep("import actual beatmap", () => Beatmaps.Import(TestResources.GetQuickTestBeatmapForImport()));
LoadSongSelect();
AddUntilStep("wait for filtered", () => SongSelect.ChildrenOfType<BeatmapCarousel>().Single().FilterCount, () => Is.EqualTo(1));
AddStep("enter gameplay", () => InputManager.Key(Key.Enter));
AddUntilStep("wait for player", () => Stack.CurrentScreen is Player);
AddUntilStep("wait for fail", () => ((Player)Stack.CurrentScreen).GameplayState.HasFailed);
AddStep("exit gameplay", () => InputManager.Key(Key.Escape));
AddStep("exit gameplay", () => InputManager.Key(Key.Escape));
AddUntilStep("wait for filtered", () => SongSelect.ChildrenOfType<BeatmapCarousel>().Single().FilterCount, () => Is.EqualTo(2));
}
[Test]
public void TestAutoplayShortcut()
{
osu.Game/Graphics/Carousel/Carousel.cs
+13 -2
View File
@@ -3,6 +3,7 @@
using System;
using System.Collections.Generic;
using System.Collections.Specialized;
using System.Diagnostics;
using System.Linq;
using System.Threading;
@@ -79,7 +80,7 @@ namespace osu.Game.Graphics.Carousel
/// <summary>
/// The number of times filter operations have been triggered.
/// </summary>
internal int FilterCount { get; private set; }
public int FilterCount { get; private set; }
/// <summary>
/// The number of displayable items currently being tracked (before filtering).
@@ -210,6 +211,12 @@ namespace osu.Game.Graphics.Carousel
return filterTask;
}
/// <summary>
/// Called when <see cref="Items"/> changes in any way.
/// </summary>
/// <returns>Whether a re-filter is required.</returns>
protected virtual bool HandleItemsChanged(NotifyCollectionChangedEventArgs args) => true;
/// <summary>
/// Fired after a filter operation completed.
/// </summary>
@@ -301,7 +308,11 @@ namespace osu.Game.Graphics.Carousel
RelativeSizeAxes = Axes.Both,
};
Items.BindCollectionChanged((_, _) => filterAfterItemsChanged.Invalidate());
Items.BindCollectionChanged((_, args) =>
{
if (HandleItemsChanged(args))
filterAfterItemsChanged.Invalidate();
});
}
[BackgroundDependencyLoader]
osu.Game/Screens/SelectV2/BeatmapCarousel.cs
+54
View File
@@ -143,6 +143,9 @@ namespace osu.Game.Screens.SelectV2
switch (changed.Action)
{
case NotifyCollectionChangedAction.Add:
if (!newItems!.Any())
return;
Items.AddRange(newItems!.SelectMany(s => s.Beatmaps));
break;
@@ -353,6 +356,57 @@ namespace osu.Game.Screens.SelectV2
}
}
protected override bool HandleItemsChanged(NotifyCollectionChangedEventArgs args)
{
switch (args.Action)
{
case NotifyCollectionChangedAction.Add:
case NotifyCollectionChangedAction.Remove:
case NotifyCollectionChangedAction.Move:
case NotifyCollectionChangedAction.Reset:
return true;
case NotifyCollectionChangedAction.Replace:
var oldBeatmaps = args.OldItems!.OfType<BeatmapInfo>().ToList();
var newBeatmaps = args.NewItems!.OfType<BeatmapInfo>().ToList();
for (int i = 0; i < oldBeatmaps.Count; i++)
{
var oldBeatmap = oldBeatmaps[i];
var newBeatmap = newBeatmaps[i];
// Ignore changes which don't concern us.
//
// Here are some examples of things that can go wrong:
// - Background difficulty calculation runs and causes a realm update.
// We use `BeatmapDifficultyCache` and don't want to know about these.
// - Background user tag population runs and causes a realm update.
// We don't display user tags so want to ignore this.
bool equalForDisplayPurposes =
// covers metadata changes
oldBeatmap.Hash == newBeatmap.Hash &&
// sanity check
oldBeatmap.OnlineID == newBeatmap.OnlineID &&
// displayed on panel
oldBeatmap.Status == newBeatmap.Status &&
// displayed on panel
oldBeatmap.DifficultyName == newBeatmap.DifficultyName &&
// hidden changed, needs re-filter
oldBeatmap.Hidden == newBeatmap.Hidden &&
// might be used for grouping, returning from gameplay
oldBeatmap.LastPlayed == newBeatmap.LastPlayed;
if (equalForDisplayPurposes)
return false;
}
return true;
default:
throw new ArgumentOutOfRangeException();
}
}
protected override void HandleFilterCompleted()
{
base.HandleFilterCompleted();