Hitsamples in keysounded maps seen to be wrongly assigned

[MaxKruse]

I saw that #24997 got closed and for good reason, that case was closed, so i decided to double-check some maps i liked and see if i can find anything else. Turns out, i can:

For comparison sake, im using the 2013 skin in lazer, as well as the default in "normal" osu! (the same skin) because as far as im concerned, every single map that was hitsounded to this day and doesnt fully replace all hitsamples was built on the sounds of the 2013 skin's included samples.

On the map https://osu.ppy.sh/beatmapsets/1633250#osu/3333745 the following section 02:38:227 (1) - has a very distinctly wrong sample assignment, a video to illustrate this: (apologies for the recording + reencoding framerate+quality, the audio however is unaffected)

Lazer:

2024-10-27.20-25-18.mp4

osu!:

2024-10-27.20-25-59.mp4

[peppy]

For clarity's sake, the beatmap you linked is not "keysounded", ie. not using per-object assignments. Hit sounds are being assigned using control points with custom banks.

Based on the videos, a point of concern is hitcircle:

237,158,158227,5,2,3:0:0:0:

affected by control point:

157937,-62.5,4,2,7,65,0,0

The hitobject does seem to be getting the correct definition:

which leads to lookup names for the sample:

succeeds on drum-hitwhistle7, which does exist in the beatmap:

So far everything looks fine. In fact, if you listen closely the sample is actually playing. It's just drowned up by drum-hitnormal also playing. Can be confirmed with this patch:

diff --git a/osu.Game/Skinning/PoolableSkinnableSample.cs b/osu.Game/Skinning/PoolableSkinnableSample.cs
index 76c2c4d7ec..9ee4b7c371 100644
--- a/osu.Game/Skinning/PoolableSkinnableSample.cs
+++ b/osu.Game/Skinning/PoolableSkinnableSample.cs
@@ -104,7 +104,8 @@ private void updateSample()
             if (sample == null)
                 return;
 
-            sampleContainer.Add(Sample = new DrawableSample(sample));
+            if (!sample.Name.Contains("hitnormal"))
+                sampleContainer.Add(Sample = new DrawableSample(sample));
 
             // Start playback internally for the new sample if the previous one was playing beforehand.
             if (wasPlaying && Looping)

The drum-hitnormal which is playing is included in the beatmap, but without a custom prefix:

In conclusion, I think the fallback logic we have doesn't match stable. If a custom suffix/sampleset number is specified and doesn't exist, the expectation seems to be "play nothing".

Checking stable's code:

Playback always prefers the cache, even when the lookup is cached as a failure -1

Cache / load code starts here; follow the chain

[peppy]

Should be resolved via #30599.