Fix "ALL MODS" display not displaying in new playlist song select#36500
Fix "ALL MODS" display not displaying in new playlist song select#36500bdach merged 5 commits intoppy:masterfrom
Conversation
|
Okay I just found out this class is copypasta from another identical usage which needs similar fixes applied. Blocking until I clean this up. |
I don't want to have to update things in multiple places with different code in each place. This also closes ppy#36412.
| public partial class ModCountText : VisibilityContainer, IHasCustomTooltip<IReadOnlyList<Mod>> | ||
| { | ||
| public readonly Bindable<IReadOnlyList<Mod>> Mods = new Bindable<IReadOnlyList<Mod>>(); | ||
| public readonly Bindable<bool> Freestyle = new Bindable<bool>(); |
There was a problem hiding this comment.
Somewhat perturbed by this leaking into the solo/non-free-mod song select implementation which should not maintain or be aware of the notion of "freestyle" at all.
I'd be much more comfortable with this if this component was dumb, i.e. it was just passed content to display by its consumers (text + mods in the overflow / tooltip) which would presumably eliminate the need to do this.
There was a problem hiding this comment.
Can do.
I'm 10x less perturbed by this kind of thing than I am with classes like this being verbatim copy pasted across multiple usages where I only realise this is the case (and bugs exist in multiple places as a result) by accident.
There was a problem hiding this comment.
I'll meet you halfway because the Mods flow is required for the tooltip to work.
github-project-automation
bot
moved this from Pending Review
to Done
in osu! team task tracker
2 participants
FreeModsbutton #36411.ModCountTextonFreeModsbutton #36412Also fixes some visual glitching which I noticed on the way:
Tooltip can get very long
Display flashed when switching freestyle on/off
Adjusted via 0c90cf3. Hard to show but basically it was updating the background dim overlay to be hidden while the overflow was empty for one frame.