Merge pull request #9340 from bdach/layered-hit-sounds

Add support for layered hit sounds skin option

osu.Game.Rulesets.Mania.Tests/Resources/SampleLookups/mania-hitobject-beatmap-custom-sample-bank.osu

@@ -0,0 +1,10 @@
+osu file format v14
+
+[General]
+Mode: 3
+
+[TimingPoints]
+0,300,4,0,2,100,1,0
+
+[HitObjects]
+444,320,1000,5,2,0:0:0:0:

osu.Game.Rulesets.Mania.Tests/Resources/SampleLookups/mania-hitobject-beatmap-normal-sample-bank.osu

@@ -0,0 +1,10 @@
+osu file format v14
+
+[General]
+Mode: 3
+
+[TimingPoints]
+0,300,4,0,2,100,1,0
+
+[HitObjects]
+444,320,1000,5,1,0:0:0:0:

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

@@ -0,0 +1,49 @@
+// 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.Reflection;
+using NUnit.Framework;
+using osu.Framework.IO.Stores;
+using osu.Game.Tests.Beatmaps;
+
+namespace osu.Game.Rulesets.Mania.Tests
+{
+    public class TestSceneManiaHitObjectSamples : HitObjectSampleTest
+    {
+        protected override Ruleset CreatePlayerRuleset() => new ManiaRuleset();
+        protected override IResourceStore<byte[]> Resources => new DllResourceStore(Assembly.GetAssembly(typeof(TestSceneManiaHitObjectSamples)));
+
+        /// <summary>
+        /// Tests that when a normal sample bank is used, the normal hitsound will be looked up.
+        /// </summary>
+        [Test]
+        public void TestManiaHitObjectNormalSampleBank()
+        {
+            const string expected_sample = "normal-hitnormal2";
+
+            SetupSkins(expected_sample, expected_sample);
+
+            CreateTestWithBeatmap("mania-hitobject-beatmap-normal-sample-bank.osu");
+
+            AssertBeatmapLookup(expected_sample);
+        }
+
+        /// <summary>
+        /// Tests that when a custom sample bank is used, layered hitsounds are not played
+        /// (only the sample from the custom bank is looked up).
+        /// </summary>
+        [Test]
+        public void TestManiaHitObjectCustomSampleBank()
+        {
+            const string expected_sample = "normal-hitwhistle2";
+            const string unwanted_sample = "normal-hitnormal2";
+
+            SetupSkins(expected_sample, unwanted_sample);
+
+            CreateTestWithBeatmap("mania-hitobject-beatmap-custom-sample-bank.osu");
+
+            AssertBeatmapLookup(expected_sample);
+            AssertNoLookup(unwanted_sample);
+        }
+    }
+}

osu.Game.Rulesets.Mania/Skinning/ManiaLegacySkinTransformer.cs

@@ -9,6 +9,9 @@ using osu.Game.Beatmaps;
 using osu.Game.Rulesets.Mania.Beatmaps;
 using osu.Game.Skinning;
 using System.Collections.Generic;
+using osu.Framework.Audio.Sample;
+using osu.Game.Audio;
+using osu.Game.Rulesets.Objects.Legacy;
 
 namespace osu.Game.Rulesets.Mania.Skinning
 {
@@ -129,6 +132,15 @@ namespace osu.Game.Rulesets.Mania.Skinning
             return this.GetAnimation(filename, true, true);
         }
 
+        public override SampleChannel GetSample(ISampleInfo sampleInfo)
+        {
+            // layered hit sounds never play in mania
+            if (sampleInfo is ConvertHitObjectParser.LegacyHitSampleInfo legacySample && legacySample.IsLayered)
+                return new SampleChannelVirtual();
+
+            return Source.GetSample(sampleInfo);
+        }
+
         public override IBindable<TValue> GetConfig<TLookup, TValue>(TLookup lookup)
         {
             if (lookup is ManiaSkinConfigurationLookup maniaLookup)

osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs

@@ -6,6 +6,7 @@ using osu.Framework.IO.Stores;
 using osu.Framework.Testing;
 using osu.Game.Rulesets;
 using osu.Game.Rulesets.Osu;
+using osu.Game.Skinning;
 using osu.Game.Tests.Beatmaps;
 using osu.Game.Tests.Resources;
 
@@ -167,5 +168,64 @@ namespace osu.Game.Tests.Gameplay
 
             AssertBeatmapLookup(expected_sample);
         }
+
+        /// <summary>
+        /// Tests that when a custom sample bank is used, both the normal and additional sounds will be looked up.
+        /// </summary>
+        [Test]
+        public void TestHitObjectCustomSampleBank()
+        {
+            string[] expectedSamples =
+            {
+                "normal-hitnormal2",
+                "normal-hitwhistle2"
+            };
+
+            SetupSkins(expectedSamples[0], expectedSamples[1]);
+
+            CreateTestWithBeatmap("hitobject-beatmap-custom-sample-bank.osu");
+
+            AssertBeatmapLookup(expectedSamples[0]);
+            AssertUserLookup(expectedSamples[1]);
+        }
+
+        /// <summary>
+        /// Tests that when a custom sample bank is used, but <see cref="GlobalSkinConfiguration.LayeredHitSounds"/> is disabled,
+        /// only the additional sound will be looked up.
+        /// </summary>
+        [Test]
+        public void TestHitObjectCustomSampleBankWithoutLayered()
+        {
+            const string expected_sample = "normal-hitwhistle2";
+            const string unwanted_sample = "normal-hitnormal2";
+
+            SetupSkins(expected_sample, unwanted_sample);
+            disableLayeredHitSounds();
+
+            CreateTestWithBeatmap("hitobject-beatmap-custom-sample-bank.osu");
+
+            AssertBeatmapLookup(expected_sample);
+            AssertNoLookup(unwanted_sample);
+        }
+
+        /// <summary>
+        /// Tests that when a normal sample bank is used and <see cref="GlobalSkinConfiguration.LayeredHitSounds"/> is disabled,
+        /// the normal sound will be looked up anyway.
+        /// </summary>
+        [Test]
+        public void TestHitObjectNormalSampleBankWithoutLayered()
+        {
+            const string expected_sample = "normal-hitnormal";
+
+            SetupSkins(expected_sample, expected_sample);
+            disableLayeredHitSounds();
+
+            CreateTestWithBeatmap("hitobject-beatmap-sample.osu");
+
+            AssertBeatmapLookup(expected_sample);
+        }
+
+        private void disableLayeredHitSounds()
+            => AddStep("set LayeredHitSounds to false", () => Skin.Configuration.ConfigDictionary[GlobalSkinConfiguration.LayeredHitSounds.ToString()] = "0");
     }
 }

osu.Game.Tests/Resources/SampleLookups/hitobject-beatmap-custom-sample-bank.osu

@@ -0,0 +1,7 @@
+osu file format v14
+
+[TimingPoints]
+0,300,4,0,2,100,1,0
+
+[HitObjects]
+444,320,1000,5,2,0:0:0:0:

osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs

@@ -12,6 +12,7 @@ using System.Linq;
 using JetBrains.Annotations;
 using osu.Framework.Utils;
 using osu.Game.Beatmaps.Legacy;
+using osu.Game.Skinning;
 
 namespace osu.Game.Rulesets.Objects.Legacy
 {
@@ -356,7 +357,10 @@ namespace osu.Game.Rulesets.Objects.Legacy
                     Bank = bankInfo.Normal,
                     Name = HitSampleInfo.HIT_NORMAL,
                     Volume = bankInfo.Volume,
-                    CustomSampleBank = bankInfo.CustomSampleBank
+                    CustomSampleBank = bankInfo.CustomSampleBank,
+                    // if the sound type doesn't have the Normal flag set, attach it anyway as a layered sample.
+                    // None also counts as a normal non-layered sample: https://osu.ppy.sh/help/wiki/osu!_File_Formats/Osu_(file_format)#hitsounds
+                    IsLayered = type != LegacyHitSoundType.None && !type.HasFlag(LegacyHitSoundType.Normal)
                 }
             };
 
@@ -409,7 +413,7 @@ namespace osu.Game.Rulesets.Objects.Legacy
             public SampleBankInfo Clone() => (SampleBankInfo)MemberwiseClone();
         }
 
-        internal class LegacyHitSampleInfo : HitSampleInfo
+        public class LegacyHitSampleInfo : HitSampleInfo
         {
             private int customSampleBank;
 
@@ -424,6 +428,15 @@ namespace osu.Game.Rulesets.Objects.Legacy
                         Suffix = value.ToString();
                 }
             }
+
+            /// <summary>
+            /// Whether this hit sample is layered.
+            /// </summary>
+            /// <remarks>
+            /// Layered hit samples are automatically added in all modes (except osu!mania), but can be disabled
+            /// using the <see cref="GlobalSkinConfiguration.LayeredHitSounds"/> skin config option.
+            /// </remarks>
+            public bool IsLayered { get; set; }
         }
 
         private class FileHitSampleInfo : LegacyHitSampleInfo

osu.Game/Skinning/GlobalSkinConfiguration.cs

@@ -5,6 +5,7 @@ namespace osu.Game.Skinning
 {
     public enum GlobalSkinConfiguration
     {
-        AnimationFramerate
+        AnimationFramerate,
+        LayeredHitSounds,
     }
 }

osu.Game/Skinning/LegacySkinTransformer.cs

@@ -6,6 +6,7 @@ using osu.Framework.Bindables;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Textures;
 using osu.Game.Audio;
+using osu.Game.Rulesets.Objects.Legacy;
 
 namespace osu.Game.Skinning
 {
@@ -28,7 +29,17 @@ namespace osu.Game.Skinning
 
         public Texture GetTexture(string componentName) => Source.GetTexture(componentName);
 
-        public virtual SampleChannel GetSample(ISampleInfo sampleInfo) => Source.GetSample(sampleInfo);
+        public virtual SampleChannel GetSample(ISampleInfo sampleInfo)
+        {
+            if (!(sampleInfo is ConvertHitObjectParser.LegacyHitSampleInfo legacySample))
+                return Source.GetSample(sampleInfo);
+
+            var playLayeredHitSounds = GetConfig<GlobalSkinConfiguration, bool>(GlobalSkinConfiguration.LayeredHitSounds);
+            if (legacySample.IsLayered && playLayeredHitSounds?.Value == false)
+                return new SampleChannelVirtual();
+
+            return Source.GetSample(sampleInfo);
+        }
 
         public abstract IBindable<TValue> GetConfig<TLookup, TValue>(TLookup lookup);
     }

osu.Game/Tests/Beatmaps/HitObjectSampleTest.cs

@@ -24,6 +24,10 @@ namespace osu.Game.Tests.Beatmaps
     public abstract class HitObjectSampleTest : PlayerTestScene
     {
         protected abstract IResourceStore<byte[]> Resources { get; }
+        protected LegacySkin Skin { get; private set; }
+
+        [Resolved]
+        private RulesetStore rulesetStore { get; set; }
 
         private readonly SkinInfo userSkinInfo = new SkinInfo();
 
@@ -64,6 +68,9 @@ namespace osu.Game.Tests.Beatmaps
                 {
                     using (var reader = new LineBufferedReader(Resources.GetStream($"Resources/SampleLookups/{filename}")))
                         currentTestBeatmap = Decoder.GetDecoder<Beatmap>(reader).Decode(reader);
+
+                    // populate ruleset for beatmap converters that require it to be present.
+                    currentTestBeatmap.BeatmapInfo.Ruleset = rulesetStore.GetRuleset(currentTestBeatmap.BeatmapInfo.RulesetID);
                 });
             });
         }
@@ -91,7 +98,7 @@ namespace osu.Game.Tests.Beatmaps
                 };
 
                 // Need to refresh the cached skin source to refresh the skin resource store.
-                dependencies.SkinSource = new SkinProvidingContainer(new LegacySkin(userSkinInfo, userSkinResourceStore, Audio));
+                dependencies.SkinSource = new SkinProvidingContainer(Skin = new LegacySkin(userSkinInfo, userSkinResourceStore, Audio));
             });
         }
 
@@ -101,6 +108,9 @@ namespace osu.Game.Tests.Beatmaps
         protected void AssertUserLookup(string name) => AddAssert($"\"{name}\" looked up from user skin",
             () => !beatmapSkinResourceStore.PerformedLookups.Contains(name) && userSkinResourceStore.PerformedLookups.Contains(name));
 
+        protected void AssertNoLookup(string name) => AddAssert($"\"{name}\" not looked up",
+            () => !beatmapSkinResourceStore.PerformedLookups.Contains(name) && !userSkinResourceStore.PerformedLookups.Contains(name));
+
         private class SkinSourceDependencyContainer : IReadOnlyDependencyContainer
         {
             public ISkinSource SkinSource;