Refactor classic osu!catch combo counter and place it in HUD overlay #26253
Conversation
frenzibyte
changed the title
| if (drawableRuleset != null) | ||
| { | ||
| var catchPlayfield = (CatchPlayfield)drawableRuleset.Playfield; | ||
| X = Parent.AsNonNull().ToLocalSpace(catchPlayfield.Catcher.ScreenSpaceDrawQuad.Centre).X; |
There was a problem hiding this comment.
I'm not 100% sure about this performance-wise, but it looks to work seamlessly.
There was a problem hiding this comment.
Probably okay. Minor refactor applied for legibility.
peppy
force-pushed
the
refactor-catch-combo-counter
branch
from
8125428 to
8fd296d
Compare
peppy
force-pushed
the
refactor-catch-combo-counter
branch
from
eacb0d0 to
a982519
Compare
See https://github.com/peppy/osu-stable-reference/blob/bb57924c1552adbed11ee3d96cdcde47cf96f2b6/osu!/GameModes/Play/Components/ComboCounterFruits.cs#L76 and https://github.com/peppy/osu-stable-reference/blob/bb57924c1552adbed11ee3d96cdcde47cf96f2b6/osu!/GameModes/Play/Components/ComboCounter.cs#L144
frenzibyte
force-pushed
the
refactor-catch-combo-counter
branch
from
80b5a7e to
f944ac5
Compare
Fixes catch editor saving test scene dying to the project referencing `osu.Game.Tests` which entails referencing all rulesets at once
|
Android project compile failures here |
|
I've brought this up-to-date. Probably needs second eyes at some point (low priority I guess), since I was involved in making this bearable. |
There was a problem hiding this comment.
That is going to make this PR harder to fix up, yeah.
| var catcher = ((CatchPlayfield)drawableRuleset.Playfield).Catcher; | ||
| X = Parent!.ToLocalSpace(catcher.ScreenSpaceDrawQuad.Centre).X; |
There was a problem hiding this comment.
I do wonder about this. Maybe it's fine for now, but I do recall that people have asked for things to be easily attachable to the catcher ("things" being an accuracy counter or something). Due to that I'd be wondering at this stage whether we want to have a SkinnableContainer rooted at the catcher or something for that express purpose.
Probably not something that is required to be acted upon immediately for merge, just food for thought maybe.
There was a problem hiding this comment.
Pretty interesting suggestion. It can't be applied at the moment however, as the available skin layers are defined at a ruleset-agnostic level (see GlobalSkinnableContainers).
|
I've removed Since the combo counter is no longer tied to the drawable ruleset, it will not be flipped alongside the catcher and all other components without hacks implemented. I've tried to handle this in cfc8b78 but the implementation is quite unbearable, especially since I've came to realise this PR is purely a code-quality refactor with the small addition of allowing the combo counter to be moved vertically. I personally think it is a better choice to close this PR and revisit it later when rulesets are allowed to create their own skinnable containers (so that we can have a "Catcher" layer that moves alongside the catcher and is flipped accordingly, combined with Closing as such. Can be reopened if anyone thinks otherwise. |
Also adjusted vertical position of legacy catch combo counter to roughly match stable (basically center).
"Closest" anchor is disabled for the combo counter since using an anchor that's not on the left side will break the calculation of the X position (also X position is calculated automatically so it doesn't make sense to change the anchor rather than locking it).
Preview:
CleanShot.2023-12-30.at.06.38.43-converted.mp4