mirror of
https://github.com/ppy/osu.git
synced 2026-06-03 22:54:23 +08:00
Fix beatmap skin sample lookups falling back to non-custom sample banks if the custom sample bank sample was not found
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.
This commit is contained in:
@@ -594,12 +594,17 @@ namespace osu.Game.Skinning
|
||||
{
|
||||
var lookupNames = hitSample.LookupNames.SelectMany(getFallbackSampleNames);
|
||||
|
||||
if (!UseCustomSampleBanks && !string.IsNullOrEmpty(hitSample.Suffix))
|
||||
if (!string.IsNullOrEmpty(hitSample.Suffix))
|
||||
{
|
||||
// for compatibility with stable, exclude the lookup names with the custom sample bank suffix, if they are not valid for use in this skin.
|
||||
// for compatibility with stable:
|
||||
// - if the skin can use custom sample banks, it MUST use the custom sample bank suffix. it is not allowed to fall back to a non-custom sound.
|
||||
// - if the skin cannot use custom sample banks, it MUST NOT use the custom sample bank suffix.
|
||||
// using .EndsWith() is intentional as it ensures parity in all edge cases
|
||||
// (see LegacyTaikoSampleInfo for an example of one - prioritising the taiko prefix should still apply, but the sample bank should not).
|
||||
lookupNames = lookupNames.Where(name => !name.EndsWith(hitSample.Suffix, StringComparison.Ordinal));
|
||||
// (see LegacyTaikoSampleInfo for an example of one - prioritising the taiko prefix should still apply).
|
||||
if (UseCustomSampleBanks)
|
||||
lookupNames = lookupNames.Where(name => name.EndsWith(hitSample.Suffix, StringComparison.Ordinal));
|
||||
else
|
||||
lookupNames = lookupNames.Where(name => !name.EndsWith(hitSample.Suffix, StringComparison.Ordinal));
|
||||
}
|
||||
|
||||
foreach (string l in lookupNames)
|
||||
|
||||
Reference in New Issue
Block a user