Merge pull request #9215 from bdach/hold-note-release-hitsounds

Fix incorrect sample choices for hold notes

osu.Game.Rulesets.Mania.Tests/ManiaBeatmapSampleConversionTest.cs

@@ -0,0 +1,76 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using NUnit.Framework;
+using osu.Framework.Utils;
+using osu.Game.Audio;
+using osu.Game.Rulesets.Mania.Objects;
+using osu.Game.Rulesets.Objects;
+using osu.Game.Tests.Beatmaps;
+
+namespace osu.Game.Rulesets.Mania.Tests
+{
+    [TestFixture]
+    public class ManiaBeatmapSampleConversionTest : BeatmapConversionTest<ConvertMapping<SampleConvertValue>, SampleConvertValue>
+    {
+        protected override string ResourceAssembly => "osu.Game.Rulesets.Mania";
+
+        [TestCase("convert-samples")]
+        [TestCase("mania-samples")]
+        public void Test(string name) => base.Test(name);
+
+        protected override IEnumerable<SampleConvertValue> CreateConvertValue(HitObject hitObject)
+        {
+            yield return new SampleConvertValue
+            {
+                StartTime = hitObject.StartTime,
+                EndTime = hitObject.GetEndTime(),
+                Column = ((ManiaHitObject)hitObject).Column,
+                NodeSamples = getSampleNames((hitObject as HoldNote)?.NodeSamples)
+            };
+        }
+
+        private IList<IList<string>> getSampleNames(List<IList<HitSampleInfo>> hitSampleInfo)
+            => hitSampleInfo?.Select(samples =>
+                                (IList<string>)samples.Select(sample => sample.LookupNames.First()).ToList())
+                            .ToList();
+
+        protected override Ruleset CreateRuleset() => new ManiaRuleset();
+    }
+
+    public struct SampleConvertValue : IEquatable<SampleConvertValue>
+    {
+        /// <summary>
+        /// A sane value to account for osu!stable using ints everywhere.
+        /// </summary>
+        private const float conversion_lenience = 2;
+
+        public double StartTime;
+        public double EndTime;
+        public int Column;
+        public IList<IList<string>> NodeSamples;
+
+        public bool Equals(SampleConvertValue other)
+            => Precision.AlmostEquals(StartTime, other.StartTime, conversion_lenience)
+               && Precision.AlmostEquals(EndTime, other.EndTime, conversion_lenience)
+               && samplesEqual(NodeSamples, other.NodeSamples);
+
+        private static bool samplesEqual(ICollection<IList<string>> firstSampleList, ICollection<IList<string>> secondSampleList)
+        {
+            if (firstSampleList == null && secondSampleList == null)
+                return true;
+
+            // both items can't be null now, so if any single one is, then they're not equal
+            if (firstSampleList == null || secondSampleList == null)
+                return false;
+
+            return firstSampleList.Count == secondSampleList.Count
+                   // cannot use .Zip() without the selector function as it doesn't compile in android test project
+                   && firstSampleList.Zip(secondSampleList, (first, second) => (first, second))
+                                     .All(samples => samples.first.SequenceEqual(samples.second));
+        }
+    }
+}

osu.Game.Rulesets.Mania/Beatmaps/ManiaBeatmapConverter.cs

@@ -6,6 +6,7 @@ using System;
 using System.Linq;
 using System.Collections.Generic;
 using osu.Framework.Utils;
+using osu.Game.Audio;
 using osu.Game.Beatmaps;
 using osu.Game.Rulesets.Objects;
 using osu.Game.Rulesets.Objects.Types;
@@ -239,7 +240,7 @@ namespace osu.Game.Rulesets.Mania.Beatmaps
                         Duration = endTimeData.Duration,
                         Column = column,
                         Samples = HitObject.Samples,
-                        NodeSamples = (HitObject as IHasRepeats)?.NodeSamples
+                        NodeSamples = (HitObject as IHasRepeats)?.NodeSamples ?? defaultNodeSamples
                     });
                 }
                 else if (HitObject is IHasXPosition)
@@ -254,6 +255,16 @@ namespace osu.Game.Rulesets.Mania.Beatmaps
 
                 return pattern;
             }
+
+            /// <remarks>
+            /// osu!mania-specific beatmaps in stable only play samples at the start of the hold note.
+            /// </remarks>
+            private List<IList<HitSampleInfo>> defaultNodeSamples
+                => new List<IList<HitSampleInfo>>
+                {
+                    HitObject.Samples,
+                    new List<HitSampleInfo>()
+                };
         }
     }
 }

osu.Game.Rulesets.Mania/Beatmaps/Patterns/Legacy/DistanceObjectPatternGenerator.cs

@@ -472,15 +472,23 @@ namespace osu.Game.Rulesets.Mania.Beatmaps.Patterns.Legacy
         /// </summary>
         /// <param name="time">The time to retrieve the sample info list from.</param>
         /// <returns></returns>
-        private IList<HitSampleInfo> sampleInfoListAt(double time)
+        private IList<HitSampleInfo> sampleInfoListAt(double time) => nodeSamplesAt(time)?.First() ?? HitObject.Samples;
+
+        /// <summary>
+        /// Retrieves the list of node samples that occur at time greater than or equal to <paramref name="time"/>.
+        /// </summary>
+        /// <param name="time">The time to retrieve node samples at.</param>
+        private List<IList<HitSampleInfo>> nodeSamplesAt(double time)
         {
             if (!(HitObject is IHasPathWithRepeats curveData))
-                return HitObject.Samples;
+                return null;
 
             double segmentTime = (EndTime - HitObject.StartTime) / spanCount;
 
             int index = (int)(segmentTime == 0 ? 0 : (time - HitObject.StartTime) / segmentTime);
-            return curveData.NodeSamples[index];
+
+            // avoid slicing the list & creating copies, if at all possible.
+            return index == 0 ? curveData.NodeSamples : curveData.NodeSamples.Skip(index).ToList();
         }
 
         /// <summary>
@@ -511,7 +519,7 @@ namespace osu.Game.Rulesets.Mania.Beatmaps.Patterns.Legacy
                     Duration = endTime - startTime,
                     Column = column,
                     Samples = HitObject.Samples,
-                    NodeSamples = (HitObject as IHasRepeats)?.NodeSamples
+                    NodeSamples = nodeSamplesAt(startTime)
                 };
             }
 

osu.Game.Rulesets.Mania/Resources/Testing/Beatmaps/convert-samples-expected-conversion.json

@@ -0,0 +1,30 @@
+{
+  "Mappings": [{
+    "StartTime": 1000.0,
+    "Objects": [{
+      "StartTime": 1000.0,
+      "EndTime": 2750.0,
+      "Column": 1,
+      "NodeSamples": [
+        ["normal-hitnormal"],
+        ["soft-hitnormal"],
+        ["drum-hitnormal"]
+      ]
+    }, {
+      "StartTime": 1875.0,
+      "EndTime": 2750.0,
+      "Column": 0,
+      "NodeSamples": [
+        ["soft-hitnormal"],
+        ["drum-hitnormal"]
+      ]
+    }]
+  }, {
+    "StartTime": 3750.0,
+    "Objects": [{
+      "StartTime": 3750.0,
+      "EndTime": 3750.0,
+      "Column": 3
+    }]
+  }]
+}

osu.Game.Rulesets.Mania/Resources/Testing/Beatmaps/convert-samples.osu

@@ -0,0 +1,16 @@
+osu file format v14
+
+[Difficulty]
+HPDrainRate:5
+CircleSize:5
+OverallDifficulty:5
+ApproachRate:5
+SliderMultiplier:1.4
+SliderTickRate:1
+
+[TimingPoints]
+0,500,4,1,0,100,1,0
+
+[HitObjects]
+88,99,1000,6,0,L|306:259,2,245,0|0|0,1:0|2:0|3:0,0:0:0:0:
+259,118,3750,1,0,0:0:0:0:

osu.Game.Rulesets.Mania/Resources/Testing/Beatmaps/mania-samples-expected-conversion.json

@@ -0,0 +1,25 @@
+{
+  "Mappings": [{
+    "StartTime": 500.0,
+    "Objects": [{
+      "StartTime": 500.0,
+      "EndTime": 1500.0,
+      "Column": 0,
+      "NodeSamples": [
+        ["normal-hitnormal"],
+        []
+      ]
+    }]
+  }, {
+    "StartTime": 2000.0,
+    "Objects": [{
+      "StartTime": 2000.0,
+      "EndTime": 3000.0,
+      "Column": 2,
+      "NodeSamples": [
+        ["drum-hitnormal"],
+        []
+      ]
+    }]
+  }]
+}

osu.Game.Rulesets.Mania/Resources/Testing/Beatmaps/mania-samples.osu

@@ -0,0 +1,19 @@
+osu file format v14
+
+[General]
+Mode: 3
+
+[Difficulty]
+HPDrainRate:5
+CircleSize:5
+OverallDifficulty:5
+ApproachRate:5
+SliderMultiplier:1.4
+SliderTickRate:1
+
+[TimingPoints]
+0,500,4,1,0,100,1,0
+
+[HitObjects]
+51,192,500,128,0,1500:1:0:0:0:
+256,192,2000,128,0,3000:3:0:0:0: