Osu: Hitsound discrepancy on legacy skins

[integerrr]

Describe the bug:

• Certain hitsounds are different on lazer compared to stable
• Reproduce:
  
  
  
  • Skin: chocomint's skin
  
  
  • Map: Panda Eyes & Teminite - Highscore [Game Over]
  
  
  • Pick any replay, hitsound should be different on both versions immediately
  
  

osu!lazer version: 2020.429.0

Logs:
database.log
network.log
performance-audio.log
performance-draw.log
performance-update.log
performance.log
runtime.log

[peppy]

could you please provide more detail? which sounds are you hearing differently? does it only happen with that combination of skin and beatmap? does turning off beatmap skins fix the issue?

[integerrr]

Sorry for being too lazy and not providing video examples
Stable: https://streamable.com/j0wqek
Lazer: https://streamable.com/90q4ox

I think mainly I am hearing different slider sounds and a different sample being played. When I looked at the skin folder I think lazer is playing normal-hitnormal2.wav while stable is playing normal-hitnormal.wav (or normal-hitnormal1.wav they sound identical I think)

• There might be other skins that exhibit this behaviour but I don't have that many skins to test
• Also this affects other beatmaps to various extent, this beatmap is the only one that I can remember off the top of my head
• Beatmap skins were never turned on

[bdach]

Looked at this today. lazer is actually playing soft-hitnormal2 (which is the same sound on the attached skin as normal-hitnormal2, so the confusion is understandable).

What I think is happening here (going off the very first hitobject) is that ConvertHitObjectParser is applying its standard layered normal-hitnormal sample, but then DrawableHitObject itself ends up applying the sample control point to it in loadSamples(). The timing point in question is

9110,-200,4,2,2,70,0,0

so naturally a normal-hitnormal ends up being a soft-hitnormal2.

I assume stable does not apply timing point sample adjustments to layered hitsounds, which I think makes this unaddressable until layered hit sounds are fully supported in lazer, but the fact that the conversion is being enforced inside of DrawableHitObject itself seems... problematic, to say the least...

[bdach]

I attempted to investigate further, but after messing around with stable some it doesn't actually seem to be related to layered hit sounds and I am really confused as to how the sample override works. I had a really hard time trying to convince stable to use indexed samples from the user skin and it looked like stable only applies the sample index when looking at the beatmap skin, but that might be completely wrong. I suppose I'll leave this one be for now.

[bdach]

Would it be possible to extract the relevant sample logic bits from stable into a gist or something, so that I can try finding the discrepancy here without having to resort to reversing stable logic?

[peppy]

I've sent you the required code.

[bdach]

I'm pretty sure I found the what the discrepancy is. The problem is that it is easy to explain but absolutely dreadful to resolve given our current structure of audio lookups. The long and short of it is, stable seems to apply the control point just the same, but then completely disregard the custom sample set index (custom sample bank in lazer vernacular) when looking up samples from the user skin.

I have two draft branches with different variations of fixes. They both suck in different ways.

• Branch A has an absolutely dreadful conditional. (It can't be a .Skip(1) either if we want complete accuracy, either, for taiko reasons.) Its upside is keeping this sort of thing relatively localised and self-contained.

• Branch B changes LookupNames to accept a skin, but as LookupNames was also invoked in skin-less contexts (tests, encoding beatmaps), the API is bad (what does doing a lookup with a null skin even mean?) It also opens the possibility of returning different samples for different skin implementations, which in theory sounds nice, but I think it is undesirable as it complicates things a lot where they otherwise don't need to be and spreads the badness around everywhere.

I am also not sure why user skins have indexed samples in them, if they're never being used as far as I can see.

(Posting this for discussion/feedback as to how to approach this.)

[peppy]

I think I definitely prefer method 1. Probably with a longer paragraph comment explaining why this is necessary.

[smoogipoo]

Yeah, method 1 looks fairly logical to me.