Merge pull request #19574 from smoogipoo/fix-skin-sample-lookup

Ensure skin samples are looked up in correct order

osu.Game.Tests/Skins/TestSceneSkinResources.cs

@@ -1,14 +1,24 @@
 // 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.
 
-#nullable disable
+using System;
-
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using Moq;
 using NUnit.Framework;
 using osu.Framework.Allocation;
+using osu.Framework.Audio.Sample;
+using osu.Framework.Bindables;
 using osu.Framework.Extensions;
+using osu.Framework.Extensions.ObjectExtensions;
+using osu.Framework.Graphics.OpenGL.Textures;
+using osu.Framework.Graphics.Textures;
+using osu.Framework.IO.Stores;
 using osu.Framework.Testing;
 using osu.Game.Audio;
 using osu.Game.Database;
+using osu.Game.IO;
 using osu.Game.Skinning;
 using osu.Game.Tests.Resources;
 using osu.Game.Tests.Visual;
@@ -19,9 +29,9 @@ namespace osu.Game.Tests.Skins
     public class TestSceneSkinResources : OsuTestScene
     {
         [Resolved]
-        private SkinManager skins { get; set; }
+        private SkinManager skins { get; set; } = null!;
 
-        private ISkin skin;
+        private ISkin skin = null!;
 
         [BackgroundDependencyLoader]
         private void load()
@@ -32,5 +42,55 @@ namespace osu.Game.Tests.Skins
 
         [Test]
         public void TestRetrieveOggSample() => AddAssert("sample is non-null", () => skin.GetSample(new SampleInfo("sample")) != null);
+
+        [Test]
+        public void TestSampleRetrievalOrder()
+        {
+            Mock<IStorageResourceProvider> mockResourceProvider = null!;
+            Mock<IResourceStore<byte[]>> mockResourceStore = null!;
+            List<string> lookedUpFileNames = null!;
+
+            AddStep("setup mock providers provider", () =>
+            {
+                lookedUpFileNames = new List<string>();
+                mockResourceProvider = new Mock<IStorageResourceProvider>();
+                mockResourceProvider.Setup(m => m.AudioManager).Returns(Audio);
+                mockResourceStore = new Mock<IResourceStore<byte[]>>();
+                mockResourceStore.Setup(r => r.Get(It.IsAny<string>()))
+                                 .Callback<string>(n => lookedUpFileNames.Add(n))
+                                 .Returns<byte>(null);
+            });
+
+            AddStep("query sample", () =>
+            {
+                TestSkin testSkin = new TestSkin(new SkinInfo(), mockResourceProvider.Object, new ResourceStore<byte[]>(mockResourceStore.Object));
+                testSkin.GetSample(new SampleInfo());
+            });
+
+            AddAssert("sample lookups were in correct order", () =>
+            {
+                string[] lookups = lookedUpFileNames.Where(f => f.StartsWith(TestSkin.SAMPLE_NAME, StringComparison.Ordinal)).ToArray();
+                return Path.GetExtension(lookups[0]) == string.Empty
+                       && Path.GetExtension(lookups[1]) == ".wav"
+                       && Path.GetExtension(lookups[2]) == ".mp3"
+                       && Path.GetExtension(lookups[3]) == ".ogg";
+            });
+        }
+
+        private class TestSkin : Skin
+        {
+            public const string SAMPLE_NAME = "test-sample";
+
+            public TestSkin(SkinInfo skin, IStorageResourceProvider? resources, IResourceStore<byte[]>? storage = null, string configurationFilename = "skin.ini")
+                : base(skin, resources, storage, configurationFilename)
+            {
+            }
+
+            public override Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) => throw new NotImplementedException();
+
+            public override IBindable<TValue> GetConfig<TLookup, TValue>(TLookup lookup) => throw new NotImplementedException();
+
+            public override ISample GetSample(ISampleInfo sampleInfo) => Samples.AsNonNull().Get(SAMPLE_NAME);
+        }
     }
 }

osu.Game/Skinning/Skin.cs

@@ -69,12 +69,14 @@ namespace osu.Game.Skinning
 
                 storage ??= realmBackedStorage = new RealmBackedResourceStore<SkinInfo>(SkinInfo, resources.Files, resources.RealmAccess);
 
-                (storage as ResourceStore<byte[]>)?.AddExtension("ogg");
-
                 var samples = resources.AudioManager?.GetSampleStore(storage);
                 if (samples != null)
                     samples.PlaybackConcurrency = OsuGameBase.SAMPLE_CONCURRENCY;
 
+                // osu-stable performs audio lookups in order of wav -> mp3 -> ogg.
+                // The GetSampleStore() call above internally adds wav and mp3, so ogg is added at the end to ensure expected ordering.
+                (storage as ResourceStore<byte[]>)?.AddExtension("ogg");
+
                 Samples = samples;
                 Textures = new TextureStore(resources.CreateTextureLoaderStore(storage));
             }