This change pulls back a significant degree of overspecialisation and
rigidity in the class structure of `HitWindows` to make subsequent
changes to hit windows, whose purpose is to improve replay playback
accuracy, possible to do cleanly.
Notably:
- `HitWindows` is full abstract now. In a few use cases, and as a
reference for ruleset implementors, `DefaultHitWindows` is provided as
a separate class instead.
This fixes the weirdness wherein `HitWindows` always declared 6 fields
for result types but some of them would never be set to a non-zero
value or read.
- `HitWindow.GetRanges()` is deleted because it is overspecialised and
prevents being able to adjust hitwindows by ±0.5ms cleanly which will
be required later.
The fallout of this is that the assertion that used `GetRanges()` in
the `HitWindows` ctor must use something else now, and the closest
thing to it was `GetAllAvailableWindows()`, which didn't return
the miss window - so I made it return the miss window and fixed the
one consumer that didn't want it (bar hit error meter) to skip it.
- Diff also contains some clean-up around `DifficultyRange` to unify
handling of it.
By falling back to the default of 40% of track duration in such cases.
Also added a safety for the restart point exceeding acceptable bounds in
case of a non-zero offset.
Closes https://github.com/ppy/osu/issues/33308.
- It was logging success before actually succeeding.
- It appears in practice that this code can somehow actually nullref.
Unfortunately logs provided in that instance were not enough to
pinpoint what (because of lack of line numbers). I'm hoping that by
logging as error, and therefore to sentry, we can actually retrieve
this information so that there's no need to work blind.
This is in response to feedback in
https://osu.ppy.sh/community/forums/topics/2056547?n=1.
Upon examining the button further, there was indeed some rather weird...
almost hysteresis in how the button behaved with respect to the area on
the screen that activated it. Because of the following scourge of a
method that continues to haunt us to this day:
https://github.com/ppy/osu/blob/31487545d0d17c4337d4b4cc5d4afb3ba1dae838/osu.Game/Graphics/Containers/OsuClickableContainer.cs#L24-L25
the button would effectively only be activated by 80% of its drawable
area when it was not hovered, because of the scale applied to the
`content` container which `Container.Content` redirected to.
This is resolved here by various rearrangements of paddings and sizes
such that the clickable area of any of the buttons of the card is always
the full top or bottom half of the button area.
Also included are some cosmetic touch-ups which happened to be
convenient like folding the loading spinner into the base
`BeatmapCardIconButton`, adding loading support for the favourite
button, using BDL more, and resolving some "virtual member call in
constructor" inspections.
Closes https://github.com/ppy/osu/issues/32420.
The failure cause here is that in editor the beatmap version for the
beatmap affected (or... any beatmap, really), is 0 (ZERO). That is
probably a regression from https://github.com/ppy/osu/pull/32315, but
like... can we universally agree that calling that change "a regression"
in any capacity is dumb? Like what was that code *doing* playing dumb
reference games and copying stuff into an arbitrary instance that could
get or not get used later on? And now you have a 50/50 chance of
accessing the *correct* model's field, depending on whether you go via
`BeatmapInfo` or `Beatmap.BeatmapInfo`?
Moving the field to `IBeatmap`, i.e. what is by now - by consensus,
since https://github.com/ppy/osu/pull/28473 - supposed to be the "decoded
and materialised" beatmap, fixes this issue.
I probably should have done this as part of
https://github.com/ppy/osu/pull/28473 but it slipped my mind. Probably
for the better too because this change has rather large chances of
breaking stuff so maybe better to examine it in isolation (via diffcalc
runs or whatever).
For added humour points, you'd say that the field on `BeatmapInfo` was
not `[Ignore]`d, so this is a realm schema change, right? No. As far as
I can tell, it's not. I opened realm studio and `BeatmapVersion` *is not
a listed column` on `Beatmap` models.
I'm also not gonna get into the fact that I think `EditorBeatmap` doing
dumb games with juggling two `BeatmapInfo` references since
https://github.com/ppy/osu/pull/15075 is bad, because I don't think I
have the mental capacity to hotfix this by going down that train of
thought.