Slider hitsounds do not save correctly

[plaguemask]

Type

Game behaviour

Bug description

Saving a beatmap in the osu! editor simply does not work when it comes to sliders' hitsounds, and also breaks after-the-fact hitsounding functionality. This problem lies only with the audio that plays; what's displayed in the UI is always correct.

Video below shows:

• After placing a slider, save and exit the editor, then re-open the editor. You will find:
  • The slider's custom hitsounds become default normal hitsounds (This also affects playing the map after saving)
  • Changing the slider's hitsound volume only applies after saving/exiting/opening again, and only applies to slider end
  • Changing the slider's hitsound bank no longer works
• Video ends with placing a new slider with the same hitsound data, as shown in the UI. Because the second slider has not been saved and reloaded, it plays correctly.

Screenshots or videos

osu-bug.mp4

Version

2023.717.0-lazer

Logs

probably not very useful but here:
updater.log
database.log
input.log
network.log
performance.log
runtime.log

[bdach]

The slider's custom hitsounds become default normal hitsounds (This also affects playing the map after saving)

How did you get those custom hitsounds in there in the first place?

Changing the slider's hitsound volume only applies after saving/exiting/opening again, and only applies to slider end

Tracked as #22187

[plaguemask]

How did you get those custom hitsounds in there in the first place?

For the video, I had made a new difficulty of cYsmix - triangles. It already has custom hitsounds overriding all the banks (screenshot of .osz contents).

Note that, even absent of custom hitsound files, any non-normal bank also reverts to playing default normal hitsounds.

[OliBomby]

The slider's custom hitsounds become default normal hitsounds (This also affects playing the map after saving)

I found the reason why the sound of the sliderhead and slidertail changes when you save and reload the beatmap.

In LegacyBeatmapSkin.GetSample there is a check that excludes LegacyHitSampleInfo with custom sample bank 0 from getting a custom sample. When you place a new hit object, it gets regular HitSampleInfo which are eligible for custom samples from the beatmap. When you save and reload, all the previously regular HitSampleInfo are now LegacyHitSampleInfo with custom sample bank 0, so they get excluded from getting a custom sample.

Perhaps we should let LegacyBeatmapEncoder encode HitSampleInfo with custom sample bank 1, so custom samples are enabled even after a save and reload?

[peppy]

I'm very confused. I think you're looking in the right place, though.

Logically, setting the custom sample bank to 1 would make them play different sounds, ie. normal-hitnormal1.wav. I don't think that's what we want. But, we do want these cases to still take the base.GetSample path because it handles the fallback logic to get the correct local filenames.

I honestly have no idea how this customSampleBank stuff works, and am struggling to find any documentation.

But there's no shortage of cryptic messages where it's being used to do something:

osu/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs

Lines 562 to 563 in df5b389

	// Force CSS=1 to make sure that the LegacyBeatmapSkin does not fall back to the user skin.
	// Note that this does not change the lookup names, as they are overridden locally.

Basically, I think we need to understand why there's this concept that having a CustomSampleBank of 0 means we don't want to use local samples. Because I don't think that makes sense...

[OliBomby]

Basically, I think we need to understand why there's this concept that having a CustomSampleBank of 0 means we don't want to use local samples. Because I don't think that makes sense...

I think that concept is there just for compatibility reasons. In osu! stable the custom sample bank 0 is used for if you dont want to use custom samples. Custom sample bank 1 is the first sample bank which allows custom samples but it is unique in that it doesnt require a 1 suffix in the lookup names, ie. normal-hitnormal.wav would be used by custom sample bank 1. Subsequent custom sample banks do need this number suffix.