As outlined in the issue thread, the tail volume wasn't saving because it wasn't actually attached to a hitobject properly, and as such the LegacyBeatmapEncoder logic, which is based on hitobjects, did not pick them up on save.
To fix that, switch to using NodeSamples for objects that are IHasRepeats. That has one added complication in that having it work properly requires changes to the decode side too. That is because the intent is to allow the user to change the sample settings for each node (which are specified via NodeSamples), as well as "the rest of the object", which generally means ticks or auxiliary samples like sliderslide (which are specified by Samples).
However, up until now, Samples always queried the control point which was active at the end time of the slider. This obviously can't work anymore when converting NodeSamples to legacy control points, because the last node's sample is also at the end time of the slider. To bypass that, add extra sample points after each node (just out of reach of the 5ms leniency), which are supposed to control volume of ticks and/or slides, and read those instead on decode into Samples when parsing an IHasRepeats. See following crude visual explanation:
Upon testing, this sort of has the intended effect in stable, with the exception of sliderslide, which seems to either respect or not respect the relevant volume spec dependent on... not sure what, and not sure I want to be debugging that. It might be frame alignment, or it might be the phase of the moon.
cc @OliBomby (not sure if you want to voice your opinion on this)
RFC. Closes https://github.com/ppy/osu/issues/28587.
As outlined in the issue thread, the tail volume wasn't saving because it wasn't actually attached to a hitobject properly, and as such the
LegacyBeatmapEncoderlogic, which is based on hitobjects, did not pick them up on save.To fix that, switch to using
NodeSamplesfor objects that areIHasRepeats. That has one added complication in that having it work properly requires changes to the decode side too. That is because the intent is to allow the user to change the sample settings for each node (which are specified viaNodeSamples), as well as "the rest of the object", which generally means ticks or auxiliary samples likesliderslide(which are specified bySamples).However, up until now,
Samplesalways queried the control point which was active at the end time of the slider. This obviously can't work anymore when convertingNodeSamplesto legacy control points, because the last node's sample is also at the end time of the slider. To bypass that, add extra sample points after each node (just out of reach of the 5ms leniency), which are supposed to control volume of ticks and/or slides, and read those instead on decode intoSampleswhen parsing anIHasRepeats. See following crude visual explanation:Upon testing, this sort of has the intended effect in stable, with the exception of
sliderslide, which seems to either respect or not respect the relevant volume spec dependent on... not sure what, and not sure I want to be debugging that. It might be frame alignment, or it might be the phase of the moon.cc @OliBomby (not sure if you want to voice your opinion on this)