Skip to content
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

Misc hover/select sound usage updates #14147

Merged
merged 6 commits into from
Aug 7, 2021
Merged

Conversation

nekodex
Copy link
Contributor

@nekodex nekodex commented Aug 6, 2021

A few misc changes:

  • Re-adds on-click feedback to the news overlay sidebar (the month headers)
  • Re-adds on-click feedback to dropdown headers
  • Adds open/close sounds to menus (re-using dropdown sounds... for now?)
  • Adds hover/select/join sounds to the multiplayer games listing

partially addresses #14082

@frenzibyte
Copy link
Member

Currently opening editor triggers a menu open sample, coming from EditorMenuBar which inherits from OsuMenu. Seems like that should not play any sounds on AnimateOpen/AnimateClose.

Copy link
Member

@frenzibyte frenzibyte left a comment

Choose a reason for hiding this comment

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

As above, otherwise looks good.

@bdach
Copy link
Collaborator

bdach commented Aug 7, 2021

I've fixed the above with a local guard checking that the menu is not a TopLevelMenu.

Possibly arguable that those menus shouldn't have Animate{Open,Close} called on them in the first place, but that'd be a framework amendment, so not sure whether that's something for now or later. Depends on how much this is desired to be in the next release or not, I suppose.

bdach
bdach previously approved these changes Aug 7, 2021
@frenzibyte
Copy link
Member

Hard one to decide, one may want to perform transforms when showing a menu bar (I can envision that potentially happening with editor at some point).

Not playing on TopLevelMenu sounds logically sufficient.

frenzibyte
frenzibyte previously approved these changes Aug 7, 2021
@peppy peppy dismissed stale reviews from frenzibyte and bdach via db270a7 August 7, 2021 18:54
@peppy peppy enabled auto-merge August 7, 2021 18:58
@peppy peppy merged commit c8a0b60 into ppy:master Aug 7, 2021
@nekodex nekodex mentioned this pull request Aug 11, 2021
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants