Restore previous beatmap when leaving scoped mode#36582
Conversation
|
Just in case, I'll give my thoughts on why I suggested this UX in the issue post, specifically this part:
There are 2 solutions: either always return to the difficulty which was selected before entering scoped mode, or only return to it if the user selects a difficulty inside scoped mode that's outside filter range. By the way, with this PR, if you enter scoped mode, then select a difficulty that's outside filter range, then exit to main menu, then enter song select, the scrolling will still be lost. I'll leave it to you if this is worth fixing (I don't know code). |
There was a problem hiding this comment.
In general I am not a fan of how this restore behaviour has smeared itself all over BeatmapCarousel. I think it's going to end badly because I tried similar once before and it resulted in carnage.
If this sort of thing is to be entertained then the "scoping" API probably has to change. It probably should not be a plain bindable anymore and instead be a method you call, something like (DISCLAIMER: I have not tried this, just spitballing):
IDisposable ScopeToBeatmap(IBeatmapSetInfo beatmapSet);The IDisposable would be an InvokeOnDisposal() that the caller should dispose of when they wish to stop showing the scoped beatmap. This disposable would be responsible for restoring the beatmap, hopefully in a way that centralises this logic to one confined operation.
The way this is currently written just gives me little to no confidence that this flow won't fire in the wrong circumstances accidentally, will fire in all right applicable circumstances, etc.
Either way any solution here probably needs some further testing with things like changing ruleset inside scoped mode, etc. to give better guarantees that this doesn't fire in some unwanted circumstance.
This is out of scope to fix even on a UX / user expectation level. Not sure why exiting out of song select should always preserve scroll position now. |
It would be better if the scroll position always got preserved; why would anyone want their scrolling to randomly be at the top? But it's fine, I personally don't care about it and it's a very minor issue, I just noticed it while testing the PR and thought I'd mention it in case it's a simple fix which might as well be included. |
|
I've added a bunch of tests for cases where the selection would be changed through some other way on top of the revert-on-unscope behavior implemented here (changing ruleset, filters. etc.). Ultimately I didn't end up using |
| leasedScopedBeatmapSet.Value = null; | ||
| } | ||
|
|
||
| private void filterCompleted() |
There was a problem hiding this comment.
Does this need to hook into the filtering flow there? Can't we just do
diff --git a/osu.Game/Screens/SelectV2/SongSelect.cs b/osu.Game/Screens/SelectV2/SongSelect.cs
index 1e40877666..6964c5034d 100644
--- a/osu.Game/Screens/SelectV2/SongSelect.cs
+++ b/osu.Game/Screens/SelectV2/SongSelect.cs
@@ -271,7 +271,6 @@ private void load(AudioManager audio, OsuConfigManager config)
RequestPresentBeatmap = b => SelectAndRun(b, OnStart),
RequestSelection = queueBeatmapSelection,
RequestRecommendedSelection = requestRecommendedSelection,
- FilterCompleted = filterCompleted,
NewItemsPresented = newItemsPresented,
},
noResultsPlaceholder = new NoResultsPlaceholder
@@ -1264,18 +1263,9 @@ public void UnscopeBeatmapSet()
return;
leasedScopedBeatmapSet.Value = null;
- }
-
- private void filterCompleted()
- {
- if (carousel.CurrentGroupedBeatmap == null)
- return;
-
- if (beforeScopedSelection == null)
- return;
-
- if (!carousel.GetCarouselItems()!.Any(i => i.Model is GroupedBeatmap g && g.Equals(carousel.CurrentGroupedBeatmap)))
+ if (beforeScopedSelection != null)
queueBeatmapSelection(beforeScopedSelection);
+ beforeScopedSelection = null;
}
#endregion
I guess the concern might have been that if it's done that way then the selection will change before the filter completes which looks kinda twitchy? But I dunno, I think that's fine, I'd rather have it look a little twitchy than be worse code. (Which maybe betrays that my priorities are backwards, but I'd be running this past @peppy anyway before a merge.)
There was a problem hiding this comment.
That's what I did initially, but this causes the selection to be always reverted, even if the current one is still visible after unscoping. This might be fine but I feel that is something that could break expectations.
The selection changing before the filter completing didn't look that bad actually when I was testing it.
There was a problem hiding this comment.
I think always reverting the selection to the one before scoping is probably OK? The reporter of the issue thought it was acceptable as well.
I'd personally say let's start there and we can complicate later as required.
|
Here's how the transition looks with the selection being reverted before the filtering happens: 2026-02-13.16-47-00.mp4I think it actually telegraphs the intention of reverting to the pre-scope selection pretty well. |
For safety, mostly.
bdach
left a comment
There was a problem hiding this comment.
Yeah I'm pretty happy with where this is code-wise now.
@peppy would appreciate a check on the concerns raised in #36582 (comment) / #36582 (comment) regarding transitions at the least.
|
FWIW I still think there may be a future where we want a special mode for the case where the current selection is not visible due to filters (like stable, where it shows the panel regardless). The use case being when a user manages to change selection by clicking a notification for, say, a a beatmap import, or something else. But this is fine for now as a means to maintain sanity. |
|
As someone who uses this feature a lot, the behavior of always going back to the originally selected difficulty when exiting scoped mode instead of staying on the currently selected difficulty even when both are inside of the filter range feels very bad. It doesn't really make sense in this case because if I wanted to go back to the original difficulty, I would select that difficulty first before exiting scoped mode. But now, if I want to exit scoped mode at a different difficulty, I can't (and I did that all the time). |
Thanks for the feedback. That's how the feature was originally implemented, but it was quite a complex implementation. I think what you are saying makes sense though, so hopefully we can make it work like that in the future. Feel free to open a discussion tracking this, I don't think we have one yet. |
Resolves #36288.
If the current selection is still available after leaving scoped mode, it's left as is. If it's not, the selection from before entering scoped mode is restored.
2026-02-04.14-07-31.mp4