Merge branch 'master' into beatmap-refactor/tournament

.config/dotnet-tools.json

@@ -15,7 +15,7 @@
       ]
     },
     "smoogipoo.nvika": {
-      "version": "1.0.1",
+      "version": "1.0.3",
       "commands": [
         "nvika"
       ]

osu.Game.Rulesets.Catch/Objects/JuiceStream.cs

@@ -145,7 +145,7 @@ namespace osu.Game.Rulesets.Catch.Objects
 
         public double Distance => Path.Distance;
 
-        public List<IList<HitSampleInfo>> NodeSamples { get; set; } = new List<IList<HitSampleInfo>>();
+        public IList<IList<HitSampleInfo>> NodeSamples { get; set; } = new List<IList<HitSampleInfo>>();
 
         public double? LegacyLastTickOffset { get; set; }
     }

osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs

@@ -66,7 +66,7 @@ namespace osu.Game.Rulesets.Catch.Skinning.Legacy
                         return null;
 
                     case CatchSkinComponents.Catcher:
-                        decimal version = GetConfig<LegacySkinConfiguration.LegacySetting, decimal>(LegacySkinConfiguration.LegacySetting.Version)?.Value ?? 1;
+                        decimal version = GetConfig<SkinConfiguration.LegacySetting, decimal>(SkinConfiguration.LegacySetting.Version)?.Value ?? 1;
 
                         if (version < 2.3m)
                         {

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

@@ -38,7 +38,7 @@ namespace osu.Game.Rulesets.Mania.Tests
         private IList<string> getSampleNames(IList<HitSampleInfo> hitSampleInfo)
             => hitSampleInfo.Select(sample => sample.LookupNames.First()).ToList();
 
-        private IList<IList<string>> getNodeSampleNames(List<IList<HitSampleInfo>> hitSampleInfo)
+        private IList<IList<string>> getNodeSampleNames(IList<IList<HitSampleInfo>> hitSampleInfo)
             => hitSampleInfo?.Select(getSampleNames)
                             .ToList();
 

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

@@ -488,7 +488,7 @@ namespace osu.Game.Rulesets.Mania.Beatmaps.Patterns.Legacy
         /// 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(int time)
+        private IList<IList<HitSampleInfo>> nodeSamplesAt(int time)
         {
             if (!(HitObject is IHasPathWithRepeats curveData))
                 return null;

osu.Game.Rulesets.Mania/Objects/HoldNote.cs

@@ -67,7 +67,7 @@ namespace osu.Game.Rulesets.Mania.Objects
             }
         }
 
-        public List<IList<HitSampleInfo>> NodeSamples { get; set; }
+        public IList<IList<HitSampleInfo>> NodeSamples { get; set; }
 
         /// <summary>
         /// The head note of the hold.

osu.Game.Rulesets.Mania/Skinning/Legacy/LegacyStageBackground.cs

@@ -98,7 +98,7 @@ namespace osu.Game.Rulesets.Mania.Skinning.Legacy
                 float rightLineWidth = skin.GetManiaSkinConfig<float>(LegacyManiaSkinConfigurationLookups.RightLineWidth, columnIndex)?.Value ?? 1;
 
                 bool hasLeftLine = leftLineWidth > 0;
-                bool hasRightLine = rightLineWidth > 0 && skin.GetConfig<LegacySkinConfiguration.LegacySetting, decimal>(LegacySkinConfiguration.LegacySetting.Version)?.Value >= 2.4m
+                bool hasRightLine = rightLineWidth > 0 && skin.GetConfig<SkinConfiguration.LegacySetting, decimal>(SkinConfiguration.LegacySetting.Version)?.Value >= 2.4m
                                     || isLastColumn;
 
                 Color4 lineColour = skin.GetManiaSkinConfig<Color4>(LegacyManiaSkinConfigurationLookups.ColumnLineColour, columnIndex)?.Value ?? Color4.White;

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

@@ -63,7 +63,7 @@ namespace osu.Game.Rulesets.Mania.Skinning.Legacy
         {
             this.beatmap = (ManiaBeatmap)beatmap;
 
-            isLegacySkin = new Lazy<bool>(() => GetConfig<LegacySkinConfiguration.LegacySetting, decimal>(LegacySkinConfiguration.LegacySetting.Version) != null);
+            isLegacySkin = new Lazy<bool>(() => GetConfig<SkinConfiguration.LegacySetting, decimal>(SkinConfiguration.LegacySetting.Version) != null);
             hasKeyTexture = new Lazy<bool>(() =>
             {
                 string keyImage = this.GetManiaSkinConfig<string>(LegacyManiaSkinConfigurationLookups.KeyImage, 0)?.Value ?? "mania-key1";

osu.Game.Rulesets.Osu/Objects/Slider.cs

@@ -79,7 +79,7 @@ namespace osu.Game.Rulesets.Osu.Objects
         /// </summary>
         internal float LazyTravelDistance;
 
-        public List<IList<HitSampleInfo>> NodeSamples { get; set; } = new List<IList<HitSampleInfo>>();
+        public IList<IList<HitSampleInfo>> NodeSamples { get; set; } = new List<IList<HitSampleInfo>>();
 
         [JsonIgnore]
         public IList<HitSampleInfo> TailSamples { get; private set; }

osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyMainCirclePiece.cs

@@ -14,7 +14,6 @@ using osu.Game.Rulesets.Osu.Objects.Drawables;
 using osu.Game.Skinning;
 using osuTK;
 using osuTK.Graphics;
-using static osu.Game.Skinning.LegacySkinConfiguration;
 
 namespace osu.Game.Rulesets.Osu.Skinning.Legacy
 {
@@ -158,7 +157,7 @@ namespace osu.Game.Rulesets.Osu.Skinning.Legacy
 
                         if (hasNumber)
                         {
-                            decimal? legacyVersion = skin.GetConfig<LegacySetting, decimal>(LegacySetting.Version)?.Value;
+                            decimal? legacyVersion = skin.GetConfig<SkinConfiguration.LegacySetting, decimal>(SkinConfiguration.LegacySetting.Version)?.Value;
 
                             if (legacyVersion >= 2.0m)
                                 // legacy skins of version 2.0 and newer only apply very short fade out to the number piece.

osu.Game.Rulesets.Taiko/Beatmaps/TaikoBeatmapConverter.cs

@@ -81,7 +81,7 @@ namespace osu.Game.Rulesets.Taiko.Beatmaps
                 {
                     if (shouldConvertSliderToHits(obj, beatmap, distanceData, out int taikoDuration, out double tickSpacing))
                     {
-                        List<IList<HitSampleInfo>> allSamples = obj is IHasPathWithRepeats curveData ? curveData.NodeSamples : new List<IList<HitSampleInfo>>(new[] { samples });
+                        IList<IList<HitSampleInfo>> allSamples = obj is IHasPathWithRepeats curveData ? curveData.NodeSamples : new List<IList<HitSampleInfo>>(new[] { samples });
 
                         int i = 0;
 

osu.Game.Rulesets.Taiko/Skinning/Legacy/LegacyInputDrum.cs

@@ -69,7 +69,7 @@ namespace osu.Game.Rulesets.Taiko.Skinning.Legacy
             // because the right half is flipped, we need to position using width - position to get the true "topleft" origin position
             float negativeScaleAdjust = content.Width / ratio;
 
-            if (skin.GetConfig<LegacySkinConfiguration.LegacySetting, decimal>(LegacySkinConfiguration.LegacySetting.Version)?.Value >= 2.1m)
+            if (skin.GetConfig<SkinConfiguration.LegacySetting, decimal>(SkinConfiguration.LegacySetting.Version)?.Value >= 2.1m)
             {
                 left.Centre.Position = new Vector2(0, taiko_bar_y) * ratio;
                 right.Centre.Position = new Vector2(negativeScaleAdjust - 56, taiko_bar_y) * ratio;

osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs

@@ -7,7 +7,7 @@ using osu.Game.Rulesets;
 using osu.Game.Rulesets.Osu;
 using osu.Game.Tests.Beatmaps;
 using osu.Game.Tests.Resources;
-using static osu.Game.Skinning.LegacySkinConfiguration;
+using static osu.Game.Skinning.SkinConfiguration;
 
 namespace osu.Game.Tests.Gameplay
 {

osu.Game.Tests/Skins/IO/ImportSkinTest.cs

@@ -4,10 +4,12 @@
 using System;
 using System.IO;
 using System.Linq;
+using System.Runtime.CompilerServices;
 using System.Threading.Tasks;
 using NUnit.Framework;
 using osu.Framework.Allocation;
 using osu.Framework.Platform;
+using osu.Game.IO;
 using osu.Game.IO.Archives;
 using osu.Game.Skinning;
 using SharpCompress.Archives.Zip;
@@ -16,155 +18,179 @@ namespace osu.Game.Tests.Skins.IO
 {
     public class ImportSkinTest : ImportTest
     {
-        [Test]
+        #region Testing filename metadata inclusion
-        public async Task TestBasicImport()
-        {
-            using (HeadlessGameHost host = new CleanRunHeadlessGameHost(nameof(ImportSkinTest)))
-            {
-                try
-                {
-                    var osu = LoadOsuIntoHost(host);
-
-                    var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("test skin", "skinner"), "skin.osk"));
-
-                    Assert.That(imported.Name, Is.EqualTo("test skin"));
-                    Assert.That(imported.Creator, Is.EqualTo("skinner"));
-                }
-                finally
-                {
-                    host.Exit();
-                }
-            }
-        }
 
         [Test]
-        public async Task TestImportTwiceWithSameMetadata()
+        public Task TestSingleImportDifferentFilename() => runSkinTest(async osu =>
         {
-            using (HeadlessGameHost host = new CleanRunHeadlessGameHost(nameof(ImportSkinTest)))
+            var import1 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("test skin", "skinner"), "skin.osk"));
-            {
-                try
-                {
-                    var osu = LoadOsuIntoHost(host);
 
-                    var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("test skin", "skinner"), "skin.osk"));
+            // When the import filename doesn't match, it should be appended (and update the skin.ini).
-                    var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("test skin", "skinner"), "skin2.osk"));
+            assertCorrectMetadata(import1, "test skin [skin]", "skinner", osu);
-
+        });
-                    Assert.That(imported2.ID, Is.Not.EqualTo(imported.ID));
-                    Assert.That(osu.Dependencies.Get<SkinManager>().GetAllUserSkins(true).Count, Is.EqualTo(1));
-
-                    // the first should be overwritten by the second import.
-                    Assert.That(osu.Dependencies.Get<SkinManager>().GetAllUserSkins(true).First().Files.First().FileInfoID, Is.EqualTo(imported2.Files.First().FileInfoID));
-                }
-                finally
-                {
-                    host.Exit();
-                }
-            }
-        }
 
         [Test]
-        public async Task TestImportTwiceWithNoMetadata()
+        public Task TestSingleImportMatchingFilename() => runSkinTest(async osu =>
         {
-            using (HeadlessGameHost host = new CleanRunHeadlessGameHost(nameof(ImportSkinTest)))
+            var import1 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("test skin", "skinner"), "test skin.osk"));
-            {
-                try
-                {
-                    var osu = LoadOsuIntoHost(host);
 
+            // When the import filename matches it shouldn't be appended.
+            assertCorrectMetadata(import1, "test skin", "skinner", osu);
+        });
+
+        [Test]
+        public Task TestSingleImportNoIniFile() => runSkinTest(async osu =>
+        {
+            var import1 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithNonIniFile(), "test skin.osk"));
+
+            // When the import filename matches it shouldn't be appended.
+            assertCorrectMetadata(import1, "test skin", "Unknown", osu);
+        });
+
+        [Test]
+        public Task TestEmptyImportImportsWithFilename() => runSkinTest(async osu =>
+        {
+            var import1 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createEmptyOsk(), "test skin.osk"));
+
+            // When the import filename matches it shouldn't be appended.
+            assertCorrectMetadata(import1, "test skin", "Unknown", osu);
+        });
+
+        #endregion
+
+        #region Cases where imports should match existing
+
+        [Test]
+        public Task TestImportTwiceWithSameMetadataAndFilename() => runSkinTest(async osu =>
+        {
+            var import1 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("test skin", "skinner"), "skin.osk"));
+            var import2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("test skin", "skinner"), "skin.osk"));
+
+            assertImportedOnce(import1, import2);
+        });
+
+        [Test]
+        public Task TestImportTwiceWithNoMetadataSameDownloadFilename() => runSkinTest(async osu =>
+        {
             // if a user downloads two skins that do have skin.ini files but don't have any creator metadata in the skin.ini, they should both import separately just for safety.
-                    var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk(string.Empty, string.Empty), "download.osk"));
+            var import1 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni(string.Empty, string.Empty), "download.osk"));
-                    var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk(string.Empty, string.Empty), "download.osk"));
+            var import2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni(string.Empty, string.Empty), "download.osk"));
 
-                    Assert.That(imported2.ID, Is.Not.EqualTo(imported.ID));
+            assertImportedOnce(import1, import2);
-                    Assert.That(osu.Dependencies.Get<SkinManager>().GetAllUserSkins(true).Count, Is.EqualTo(2));
+        });
-
-                    Assert.That(osu.Dependencies.Get<SkinManager>().GetAllUserSkins(true).First().Files.First().FileInfoID, Is.EqualTo(imported.Files.First().FileInfoID));
-                    Assert.That(osu.Dependencies.Get<SkinManager>().GetAllUserSkins(true).Last().Files.First().FileInfoID, Is.EqualTo(imported2.Files.First().FileInfoID));
-                }
-                finally
-                {
-                    host.Exit();
-                }
-            }
-        }
 
         [Test]
-        public async Task TestImportTwiceWithDifferentMetadata()
+        public Task TestImportUpperCasedOskArchive() => runSkinTest(async osu =>
         {
-            using (HeadlessGameHost host = new CleanRunHeadlessGameHost(nameof(ImportSkinTest)))
+            var import1 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("name 1", "author 1"), "name 1.OsK"));
-            {
+            assertCorrectMetadata(import1, "name 1", "author 1", osu);
-                try
-                {
-                    var osu = LoadOsuIntoHost(host);
 
-                    var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("test skin v2", "skinner"), "skin.osk"));
+            var import2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("name 1", "author 1"), "name 1.oSK"));
-                    var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("test skin v2.1", "skinner"), "skin2.osk"));
 
-                    Assert.That(imported2.ID, Is.Not.EqualTo(imported.ID));
+            assertImportedOnce(import1, import2);
-                    Assert.That(osu.Dependencies.Get<SkinManager>().GetAllUserSkins(true).Count, Is.EqualTo(2));
+        });
-
-                    Assert.That(osu.Dependencies.Get<SkinManager>().GetAllUserSkins(true).First().Files.First().FileInfoID, Is.EqualTo(imported.Files.First().FileInfoID));
-                    Assert.That(osu.Dependencies.Get<SkinManager>().GetAllUserSkins(true).Last().Files.First().FileInfoID, Is.EqualTo(imported2.Files.First().FileInfoID));
-                }
-                finally
-                {
-                    host.Exit();
-                }
-            }
-        }
 
         [Test]
-        public async Task TestImportUpperCasedOskArchive()
+        public Task TestSameMetadataNameSameFolderName() => runSkinTest(async osu =>
         {
-            using (HeadlessGameHost host = new CleanRunHeadlessGameHost(nameof(ImportSkinTest)))
+            var import1 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("name 1", "author 1"), "my custom skin 1"));
-            {
+            var import2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("name 1", "author 1"), "my custom skin 1"));
-                try
-                {
-                    var osu = LoadOsuIntoHost(host);
 
-                    var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("name 1", "author 1"), "skin1.OsK"));
+            assertImportedOnce(import1, import2);
+            assertCorrectMetadata(import1, "name 1 [my custom skin 1]", "author 1", osu);
+        });
 
-                    Assert.That(imported.Name, Is.EqualTo("name 1"));
+        #endregion
-                    Assert.That(imported.Creator, Is.EqualTo("author 1"));
 
-                    var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("name 1", "author 1"), "skin1.oSK"));
+        #region Cases where imports should be uniquely imported
-
-                    Assert.That(imported2.Hash, Is.EqualTo(imported.Hash));
-                }
-                finally
-                {
-                    host.Exit();
-                }
-            }
-        }
 
         [Test]
-        public async Task TestSameMetadataNameDifferentFolderName()
+        public Task TestImportTwiceWithSameMetadataButDifferentFilename() => runSkinTest(async osu =>
         {
-            using (HeadlessGameHost host = new CleanRunHeadlessGameHost(nameof(ImportSkinTest)))
+            var import1 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("test skin", "skinner"), "skin.osk"));
-            {
+            var import2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("test skin", "skinner"), "skin2.osk"));
-                try
-                {
-                    var osu = LoadOsuIntoHost(host);
 
-                    var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("name 1", "author 1", false), "my custom skin 1"));
+            assertImportedBoth(import1, import2);
-                    Assert.That(imported.Name, Is.EqualTo("name 1 [my custom skin 1]"));
+        });
-                    Assert.That(imported.Creator, Is.EqualTo("author 1"));
 
-                    var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("name 1", "author 1", false), "my custom skin 2"));
+        [Test]
-                    Assert.That(imported2.Name, Is.EqualTo("name 1 [my custom skin 2]"));
+        public Task TestImportTwiceWithNoMetadataDifferentDownloadFilename() => runSkinTest(async osu =>
-                    Assert.That(imported2.Creator, Is.EqualTo("author 1"));
-
-                    Assert.That(imported2.Hash, Is.Not.EqualTo(imported.Hash));
-                }
-                finally
         {
-                    host.Exit();
+            // if a user downloads two skins that do have skin.ini files but don't have any creator metadata in the skin.ini, they should both import separately just for safety.
-                }
+            var import1 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni(string.Empty, string.Empty), "download.osk"));
-            }
+            var import2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni(string.Empty, string.Empty), "download2.osk"));
+
+            assertImportedBoth(import1, import2);
+        });
+
+        [Test]
+        public Task TestImportTwiceWithSameFilenameDifferentMetadata() => runSkinTest(async osu =>
+        {
+            var import1 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("test skin v2", "skinner"), "skin.osk"));
+            var import2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("test skin v2.1", "skinner"), "skin.osk"));
+
+            assertImportedBoth(import1, import2);
+            assertCorrectMetadata(import1, "test skin v2 [skin]", "skinner", osu);
+            assertCorrectMetadata(import2, "test skin v2.1 [skin]", "skinner", osu);
+        });
+
+        [Test]
+        public Task TestSameMetadataNameDifferentFolderName() => runSkinTest(async osu =>
+        {
+            var import1 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("name 1", "author 1"), "my custom skin 1"));
+            var import2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("name 1", "author 1"), "my custom skin 2"));
+
+            assertImportedBoth(import1, import2);
+            assertCorrectMetadata(import1, "name 1 [my custom skin 1]", "author 1", osu);
+            assertCorrectMetadata(import2, "name 1 [my custom skin 2]", "author 1", osu);
+        });
+
+        #endregion
+
+        private void assertCorrectMetadata(SkinInfo import1, string name, string creator, OsuGameBase osu)
+        {
+            Assert.That(import1.Name, Is.EqualTo(name));
+            Assert.That(import1.Creator, Is.EqualTo(creator));
+
+            // for extra safety let's reconstruct the skin, reading from the skin.ini.
+            var instance = import1.CreateInstance((IStorageResourceProvider)osu.Dependencies.Get(typeof(SkinManager)));
+
+            Assert.That(instance.Configuration.SkinInfo.Name, Is.EqualTo(name));
+            Assert.That(instance.Configuration.SkinInfo.Creator, Is.EqualTo(creator));
         }
 
-        private MemoryStream createOsk(string name, string author, bool makeUnique = true)
+        private void assertImportedBoth(SkinInfo import1, SkinInfo import2)
+        {
+            Assert.That(import2.ID, Is.Not.EqualTo(import1.ID));
+            Assert.That(import2.Hash, Is.Not.EqualTo(import1.Hash));
+            Assert.That(import2.Files.Select(f => f.FileInfoID), Is.Not.EquivalentTo(import1.Files.Select(f => f.FileInfoID)));
+        }
+
+        private void assertImportedOnce(SkinInfo import1, SkinInfo import2)
+        {
+            Assert.That(import2.ID, Is.EqualTo(import1.ID));
+            Assert.That(import2.Hash, Is.EqualTo(import1.Hash));
+            Assert.That(import2.Files.Select(f => f.FileInfoID), Is.EquivalentTo(import1.Files.Select(f => f.FileInfoID)));
+        }
+
+        private MemoryStream createEmptyOsk()
+        {
+            var zipStream = new MemoryStream();
+            using var zip = ZipArchive.Create();
+            zip.SaveTo(zipStream);
+            return zipStream;
+        }
+
+        private MemoryStream createOskWithNonIniFile()
+        {
+            var zipStream = new MemoryStream();
+            using var zip = ZipArchive.Create();
+            zip.AddEntry("hitcircle.png", new MemoryStream(new byte[] { 0, 1, 2, 3 }));
+            zip.SaveTo(zipStream);
+            return zipStream;
+        }
+
+        private MemoryStream createOskWithIni(string name, string author, bool makeUnique = false)
         {
             var zipStream = new MemoryStream();
             using var zip = ZipArchive.Create();
@@ -193,6 +219,22 @@ namespace osu.Game.Tests.Skins.IO
             return stream;
         }
 
+        private async Task runSkinTest(Func<OsuGameBase, Task> action, [CallerMemberName] string callingMethodName = @"")
+        {
+            using (HeadlessGameHost host = new CleanRunHeadlessGameHost(callingMethodName))
+            {
+                try
+                {
+                    var osu = LoadOsuIntoHost(host);
+                    await action(osu);
+                }
+                finally
+                {
+                    host.Exit();
+                }
+            }
+        }
+
         private async Task<SkinInfo> loadSkinIntoOsu(OsuGameBase osu, ArchiveReader archive = null)
         {
             var skinManager = osu.Dependencies.Get<SkinManager>();

osu.Game.Tests/Skins/LegacySkinDecoderTest.cs

@@ -106,7 +106,7 @@ namespace osu.Game.Tests.Skins
             var decoder = new LegacySkinDecoder();
             using (var resStream = TestResources.OpenResource("skin-latest.ini"))
             using (var stream = new LineBufferedReader(resStream))
-                Assert.AreEqual(LegacySkinConfiguration.LATEST_VERSION, decoder.Decode(stream).LegacyVersion);
+                Assert.AreEqual(SkinConfiguration.LATEST_VERSION, decoder.Decode(stream).LegacyVersion);
         }
 
         [Test]

osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs

@@ -151,7 +151,7 @@ namespace osu.Game.Tests.Skins
         {
             AddStep("Set user skin version 2.3", () => userSource.Configuration.LegacyVersion = 2.3m);
             AddStep("Set beatmap skin version null", () => beatmapSource.Configuration.LegacyVersion = null);
-            AddAssert("Check legacy version lookup", () => requester.GetConfig<LegacySkinConfiguration.LegacySetting, decimal>(LegacySkinConfiguration.LegacySetting.Version)?.Value == 2.3m);
+            AddAssert("Check legacy version lookup", () => requester.GetConfig<SkinConfiguration.LegacySetting, decimal>(SkinConfiguration.LegacySetting.Version)?.Value == 2.3m);
         }
 
         [Test]
@@ -160,7 +160,7 @@ namespace osu.Game.Tests.Skins
             // completely ignoring beatmap versions for simplicity.
             AddStep("Set user skin version 2.3", () => userSource.Configuration.LegacyVersion = 2.3m);
             AddStep("Set beatmap skin version null", () => beatmapSource.Configuration.LegacyVersion = 1.7m);
-            AddAssert("Check legacy version lookup", () => requester.GetConfig<LegacySkinConfiguration.LegacySetting, decimal>(LegacySkinConfiguration.LegacySetting.Version)?.Value == 2.3m);
+            AddAssert("Check legacy version lookup", () => requester.GetConfig<SkinConfiguration.LegacySetting, decimal>(SkinConfiguration.LegacySetting.Version)?.Value == 2.3m);
         }
 
         [Test]
@@ -169,14 +169,14 @@ namespace osu.Game.Tests.Skins
             AddStep("Set user skin version 2.3", () => userSource.Configuration.LegacyVersion = null);
             AddStep("Set beatmap skin version null", () => beatmapSource.Configuration.LegacyVersion = null);
             AddAssert("Check legacy version lookup",
-                () => requester.GetConfig<LegacySkinConfiguration.LegacySetting, decimal>(LegacySkinConfiguration.LegacySetting.Version)?.Value == LegacySkinConfiguration.LATEST_VERSION);
+                () => requester.GetConfig<SkinConfiguration.LegacySetting, decimal>(SkinConfiguration.LegacySetting.Version)?.Value == SkinConfiguration.LATEST_VERSION);
         }
 
         [Test]
         public void TestIniWithNoVersionFallsBackTo1()
         {
             AddStep("Parse skin with no version", () => userSource.Configuration = new LegacySkinDecoder().Decode(new LineBufferedReader(new MemoryStream())));
-            AddAssert("Check legacy version lookup", () => requester.GetConfig<LegacySkinConfiguration.LegacySetting, decimal>(LegacySkinConfiguration.LegacySetting.Version)?.Value == 1.0m);
+            AddAssert("Check legacy version lookup", () => requester.GetConfig<SkinConfiguration.LegacySetting, decimal>(SkinConfiguration.LegacySetting.Version)?.Value == 1.0m);
         }
 
         public enum LookupType

osu.Game.Tests/Visual/Multiplayer/TestSceneLoungeRoomsContainer.cs

@@ -19,7 +19,7 @@ namespace osu.Game.Tests.Visual.Multiplayer
 {
     public class TestSceneLoungeRoomsContainer : OnlinePlayTestScene
     {
-        protected new TestRequestHandlingRoomManager RoomManager => (TestRequestHandlingRoomManager)base.RoomManager;
+        protected new TestRoomManager RoomManager => (TestRoomManager)base.RoomManager;
 
         private RoomsContainer container;
 

osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs

@@ -17,6 +17,7 @@ using osu.Framework.Utils;
 using osu.Game.Beatmaps;
 using osu.Game.Database;
 using osu.Game.Graphics.UserInterface;
+using osu.Game.Online.API;
 using osu.Game.Online.Multiplayer;
 using osu.Game.Online.Rooms;
 using osu.Game.Overlays.Mods;
@@ -33,6 +34,7 @@ using osu.Game.Screens.OnlinePlay.Multiplayer.Match;
 using osu.Game.Screens.Play;
 using osu.Game.Screens.Ranking;
 using osu.Game.Tests.Resources;
+using osu.Game.Tests.Visual.OnlinePlay;
 using osu.Game.Users;
 using osuTK.Input;
 
@@ -48,7 +50,7 @@ namespace osu.Game.Tests.Visual.Multiplayer
         private TestMultiplayer multiplayerScreen;
         private TestMultiplayerClient client;
 
-        private TestRequestHandlingMultiplayerRoomManager roomManager => multiplayerScreen.RoomManager;
+        private TestMultiplayerRoomManager roomManager => multiplayerScreen.RoomManager;
 
         [Cached(typeof(UserLookupCache))]
         private UserLookupCache lookupCache = new TestUserLookupCache();
@@ -616,17 +618,26 @@ namespace osu.Game.Tests.Visual.Multiplayer
             [Cached(typeof(MultiplayerClient))]
             public readonly TestMultiplayerClient Client;
 
+            [Cached]
+            public readonly TestRoomRequestsHandler RequestsHandler = new TestRoomRequestsHandler();
+
             public DependenciesScreen(TestMultiplayerClient client)
             {
                 Client = client;
             }
+
+            [BackgroundDependencyLoader]
+            private void load(IAPIProvider api, OsuGameBase game)
+            {
+                ((DummyAPIAccess)api).HandleRequest = request => RequestsHandler.HandleRequest(request, api.LocalUser.Value, game);
+            }
         }
 
         private class TestMultiplayer : Screens.OnlinePlay.Multiplayer.Multiplayer
         {
-            public new TestRequestHandlingMultiplayerRoomManager RoomManager { get; private set; }
+            public new TestMultiplayerRoomManager RoomManager { get; private set; }
 
-            protected override RoomManager CreateRoomManager() => RoomManager = new TestRequestHandlingMultiplayerRoomManager();
+            protected override RoomManager CreateRoomManager() => RoomManager = new TestMultiplayerRoomManager();
         }
     }
 }

osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerGameplayLeaderboard.cs

@@ -8,7 +8,6 @@ using NUnit.Framework;
 using osu.Framework.Allocation;
 using osu.Framework.Extensions.ObjectExtensions;
 using osu.Framework.Graphics;
-using osu.Framework.Testing;
 using osu.Framework.Utils;
 using osu.Game.Configuration;
 using osu.Game.Online.API;
@@ -40,9 +39,10 @@ namespace osu.Game.Tests.Visual.Multiplayer
             Dependencies.Cache(config = new OsuConfigManager(LocalStorage));
         }
 
-        [SetUpSteps]
         public override void SetUpSteps()
         {
+            base.SetUpSteps();
+
             AddStep("set local user", () => ((DummyAPIAccess)API).LocalUser.Value = LookupCache.GetUserAsync(1).Result);
 
             AddStep("create leaderboard", () =>

osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerGameplayLeaderboardTeams.cs

@@ -5,7 +5,6 @@ using System.Collections.Generic;
 using System.Linq;
 using NUnit.Framework;
 using osu.Framework.Graphics;
-using osu.Framework.Testing;
 using osu.Framework.Utils;
 using osu.Game.Online.API;
 using osu.Game.Online.Multiplayer;
@@ -44,9 +43,10 @@ namespace osu.Game.Tests.Visual.Multiplayer
             return room;
         }
 
-        [SetUpSteps]
         public override void SetUpSteps()
         {
+            base.SetUpSteps();
+
             AddStep("set local user", () => ((DummyAPIAccess)API).LocalUser.Value = LookupCache.GetUserAsync(1).Result);
 
             AddStep("create leaderboard", () =>

osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerLoungeSubScreen.cs

@@ -18,7 +18,7 @@ namespace osu.Game.Tests.Visual.Multiplayer
 {
     public class TestSceneMultiplayerLoungeSubScreen : OnlinePlayTestScene
     {
-        protected new TestRequestHandlingRoomManager RoomManager => (TestRequestHandlingRoomManager)base.RoomManager;
+        protected new TestRoomManager RoomManager => (TestRoomManager)base.RoomManager;
 
         private LoungeSubScreen loungeScreen;
 

osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchFooter.cs

@@ -4,7 +4,6 @@
 using NUnit.Framework;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
-using osu.Game.Online.Rooms;
 using osu.Game.Screens.OnlinePlay.Multiplayer.Match;
 
 namespace osu.Game.Tests.Visual.Multiplayer
@@ -14,8 +13,6 @@ namespace osu.Game.Tests.Visual.Multiplayer
         [SetUp]
         public new void Setup() => Schedule(() =>
         {
-            SelectedRoom.Value = new Room();
-
             Child = new Container
             {
                 Anchor = Anchor.Centre,

osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs

@@ -11,6 +11,7 @@ using osu.Framework.Platform;
 using osu.Framework.Testing;
 using osu.Game.Beatmaps;
 using osu.Game.Database;
+using osu.Game.Online.API;
 using osu.Game.Online.Multiplayer;
 using osu.Game.Online.Multiplayer.MatchTypes.TeamVersus;
 using osu.Game.Online.Rooms;
@@ -23,6 +24,7 @@ using osu.Game.Screens.OnlinePlay.Multiplayer;
 using osu.Game.Screens.OnlinePlay.Multiplayer.Match;
 using osu.Game.Screens.OnlinePlay.Multiplayer.Participants;
 using osu.Game.Tests.Resources;
+using osu.Game.Tests.Visual.OnlinePlay;
 using osuTK.Input;
 
 namespace osu.Game.Tests.Visual.Multiplayer
@@ -176,17 +178,26 @@ namespace osu.Game.Tests.Visual.Multiplayer
             [Cached(typeof(MultiplayerClient))]
             public readonly TestMultiplayerClient Client;
 
+            [Cached]
+            public readonly TestRoomRequestsHandler RequestsHandler = new TestRoomRequestsHandler();
+
             public DependenciesScreen(TestMultiplayerClient client)
             {
                 Client = client;
             }
+
+            [BackgroundDependencyLoader]
+            private void load(IAPIProvider api, OsuGameBase game)
+            {
+                ((DummyAPIAccess)api).HandleRequest = request => RequestsHandler.HandleRequest(request, api.LocalUser.Value, game);
+            }
         }
 
         private class TestMultiplayer : Screens.OnlinePlay.Multiplayer.Multiplayer
         {
-            public new TestRequestHandlingMultiplayerRoomManager RoomManager { get; private set; }
+            public new TestMultiplayerRoomManager RoomManager { get; private set; }
 
-            protected override RoomManager CreateRoomManager() => RoomManager = new TestRequestHandlingMultiplayerRoomManager();
+            protected override RoomManager CreateRoomManager() => RoomManager = new TestMultiplayerRoomManager();
         }
     }
 }

osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs

@@ -9,6 +9,7 @@ using osu.Framework.Screens;
 using osu.Framework.Testing;
 using osu.Game.Beatmaps;
 using osu.Game.Graphics.UserInterface;
+using osu.Game.Online.API;
 using osu.Game.Online.Multiplayer;
 using osu.Game.Overlays;
 using osu.Game.Overlays.Mods;
@@ -24,6 +25,7 @@ using osu.Game.Screens.Select;
 using osu.Game.Screens.Select.Options;
 using osu.Game.Tests.Beatmaps.IO;
 using osu.Game.Tests.Visual.Multiplayer;
+using osu.Game.Tests.Visual.OnlinePlay;
 using osuTK;
 using osuTK.Input;
 
@@ -459,12 +461,22 @@ namespace osu.Game.Tests.Visual.Navigation
             [Cached(typeof(MultiplayerClient))]
             public readonly TestMultiplayerClient Client;
 
+            [Cached]
+            public readonly TestRoomRequestsHandler RequestsHandler;
+
             public TestMultiplayer()
             {
-                Client = new TestMultiplayerClient((TestRequestHandlingMultiplayerRoomManager)RoomManager);
+                Client = new TestMultiplayerClient((TestMultiplayerRoomManager)RoomManager);
+                RequestsHandler = new TestRoomRequestsHandler();
             }
 
-            protected override RoomManager CreateRoomManager() => new TestRequestHandlingMultiplayerRoomManager();
+            [BackgroundDependencyLoader]
+            private void load(IAPIProvider api, OsuGameBase game)
+            {
+                ((DummyAPIAccess)api).HandleRequest = request => RequestsHandler.HandleRequest(request, api.LocalUser.Value, game);
+            }
+
+            protected override RoomManager CreateRoomManager() => new TestMultiplayerRoomManager();
         }
     }
 }

osu.Game.Tests/Visual/Online/TestSceneBeatmapListingOverlay.cs

@@ -90,7 +90,7 @@ namespace osu.Game.Tests.Visual.Online
         {
             AddAssert("is visible", () => overlay.State.Value == Visibility.Visible);
 
-            AddStep("show many results", () => fetchFor(Enumerable.Repeat(CreateAPIBeatmapSet(Ruleset.Value), 10).ToArray()));
+            AddStep("show many results", () => fetchFor(Enumerable.Repeat(CreateAPIBeatmapSet(Ruleset.Value), 100).ToArray()));
 
             AddUntilStep("placeholder hidden", () => !overlay.ChildrenOfType<BeatmapListingOverlay.NotFoundDrawable>().Any(d => d.IsPresent));
 

osu.Game.Tests/Visual/Playlists/TestScenePlaylistsLoungeSubScreen.cs

@@ -17,7 +17,7 @@ namespace osu.Game.Tests.Visual.Playlists
 {
     public class TestScenePlaylistsLoungeSubScreen : OnlinePlayTestScene
     {
-        protected new TestRequestHandlingRoomManager RoomManager => (TestRequestHandlingRoomManager)base.RoomManager;
+        protected new TestRoomManager RoomManager => (TestRoomManager)base.RoomManager;
 
         private TestLoungeSubScreen loungeScreen;
 

osu.Game/Database/ArchiveModelManager.cs

@@ -32,7 +32,7 @@ namespace osu.Game.Database
     /// <typeparam name="TFileModel">The associated file join type.</typeparam>
     public abstract class ArchiveModelManager<TModel, TFileModel> : IModelManager<TModel>, IModelFileManager<TModel, TFileModel>
         where TModel : class, IHasFiles<TFileModel>, IHasPrimaryKey, ISoftDelete
-        where TFileModel : class, INamedFileInfo, new()
+        where TFileModel : class, INamedFileInfo, IHasPrimaryKey, new()
     {
         private const int import_queue_request_concurrency = 1;
 
@@ -315,16 +315,19 @@ namespace osu.Game.Database
         /// <remarks>
         ///  In the case of no matching files, a hash will be generated from the passed archive's <see cref="ArchiveReader.Name"/>.
         /// </remarks>
-        protected virtual string ComputeHash(TModel item, ArchiveReader reader = null)
+        protected virtual string ComputeHash(TModel item)
         {
-            if (reader != null)
+            var hashableFiles = item.Files
-                // fast hashing for cases where the item's files may not be populated.
+                                    .Where(f => HashableFileTypes.Any(ext => f.Filename.EndsWith(ext, StringComparison.OrdinalIgnoreCase)))
-                return computeHashFast(reader);
+                                    .OrderBy(f => f.Filename)
+                                    .ToArray();
 
+            if (hashableFiles.Length > 0)
+            {
                 // for now, concatenate all hashable files in the set to create a unique hash.
                 MemoryStream hashable = new MemoryStream();
 
-            foreach (TFileModel file in item.Files.Where(f => HashableFileTypes.Any(ext => f.Filename.EndsWith(ext, StringComparison.OrdinalIgnoreCase))).OrderBy(f => f.Filename))
+                foreach (TFileModel file in hashableFiles)
                 {
                     using (Stream s = Files.Store.GetStream(file.FileInfo.StoragePath))
                         s.CopyTo(hashable);
@@ -332,8 +335,9 @@ namespace osu.Game.Database
 
                 if (hashable.Length > 0)
                     return hashable.ComputeSHA2Hash();
+            }
 
-            return item.Hash;
+            return generateFallbackHash();
         }
 
         /// <summary>
@@ -393,7 +397,7 @@ namespace osu.Game.Database
                 LogForModel(item, @"Beginning import...");
 
                 item.Files = archive != null ? createFileInfos(archive, Files) : new List<TFileModel>();
-                item.Hash = ComputeHash(item, archive);
+                item.Hash = ComputeHash(item);
 
                 await Populate(item, archive, cancellationToken).ConfigureAwait(false);
 
@@ -516,10 +520,13 @@ namespace osu.Game.Database
                 {
                     Files.Dereference(file.FileInfo);
 
+                    if (file.ID > 0)
+                    {
                         // This shouldn't be required, but here for safety in case the provided TModel is not being change tracked
                         // Definitely can be removed once we rework the database backend.
                         usage.Context.Set<TFileModel>().Remove(file);
                     }
+                }
 
                 model.Files.Remove(file);
             }
@@ -540,9 +547,10 @@ namespace osu.Game.Database
                     Filename = filename,
                     FileInfo = Files.Add(contents)
                 });
-
-                Update(model);
             }
+
+            if (model.ID > 0)
+                Update(model);
         }
 
         /// <summary>
@@ -684,7 +692,7 @@ namespace osu.Game.Database
             if (hashable.Length > 0)
                 return hashable.ComputeSHA2Hash();
 
-            return reader.Name.ComputeSHA2Hash();
+            return generateFallbackHash();
         }
 
         /// <summary>
@@ -897,6 +905,14 @@ namespace osu.Game.Database
 
         #endregion
 
+        private static string generateFallbackHash()
+        {
+            // if a hash could no be generated from file content, presume a unique / new import.
+            // therefore, let's use a guaranteed unique hash.
+            // this doesn't follow the SHA2 hashing schema intentionally, so such entries on the data store can be identified.
+            return Guid.NewGuid().ToString();
+        }
+
         private string getValidFilename(string filename)
         {
             foreach (char c in Path.GetInvalidFileNameChars())

osu.Game/Migrations/20211020081609_ResetSkinHashes.cs

@@ -0,0 +1,23 @@
+// 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 Microsoft.EntityFrameworkCore.Infrastructure;
+using Microsoft.EntityFrameworkCore.Migrations;
+using osu.Game.Database;
+
+namespace osu.Game.Migrations
+{
+    [DbContext(typeof(OsuDbContext))]
+    [Migration("20211020081609_ResetSkinHashes")]
+    public partial class ResetSkinHashes : Migration
+    {
+        protected override void Up(MigrationBuilder migrationBuilder)
+        {
+            migrationBuilder.Sql($"UPDATE SkinInfo SET Hash = null");
+        }
+
+        protected override void Down(MigrationBuilder migrationBuilder)
+        {
+        }
+    }
+}

osu.Game/Online/API/Requests/Responses/APIBeatmap.cs

@@ -54,10 +54,15 @@ namespace osu.Game.Online.API.Requests.Responses
         [JsonProperty(@"accuracy")]
         private float overallDifficulty { get; set; }
 
-        public double Length => TimeSpan.FromSeconds(lengthInSeconds).TotalMilliseconds;
+        [JsonIgnore]
+        public double Length { get; set; }
 
         [JsonProperty(@"total_length")]
-        private double lengthInSeconds { get; set; }
+        private double lengthInSeconds
+        {
+            get => TimeSpan.FromMilliseconds(Length).TotalSeconds;
+            set => Length = TimeSpan.FromSeconds(value).TotalMilliseconds;
+        }
 
         [JsonProperty(@"count_circles")]
         public int CircleCount { get; set; }

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

@@ -37,7 +37,7 @@ namespace osu.Game.Rulesets.Objects.Legacy.Catch
         }
 
         protected override HitObject CreateSlider(Vector2 position, bool newCombo, int comboOffset, PathControlPoint[] controlPoints, double? length, int repeatCount,
-                                                  List<IList<HitSampleInfo>> nodeSamples)
+                                                  IList<IList<HitSampleInfo>> nodeSamples)
         {
             newCombo |= forceNewCombo;
             comboOffset += extraComboOffset;

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

@@ -408,7 +408,7 @@ namespace osu.Game.Rulesets.Objects.Legacy
         /// <param name="nodeSamples">The samples to be played when the slider nodes are hit. This includes the head and tail of the slider.</param>
         /// <returns>The hit object.</returns>
         protected abstract HitObject CreateSlider(Vector2 position, bool newCombo, int comboOffset, PathControlPoint[] controlPoints, double? length, int repeatCount,
-                                                  List<IList<HitSampleInfo>> nodeSamples);
+                                                  IList<IList<HitSampleInfo>> nodeSamples);
 
         /// <summary>
         /// Creates a legacy Spinner-type hit object.
@@ -481,7 +481,7 @@ namespace osu.Game.Rulesets.Objects.Legacy
             /// </summary>
             /// <remarks>
             /// Layered hit samples are automatically added in all modes (except osu!mania), but can be disabled
-            /// using the <see cref="LegacySkinConfiguration.LegacySetting.LayeredHitSounds"/> skin config option.
+            /// using the <see cref="SkinConfiguration.LegacySetting.LayeredHitSounds"/> skin config option.
             /// </remarks>
             public readonly bool IsLayered;
 

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

@@ -24,7 +24,7 @@ namespace osu.Game.Rulesets.Objects.Legacy
 
         public double Distance => Path.Distance;
 
-        public List<IList<HitSampleInfo>> NodeSamples { get; set; }
+        public IList<IList<HitSampleInfo>> NodeSamples { get; set; }
         public int RepeatCount { get; set; }
 
         [JsonIgnore]

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

@@ -26,7 +26,7 @@ namespace osu.Game.Rulesets.Objects.Legacy.Mania
         }
 
         protected override HitObject CreateSlider(Vector2 position, bool newCombo, int comboOffset, PathControlPoint[] controlPoints, double? length, int repeatCount,
-                                                  List<IList<HitSampleInfo>> nodeSamples)
+                                                  IList<IList<HitSampleInfo>> nodeSamples)
         {
             return new ConvertSlider
             {

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

@@ -37,7 +37,7 @@ namespace osu.Game.Rulesets.Objects.Legacy.Osu
         }
 
         protected override HitObject CreateSlider(Vector2 position, bool newCombo, int comboOffset, PathControlPoint[] controlPoints, double? length, int repeatCount,
-                                                  List<IList<HitSampleInfo>> nodeSamples)
+                                                  IList<IList<HitSampleInfo>> nodeSamples)
         {
             newCombo |= forceNewCombo;
             comboOffset += extraComboOffset;

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

@@ -23,7 +23,7 @@ namespace osu.Game.Rulesets.Objects.Legacy.Taiko
         }
 
         protected override HitObject CreateSlider(Vector2 position, bool newCombo, int comboOffset, PathControlPoint[] controlPoints, double? length, int repeatCount,
-                                                  List<IList<HitSampleInfo>> nodeSamples)
+                                                  IList<IList<HitSampleInfo>> nodeSamples)
         {
             return new ConvertSlider
             {

osu.Game/Rulesets/Objects/Types/IHasRepeats.cs

@@ -25,7 +25,7 @@ namespace osu.Game.Rulesets.Objects.Types
         /// n-1: The last repeat.<br />
         /// n: The last node.
         /// </summary>
-        List<IList<HitSampleInfo>> NodeSamples { get; }
+        IList<IList<HitSampleInfo>> NodeSamples { get; }
     }
 
     public static class HasRepeatsExtensions

osu.Game/Screens/OnlinePlay/Components/ReadyButton.cs

@@ -3,13 +3,15 @@
 
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
+using osu.Framework.Graphics.Cursor;
+using osu.Framework.Localisation;
 using osu.Game.Graphics.UserInterface;
 using osu.Game.Online;
 using osu.Game.Online.Rooms;
 
 namespace osu.Game.Screens.OnlinePlay.Components
 {
-    public abstract class ReadyButton : TriangleButton
+    public abstract class ReadyButton : TriangleButton, IHasTooltip
     {
         public new readonly BindableBool Enabled = new BindableBool();
 
@@ -24,6 +26,18 @@ namespace osu.Game.Screens.OnlinePlay.Components
             Enabled.BindValueChanged(_ => updateState(), true);
         }
 
-        private void updateState() => base.Enabled.Value = availability.Value.State == DownloadState.LocallyAvailable && Enabled.Value;
+        private void updateState() =>
+            base.Enabled.Value = availability.Value.State == DownloadState.LocallyAvailable && Enabled.Value;
+
+        public virtual LocalisableString TooltipText
+        {
+            get
+            {
+                if (Enabled.Value)
+                    return string.Empty;
+
+                return "Beatmap not downloaded";
+            }
+        }
     }
 }

osu.Game/Screens/OnlinePlay/Components/RoomLocalUserInfo.cs

@@ -14,6 +14,9 @@ namespace osu.Game.Screens.OnlinePlay.Components
     {
         private OsuSpriteText attemptDisplay;
 
+        [Resolved]
+        private OsuColour colours { get; set; }
+
         public RoomLocalUserInfo()
         {
             AutoSizeAxes = Axes.Both;
@@ -54,6 +57,9 @@ namespace osu.Game.Screens.OnlinePlay.Components
                 {
                     int remaining = MaxAttempts.Value.Value - UserScore.Value.PlaylistItemAttempts.Sum(a => a.Attempts);
                     attemptDisplay.Text += $" ({remaining} remaining)";
+
+                    if (remaining == 0)
+                        attemptDisplay.Colour = colours.RedLight;
                 }
             }
             else

osu.Game/Screens/OnlinePlay/Playlists/PlaylistsReadyButton.cs

@@ -2,8 +2,10 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using System;
+using System.Linq;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
+using osu.Framework.Localisation;
 using osu.Game.Beatmaps;
 using osu.Game.Graphics;
 using osu.Game.Online.Rooms;
@@ -16,6 +18,12 @@ namespace osu.Game.Screens.OnlinePlay.Playlists
         [Resolved(typeof(Room), nameof(Room.EndDate))]
         private Bindable<DateTimeOffset?> endDate { get; set; }
 
+        [Resolved(typeof(Room), nameof(Room.MaxAttempts))]
+        private Bindable<int?> maxAttempts { get; set; }
+
+        [Resolved(typeof(Room), nameof(Room.UserScore))]
+        private Bindable<PlaylistAggregateScore> userScore { get; set; }
+
         [Resolved]
         private IBindable<WorkingBeatmap> gameBeatmap { get; set; }
 
@@ -32,11 +40,49 @@ namespace osu.Game.Screens.OnlinePlay.Playlists
             Triangles.ColourLight = colours.GreenLight;
         }
 
+        private bool hasRemainingAttempts = true;
+
+        protected override void LoadComplete()
+        {
+            base.LoadComplete();
+
+            userScore.BindValueChanged(aggregate =>
+            {
+                if (maxAttempts.Value == null)
+                    return;
+
+                int remaining = maxAttempts.Value.Value - aggregate.NewValue.PlaylistItemAttempts.Sum(a => a.Attempts);
+
+                hasRemainingAttempts = remaining > 0;
+            });
+        }
+
         protected override void Update()
         {
             base.Update();
 
-            Enabled.Value = endDate.Value != null && DateTimeOffset.UtcNow.AddSeconds(30).AddMilliseconds(gameBeatmap.Value.Track.Length) < endDate.Value;
+            Enabled.Value = hasRemainingAttempts && enoughTimeLeft;
+        }
+
+        public override LocalisableString TooltipText
+        {
+            get
+            {
+                if (Enabled.Value)
+                    return string.Empty;
+
+                if (!enoughTimeLeft)
+                    return "No time left!";
+
+                if (!hasRemainingAttempts)
+                    return "Attempts exhausted!";
+
+                return base.TooltipText;
             }
         }
+
+        private bool enoughTimeLeft =>
+            // This should probably consider the length of the currently selected item, rather than a constant 30 seconds.
+            endDate.Value != null && DateTimeOffset.UtcNow.AddSeconds(30).AddMilliseconds(gameBeatmap.Value.Track.Length) < endDate.Value;
+    }
 }

osu.Game/Screens/OnlinePlay/Playlists/PlaylistsRoomSubScreen.cs

@@ -37,6 +37,8 @@ namespace osu.Game.Screens.OnlinePlay.Playlists
         private MatchLeaderboard leaderboard;
         private SelectionPollingComponent selectionPollingComponent;
 
+        private FillFlowContainer progressSection;
+
         public PlaylistsRoomSubScreen(Room room)
             : base(room, false) // Editing is temporarily not allowed.
         {
@@ -67,6 +69,8 @@ namespace osu.Game.Screens.OnlinePlay.Playlists
                     Schedule(() => SelectedItem.Value = Room.Playlist.FirstOrDefault());
                 }
             }, true);
+
+            Room.MaxAttempts.BindValueChanged(attempts => progressSection.Alpha = Room.MaxAttempts.Value != null ? 1 : 0, true);
         }
 
         protected override Drawable CreateMainContent() => new GridContainer
@@ -153,6 +157,22 @@ namespace osu.Game.Screens.OnlinePlay.Playlists
                                 },
                             },
                             new Drawable[]
+                            {
+                                progressSection = new FillFlowContainer
+                                {
+                                    RelativeSizeAxes = Axes.X,
+                                    AutoSizeAxes = Axes.Y,
+                                    Alpha = 0,
+                                    Margin = new MarginPadding { Bottom = 10 },
+                                    Direction = FillDirection.Vertical,
+                                    Children = new Drawable[]
+                                    {
+                                        new OverlinedHeader("Progress"),
+                                        new RoomLocalUserInfo(),
+                                    }
+                                },
+                            },
+                            new Drawable[]
                             {
                                 new OverlinedHeader("Leaderboard")
                             },
@@ -162,6 +182,7 @@ namespace osu.Game.Screens.OnlinePlay.Playlists
                         },
                         RowDimensions = new[]
                         {
+                            new Dimension(GridSizeMode.AutoSize),
                             new Dimension(GridSizeMode.AutoSize),
                             new Dimension(GridSizeMode.AutoSize),
                             new Dimension(),

osu.Game/Skinning/DefaultLegacySkin.cs

@@ -19,7 +19,14 @@ namespace osu.Game.Skinning
 
         [UsedImplicitly(ImplicitUseKindFlags.InstantiatedWithFixedConstructorSignature)]
         public DefaultLegacySkin(SkinInfo skin, IStorageResourceProvider resources)
-            : base(skin, new NamespacedResourceStore<byte[]>(resources.Resources, "Skins/Legacy"), resources, string.Empty)
+            : base(
+                skin,
+                new NamespacedResourceStore<byte[]>(resources.Resources, "Skins/Legacy"),
+                resources,
+                // A default legacy skin may still have a skin.ini if it is modified by the user.
+                // We must specify the stream directly as we are redirecting storage to the osu-resources location for other files.
+                new LegacySkinResourceStore<SkinFileInfo>(skin, resources.Files).GetStream("skin.ini")
+            )
         {
             Configuration.CustomColours["SliderBall"] = new Color4(2, 170, 255, 255);
             Configuration.CustomComboColours = new List<Color4>

osu.Game/Skinning/DefaultSkin.cs

@@ -35,7 +35,6 @@ namespace osu.Game.Skinning
             : base(skin, resources)
         {
             this.resources = resources;
-            Configuration = new DefaultSkinConfiguration();
         }
 
         public override Texture GetTexture(string componentName, WrapMode wrapModeS, WrapMode wrapModeT) => null;

osu.Game/Skinning/DefaultSkinConfiguration.cs

@@ -1,12 +0,0 @@
-// 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.
-
-namespace osu.Game.Skinning
-{
-    /// <summary>
-    /// A skin configuration pre-populated with sane defaults.
-    /// </summary>
-    public class DefaultSkinConfiguration : SkinConfiguration
-    {
-    }
-}

osu.Game/Skinning/LegacyBeatmapSkin.cs

@@ -50,7 +50,7 @@ namespace osu.Game.Skinning
         {
             switch (lookup)
             {
-                case LegacySkinConfiguration.LegacySetting s when s == LegacySkinConfiguration.LegacySetting.Version:
+                case SkinConfiguration.LegacySetting s when s == SkinConfiguration.LegacySetting.Version:
                     // For lookup simplicity, ignore beatmap-level versioning completely.
 
                     // If it is decided that we need this due to beatmaps somehow using it, the default (1.0 specified in LegacySkinDecoder.CreateTemplateObject)

osu.Game/Skinning/LegacySkin.cs

@@ -47,12 +47,6 @@ namespace osu.Game.Skinning
         /// </summary>
         protected virtual bool UseCustomSampleBanks => false;
 
-        public new LegacySkinConfiguration Configuration
-        {
-            get => base.Configuration as LegacySkinConfiguration;
-            set => base.Configuration = value;
-        }
-
         private readonly Dictionary<int, LegacyManiaSkinConfiguration> maniaConfigurations = new Dictionary<int, LegacyManiaSkinConfiguration>();
 
         [UsedImplicitly(ImplicitUseKindFlags.InstantiatedWithFixedConstructorSignature)]
@@ -69,29 +63,20 @@ namespace osu.Game.Skinning
         /// <param name="resources">Access to raw game resources.</param>
         /// <param name="configurationFilename">The user-facing filename of the configuration file to be parsed. Can accept an .osu or skin.ini file.</param>
         protected LegacySkin(SkinInfo skin, [CanBeNull] IResourceStore<byte[]> storage, [CanBeNull] IStorageResourceProvider resources, string configurationFilename)
-            : base(skin, resources)
+            : this(skin, storage, resources, storage?.GetStream(configurationFilename))
         {
-            using (var stream = storage?.GetStream(configurationFilename))
-            {
-                if (stream != null)
-                {
-                    using (LineBufferedReader reader = new LineBufferedReader(stream, true))
-                        Configuration = new LegacySkinDecoder().Decode(reader);
-
-                    stream.Seek(0, SeekOrigin.Begin);
-
-                    using (LineBufferedReader reader = new LineBufferedReader(stream))
-                    {
-                        var maniaList = new LegacyManiaSkinDecoder().Decode(reader);
-
-                        foreach (var config in maniaList)
-                            maniaConfigurations[config.Keys] = config;
-                    }
-                }
-                else
-                    Configuration = new LegacySkinConfiguration();
         }
 
+        /// <summary>
+        /// Construct a new legacy skin instance.
+        /// </summary>
+        /// <param name="skin">The model for this skin.</param>
+        /// <param name="storage">A storage for looking up files within this skin using user-facing filenames.</param>
+        /// <param name="resources">Access to raw game resources.</param>
+        /// <param name="configurationStream">An optional stream containing the contents of a skin.ini file.</param>
+        protected LegacySkin(SkinInfo skin, [CanBeNull] IResourceStore<byte[]> storage, [CanBeNull] IStorageResourceProvider resources, [CanBeNull] Stream configurationStream)
+            : base(skin, resources, configurationStream)
+        {
             if (storage != null)
             {
                 var samples = resources?.AudioManager?.GetSampleStore(storage);
@@ -110,6 +95,21 @@ namespace osu.Game.Skinning
                 true) != null);
         }
 
+        protected override void ParseConfigurationStream(Stream stream)
+        {
+            base.ParseConfigurationStream(stream);
+
+            stream.Seek(0, SeekOrigin.Begin);
+
+            using (LineBufferedReader reader = new LineBufferedReader(stream))
+            {
+                var maniaList = new LegacyManiaSkinDecoder().Decode(reader);
+
+                foreach (var config in maniaList)
+                    maniaConfigurations[config.Keys] = config;
+            }
+        }
+
         public override IBindable<TValue> GetConfig<TLookup, TValue>(TLookup lookup)
         {
             switch (lookup)
@@ -146,7 +146,7 @@ namespace osu.Game.Skinning
 
                     break;
 
-                case LegacySkinConfiguration.LegacySetting legacy:
+                case SkinConfiguration.LegacySetting legacy:
                     return legacySettingLookup<TValue>(legacy);
 
                 default:
@@ -189,7 +189,7 @@ namespace osu.Game.Skinning
                 case LegacyManiaSkinConfigurationLookups.ExplosionScale:
                     Debug.Assert(maniaLookup.TargetColumn != null);
 
-                    if (GetConfig<LegacySkinConfiguration.LegacySetting, decimal>(LegacySkinConfiguration.LegacySetting.Version)?.Value < 2.5m)
+                    if (GetConfig<SkinConfiguration.LegacySetting, decimal>(SkinConfiguration.LegacySetting.Version)?.Value < 2.5m)
                         return SkinUtils.As<TValue>(new Bindable<float>(1));
 
                     if (existing.ExplosionWidth[maniaLookup.TargetColumn.Value] != 0)
@@ -236,7 +236,7 @@ namespace osu.Game.Skinning
                 case LegacyManiaSkinConfigurationLookups.HoldNoteLightScale:
                     Debug.Assert(maniaLookup.TargetColumn != null);
 
-                    if (GetConfig<LegacySkinConfiguration.LegacySetting, decimal>(LegacySkinConfiguration.LegacySetting.Version)?.Value < 2.5m)
+                    if (GetConfig<SkinConfiguration.LegacySetting, decimal>(SkinConfiguration.LegacySetting.Version)?.Value < 2.5m)
                         return SkinUtils.As<TValue>(new Bindable<float>(1));
 
                     if (existing.HoldNoteLightWidth[maniaLookup.TargetColumn.Value] != 0)
@@ -309,15 +309,15 @@ namespace osu.Game.Skinning
             => source.ImageLookups.TryGetValue(lookup, out string image) ? new Bindable<string>(image) : null;
 
         [CanBeNull]
-        private IBindable<TValue> legacySettingLookup<TValue>(LegacySkinConfiguration.LegacySetting legacySetting)
+        private IBindable<TValue> legacySettingLookup<TValue>(SkinConfiguration.LegacySetting legacySetting)
         {
             switch (legacySetting)
             {
-                case LegacySkinConfiguration.LegacySetting.Version:
+                case SkinConfiguration.LegacySetting.Version:
-                    return SkinUtils.As<TValue>(new Bindable<decimal>(Configuration.LegacyVersion ?? LegacySkinConfiguration.LATEST_VERSION));
+                    return SkinUtils.As<TValue>(new Bindable<decimal>(Configuration.LegacyVersion ?? SkinConfiguration.LATEST_VERSION));
 
                 default:
-                    return genericLookup<LegacySkinConfiguration.LegacySetting, TValue>(legacySetting);
+                    return genericLookup<SkinConfiguration.LegacySetting, TValue>(legacySetting);
             }
         }
 

osu.Game/Skinning/LegacySkinConfiguration.cs

@@ -1,28 +0,0 @@
-// 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.
-
-namespace osu.Game.Skinning
-{
-    public class LegacySkinConfiguration : SkinConfiguration
-    {
-        public const decimal LATEST_VERSION = 2.7m;
-
-        /// <summary>
-        /// Legacy version of this skin.
-        /// </summary>
-        public decimal? LegacyVersion { get; internal set; }
-
-        public enum LegacySetting
-        {
-            Version,
-            ComboPrefix,
-            ComboOverlap,
-            ScorePrefix,
-            ScoreOverlap,
-            HitCirclePrefix,
-            HitCircleOverlap,
-            AnimationFramerate,
-            LayeredHitSounds
-        }
-    }
-}

osu.Game/Skinning/LegacySkinDecoder.cs

@@ -6,14 +6,14 @@ using osu.Game.Beatmaps.Formats;
 
 namespace osu.Game.Skinning
 {
-    public class LegacySkinDecoder : LegacyDecoder<LegacySkinConfiguration>
+    public class LegacySkinDecoder : LegacyDecoder<SkinConfiguration>
     {
         public LegacySkinDecoder()
             : base(1)
         {
         }
 
-        protected override void ParseLine(LegacySkinConfiguration skin, Section section, string line)
+        protected override void ParseLine(SkinConfiguration skin, Section section, string line)
         {
             if (section != Section.Colours)
             {
@@ -34,7 +34,7 @@ namespace osu.Game.Skinning
 
                             case @"Version":
                                 if (pair.Value == "latest")
-                                    skin.LegacyVersion = LegacySkinConfiguration.LATEST_VERSION;
+                                    skin.LegacyVersion = SkinConfiguration.LATEST_VERSION;
                                 else if (decimal.TryParse(pair.Value, NumberStyles.AllowDecimalPoint, CultureInfo.InvariantCulture, out decimal version))
                                     skin.LegacyVersion = version;
 
@@ -57,7 +57,7 @@ namespace osu.Game.Skinning
             base.ParseLine(skin, section, line);
         }
 
-        protected override LegacySkinConfiguration CreateTemplateObject()
+        protected override SkinConfiguration CreateTemplateObject()
         {
             var config = base.CreateTemplateObject();
             config.LegacyVersion = 1.0m;

osu.Game/Skinning/LegacySkinExtensions.cs

@@ -12,7 +12,7 @@ using osu.Framework.Graphics.Animations;
 using osu.Framework.Graphics.OpenGL.Textures;
 using osu.Framework.Graphics.Sprites;
 using osu.Framework.Graphics.Textures;
-using static osu.Game.Skinning.LegacySkinConfiguration;
+using static osu.Game.Skinning.SkinConfiguration;
 
 namespace osu.Game.Skinning
 {

osu.Game/Skinning/LegacySkinTransformer.cs

@@ -10,7 +10,7 @@ using osu.Framework.Graphics.OpenGL.Textures;
 using osu.Framework.Graphics.Textures;
 using osu.Game.Audio;
 using osu.Game.Rulesets.Objects.Legacy;
-using static osu.Game.Skinning.LegacySkinConfiguration;
+using static osu.Game.Skinning.SkinConfiguration;
 
 namespace osu.Game.Skinning
 {

osu.Game/Skinning/Skin.cs

@@ -3,8 +3,10 @@
 
 using System;
 using System.Collections.Generic;
+using System.IO;
 using System.Linq;
 using System.Text;
+using JetBrains.Annotations;
 using Newtonsoft.Json;
 using osu.Framework.Audio.Sample;
 using osu.Framework.Bindables;
@@ -21,8 +23,9 @@ namespace osu.Game.Skinning
     public abstract class Skin : IDisposable, ISkin
     {
         public readonly SkinInfo SkinInfo;
+        private readonly IStorageResourceProvider resources;
 
-        public SkinConfiguration Configuration { get; protected set; }
+        public SkinConfiguration Configuration { get; set; }
 
         public IDictionary<SkinnableTarget, SkinnableInfo[]> DrawableComponentInfo => drawableComponentInfo;
 
@@ -36,9 +39,18 @@ namespace osu.Game.Skinning
 
         public abstract IBindable<TValue> GetConfig<TLookup, TValue>(TLookup lookup);
 
-        protected Skin(SkinInfo skin, IStorageResourceProvider resources)
+        protected Skin(SkinInfo skin, IStorageResourceProvider resources, [CanBeNull] Stream configurationStream = null)
         {
             SkinInfo = skin;
+            this.resources = resources;
+
+            configurationStream ??= getConfigurationStream();
+
+            if (configurationStream != null)
+                // stream will be closed after use by LineBufferedReader.
+                ParseConfigurationStream(configurationStream);
+            else
+                Configuration = new SkinConfiguration();
 
             // we may want to move this to some kind of async operation in the future.
             foreach (SkinnableTarget skinnableTarget in Enum.GetValues(typeof(SkinnableTarget)))
@@ -73,6 +85,22 @@ namespace osu.Game.Skinning
             }
         }
 
+        protected virtual void ParseConfigurationStream(Stream stream)
+        {
+            using (LineBufferedReader reader = new LineBufferedReader(stream, true))
+                Configuration = new LegacySkinDecoder().Decode(reader);
+        }
+
+        private Stream getConfigurationStream()
+        {
+            string path = SkinInfo.Files.SingleOrDefault(f => f.Filename == "skin.ini")?.FileInfo.StoragePath;
+
+            if (string.IsNullOrEmpty(path))
+                return null;
+
+            return resources?.Files.GetStream(path);
+        }
+
         /// <summary>
         /// Remove all stored customisations for the provided target.
         /// </summary>

osu.Game/Skinning/SkinConfiguration.cs

@@ -14,11 +14,31 @@ namespace osu.Game.Skinning
     {
         public readonly SkinInfo SkinInfo = new SkinInfo();
 
+        public const decimal LATEST_VERSION = 2.7m;
+
         /// <summary>
         /// Whether to allow <see cref="DefaultComboColours"/> as a fallback list for when no combo colours are provided.
         /// </summary>
         internal bool AllowDefaultComboColoursFallback = true;
 
+        /// <summary>
+        /// Legacy version of this skin.
+        /// </summary>
+        public decimal? LegacyVersion { get; internal set; }
+
+        public enum LegacySetting
+        {
+            Version,
+            ComboPrefix,
+            ComboOverlap,
+            ScorePrefix,
+            ScoreOverlap,
+            HitCirclePrefix,
+            HitCircleOverlap,
+            AnimationFramerate,
+            LayeredHitSounds
+        }
+
         public static List<Color4> DefaultComboColours { get; } = new List<Color4>
         {
             new Color4(255, 192, 0, 255),

osu.Game/Skinning/SkinInfo.cs

@@ -18,12 +18,12 @@ namespace osu.Game.Skinning
 
         public int ID { get; set; }
 
-        public string Name { get; set; }
+        public string Name { get; set; } = string.Empty;
+
+        public string Creator { get; set; } = string.Empty;
 
         public string Hash { get; set; }
 
-        public string Creator { get; set; }
-
         public string InstantiationInfo { get; set; }
 
         public virtual Skin CreateInstance(IStorageResourceProvider resources)

osu.Game/Skinning/SkinManager.cs

@@ -14,11 +14,11 @@ using Newtonsoft.Json;
 using osu.Framework.Audio;
 using osu.Framework.Audio.Sample;
 using osu.Framework.Bindables;
-using osu.Framework.Extensions;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.OpenGL.Textures;
 using osu.Framework.Graphics.Textures;
 using osu.Framework.IO.Stores;
+using osu.Framework.Logging;
 using osu.Framework.Platform;
 using osu.Framework.Testing;
 using osu.Framework.Utils;
@@ -51,7 +51,7 @@ namespace osu.Game.Skinning
 
         public override IEnumerable<string> HandledExtensions => new[] { ".osk" };
 
-        protected override string[] HashableFileTypes => new[] { ".ini" };
+        protected override string[] HashableFileTypes => new[] { ".ini", ".json" };
 
         protected override string ImportFromStablePath => "Skins";
 
@@ -85,6 +85,27 @@ namespace osu.Game.Skinning
 
                 SourceChanged?.Invoke();
             };
+
+            // can be removed 20220420.
+            populateMissingHashes();
+        }
+
+        private void populateMissingHashes()
+        {
+            var skinsWithoutHashes = ModelStore.ConsumableItems.Where(i => i.Hash == null).ToArray();
+
+            foreach (SkinInfo skin in skinsWithoutHashes)
+            {
+                try
+                {
+                    Update(skin);
+                }
+                catch (Exception e)
+                {
+                    Delete(skin);
+                    Logger.Error(e, $"Existing skin {skin} has been deleted during hash recomputation due to being invalid");
+                }
+            }
         }
 
         protected override bool ShouldDeleteArchive(string path) => Path.GetExtension(path)?.ToLowerInvariant() == ".osk";
@@ -128,28 +149,118 @@ namespace osu.Game.Skinning
             CurrentSkinInfo.Value = ModelStore.ConsumableItems.Single(i => i.ID == chosen.ID);
         }
 
-        protected override SkinInfo CreateModel(ArchiveReader archive) => new SkinInfo { Name = archive.Name };
+        protected override SkinInfo CreateModel(ArchiveReader archive) => new SkinInfo { Name = archive.Name ?? "No name" };
 
         private const string unknown_creator_string = "Unknown";
 
         protected override bool HasCustomHashFunction => true;
 
-        protected override string ComputeHash(SkinInfo item, ArchiveReader reader = null)
+        protected override string ComputeHash(SkinInfo item)
         {
             var instance = GetSkin(item);
 
-            // in the case the skin has a skin.ini file, we are going to create a hash based on that.
+            // This function can be run on fresh import or save. The logic here ensures a skin.ini file is in a good state for both operations.
-            // we don't want to do this in the case we don't have a skin.ini, as it would match only on the filename portion,
-            // causing potentially unique skin imports to be considered as a duplicate.
-            if (!string.IsNullOrEmpty(instance.Configuration.SkinInfo.Name))
-            {
-                // we need to populate early to create a hash based off skin.ini contents
-                populateMetadata(item, instance, reader?.Name);
 
-                return item.ToString().ComputeSHA2Hash();
+            // `Skin` will parse the skin.ini and populate `Skin.Configuration` during construction above.
+            string skinIniSourcedName = instance.Configuration.SkinInfo.Name;
+            string skinIniSourcedCreator = instance.Configuration.SkinInfo.Creator;
+            string archiveName = item.Name.Replace(".osk", "", StringComparison.OrdinalIgnoreCase);
+
+            bool isImport = item.ID == 0;
+
+            if (isImport)
+            {
+                item.Name = !string.IsNullOrEmpty(skinIniSourcedName) ? skinIniSourcedName : archiveName;
+                item.Creator = !string.IsNullOrEmpty(skinIniSourcedCreator) ? skinIniSourcedCreator : unknown_creator_string;
+
+                // For imports, we want to use the archive or folder name as part of the metadata, in addition to any existing skin.ini metadata.
+                // In an ideal world, skin.ini would be the only source of metadata, but a lot of skin creators and users don't update it when making modifications.
+                // In both of these cases, the expectation from the user is that the filename or folder name is displayed somewhere to identify the skin.
+                if (archiveName != item.Name)
+                    item.Name = $"{item.Name} [{archiveName}]";
             }
 
-            return base.ComputeHash(item, reader);
+            // By this point, the metadata in SkinInfo will be correct.
+            // Regardless of whether this is an import or not, let's write the skin.ini if non-existing or non-matching.
+            // This is (weirdly) done inside ComputeHash to avoid adding a new method to handle this case. After switching to realm it can be moved into another place.
+            if (skinIniSourcedName != item.Name)
+                updateSkinIniMetadata(item);
+
+            return base.ComputeHash(item);
+        }
+
+        private void updateSkinIniMetadata(SkinInfo item)
+        {
+            string nameLine = $"Name: {item.Name}";
+            string authorLine = $"Author: {item.Creator}";
+
+            var existingFile = item.Files.SingleOrDefault(f => f.Filename == "skin.ini");
+
+            if (existingFile != null)
+            {
+                List<string> outputLines = new List<string>();
+
+                bool addedName = false;
+                bool addedAuthor = false;
+
+                using (var stream = Files.Storage.GetStream(existingFile.FileInfo.StoragePath))
+                using (var sr = new StreamReader(stream))
+                {
+                    string line;
+
+                    while ((line = sr.ReadLine()) != null)
+                    {
+                        if (line.StartsWith("Name:", StringComparison.Ordinal))
+                        {
+                            outputLines.Add(nameLine);
+                            addedName = true;
+                        }
+                        else if (line.StartsWith("Author:", StringComparison.Ordinal))
+                        {
+                            outputLines.Add(authorLine);
+                            addedAuthor = true;
+                        }
+                        else
+                            outputLines.Add(line);
+                    }
+                }
+
+                if (!addedName || !addedAuthor)
+                {
+                    outputLines.AddRange(new[]
+                    {
+                        "[General]",
+                        nameLine,
+                        authorLine,
+                    });
+                }
+
+                using (Stream stream = new MemoryStream())
+                {
+                    using (var sw = new StreamWriter(stream, Encoding.UTF8, 1024, true))
+                    {
+                        foreach (string line in outputLines)
+                            sw.WriteLine(line);
+                    }
+
+                    ReplaceFile(item, existingFile, stream);
+                }
+            }
+            else
+            {
+                using (Stream stream = new MemoryStream())
+                {
+                    using (var sw = new StreamWriter(stream, Encoding.UTF8, 1024, true))
+                    {
+                        sw.WriteLine("[General]");
+                        sw.WriteLine(nameLine);
+                        sw.WriteLine(authorLine);
+                        sw.WriteLine("Version: latest");
+                    }
+
+                    AddFile(item, stream, "skin.ini");
+                }
+            }
         }
 
         protected override Task Populate(SkinInfo model, ArchiveReader archive, CancellationToken cancellationToken = default)
@@ -158,32 +269,12 @@ namespace osu.Game.Skinning
 
             model.InstantiationInfo ??= instance.GetType().GetInvariantInstantiationInfo();
 
-            populateMetadata(model, instance, archive?.Name);
+            model.Name = instance.Configuration.SkinInfo.Name;
+            model.Creator = instance.Configuration.SkinInfo.Creator;
 
             return Task.CompletedTask;
         }
 
-        private void populateMetadata(SkinInfo item, Skin instance, string archiveName)
-        {
-            if (!string.IsNullOrEmpty(instance.Configuration.SkinInfo.Name))
-            {
-                item.Name = instance.Configuration.SkinInfo.Name;
-                item.Creator = instance.Configuration.SkinInfo.Creator;
-            }
-            else
-            {
-                item.Name = item.Name.Replace(".osk", "", StringComparison.OrdinalIgnoreCase);
-                item.Creator ??= unknown_creator_string;
-            }
-
-            // generally when importing from a folder, the ".osk" extension will not be present.
-            // if we ever need a more reliable method of determining this, the type of `ArchiveReader` can be checked.
-            bool isArchiveImport = archiveName?.Contains(".osk", StringComparison.OrdinalIgnoreCase) == true;
-
-            if (archiveName != null && !isArchiveImport && archiveName != item.Name)
-                item.Name = $"{item.Name} [{archiveName}]";
-        }
-
         /// <summary>
         /// Retrieve a <see cref="Skin"/> instance for the provided <see cref="SkinInfo"/>
         /// </summary>

osu.Game/Tests/Visual/EditorTestScene.cs

@@ -10,7 +10,6 @@ using osu.Framework.Platform;
 using osu.Framework.Testing;
 using osu.Game.Beatmaps;
 using osu.Game.Database;
-using osu.Game.IO.Archives;
 using osu.Game.Online.API;
 using osu.Game.Rulesets;
 using osu.Game.Rulesets.Edit;
@@ -154,7 +153,7 @@ namespace osu.Game.Tests.Visual
                 {
                 }
 
-                protected override string ComputeHash(BeatmapSetInfo item, ArchiveReader reader = null)
+                protected override string ComputeHash(BeatmapSetInfo item)
                     => string.Empty;
             }
 

osu.Game/Tests/Visual/Multiplayer/IMultiplayerTestSceneDependencies.cs

@@ -22,7 +22,7 @@ namespace osu.Game.Tests.Visual.Multiplayer
         /// <summary>
         /// The cached <see cref="IRoomManager"/>.
         /// </summary>
-        new TestRequestHandlingMultiplayerRoomManager RoomManager { get; }
+        new TestMultiplayerRoomManager RoomManager { get; }
 
         /// <summary>
         /// The cached <see cref="UserLookupCache"/>.

osu.Game/Tests/Visual/Multiplayer/MultiplayerTestScene.cs

@@ -18,7 +18,7 @@ namespace osu.Game.Tests.Visual.Multiplayer
         public const int PLAYER_2_ID = 56;
 
         public TestMultiplayerClient Client => OnlinePlayDependencies.Client;
-        public new TestRequestHandlingMultiplayerRoomManager RoomManager => OnlinePlayDependencies.RoomManager;
+        public new TestMultiplayerRoomManager RoomManager => OnlinePlayDependencies.RoomManager;
         public TestUserLookupCache LookupCache => OnlinePlayDependencies?.LookupCache;
         public TestSpectatorClient SpectatorClient => OnlinePlayDependencies?.SpectatorClient;
 
@@ -35,12 +35,7 @@ namespace osu.Game.Tests.Visual.Multiplayer
         public new void Setup() => Schedule(() =>
         {
             if (joinRoom)
-            {
+                SelectedRoom.Value = CreateRoom();
-                var room = CreateRoom();
-
-                RoomManager.CreateRoom(room);
-                SelectedRoom.Value = room;
-            }
         });
 
         protected virtual Room CreateRoom()
@@ -64,8 +59,11 @@ namespace osu.Game.Tests.Visual.Multiplayer
             base.SetUpSteps();
 
             if (joinRoom)
+            {
+                AddStep("join room", () => RoomManager.CreateRoom(SelectedRoom.Value));
                 AddUntilStep("wait for room join", () => Client.Room != null);
             }
+        }
 
         protected override OnlinePlayTestSceneDependencies CreateOnlinePlayDependencies() => new MultiplayerTestSceneDependencies();
     }

osu.Game/Tests/Visual/Multiplayer/MultiplayerTestSceneDependencies.cs

@@ -18,7 +18,7 @@ namespace osu.Game.Tests.Visual.Multiplayer
         public TestMultiplayerClient Client { get; }
         public TestUserLookupCache LookupCache { get; }
         public TestSpectatorClient SpectatorClient { get; }
-        public new TestRequestHandlingMultiplayerRoomManager RoomManager => (TestRequestHandlingMultiplayerRoomManager)base.RoomManager;
+        public new TestMultiplayerRoomManager RoomManager => (TestMultiplayerRoomManager)base.RoomManager;
 
         public MultiplayerTestSceneDependencies()
         {
@@ -31,7 +31,7 @@ namespace osu.Game.Tests.Visual.Multiplayer
             CacheAs<SpectatorClient>(SpectatorClient);
         }
 
-        protected override IRoomManager CreateRoomManager() => new TestRequestHandlingMultiplayerRoomManager();
+        protected override IRoomManager CreateRoomManager() => new TestMultiplayerRoomManager();
 
         protected virtual TestSpectatorClient CreateSpectatorClient() => new TestSpectatorClient();
     }

osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs

@@ -39,9 +39,9 @@ namespace osu.Game.Tests.Visual.Multiplayer
         [Resolved]
         private BeatmapManager beatmaps { get; set; } = null!;
 
-        private readonly TestRequestHandlingMultiplayerRoomManager roomManager;
+        private readonly TestMultiplayerRoomManager roomManager;
 
-        public TestMultiplayerClient(TestRequestHandlingMultiplayerRoomManager roomManager)
+        public TestMultiplayerClient(TestMultiplayerRoomManager roomManager)
         {
             this.roomManager = roomManager;
         }

osu.Game/Tests/Visual/Multiplayer/TestMultiplayerRoomManager.cs

@@ -3,7 +3,6 @@
 
 using System.Collections.Generic;
 using osu.Framework.Allocation;
-using osu.Game.Online.API;
 using osu.Game.Online.Rooms;
 using osu.Game.Screens.OnlinePlay.Components;
 using osu.Game.Screens.OnlinePlay.Multiplayer;
@@ -12,25 +11,20 @@ using osu.Game.Tests.Visual.OnlinePlay;
 namespace osu.Game.Tests.Visual.Multiplayer
 {
     /// <summary>
-    /// A <see cref="RoomManager"/> for use in multiplayer test scenes, backed by a <see cref="TestRoomRequestsHandler"/>.
+    /// A <see cref="RoomManager"/> for use in multiplayer test scenes.
     /// Should generally not be used by itself outside of a <see cref="MultiplayerTestScene"/>.
     /// </summary>
-    public class TestRequestHandlingMultiplayerRoomManager : MultiplayerRoomManager
+    public class TestMultiplayerRoomManager : MultiplayerRoomManager
     {
-        public IReadOnlyList<Room> ServerSideRooms => handler.ServerSideRooms;
+        [Resolved]
+        private TestRoomRequestsHandler requestsHandler { get; set; }
 
-        private readonly TestRoomRequestsHandler handler = new TestRoomRequestsHandler();
+        public IReadOnlyList<Room> ServerSideRooms => requestsHandler.ServerSideRooms;
-
-        [BackgroundDependencyLoader]
-        private void load(IAPIProvider api, OsuGameBase game)
-        {
-            ((DummyAPIAccess)api).HandleRequest = request => handler.HandleRequest(request, api.LocalUser.Value, game);
-        }
 
         /// <summary>
         /// Adds a room to a local "server-side" list that's returned when a <see cref="GetRoomsRequest"/> is fired.
         /// </summary>
         /// <param name="room">The room.</param>
-        public void AddServerSideRoom(Room room) => handler.AddServerSideRoom(room);
+        public void AddServerSideRoom(Room room) => requestsHandler.AddServerSideRoom(room);
     }
 }

osu.Game/Tests/Visual/OnlinePlay/OnlinePlayTestScene.cs

@@ -7,6 +7,7 @@ using osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
+using osu.Game.Online.API;
 using osu.Game.Online.Rooms;
 using osu.Game.Screens.OnlinePlay;
 
@@ -27,11 +28,14 @@ namespace osu.Game.Tests.Visual.OnlinePlay
         /// </summary>
         protected OnlinePlayTestSceneDependencies OnlinePlayDependencies => dependencies?.OnlinePlayDependencies;
 
-        private DelegatedDependencyContainer dependencies;
-
         protected override Container<Drawable> Content => content;
+
+        [Resolved]
+        private OsuGameBase game { get; set; }
+
         private readonly Container content;
         private readonly Container drawableDependenciesContainer;
+        private DelegatedDependencyContainer dependencies;
 
         protected OnlinePlayTestScene()
         {
@@ -57,6 +61,12 @@ namespace osu.Game.Tests.Visual.OnlinePlay
             drawableDependenciesContainer.AddRange(OnlinePlayDependencies.DrawableComponents);
         });
 
+        public override void SetUpSteps()
+        {
+            base.SetUpSteps();
+            AddStep("setup API", () => ((DummyAPIAccess)API).HandleRequest = request => OnlinePlayDependencies.RequestsHandler.HandleRequest(request, API.LocalUser.Value, game));
+        }
+
         /// <summary>
         /// Creates the room dependencies. Called every <see cref="Setup"/>.
         /// </summary>

osu.Game/Tests/Visual/OnlinePlay/OnlinePlayTestSceneDependencies.cs

@@ -21,6 +21,7 @@ namespace osu.Game.Tests.Visual.OnlinePlay
         public IRoomManager RoomManager { get; }
         public OngoingOperationTracker OngoingOperationTracker { get; }
         public OnlinePlayBeatmapAvailabilityTracker AvailabilityTracker { get; }
+        public TestRoomRequestsHandler RequestsHandler { get; }
 
         /// <summary>
         /// All cached dependencies which are also <see cref="Drawable"/> components.
@@ -36,9 +37,11 @@ namespace osu.Game.Tests.Visual.OnlinePlay
             RoomManager = CreateRoomManager();
             OngoingOperationTracker = new OngoingOperationTracker();
             AvailabilityTracker = new OnlinePlayBeatmapAvailabilityTracker();
+            RequestsHandler = new TestRoomRequestsHandler();
 
             dependencies = new DependencyContainer(new CachedModelDependencyContainer<Room>(null) { Model = { BindTarget = SelectedRoom } });
 
+            CacheAs(RequestsHandler);
             CacheAs(SelectedRoom);
             CacheAs(RoomManager);
             CacheAs(OngoingOperationTracker);
@@ -71,6 +74,6 @@ namespace osu.Game.Tests.Visual.OnlinePlay
                 drawableComponents.Add(drawable);
         }
 
-        protected virtual IRoomManager CreateRoomManager() => new TestRequestHandlingRoomManager();
+        protected virtual IRoomManager CreateRoomManager() => new TestRoomManager();
     }
 }

osu.Game/Tests/Visual/OnlinePlay/TestRoomManager.cs

@@ -2,9 +2,7 @@
 // See the LICENCE file in the repository root for full licence text.
 
 using System;
-using osu.Framework.Allocation;
 using osu.Game.Beatmaps;
-using osu.Game.Online.API;
 using osu.Game.Online.Rooms;
 using osu.Game.Rulesets;
 using osu.Game.Screens.OnlinePlay.Components;
@@ -15,20 +13,12 @@ namespace osu.Game.Tests.Visual.OnlinePlay
     /// <summary>
     /// A very simple <see cref="RoomManager"/> for use in online play test scenes.
     /// </summary>
-    public class TestRequestHandlingRoomManager : RoomManager
+    public class TestRoomManager : RoomManager
     {
         public Action<Room, string> JoinRoomRequested;
 
         private int currentRoomId;
 
-        private readonly TestRoomRequestsHandler handler = new TestRoomRequestsHandler();
-
-        [BackgroundDependencyLoader]
-        private void load(IAPIProvider api, OsuGameBase game)
-        {
-            ((DummyAPIAccess)api).HandleRequest = request => handler.HandleRequest(request, api.LocalUser.Value, game);
-        }
-
         public override void JoinRoom(Room room, string password = null, Action<Room> onSuccess = null, Action<string> onError = null)
         {
             JoinRoomRequested?.Invoke(room, password);