Merge pull request #10619 from peppy/sample-lookup-improvements

Change sample lookup structure to allow specifications outside of "Gameplay" prefix

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

@@ -6,20 +6,20 @@
       "EndTime": 2750.0,
       "Column": 1,
       "NodeSamples": [
-        ["normal-hitnormal"],
+        ["Gameplay/normal-hitnormal"],
-        ["soft-hitnormal"],
+        ["Gameplay/soft-hitnormal"],
-        ["drum-hitnormal"]
+        ["Gameplay/drum-hitnormal"]
       ],
-      "Samples": ["-hitnormal"]
+      "Samples": ["Gameplay/-hitnormal"]
     }, {
       "StartTime": 1875.0,
       "EndTime": 2750.0,
       "Column": 0,
       "NodeSamples": [
-        ["soft-hitnormal"],
+        ["Gameplay/soft-hitnormal"],
-        ["drum-hitnormal"]
+        ["Gameplay/drum-hitnormal"]
       ],
-      "Samples": ["-hitnormal"]
+      "Samples": ["Gameplay/-hitnormal"]
     }]
   }, {
     "StartTime": 3750.0,
@@ -27,7 +27,7 @@
       "StartTime": 3750.0,
       "EndTime": 3750.0,
       "Column": 3,
-      "Samples": ["normal-hitnormal"]
+      "Samples": ["Gameplay/normal-hitnormal"]
     }]
   }]
 }

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

@@ -6,10 +6,10 @@
       "EndTime": 1500.0,
       "Column": 0,
       "NodeSamples": [
-        ["normal-hitnormal"],
+        ["Gameplay/normal-hitnormal"],
         []
       ],
-      "Samples": ["normal-hitnormal"]
+      "Samples": ["Gameplay/normal-hitnormal"]
     }]
   }, {
     "StartTime": 2000.0,
@@ -18,10 +18,10 @@
       "EndTime": 3000.0,
       "Column": 2,
       "NodeSamples": [
-        ["drum-hitnormal"],
+        ["Gameplay/drum-hitnormal"],
         []
       ],
-      "Samples": ["drum-hitnormal"]
+      "Samples": ["Gameplay/drum-hitnormal"]
     }]
   }]
 }

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

@@ -5,17 +5,17 @@
       "StartTime": 8470.0,
       "EndTime": 8470.0,
       "Column": 0,
-      "Samples": ["normal-hitnormal", "normal-hitclap"]
+      "Samples": ["Gameplay/normal-hitnormal", "Gameplay/normal-hitclap"]
     }, {
       "StartTime": 8626.470587768974,
       "EndTime": 8626.470587768974,
       "Column": 1,
-      "Samples": ["normal-hitnormal"]
+      "Samples": ["Gameplay/normal-hitnormal"]
     }, {
       "StartTime": 8782.941175537948,
       "EndTime": 8782.941175537948,
       "Column": 2,
-      "Samples": ["normal-hitnormal", "normal-hitclap"]
+      "Samples": ["Gameplay/normal-hitnormal", "Gameplay/normal-hitclap"]
     }]
   }]
 }

osu.Game.Rulesets.Taiko/Skinning/TaikoLegacySkinTransformer.cs

@@ -162,7 +162,7 @@ namespace osu.Game.Rulesets.Taiko.Skinning
                 get
                 {
                     foreach (var name in source.LookupNames)
-                        yield return $"taiko-{name}";
+                        yield return name.Insert(name.LastIndexOf('/') + 1, "taiko-");
 
                     foreach (var name in source.LookupNames)
                         yield return name;

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

@@ -410,13 +410,13 @@ namespace osu.Game.Tests.Beatmaps.Formats
             {
                 var hitObjects = decoder.Decode(stream).HitObjects;
 
-                Assert.AreEqual("normal-hitnormal", getTestableSampleInfo(hitObjects[0]).LookupNames.First());
+                Assert.AreEqual("Gameplay/normal-hitnormal", getTestableSampleInfo(hitObjects[0]).LookupNames.First());
-                Assert.AreEqual("normal-hitnormal", getTestableSampleInfo(hitObjects[1]).LookupNames.First());
+                Assert.AreEqual("Gameplay/normal-hitnormal", getTestableSampleInfo(hitObjects[1]).LookupNames.First());
-                Assert.AreEqual("normal-hitnormal2", getTestableSampleInfo(hitObjects[2]).LookupNames.First());
+                Assert.AreEqual("Gameplay/normal-hitnormal2", getTestableSampleInfo(hitObjects[2]).LookupNames.First());
-                Assert.AreEqual("normal-hitnormal", getTestableSampleInfo(hitObjects[3]).LookupNames.First());
+                Assert.AreEqual("Gameplay/normal-hitnormal", getTestableSampleInfo(hitObjects[3]).LookupNames.First());
 
                 // The control point at the end time of the slider should be applied
-                Assert.AreEqual("soft-hitnormal8", getTestableSampleInfo(hitObjects[4]).LookupNames.First());
+                Assert.AreEqual("Gameplay/soft-hitnormal8", getTestableSampleInfo(hitObjects[4]).LookupNames.First());
             }
 
             static HitSampleInfo getTestableSampleInfo(HitObject hitObject) => hitObject.SampleControlPoint.ApplyTo(hitObject.Samples[0]);
@@ -432,9 +432,9 @@ namespace osu.Game.Tests.Beatmaps.Formats
             {
                 var hitObjects = decoder.Decode(stream).HitObjects;
 
-                Assert.AreEqual("normal-hitnormal", getTestableSampleInfo(hitObjects[0]).LookupNames.First());
+                Assert.AreEqual("Gameplay/normal-hitnormal", getTestableSampleInfo(hitObjects[0]).LookupNames.First());
-                Assert.AreEqual("normal-hitnormal2", getTestableSampleInfo(hitObjects[1]).LookupNames.First());
+                Assert.AreEqual("Gameplay/normal-hitnormal2", getTestableSampleInfo(hitObjects[1]).LookupNames.First());
-                Assert.AreEqual("normal-hitnormal3", getTestableSampleInfo(hitObjects[2]).LookupNames.First());
+                Assert.AreEqual("Gameplay/normal-hitnormal3", getTestableSampleInfo(hitObjects[2]).LookupNames.First());
             }
 
             static HitSampleInfo getTestableSampleInfo(HitObject hitObject) => hitObject.SampleControlPoint.ApplyTo(hitObject.Samples[0]);
@@ -452,7 +452,7 @@ namespace osu.Game.Tests.Beatmaps.Formats
 
                 Assert.AreEqual("hit_1.wav", getTestableSampleInfo(hitObjects[0]).LookupNames.First());
                 Assert.AreEqual("hit_2.wav", getTestableSampleInfo(hitObjects[1]).LookupNames.First());
-                Assert.AreEqual("normal-hitnormal2", getTestableSampleInfo(hitObjects[2]).LookupNames.First());
+                Assert.AreEqual("Gameplay/normal-hitnormal2", getTestableSampleInfo(hitObjects[2]).LookupNames.First());
                 Assert.AreEqual("hit_1.wav", getTestableSampleInfo(hitObjects[3]).LookupNames.First());
                 Assert.AreEqual(70, getTestableSampleInfo(hitObjects[3]).Volume);
             }

osu.Game.Tests/Visual/Gameplay/TestSceneSkinnableSound.cs

@@ -34,7 +34,7 @@ namespace osu.Game.Tests.Visual.Gameplay
                     skinSource = new TestSkinSourceContainer
                     {
                         RelativeSizeAxes = Axes.Both,
-                        Child = skinnableSound = new PausableSkinnableSound(new SampleInfo("normal-sliderslide"))
+                        Child = skinnableSound = new PausableSkinnableSound(new SampleInfo("Gameplay/normal-sliderslide"))
                     },
                 };
             });

osu.Game/Audio/HitSampleInfo.cs

@@ -50,9 +50,9 @@ namespace osu.Game.Audio
             get
             {
                 if (!string.IsNullOrEmpty(Suffix))
-                    yield return $"{Bank}-{Name}{Suffix}";
+                    yield return $"Gameplay/{Bank}-{Name}{Suffix}";
 
-                yield return $"{Bank}-{Name}";
+                yield return $"Gameplay/{Bank}-{Name}";
             }
         }
 

osu.Game/Audio/SampleInfo.cs

@@ -10,14 +10,14 @@ namespace osu.Game.Audio
     /// </summary>
     public class SampleInfo : ISampleInfo
     {
-        private readonly string sampleName;
+        private readonly string[] sampleNames;
 
-        public SampleInfo(string sampleName)
+        public SampleInfo(params string[] sampleNames)
         {
-            this.sampleName = sampleName;
+            this.sampleNames = sampleNames;
         }
 
-        public IEnumerable<string> LookupNames => new[] { sampleName };
+        public IEnumerable<string> LookupNames => sampleNames;
 
         public int Volume { get; } = 100;
     }

osu.Game/Rulesets/Mods/ModNightcore.cs

@@ -69,10 +69,10 @@ namespace osu.Game.Rulesets.Mods
             {
                 InternalChildren = new Drawable[]
                 {
-                    hatSample = new PausableSkinnableSound(new SampleInfo("nightcore-hat")),
+                    hatSample = new PausableSkinnableSound(new SampleInfo("Gameplay/nightcore-hat")),
-                    clapSample = new PausableSkinnableSound(new SampleInfo("nightcore-clap")),
+                    clapSample = new PausableSkinnableSound(new SampleInfo("Gameplay/nightcore-clap")),
-                    kickSample = new PausableSkinnableSound(new SampleInfo("nightcore-kick")),
+                    kickSample = new PausableSkinnableSound(new SampleInfo("Gameplay/nightcore-kick")),
-                    finishSample = new PausableSkinnableSound(new SampleInfo("nightcore-finish")),
+                    finishSample = new PausableSkinnableSound(new SampleInfo("Gameplay/nightcore-finish")),
                 };
             }
 

osu.Game/Screens/Play/ComboEffects.cs

@@ -28,7 +28,7 @@ namespace osu.Game.Screens.Play
         [BackgroundDependencyLoader]
         private void load(OsuConfigManager config)
         {
-            InternalChild = comboBreakSample = new SkinnableSound(new SampleInfo("combobreak"));
+            InternalChild = comboBreakSample = new SkinnableSound(new SampleInfo("Gameplay/combobreak"));
             alwaysPlay = config.GetBindable<bool>(OsuSetting.AlwaysPlayFirstComboBreak);
         }
 

osu.Game/Screens/Play/PauseOverlay.cs

@@ -33,7 +33,7 @@ namespace osu.Game.Screens.Play
             AddButton("Retry", colours.YellowDark, () => OnRetry?.Invoke());
             AddButton("Quit", new Color4(170, 27, 39, 255), () => OnQuit?.Invoke());
 
-            AddInternal(pauseLoop = new SkinnableSound(new SampleInfo("pause-loop"))
+            AddInternal(pauseLoop = new SkinnableSound(new SampleInfo("Gameplay/pause-loop"))
             {
                 Looping = true,
                 Volume = { Value = 0 }

osu.Game/Skinning/LegacySkin.cs

@@ -417,10 +417,14 @@ namespace osu.Game.Skinning
 
         public override SampleChannel GetSample(ISampleInfo sampleInfo)
         {
-            var lookupNames = sampleInfo.LookupNames;
+            IEnumerable<string> lookupNames;
 
             if (sampleInfo is HitSampleInfo hitSample)
                 lookupNames = getLegacyLookupNames(hitSample);
+            else
+            {
+                lookupNames = sampleInfo.LookupNames.SelectMany(getFallbackNames);
+            }
 
             foreach (var lookup in lookupNames)
             {
@@ -433,6 +437,27 @@ namespace osu.Game.Skinning
             return null;
         }
 
+        private IEnumerable<string> getLegacyLookupNames(HitSampleInfo hitSample)
+        {
+            var lookupNames = hitSample.LookupNames.SelectMany(getFallbackNames);
+
+            if (!UseCustomSampleBanks && !string.IsNullOrEmpty(hitSample.Suffix))
+            {
+                // for compatibility with stable, exclude the lookup names with the custom sample bank suffix, if they are not valid for use in this skin.
+                // using .EndsWith() is intentional as it ensures parity in all edge cases
+                // (see LegacyTaikoSampleInfo for an example of one - prioritising the taiko prefix should still apply, but the sample bank should not).
+                lookupNames = lookupNames.Where(name => !name.EndsWith(hitSample.Suffix, StringComparison.Ordinal));
+            }
+
+            foreach (var l in lookupNames)
+                yield return l;
+
+            // also for compatibility, try falling back to non-bank samples (so-called "universal" samples) as the last resort.
+            // going forward specifying banks shall always be required, even for elements that wouldn't require it on stable,
+            // which is why this is done locally here.
+            yield return hitSample.Name;
+        }
+
         private IEnumerable<string> getFallbackNames(string componentName)
         {
             // May be something like "Gameplay/osu/approachcircle" from lazer, or "Arrows/note1" from a user skin.
@@ -442,23 +467,5 @@ namespace osu.Game.Skinning
             string lastPiece = componentName.Split('/').Last();
             yield return componentName.StartsWith("Gameplay/taiko/", StringComparison.Ordinal) ? "taiko-" + lastPiece : lastPiece;
         }
-
-        private IEnumerable<string> getLegacyLookupNames(HitSampleInfo hitSample)
-        {
-            var lookupNames = hitSample.LookupNames;
-
-            if (!UseCustomSampleBanks && !string.IsNullOrEmpty(hitSample.Suffix))
-                // for compatibility with stable, exclude the lookup names with the custom sample bank suffix, if they are not valid for use in this skin.
-                // using .EndsWith() is intentional as it ensures parity in all edge cases
-                // (see LegacyTaikoSampleInfo for an example of one - prioritising the taiko prefix should still apply, but the sample bank should not).
-                lookupNames = hitSample.LookupNames.Where(name => !name.EndsWith(hitSample.Suffix, StringComparison.Ordinal));
-
-            // also for compatibility, try falling back to non-bank samples (so-called "universal" samples) as the last resort.
-            // going forward specifying banks shall always be required, even for elements that wouldn't require it on stable,
-            // which is why this is done locally here.
-            lookupNames = lookupNames.Append(hitSample.Name);
-
-            return lookupNames;
-        }
     }
 }

osu.Game/Skinning/SkinnableSound.cs

@@ -88,7 +88,7 @@ namespace osu.Game.Skinning
                 {
                     foreach (var lookup in s.LookupNames)
                     {
-                        if ((ch = samples.Get($"Gameplay/{lookup}")) != null)
+                        if ((ch = samples.Get(lookup)) != null)
                             break;
                     }
                 }