This is a set of model changes which is supposed to facilitate support
for custom sample sets to the beatmap editor that is on par with stable.
It is the minimal set of changes. Because of this, it can probably be
considered "ugly" or however else you want to put it - but before you
say that, I want to try and pre-empt that criticism by explaining where
the problems lie.
Problem #1: duality in sample models
---
There is currently a weird duality of what a `HitObject`'s samples will
be.
- If an object has just been placed in the editor, and not saved /
decoded yet, it will use `HitSampleInfo`.
- If an object has already been encoded to the beatmap at least once, it
will use `ConvertHitObjectParser.LegacyHitSampleInfo`.
As long as that state of affairs remains, `HitSampleInfo` must be able
to represent anything that `LegacyHitSampleInfo` can, if feature parity
is to be achieved.
Problem 2: The 0 & 1 sample banks
---
Custom sample banks of 2 and above are a pretty clean affair. They map to
a suffix on the sample filename, and said samples are allowed to be
looked up from the beatmap skin. `Suffix` already exists in
`HitSampleInfo`.
However, the 1 custom sample bank is evil. It uses *non-suffixed*
samples, *allows lookups from the beatmap skins*, contrary to no bank /
bank 0, which *also* uses non-suffixed samples, but *doesn't* allow them
to be looked up from the beatmap skin.
This is why `HitSampleInfo.UseBeatmapSamples` has been called to
existence - without it there is no way to represent the ability of using
or not using the beatmap skin assets.
As has been stated previously in discussions about this feature, it's
both a *mapping* and a *skinning* concern.
There are many things you could do about either of these problems, but I
am pretty sure tackling either one is going to take *many* more lines of
code than this commit does. Which is why this is the starting point of
negotiation.
Closes https://github.com/ppy/osu/issues/34959.
`ArgonCounterTextComponent` is pretty terrible and prevents being able
to fix the issue easily. The core issue is that this is the first
instance of the component's usage where the label text can be longer
than the counter in the X dimension, so the total width of any counter
is equal to max(label width, counter width), and the label will be
aligned to the left of that width, while the counter will be aligned to
the right of that width.
The fix sort of relies on the fact that I don't expect *any* consumer of
`ArgonCounterTextComponent` that meaningfully uses the wireframe digits
to want the non-wireframe digits to be aligned to the *left* rather than
the right. It's not what I'd expect any segmented display to work.
(There are usages that specify `TopLeft` anchor, but they usually
display the same number of wireframe and non-wireframe digits, so for
them it doesn't really matter if the digits are left-aligned to the
wireframes or not.)
I can't see how this can happen in a normal flow, so just doing it as a
safety measure. Pointed out in https://github.com/ppy/osu/issues/34940
but likely due to a third party fuck being loaded.
This change refactors `GetAdjustedDisplayDifficulty()` and
`GetBeatmapAttributesToDisplay()` in two ways:
- Both methods now accept `IBeatmapInfo` instead of
`IBeatmapDifficultyInfo`. This is done in order to make mania key
count display to work, wherein `IBeatmapDifficultyInfo` is not enough
to calculate the final key count.
- `GetAdjustedDisplayDifficulty()` now applies all
`IApplicableToDifficulty` mods itself. I did this after noticing that
every real consumer of this method had to do that themselves for very
little reason.
In stable mania, Hard Rock and Easy mods do not work the same way as
they do on all of the rulesets. The difference is that mania HR and EZ,
rather than apply a multiplier to the map's original Overall Difficulty,
apply multipliers to *the durations of hit windows themselves*.
Prior to the last release, lazer was oblivious to this reality and just
treated mania HR / EZ as it did every other ruleset. Last release, for
the sake for gameplay parity across rulesets, the mods in question were
adjusted to match stable, but in the process, it started looking like HR
/ EZ did not change OD anymore.
The problem is that they do, but applying a multiplier to the map's OD
and applying a multiplier to the hit window duration is not the same
thing. The second thing is actually *much harsher* in magnitude, to the
point where applying HR to any map is almost guaranteed to exceed "the
effective OD" of 10, and applying EZ to any map is almost guaranteed to
result in "negative effective OD".
This change attempts to convey that reality by displaying "effective
OD", similar to what's already done in other rulesets when rate-changing
mods are active. Note that the values this will display *do not match*
stable *and that is correct*, because stable song select *lies* about
the actual impact on OD by just assuming it can treat all rulesets in
the same way.
---
Would close https://github.com/ppy/osu/issues/34150 I guess.
And yes I would like *all of the above* to land on the changelog if
possible if this is merged.
For further convincing that this makes any semblance of sense please see
the following: https://www.desmos.com/calculator/yigt7jycdv
Reported at https://osu.ppy.sh/comments/3681620, with appropriate levels
of rage bait (DID ANYONE TEST THIS?!?!?!?!?!?!?!?!?!111!!)
Reasoning for this is that without this, users' skin names can be
dropped after an external edit because they're never persisted anywhere
outside of realm.
The only other choice I see is to stop re-populating skin metadata from
the `.ini` upon completing an external edit, which is very doable but
seems worse than this. Dunno.
The fact that the stuff "just worked" previously due to one load-bearing
detach in a random location is really scary because a lot of this was
just not written the way it is supposed to be.
Closes https://github.com/ppy/osu/issues/33900. I think. Stable's sample
lookup logic is horrible.
The user in the issue claimed they were hearing `drum-hitfinish2`, but
they were really hearing `drum-hitfinish`, because they're the same
`.wav` file in the beatmap. Now the reason *why* they were hearind
`drum-hitfinish` is that the sample control point was specifying
something like:
23946,-200,4,2,4,40,0,0
To decipher, this is:
- default sample bank of soft
- custom sample bank of 4
Taking one of the objects affected, namely 00:23:946 (2) - that's a
slider with finish addition and drum addition bank on the slider head.
The slider head is thus attempting to play `soft-hitnormal4` and
`drum-hitfinish4`.
Neither `soft-hitnormal4` or `soft-hitnormal` exist in the beatmap, so
that plays fine via falling back to user skin's `soft-hitnormal`, but
`drum-hitfinish4` ends up falling back to `drum-hitfinish` which *does*
exist in the beatmap skin and thus plays wrongly from the beatmap skin
rather than the user skin.
I have no idea how to ensure this is correct across every beatmap and
skin out there so my approach is to just spray and pray (and rely on
issue reports I guess). I *think* this matches the stable logic which is
nestled within
https://github.com/peppy/osu-stable-reference/blob/a5e5fe6ef240505d13526cf32783cad261e9bd8b/osu!/Audio/AudioEngine.cs#L1136-L1230
but honestly if you put a gun to my head I couldn't be sure if it
matches completely in every possible circumstance or not.
We have other safeties which mean that this is not an issue during
gameplay, but in the new `TestSceneHUDOverlayRulesetLayouts` it became
apparent that allowing this to fallback (via `null` return) could lead
to weirdness.
This PR converts the leaderboard into a full-fledged skinnable component
that can be manipulated by users at will.
Notably, this finally allows https://github.com/ppy/osu/issues/20422 to
be fixed - although it's a very mixed bag, for several reasons:
- Because of taiko players' refusal to see reason^W^W^W^Winsistence on
keeping stable behaviours related to aspect ratio treatment, I have to
assume the worst case scenario, which means than on typical
resolutions like 16:9 (or even worse, 4:3), the leaderboard will
likely not occupy as much vertical space as it probably could.
- Additionally, there's the problem of where to put the spectator list.
I settled on putting it to the right of the leaderboard, but that's
kind of janky, because the leaderboard sometimes collapses and
sometimes fully hides, leading to a very awkward space left behind. If
we had the capability to anchor elements to other elements, maybe this
could be resolved, but for now, I'm not sure what to do. I think if
some users are bothered by it they can move it where they want it. Or
delete it.
It's all a bit weird so let's just disable it for now. For instance,
this is exposed as "text" font / colour but only affects the header.
Also, no other headers are cusotmisable in similar components.