Skip to content

Remove filtering & rearranging controls from now playing overlay #32679

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Apr 7, 2025

Conversation

bdach
Copy link
Collaborator

@bdach bdach commented Apr 4, 2025

Screen.Recording.2025-04-04.at.13.17.26.mov

Reasons for removal:

  • Neither the filtering nor the rearranging was respected for playback order purposes.
  • The half-baked "support" for the above causes performance degradation of the view for users with large beatmap collections.

RFC. Wanting to get this one off my forever task lists.

While the initial discussion was that filtering should stay, it's not easy (if at all possible) to make it work with VirtualisedListContainer. That consensus seemed weak anyway based on #28968 (comment). Filtering can maybe be salvaged if I try but I want to see the initial reaction to this as is.

Overflowing items now scroll rather than wrap. This is forced because of VirtualisedListContainer's constraints (items need to have a fixed height).

@peppy
Copy link
Member

peppy commented Apr 4, 2025

If filtering is to stay, I think it should be collections only, and it should match song select (ie selected collection should be a global game state).

@bdach
Copy link
Collaborator Author

bdach commented Apr 4, 2025

I think it should be collections only, and it should match song select (ie selected collection should be a global game state).

How far would you see that going? I guess this would also limit track playback game-wide to the selected collection?

Can probably do, but may be best to do in a separate PR, this one is XL already and basically none of the old implementation is reusable for this so I'd essentially be reimplementing from scratch. Let me know if you want that here or no.

@peppy peppy self-requested a review April 4, 2025 11:55
@peppy
Copy link
Member

peppy commented Apr 4, 2025

How far would you see that going? I guess this would also limit track playback game-wide to the selected collection?

Pretty much yeah. Separate / future is fine.

@peppy peppy self-requested a review April 5, 2025 13:23
RelativeSizeAxes = Axes.X;
Height = 20;

InternalChild = text = new OverflowScrollingContainer
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sold on every long item always scrolling. Probably better only when hovered?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried, please see how you find b8360a1 (especially the behaviour when un-hovering mid-scroll).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd expect the scroll to reset to original position on unhovering.

Copy link
Collaborator Author

@bdach bdach Apr 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's how I had it initially and it was a much simpler implementation but I changed because it was a bit jarring. Guess I'll roll back...

@peppy peppy self-requested a review April 7, 2025 11:34
@peppy
Copy link
Member

peppy commented Apr 7, 2025

Made some minor changes, please make sure you're okay with them!

peppy
peppy previously approved these changes Apr 7, 2025
@bdach
Copy link
Collaborator Author

bdach commented Apr 7, 2025

Changes look ok, thanks 👍

@peppy peppy merged commit 10abd55 into ppy:master Apr 7, 2025
10 checks passed
@bdach bdach deleted the playlist-hacksawing branch April 8, 2025 05:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants