Replace new combo button icons with ruleset-specifc ones#37848
Replace new combo button icons with ruleset-specifc ones#37848LiquidPL wants to merge 3 commits into
Conversation
Depends on ppy/osu-resources#425. This makes the new combo button use the new icons added in ppy#37804. Instead of having four separate icons per ruleset, the "sparkle" texture is overlaid on top of the appropriate icon. I'm not sure if I've overdone it with how every ruleset copypastes the same code for the icon (in `<ruleset>BlueprintContainer`), so that can be scaled down if necessary.
| { | ||
| Children = new Drawable[] | ||
| { | ||
| new SpriteIcon |
There was a problem hiding this comment.
was kinda hoping this would be automatic by just taking the first object type icon, haven't looked into how possible that is though.
There was a problem hiding this comment.
will try to check that out
There was a problem hiding this comment.
Might be somewhat complicated without refactoring some of the button code, since to my understanding (using osu as an example) at the top level there's OsuHitObjectComposer, which defines the hitobject buttons, aka the Toolbox section. However it also pulls in the ternary buttons, New combo among them from OsuBlueprintContainer.
By all this I mean that the data flow goes in the other direction, and it would feel awkward to have OsuBlueprintContainer try to pull the first hitobject to get its icon (which it can't right now since HitObjectComposer.CompositionTools is protected). Generally speaking it is confusing to me why is OsuBlueprintContainer, which is generally concerned with object blueprints, also responsible for all those ternary buttons?
There was a problem hiding this comment.
Given there's no internal overrides of CreateTernaryButtons and no other usages of MainTernaryStates, I think you should be safe with moving the whole shebang to HitObjectComposer. These classes got a bit confused over the years, and have been tidied up progressively. This stuff was likely just remaining to be migrated across.
Wanna give that a try?
There was a problem hiding this comment.
I can try, however my concern is that since CreateTernaryButtons depends on SelectionHandler which will take more effort to move out since it's defined on the base BlueprintContainer.
I figure I would need to move SelectionHandler up to HitObjectComposer as well, however I'd need to figure out a similar thing for the skin editor as well.
Either way it's probably best to merge this first and deal with said refactor in a future PR.
2 participants
This makes the new combo button use the new icons added in #37804. Instead of having four separate icons per ruleset, the "sparkle" texture is overlaid on top of the appropriate icon.
I'm not sure if I've overdone it with how every ruleset copypastes the same code for the icon (in
<ruleset>BlueprintContainer), so that can be scaled down if necessary.