Closes https://github.com/ppy/osu/issues/24185
The stable code has had a bug in this logic forever. So we'll need to
reimplement the bug.
Basically, sliders have to have `UpdateCalculations` run in order to
have a correct `Position2` and `EndTime`, but this wasn't being called
in the inner loop before use of `EndTime` at
1531237b63/osu!/GameplayElements/HitObjectManager.cs#L1813.
To fix this, we use `StartTime` in the inner loop to reproduce the bug.
Firstly, this is intended to be a float division.
Secondly, dividing integers by 0 results in an exception, but dividing
non-zero floats by 0 results in +/- infinity which will be clamped to
the upper range.
In particular, this occurs when the beatmap has 1 hitobject (0 drain
length).
Closes#23862.
Score V2 is a scoring algorithm, which aside from the raw numerical
values of each judgement, incorporates a combo component, wherein each
judgement's "combo score" is derived from both the raw numerical value
of the object and the current combo after the given judgement. In
particular, this means that Score V2 is sensitive to the _order_ of
judging objects, as if two objects with the same start time are judged
using different ordering, they can end up having a different "combo
score".
The issue that this change is fixing is an instance of one such
reordering. Upon inspection, it turned out that the simulated autoplay
run, which is used to determine max possible score so that it can be
standardised to 1 million again, was processing a slider repeat before a
slider tail circle, while actual gameplay was processing the same slider
repeat _after_ the slider tail circle.
The cause of that behaviour is unfortunately due to `LegacyLastTick`.
The sliders which cause the issue are extremely short. Stable had a
behaviour, in which to provide leniency, slider tails were artificially
offset back by 36ms. However, if the slider is not long enough to make
this possible, the last tick is placed in the middle of the slider. If
that slider also happens to have exactly 1 repeat, then this means that
the last tick and the repeat have the same time instant.
Because of the time equality, what begins to matter now is the _order_
of processing the elements of the drawable slider in the hierarchy. For
the purposes of legacy skins, tail circles were moved below ticks in
fce3eacd7d - but in this particular case,
it means that the order of processing the slider elements is now
inadvertently inverted, causing the entire debacle.
While the fact that scoring depends on order of processing of visuals is
suboptimal, there isn't a great way to address this without significant
restructuring. Due to the structure of processing judgements currently
in place, in which each judgement is processed independently from others
by its corresponding drawable hit object, this is probably the best that
can be done for the time being at least.
There were no usages of more than one column being provided per row, so
it seemed like unnecessarily complexity. I'm currently trying to reduce
complexity so we can improve the layout of the results screen, which
currently has up to three levels of nested `GridContainer`s.
Of note, I can't add backwards compatibility because the method
signature has not changed in `Ruleset` (only the return type). If we do
want to keep compatibility with other rulesets, we could designate a new
name for the updated method.
1cd69220ef only disabled snapping the
aforementioned nodes to distance grid for already-placed sliders.
`SliderPlacementBlueprint` has its own logic for placement, so the fix
needs to be mirrored there too.
Moved accent color assignment from OnUpdate to ApplyCustomUpdateState. In order to get this to work, a flag needed to be added to DrawableHitObject.cs to disable combo color updates also being applied.
Until now, these were haphazardly enforce inline in blueprint
implementations. The only thing stopping complete breakage is that
`EndPlacement` wasn't called (too much) from outside the blueprint,
leaving them responsible for their own placement.
By moving this conditional out of the provided paramters to
`EndPlacement`, it allows more flexible usage of that method externally.
Coming in a future PR.
