Merge pull request #29916 from peppy/fix-song-select-crash-selection-after-loading

Fix occasional song select crash when entering gameplay
This commit is contained in:
Bartłomiej Dach 2024-09-18 11:44:59 +02:00 committed by GitHub
commit cf9f8c7f66
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 47 additions and 5 deletions

View File

@ -144,6 +144,28 @@ public void TestExitSongSelectWithEscape()
exitViaEscapeAndConfirm();
}
[Test]
public void TestEnterGameplayWhileFilteringToNoSelection()
{
TestPlaySongSelect songSelect = null;
PushAndConfirm(() => songSelect = new TestPlaySongSelect());
AddUntilStep("wait for song select", () => songSelect.BeatmapSetsLoaded);
AddStep("import beatmap", () => BeatmapImportHelper.LoadQuickOszIntoOsu(Game).WaitSafely());
AddUntilStep("wait for selected", () => !Game.Beatmap.IsDefault);
AddStep("force selection", () =>
{
songSelect.FinaliseSelection();
songSelect.FilterControl.CurrentTextSearch.Value = "test";
});
AddUntilStep("wait for player", () => !songSelect.IsCurrentScreen());
AddStep("return to song select", () => songSelect.MakeCurrent());
AddUntilStep("wait for selection lost", () => songSelect.Beatmap.IsDefault);
}
[Test]
public void TestSongSelectBackActionHandling()
{

View File

@ -706,7 +706,7 @@ public void Filter(FilterCriteria? newCriteria)
private bool beatmapsSplitOut;
private void applyActiveCriteria(bool debounce, bool alwaysResetScrollPosition = true)
private void applyActiveCriteria(bool debounce)
{
PendingFilter?.Cancel();
PendingFilter = null;
@ -735,8 +735,7 @@ void perform()
root.Filter(activeCriteria);
itemsCache.Invalidate();
if (alwaysResetScrollPosition || !Scroll.UserScrolling)
ScrollToSelected(true);
ScrollToSelected(true);
FilterApplied?.Invoke();
}
@ -1036,7 +1035,7 @@ private void updateYPositions()
itemsCache.Validate();
// update and let external consumers know about selection loss.
if (BeatmapSetsLoaded)
if (BeatmapSetsLoaded && AllowSelection)
{
bool selectionLost = selectedBeatmapSet != null && selectedBeatmapSet.State.Value != CarouselItemState.Selected;

View File

@ -127,6 +127,8 @@ public abstract partial class SongSelect : ScreenWithBeatmapBackground, IKeyBind
private Sample sampleChangeDifficulty = null!;
private Sample sampleChangeBeatmap = null!;
private bool pendingFilterApplication;
private Container carouselContainer = null!;
protected BeatmapDetailArea BeatmapDetails { get; private set; } = null!;
@ -328,7 +330,20 @@ private void load(AudioManager audio, OsuColour colours, ManageCollectionsDialog
GetRecommendedBeatmap = s => recommender?.GetRecommendedBeatmap(s),
}, c => carouselContainer.Child = c);
FilterControl.FilterChanged = Carousel.Filter;
FilterControl.FilterChanged = criteria =>
{
// If a filter operation is applied when we're in a state that doesn't allow selection,
// we might end up in an unexpected state. This is because currently carousel panels are in charge
// of updating the global selection (which is very hard to deal with).
//
// For now let's just avoid filtering when selection isn't allowed locally.
// This should be nuked from existence when we get around to fixing the complexity of song select <-> beatmap carousel.
// The debounce part of BeatmapCarousel's filtering should probably also be removed and handled locally.
if (Carousel.AllowSelection)
Carousel.Filter(criteria);
else
pendingFilterApplication = true;
};
if (ShowSongSelectFooter)
{
@ -701,6 +716,12 @@ public override void OnResuming(ScreenTransitionEvent e)
Carousel.AllowSelection = true;
if (pendingFilterApplication)
{
Carousel.Filter(FilterControl.CreateCriteria());
pendingFilterApplication = false;
}
BeatmapDetails.Refresh();
beginLooping();