Fix sample banks not transferring when adjusting via editor #23564
Conversation
| @@ -44,7 +45,7 @@ public class HitSampleInfo : ISampleInfo, IEquatable<HitSampleInfo> | |||
| /// </summary> | |||
| public int Volume { get; } | |||
|
|
|||
| public HitSampleInfo(string name, string? bank = null, string? suffix = null, int volume = 0) | |||
| public HitSampleInfo(string name, string bank = SampleControlPoint.DEFAULT_BANK, string? suffix = null, int volume = 100) | |||
There was a problem hiding this comment.
Now that the volume is 100 by default you also have to make sure that the legacy decoder properly overrides hit object volumes with volume from the control points. If the volume is 100 then it will not take the volume from the control points.
There was a problem hiding this comment.
Where's the code for this? Sounds flaky as hell, and untested.
There was a problem hiding this comment.
Okay turns out my concerns were misplaced. The ConvertHitObjectParser which creates the LegacyHitSampleInfo always assigns the volume of the SampleBankInfo whose volume is still 0 by default, so it works out just fine. For a moment I thought there were occurances of LegacyHitSampleInfo being created while decoding without a volume argument given.
There was a problem hiding this comment.
And even if that wasn't the case, ConvertHitObjectParser only constructs LegacyHitSampleInfos, and LegacyHitSampleInfo ctor still has 0 volume as a default, which makes the change of the default value in the base class largely irrelevant.
| { | ||
| return legacy.With( | ||
| newCustomSampleBank: legacy.CustomSampleBank > 0 ? legacy.CustomSampleBank : CustomSampleBank, | ||
| newVolume: hitSampleInfo.Volume > 0 ? hitSampleInfo.Volume : SampleVolume, |
There was a problem hiding this comment.
Probably good to add a VolumeSpecified property as well and use that for the sample control point override instead of checking if its greater than zero.
There was a problem hiding this comment.
Hmm, given what you found above I don't think this is necessary as it's a strictly legacy flow.
This brings across a fix from 88d840a, but also aims to avoid anything like this in the future by making
HitSampleInfo.Banknon-nullable.In order to allow for the legacy flow, a
boolhas been added (to track the case where a hit object doesn't specify a sample inline, but needs to get one from the most relevant control point at the end of the decode process).