Merge pull request #2908 from smoogipoo/legacy-custom-banks

Add support for multiple sample banks

osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapDecoderTest.cs

@@ -11,6 +11,7 @@ using osu.Game.Audio;
 using osu.Game.Rulesets.Objects.Types;
 using osu.Game.Beatmaps.Formats;
 using osu.Game.Beatmaps.Timing;
+using osu.Game.Rulesets.Objects;
 using osu.Game.Skinning;
 
 namespace osu.Game.Tests.Beatmaps.Formats
@@ -211,5 +212,23 @@ namespace osu.Game.Tests.Beatmaps.Formats
                 Assert.IsTrue(hitObjects[1].Samples.Any(s => s.Name == SampleInfo.HIT_CLAP));
             }
         }
+
+        [Test]
+        public void TestDecodeCustomSamples()
+        {
+            var decoder = new LegacyBeatmapDecoder { ApplyOffsets = false };
+            using (var resStream = Resource.OpenResource("custom-samples.osu"))
+            using (var stream = new StreamReader(resStream))
+            {
+                var hitObjects = decoder.Decode(stream).HitObjects;
+
+                Assert.AreEqual("hitnormal", getTestableSampleInfo(hitObjects[0]).Name);
+                Assert.AreEqual("hitnormal", getTestableSampleInfo(hitObjects[1]).Name);
+                Assert.AreEqual("hitnormal2", getTestableSampleInfo(hitObjects[2]).Name);
+                Assert.AreEqual("hitnormal", getTestableSampleInfo(hitObjects[3]).Name);
+            }
+
+            SampleInfo getTestableSampleInfo(HitObject hitObject) => hitObject.SampleControlPoint.ApplyTo(new SampleInfo { Name = "hitnormal" });
+        }
     }
 }

osu.Game.Tests/Beatmaps/Formats/OsuJsonDecoderTest.cs

@@ -118,7 +118,11 @@ namespace osu.Game.Tests.Beatmaps.Formats
         public void TestParity(string beatmap)
         {
             var legacy = decode(beatmap, out Beatmap json);
-            json.WithDeepEqual(legacy).IgnoreProperty(r => r.DeclaringType == typeof(HitWindows)).Assert();
+            json.WithDeepEqual(legacy)
+                .IgnoreProperty(r => r.DeclaringType == typeof(HitWindows)
+                                     // Todo: CustomSampleBank shouldn't exist going forward, we need a conversion mechanism
+                                     || r.Name == nameof(LegacyDecoder<Beatmap>.LegacySampleControlPoint.CustomSampleBank))
+                .Assert();
         }
 
         /// <summary>

osu.Game.Tests/Resources/custom-samples.osu

@@ -0,0 +1,16 @@
+osu file format v14
+
+[General]
+SampleSet: Normal
+
+[TimingPoints]
+2170,468.75,4,1,0,40,1,0
+2638,-100,4,1,1,40,0,0
+3107,-100,4,1,2,40,0,0
+3576,-100,4,1,0,40,0,0
+
+[HitObjects]
+255,193,2170,1,0,0:0:0:0:
+256,191,2638,5,0,0:0:0:0:
+255,193,3107,1,0,0:0:0:0:
+256,191,3576,1,0,0:0:0:0:

osu.Game/Audio/SampleInfo.cs

@@ -32,5 +32,7 @@ namespace osu.Game.Audio
         /// The sample volume.
         /// </summary>
         public int Volume;
+
+        public SampleInfo Clone() => (SampleInfo)MemberwiseClone();
     }
 }

osu.Game/Beatmaps/ControlPoints/ControlPoint.cs

@@ -14,6 +14,15 @@ namespace osu.Game.Beatmaps.ControlPoints
 
         public int CompareTo(ControlPoint other) => Time.CompareTo(other.Time);
 
-        public bool Equals(ControlPoint other) => Time.Equals(other?.Time);
+        /// <summary>
+        /// Whether this <see cref="ControlPoint"/> provides the same parametric changes as another <see cref="ControlPoint"/>.
+        /// Basically an equality check without considering the <see cref="Time"/>.
+        /// </summary>
+        /// <param name="other">The <see cref="ControlPoint"/> to compare to.</param>
+        /// <returns>Whether this <see cref="ControlPoint"/> is equivalent to <paramref name="other"/>.</returns>
+        public virtual bool EquivalentTo(ControlPoint other) => true;
+
+        public bool Equals(ControlPoint other)
+            => EquivalentTo(other) && Time.Equals(other?.Time);
     }
 }

osu.Game/Beatmaps/ControlPoints/DifficultyControlPoint.cs

@@ -17,5 +17,10 @@ namespace osu.Game.Beatmaps.ControlPoints
         }
 
         private double speedMultiplier = 1;
+
+        public override bool EquivalentTo(ControlPoint other)
+            => base.EquivalentTo(other)
+               && other is DifficultyControlPoint difficulty
+               && SpeedMultiplier.Equals(difficulty.SpeedMultiplier);
     }
 }

osu.Game/Beatmaps/ControlPoints/EffectControlPoint.cs

@@ -14,5 +14,11 @@ namespace osu.Game.Beatmaps.ControlPoints
         /// Whether the first bar line of this control point is ignored.
         /// </summary>
         public bool OmitFirstBarLine;
+
+        public override bool EquivalentTo(ControlPoint other)
+            => base.EquivalentTo(other)
+               && other is EffectControlPoint effect
+               && KiaiMode.Equals(effect.KiaiMode)
+               && OmitFirstBarLine.Equals(effect.OmitFirstBarLine);
     }
 }

osu.Game/Beatmaps/ControlPoints/SampleControlPoint.cs

@@ -30,5 +30,25 @@ namespace osu.Game.Beatmaps.ControlPoints
             Name = sampleName,
             Volume = SampleVolume,
         };
+
+        /// <summary>
+        /// Applies <see cref="SampleBank"/> and <see cref="SampleVolume"/> to a <see cref="SampleInfo"/> if necessary, returning the modified <see cref="SampleInfo"/>.
+        /// </summary>
+        /// <param name="sampleInfo">The <see cref="SampleInfo"/>. This will not be modified.</param>
+        /// <returns>The modified <see cref="SampleInfo"/>. This does not share a reference with <paramref name="sampleInfo"/>.</returns>
+        public virtual SampleInfo ApplyTo(SampleInfo sampleInfo)
+        {
+            var newSampleInfo = sampleInfo.Clone();
+            newSampleInfo.Bank = sampleInfo.Bank ?? SampleBank;
+            newSampleInfo.Name = sampleInfo.Name;
+            newSampleInfo.Volume = sampleInfo.Volume > 0 ? sampleInfo.Volume : SampleVolume;
+            return newSampleInfo;
+        }
+
+        public override bool EquivalentTo(ControlPoint other)
+            => base.EquivalentTo(other)
+               && other is SampleControlPoint sample
+               && SampleBank.Equals(sample.SampleBank)
+               && SampleVolume.Equals(sample.SampleVolume);
     }
 }

osu.Game/Beatmaps/ControlPoints/TimingControlPoint.cs

@@ -23,5 +23,11 @@ namespace osu.Game.Beatmaps.ControlPoints
         }
 
         private double beatLength = 1000;
+
+        public override bool EquivalentTo(ControlPoint other)
+            => base.EquivalentTo(other)
+               && other is TimingControlPoint timing
+               && TimeSignature.Equals(timing.TimeSignature)
+               && BeatLength.Equals(timing.BeatLength);
     }
 }

osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs

@@ -289,9 +289,9 @@ namespace osu.Game.Beatmaps.Formats
                 if (split.Length >= 4)
                     sampleSet = (LegacySampleBank)int.Parse(split[3]);
 
-                //SampleBank sampleBank = SampleBank.Default;
-                //if (split.Length >= 5)
-                //    sampleBank = (SampleBank)int.Parse(split[4]);
+                int customSampleBank = 0;
+                if (split.Length >= 5)
+                    customSampleBank = int.Parse(split[4]);
 
                 int sampleVolume = defaultSampleVolume;
                 if (split.Length >= 6)
@@ -314,13 +314,9 @@ namespace osu.Game.Beatmaps.Formats
                 if (stringSampleSet == @"none")
                     stringSampleSet = @"normal";
 
-                DifficultyControlPoint difficultyPoint = beatmap.ControlPointInfo.DifficultyPointAt(time);
-                SampleControlPoint samplePoint = beatmap.ControlPointInfo.SamplePointAt(time);
-                EffectControlPoint effectPoint = beatmap.ControlPointInfo.EffectPointAt(time);
-
                 if (timingChange)
                 {
-                    beatmap.ControlPointInfo.TimingPoints.Add(new TimingControlPoint
+                    handleTimingControlPoint(new TimingControlPoint
                     {
                         Time = time,
                         BeatLength = beatLength,
@@ -328,41 +324,68 @@ namespace osu.Game.Beatmaps.Formats
                     });
                 }
 
-                if (speedMultiplier != difficultyPoint.SpeedMultiplier)
-                {
-                    beatmap.ControlPointInfo.DifficultyPoints.RemoveAll(x => x.Time == time);
-                    beatmap.ControlPointInfo.DifficultyPoints.Add(new DifficultyControlPoint
+                handleDifficultyControlPoint(new DifficultyControlPoint
                 {
                     Time = time,
                     SpeedMultiplier = speedMultiplier
                 });
-                }
 
-                if (stringSampleSet != samplePoint.SampleBank || sampleVolume != samplePoint.SampleVolume)
-                {
-                    beatmap.ControlPointInfo.SamplePoints.Add(new SampleControlPoint
-                    {
-                        Time = time,
-                        SampleBank = stringSampleSet,
-                        SampleVolume = sampleVolume
-                    });
-                }
-
-                if (kiaiMode != effectPoint.KiaiMode || omitFirstBarSignature != effectPoint.OmitFirstBarLine)
-                {
-                    beatmap.ControlPointInfo.EffectPoints.Add(new EffectControlPoint
+                handleEffectControlPoint(new EffectControlPoint
                 {
                     Time = time,
                     KiaiMode = kiaiMode,
                     OmitFirstBarLine = omitFirstBarSignature
                 });
-                }
+
+                handleSampleControlPoint(new LegacySampleControlPoint
+                {
+                    Time = time,
+                    SampleBank = stringSampleSet,
+                    SampleVolume = sampleVolume,
+                    CustomSampleBank = customSampleBank
+                });
             }
             catch (FormatException e)
             {
             }
         }
 
+        private void handleTimingControlPoint(TimingControlPoint newPoint)
+        {
+            beatmap.ControlPointInfo.TimingPoints.Add(newPoint);
+        }
+
+        private void handleDifficultyControlPoint(DifficultyControlPoint newPoint)
+        {
+            var existing = beatmap.ControlPointInfo.DifficultyPointAt(newPoint.Time);
+
+            if (newPoint.EquivalentTo(existing))
+                return;
+
+            beatmap.ControlPointInfo.DifficultyPoints.RemoveAll(x => x.Time == newPoint.Time);
+            beatmap.ControlPointInfo.DifficultyPoints.Add(newPoint);
+        }
+
+        private void handleEffectControlPoint(EffectControlPoint newPoint)
+        {
+            var existing = beatmap.ControlPointInfo.EffectPointAt(newPoint.Time);
+
+            if (newPoint.EquivalentTo(existing))
+                return;
+
+            beatmap.ControlPointInfo.EffectPoints.Add(newPoint);
+        }
+
+        private void handleSampleControlPoint(SampleControlPoint newPoint)
+        {
+            var existing = beatmap.ControlPointInfo.SamplePointAt(newPoint.Time);
+
+            if (newPoint.EquivalentTo(existing))
+                return;
+
+            beatmap.ControlPointInfo.SamplePoints.Add(newPoint);
+        }
+
         private void handleHitObject(string line)
         {
             // If the ruleset wasn't specified, assume the osu!standard ruleset.

osu.Game/Beatmaps/Formats/LegacyDecoder.cs

@@ -5,6 +5,8 @@ using System;
 using System.Collections.Generic;
 using System.IO;
 using osu.Framework.Logging;
+using osu.Game.Audio;
+using osu.Game.Beatmaps.ControlPoints;
 using OpenTK.Graphics;
 
 namespace osu.Game.Beatmaps.Formats
@@ -167,5 +169,25 @@ namespace osu.Game.Beatmaps.Formats
             Pass = 2,
             Foreground = 3
         }
+
+        internal class LegacySampleControlPoint : SampleControlPoint
+        {
+            public int CustomSampleBank;
+
+            public override SampleInfo ApplyTo(SampleInfo sampleInfo)
+            {
+                var baseInfo = base.ApplyTo(sampleInfo);
+
+                if (CustomSampleBank > 1)
+                    baseInfo.Name += CustomSampleBank;
+
+                return baseInfo;
+            }
+
+            public override bool EquivalentTo(ControlPoint other)
+                => base.EquivalentTo(other)
+                   && other is LegacySampleControlPoint legacy
+                   && CustomSampleBank == legacy.CustomSampleBank;
+        }
     }
 }

osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs

@@ -89,13 +89,12 @@ namespace osu.Game.Rulesets.Objects.Drawables
                 if (HitObject.SampleControlPoint == null)
                     throw new ArgumentNullException(nameof(HitObject.SampleControlPoint), $"{nameof(HitObject)}s must always have an attached {nameof(HitObject.SampleControlPoint)}."
                                                                                           + $" This is an indication that {nameof(HitObject.ApplyDefaults)} has not been invoked on {this}.");
-                AddInternal(Samples = new SkinnableSound(samples.Select(s => new SampleInfo
-                {
-                    Bank = s.Bank ?? HitObject.SampleControlPoint.SampleBank,
-                    Name = s.Name,
-                    Volume = s.Volume > 0 ? s.Volume : HitObject.SampleControlPoint.SampleVolume,
-                    Namespace = SampleNamespace
-                }).ToArray()));
+
+                samples = samples.Select(s => HitObject.SampleControlPoint.ApplyTo(s)).ToArray();
+                foreach (var s in samples)
+                    s.Namespace = SampleNamespace;
+
+                AddInternal(Samples = new SkinnableSound(samples));
             }
         }