ppy / osu

rhythm is just a *click* away!
https://osu.ppy.sh
MIT License
15.55k stars 2.31k forks source link

Custom hitsounds do not persist using "Edit externally" #29028

Open kstefanowicz opened 4 months ago

kstefanowicz commented 4 months ago

Type

Game behaviour

Bug description

The "Edit externally" feature does work for adding custom hitsounds, but they do not persist in the editor after save > exit > reload, and do not persist during gameplay.

Screenshots or videos

2024-07-23 12-48-56.webm

Version

2024.718.1-lazer

Logs

compressed-logs.zip

peppy commented 4 months ago

I'm guessing it works fine after a game restart or a beatmap switch?

bdach commented 4 months ago

Could be https://github.com/ppy/osu/issues/24287 too (the part of that that pertains to custom hitsounds specifically).

peppy commented 4 months ago

I tested this and it seemed to work correctly, without any reload.

Going to need more information here..

kstefanowicz commented 4 months ago

In my testing, custom hitsounds are saved to the beatmap after the "Edit External" workflow finishes, but existing matching hitsounds in the editor preview do not update automatically. After being Cut/Pasted back in. they do. After exiting > re-entering the editor, they revert back to default hitsounds in the editor preview until being placed again. In Test and Play, only default hitsounds play.

custom-hitsounds-not-persisting-2.webm

Interestingly, when Testing without leaving the editor, the custom hitsounds play in the editor preview before and after the test, but not during gameplay:

custom-hitsounds-test.webm

It is possible that I'm doing something wrong. I know in stable the Sampleset needs to be set to Custom 1 to have custom hitsounds, but I'm not sure how to access that functionality in Lazer: image

compressed-logs.zip

kstefanowicz commented 4 months ago

Update - after doing the following:

  1. Export the beatmap to .osz
  2. Import to Stable,
  3. Change the Sampleset in the Timing and Control Points window to Custom 1
  4. Export from Stable
  5. Import back to Lazer

The behavior stops happening and the hitsounds persist. Doing the inverse and going back to Default brings the behavior back.

changing-custom-soundbank-in-stable-fixes.webm

So it does seem to be related to that setting.

kstefanowicz commented 4 months ago

Some code observations as to why this is happening -

With a breakpoint set at ConvertHitObjectParser.cs line 552, the following call stack shows after making a change to a timing point: image

BeatmapEditorHandleChange.cs

Line 31

protected override void WriteCurrentStateToStream(MemoryStream stream)
{
    using (var sw = new StreamWriter(stream, Encoding.UTF8, 1024, true))
        new LegacyBeatmapEncoder(editorBeatmap, editorBeatmap.BeatmapSkin).Encode(sw);
}

As part of WriteCurrentStateToStream, the legacy encoder is called.

LegacyBeatmapEncoder.cs

Line 223

(As part of private void handleControlPoints(TextWriter writer) > LegacyControlPointProperties getLegacyControlPointProperties(ControlPointGroup group, bool updateSampleBank))

// Apply the control point to a hit sample to uncover legacy properties (e.g. suffix)
HitSampleInfo tempHitSample = samplePoint.ApplyTo(new ConvertHitObjectParser.LegacyHitSampleInfo(string.Empty));
int customSampleBank = toLegacyCustomSampleBank(tempHitSample);

Create a new HitSampleInfo via ConvertHitObjectParser.LegacyHitSampleInfo

Line 615

 private int toLegacyCustomSampleBank(HitSampleInfo? hitSampleInfo)
 {
     if (hitSampleInfo is ConvertHitObjectParser.LegacyHitSampleInfo legacy)
         return legacy.CustomSampleBank;

     return 0;
 }

If the hit sample info is legacy, return the legacy CustomSampleBank - else return 0

Line 234

(as part of LegacyControlPointProperties getLegacyControlPointProperties(ControlPointGroup group, bool updateSampleBank)

return new LegacyControlPointProperties
{
    SliderVelocity = difficultyPoint.SliderVelocity,
    TimingSignature = timingPoint.TimeSignature.Numerator,
    SampleBank = updateSampleBank ? (int)toLegacySampleBank(tempHitSample.Bank) : lastControlPointProperties.SampleBank,
    // Inherit the previous custom sample bank if the current custom sample bank is not set
    CustomSampleBank = customSampleBank >= 0 ? customSampleBank : lastControlPointProperties.CustomSampleBank,
    SampleVolume = tempHitSample.Volume,
    EffectFlags = effectFlags
};

ConvertHitObjectParser.cs

Line 552

public LegacyHitSampleInfo(string name, string? bank = null, int volume = 0, int customSampleBank = 0, bool isLayered = false)
    : base(name, bank ?? SampleControlPoint.DEFAULT_BANK, customSampleBank >= 2 ? customSampleBank.ToString() : null, volume)
{
    CustomSampleBank = customSampleBank;
    BankSpecified = !string.IsNullOrEmpty(bank);
    IsLayered = isLayered;
}

This is what actually sets the default sampleset value to 0 for timing points created in Lazer.

New timing points created in Lazer don't have a customSampleBank set, so the encoder sets them to the default of 0, and timing points are written to the .osu file with sampleIndex set to 0.

osu-timing-points-are-0.webm

LegacyBeatmapDecoder.cs

Line 516

(as part of private void handleTimingPoint(string line))

int customSampleBank = 0;
if (split.Length >= 5)
    customSampleBank = Parsing.ParseInt(split[4]);

Line 575

addControlPoint(time, new LegacySampleControlPoint
{
    SampleBank = stringSampleSet,
    SampleVolume = sampleVolume,
    CustomSampleBank = customSampleBank,
}, timingChange);

As the map reloads through the legacy decoder, the timing points are read with sampleIndex 0 and imported to the editor that way.