As I look into re-implementing the ability to choose combo colour for an
object (also known as "colourhax") from the editor UI, I stumble upon
these wretched ternary items again and sigh a deep sigh of annoyance.
The structure is overly rigid. `TernaryItem` does nothing that
`DrawableTernaryItem` couldn't, except make it more annoying to add
specific sub-variants of `DrawableTernaryItem` that could do more
things.
Yes you could sprinkle more levels of virtuals to
`CreateDrawableButton()` or something, but after all, as Saint Exupéry
says, "perfection is finally attained not when there is no longer
anything to add, but when there is no longer anything to take away."
So I'm leaning for taking one step towards perfection.
- Closes https://github.com/ppy/osu/issues/31423.
- Regressed in https://github.com/ppy/osu/pull/30411.
Admittedly, I don't completely understand all of the pieces here,
because code quality of this placement blueprint code is ALL-CAPS
ATROCIOUS, but I believe the failure mode to be something along the
lines of:
- User activates juice stream tool, blueprint gets created in initial
state. It reads in a mouse position far outside of the playfield, and
sets internal positioning appropriately.
- When the user moves the mouse into the bounds of the playfield, some
positions update (the ones inside `UpdateTimeAndPosition()`, but the
fruit markers are for *nested* objects, and
`updateHitObjectFromPath()` is responsible for updating those...
however, it only fires if the `editablePath.PathId` changes, which it
won't here, because there is only one path vertex until the user
commits the starting point of the juice stream and it's always at
(0,0).
- Therefore the position of the starting fruit marker remains bogus
until left click, at which point the path changes and everything
returns to *relative* sanity.
The solution essentially relies on inlining the broken method and only
guarding the relevant part of processing behind the path version check
(which is actually updating the path). Everything else that can touch
positions of nesteds (like default application, and the drawable piece
updates) is allowed to happen unconditionally.
No issue thread for this, was pointed out internally:
https://discord.com/channels/90072389919997952/1259818301517725707/1316604605777444905
Due to the custom setup that editor has with its nested
"screens-that-aren't-screens", the logic that selects the closest
timing point to the current time would only fire on the first open of
the screen. Seems like a good idea to have it fire every time instead.
No issue thread for this again. Reported internally on discord:
https://discord.com/channels/90072389919997952/1259818301517725707/1320420768814727229
Placing this logic in the beatmap processor, as a post-processing step,
means that the new combo force won't be visible until a placement has
been committed. That can be seen as subpar, but I tried putting this
logic in the placement and it sucked anyway:
- While the combo number was correct, the colour looked off, because it
would use the same combo colour as the already-placed objects after
said break, which would only cycle to the next, correct one on
placement
- Not all scenarios can be handled in the placement. Refer to one of the
test cases added in the preceding commit, wherein two objects are
placed far apart from each other, and an automated break is inserted
between them - the placement has no practical way of knowing whether
it's going to have a break inserted automatically before it or not.
