From 6632367c6db4cff221e35386a7447e771ca44c61 Mon Sep 17 00:00:00 2001
From: Dan Balasescu <smoogipoo@smgi.me>
Date: Thu, 4 Aug 2022 14:48:12 +0900
Subject: [PATCH] Ensure skin samples are looked up in correct order

---
 .../Skins/TestSceneSkinResources.cs           | 69 +++++++++++++++++--
 osu.Game/Skinning/Skin.cs                     |  6 +-
 2 files changed, 69 insertions(+), 6 deletions(-)

diff --git a/osu.Game.Tests/Skins/TestSceneSkinResources.cs b/osu.Game.Tests/Skins/TestSceneSkinResources.cs
index 42c1eeb6d1..20f5c3d911 100644
--- a/osu.Game.Tests/Skins/TestSceneSkinResources.cs
+++ b/osu.Game.Tests/Skins/TestSceneSkinResources.cs
@@ -1,14 +1,25 @@
 // 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 System.Threading;
+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 +30,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 +43,55 @@ private void load()
 
         [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);
+        }
     }
 }
diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs
index 7d93aeb897..86347d9a3a 100644
--- a/osu.Game/Skinning/Skin.cs
+++ b/osu.Game/Skinning/Skin.cs
@@ -69,12 +69,14 @@ protected Skin(SkinInfo skin, IStorageResourceProvider? resources, IResourceStor
 
                 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));
             }