From ce864011f20366179cee5281fbf46f0f2b29c234 Mon Sep 17 00:00:00 2001 From: Joehu Date: Sun, 10 Oct 2021 12:19:56 -0700 Subject: [PATCH 01/68] Add ability to change ui scaling via keyboard --- osu.Game/OsuGame.cs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index 8a018f17d9..e604e91471 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -986,6 +986,31 @@ namespace osu.Game return false; } + public override bool OnPressed(KeyBindingPressEvent e) + { + switch (e.Action) + { + case PlatformAction.ZoomIn: + adjustUIScaling(1); + return true; + + case PlatformAction.ZoomOut: + adjustUIScaling(-1); + return true; + + case PlatformAction.ZoomDefault: + LocalConfig.GetBindable(OsuSetting.UIScale).SetDefault(); + return true; + } + + void adjustUIScaling(int amount) + { + LocalConfig.SetValue(OsuSetting.UIScale, LocalConfig.Get(OsuSetting.UIScale) + (0.05f * amount)); + } + + return base.OnPressed(e); + } + #region Inactive audio dimming private readonly BindableDouble inactiveVolumeFade = new BindableDouble(); From 789c715f136254e68cdccce5d986bbfd9a05e1d6 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 20 Oct 2021 15:22:47 +0900 Subject: [PATCH 02/68] Add `skin.ini` write support to allow for more correct hashing --- osu.Game.Tests/Skins/IO/ImportSkinTest.cs | 33 +++++- osu.Game/Database/ArchiveModelManager.cs | 43 +++++-- osu.Game/Skinning/SkinManager.cs | 132 +++++++++++++++++----- 3 files changed, 162 insertions(+), 46 deletions(-) diff --git a/osu.Game.Tests/Skins/IO/ImportSkinTest.cs b/osu.Game.Tests/Skins/IO/ImportSkinTest.cs index b2600bb887..67c94413ca 100644 --- a/osu.Game.Tests/Skins/IO/ImportSkinTest.cs +++ b/osu.Game.Tests/Skins/IO/ImportSkinTest.cs @@ -38,7 +38,32 @@ namespace osu.Game.Tests.Skins.IO } [Test] - public async Task TestImportTwiceWithSameMetadata() + public async Task TestImportTwiceWithSameMetadataAndFilename() + { + 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")); + var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("test skin", "skinner"), "skin.osk")); + + Assert.That(imported2.ID, Is.EqualTo(imported.ID)); + Assert.That(osu.Dependencies.Get().GetAllUserSkins(true).Count, Is.EqualTo(1)); + + // the first should be overwritten by the second import. + Assert.That(imported.Files.First().FileInfoID, Is.EqualTo(imported2.Files.First().FileInfoID)); + } + finally + { + host.Exit(); + } + } + } + + [Test] + public async Task TestImportTwiceWithSameMetadataButDifferentFilename() { using (HeadlessGameHost host = new CleanRunHeadlessGameHost(nameof(ImportSkinTest))) { @@ -50,10 +75,10 @@ namespace osu.Game.Tests.Skins.IO var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("test skin", "skinner"), "skin2.osk")); Assert.That(imported2.ID, Is.Not.EqualTo(imported.ID)); - Assert.That(osu.Dependencies.Get().GetAllUserSkins(true).Count, Is.EqualTo(1)); + Assert.That(osu.Dependencies.Get().GetAllUserSkins(true).Count, Is.EqualTo(2)); - // the first should be overwritten by the second import. - Assert.That(osu.Dependencies.Get().GetAllUserSkins(true).First().Files.First().FileInfoID, Is.EqualTo(imported2.Files.First().FileInfoID)); + // skin.ini will be rewritten and therefore not match. + Assert.That(imported.Files.First().FileInfoID, Is.Not.EqualTo(imported2.Files.First().FileInfoID)); } finally { diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index 9c777d324b..e52e5bdfe7 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -507,18 +507,26 @@ namespace osu.Game.Database /// The existing file to be deleted. public void DeleteFile(TModel model, TFileModel file) { - using (var usage = ContextFactory.GetForWrite()) + if (model.ID > 0) { - // Dereference the existing file info, since the file model will be removed. - if (file.FileInfo != null) + using (var usage = ContextFactory.GetForWrite()) { - Files.Dereference(file.FileInfo); + // Dereference the existing file info, since the file model will be removed. + if (file.FileInfo != null) + { + Files.Dereference(file.FileInfo); - // 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().Remove(file); + // 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().Remove(file); + } + + model.Files.Remove(file); } - + } + else + { + Files.Dereference(file.FileInfo); model.Files.Remove(file); } } @@ -531,15 +539,28 @@ namespace osu.Game.Database /// The filename for the new file. public void AddFile(TModel model, Stream contents, string filename) { - using (ContextFactory.GetForWrite()) + if (model.ID > 0) { + using (ContextFactory.GetForWrite()) + { + model.Files.Add(new TFileModel + { + Filename = filename, + FileInfo = Files.Add(contents) + }); + + Update(model); + } + } + else + { + // This function may be called during the import process. + // Should not exist like this once we have switched to realm. model.Files.Add(new TFileModel { Filename = filename, FileInfo = Files.Add(contents) }); - - Update(model); } } diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index 3842acab74..724ca3bc62 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -14,7 +14,6 @@ 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; @@ -138,52 +137,123 @@ namespace osu.Game.Skinning { var instance = GetSkin(item); - // in the case the skin has a skin.ini file, we are going to create a hash based on that. - // 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); + // 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. - return item.ToString().ComputeSHA2Hash(); + // LegacySkin will parse the skin.ini and populate `Skin.Configuration` during construction above. + bool hasSkinIni = string.IsNullOrEmpty(instance.Configuration.SkinInfo.Name); + + if (hasSkinIni) + { + item.Name = instance.Configuration.SkinInfo.Name; + item.Creator = instance.Configuration.SkinInfo.Creator; } + item.Creator ??= unknown_creator_string; + + bool isImport = reader != null; + + if (isImport) + { + // 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. + + string archiveName = item.Name.Replace(".osk", "", StringComparison.OrdinalIgnoreCase); + + if (archiveName != item.Name) + item.Name = $"{item.Name} [{archiveName}]"; + } + + // 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 (instance.Configuration.SkinInfo.Name != item.Name) + updateSkinIniMetadata(item, hasSkinIni); + return base.ComputeHash(item, reader); } + private void updateSkinIniMetadata(SkinInfo item, bool hasSkinIni) + { + string nameLine = $"Name: {item.Name}"; + string authorLine = $"Author: {item.Name}"; + + if (hasSkinIni) + { + List outputLines = new List(); + + bool addedName = false; + bool addedAuthor = false; + + var existingFile = item.Files.First(f => f.Filename == "skin.ini"); + + 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); + + AddFile(item, stream, "skin.ini"); + } + } + } + protected override Task Populate(SkinInfo model, ArchiveReader archive, CancellationToken cancellationToken = default) { var instance = GetSkin(model); 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}]"; - } - /// /// Retrieve a instance for the provided /// From fa542f254778747c2dfb8e8c0d17fe5f2e995de9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 20 Oct 2021 14:05:32 +0900 Subject: [PATCH 03/68] Include `json` files in skin hashing This covers the new layout storage we are doing. --- osu.Game/Skinning/SkinManager.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index 724ca3bc62..9c7ea28cc4 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -50,7 +50,7 @@ namespace osu.Game.Skinning public override IEnumerable HandledExtensions => new[] { ".osk" }; - protected override string[] HashableFileTypes => new[] { ".ini" }; + protected override string[] HashableFileTypes => new[] { ".ini", ".json" }; protected override string ImportFromStablePath => "Skins"; @@ -140,7 +140,7 @@ namespace osu.Game.Skinning // 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. // LegacySkin will parse the skin.ini and populate `Skin.Configuration` during construction above. - bool hasSkinIni = string.IsNullOrEmpty(instance.Configuration.SkinInfo.Name); + bool hasSkinIni = !string.IsNullOrEmpty(instance.Configuration.SkinInfo.Name); if (hasSkinIni) { From 439e90fce3d3c804fceb80e928af0ed2f87836bb Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 20 Oct 2021 15:51:14 +0900 Subject: [PATCH 04/68] Disallow archive imports with no hashable files --- osu.Game/Database/ArchiveModelManager.cs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index e52e5bdfe7..5b07bfe9a5 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -317,14 +317,18 @@ namespace osu.Game.Database /// protected virtual string ComputeHash(TModel item, ArchiveReader reader = null) { - if (reader != null) - // fast hashing for cases where the item's files may not be populated. - return computeHashFast(reader); + var hashableFiles = item.Files + .Where(f => HashableFileTypes.Any(ext => f.Filename.EndsWith(ext, StringComparison.OrdinalIgnoreCase))) + .OrderBy(f => f.Filename) + .ToArray(); + + if (hashableFiles.Length == 0) + throw new InvalidOperationException("Attempted to hash an archive with no files"); // 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); @@ -700,10 +704,10 @@ namespace osu.Game.Database s.CopyTo(hashable); } - if (hashable.Length > 0) - return hashable.ComputeSHA2Hash(); + if (hashable.Length == 0) + throw new InvalidOperationException("Attempted to hash an archive with no files"); - return reader.Name.ComputeSHA2Hash(); + return hashable.ComputeSHA2Hash(); } /// From 19f30177eacb10e733216d3534ec997b1dc9a5a8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 20 Oct 2021 16:48:32 +0900 Subject: [PATCH 05/68] Rewrite tests completely --- osu.Game.Tests/Skins/IO/ImportSkinTest.cs | 249 ++++++++++------------ 1 file changed, 108 insertions(+), 141 deletions(-) diff --git a/osu.Game.Tests/Skins/IO/ImportSkinTest.cs b/osu.Game.Tests/Skins/IO/ImportSkinTest.cs index 67c94413ca..8449b0bbd3 100644 --- a/osu.Game.Tests/Skins/IO/ImportSkinTest.cs +++ b/osu.Game.Tests/Skins/IO/ImportSkinTest.cs @@ -4,6 +4,7 @@ using System; using System.IO; using System.Linq; +using System.Runtime.CompilerServices; using System.Threading.Tasks; using NUnit.Framework; using osu.Framework.Allocation; @@ -16,180 +17,130 @@ namespace osu.Game.Tests.Skins.IO { public class ImportSkinTest : ImportTest { - [Test] - 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(); - } - } - } + #region Testing filename metadata inclusion [Test] - public async Task TestImportTwiceWithSameMetadataAndFilename() + public Task TestSingleImportDifferentFilename() => runSkinTest(async osu => { - 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")); - var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("test skin", "skinner"), "skin.osk")); - var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("test skin", "skinner"), "skin.osk")); - - Assert.That(imported2.ID, Is.EqualTo(imported.ID)); - Assert.That(osu.Dependencies.Get().GetAllUserSkins(true).Count, Is.EqualTo(1)); - - // the first should be overwritten by the second import. - Assert.That(imported.Files.First().FileInfoID, Is.EqualTo(imported2.Files.First().FileInfoID)); - } - finally - { - host.Exit(); - } - } - } + // When the import filename doesn't match, it should be appended (and update the skin.ini). + Assert.That(imported.Name, Is.EqualTo("test skin [skin]")); + Assert.That(imported.Creator, Is.EqualTo("skinner")); + }); [Test] - public async Task TestImportTwiceWithSameMetadataButDifferentFilename() + public Task TestSingleImportMatchingFilename() => runSkinTest(async osu => { - using (HeadlessGameHost host = new CleanRunHeadlessGameHost(nameof(ImportSkinTest))) - { - try - { - var osu = LoadOsuIntoHost(host); + var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("test skin", "skinner"), "test skin.osk")); - var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("test skin", "skinner"), "skin.osk")); - var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("test skin", "skinner"), "skin2.osk")); + // When the import filename matches it shouldn't be appended. + Assert.That(imported.Name, Is.EqualTo("test skin")); + Assert.That(imported.Creator, Is.EqualTo("skinner")); + }); - Assert.That(imported2.ID, Is.Not.EqualTo(imported.ID)); - Assert.That(osu.Dependencies.Get().GetAllUserSkins(true).Count, Is.EqualTo(2)); + #endregion - // skin.ini will be rewritten and therefore not match. - Assert.That(imported.Files.First().FileInfoID, Is.Not.EqualTo(imported2.Files.First().FileInfoID)); - } - finally - { - host.Exit(); - } - } - } + #region Cases where imports should match existing [Test] - public async Task TestImportTwiceWithNoMetadata() + public Task TestImportTwiceWithSameMetadataAndFilename() => runSkinTest(async osu => { - 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")); + var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("test skin", "skinner"), "skin.osk")); - // 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 imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk(string.Empty, string.Empty), "download.osk")); - - Assert.That(imported2.ID, Is.Not.EqualTo(imported.ID)); - Assert.That(osu.Dependencies.Get().GetAllUserSkins(true).Count, Is.EqualTo(2)); - - Assert.That(osu.Dependencies.Get().GetAllUserSkins(true).First().Files.First().FileInfoID, Is.EqualTo(imported.Files.First().FileInfoID)); - Assert.That(osu.Dependencies.Get().GetAllUserSkins(true).Last().Files.First().FileInfoID, Is.EqualTo(imported2.Files.First().FileInfoID)); - } - finally - { - host.Exit(); - } - } - } + assertImportedOnce(imported, imported2); + }); [Test] - public async Task TestImportTwiceWithDifferentMetadata() + public Task TestImportTwiceWithNoMetadataSameDownloadFilename() => runSkinTest(async osu => { - using (HeadlessGameHost host = new CleanRunHeadlessGameHost(nameof(ImportSkinTest))) - { - try - { - var osu = LoadOsuIntoHost(host); + // 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 imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk(string.Empty, string.Empty), "download.osk")); - var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("test skin v2", "skinner"), "skin.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)); - Assert.That(osu.Dependencies.Get().GetAllUserSkins(true).Count, Is.EqualTo(2)); - - Assert.That(osu.Dependencies.Get().GetAllUserSkins(true).First().Files.First().FileInfoID, Is.EqualTo(imported.Files.First().FileInfoID)); - Assert.That(osu.Dependencies.Get().GetAllUserSkins(true).Last().Files.First().FileInfoID, Is.EqualTo(imported2.Files.First().FileInfoID)); - } - finally - { - host.Exit(); - } - } - } + assertImportedOnce(imported, imported2); + }); [Test] - public async Task TestImportUpperCasedOskArchive() + public Task TestImportUpperCasedOskArchive() => runSkinTest(async osu => { - using (HeadlessGameHost host = new CleanRunHeadlessGameHost(nameof(ImportSkinTest))) - { - try - { - var osu = LoadOsuIntoHost(host); + var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("name 1", "author 1"), "name 1.OsK")); + assertCorrectMetadata(imported, "name 1", "author 1"); - var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("name 1", "author 1"), "skin1.OsK")); + var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("name 1", "author 1"), "name 1.oSK")); - Assert.That(imported.Name, Is.EqualTo("name 1")); - Assert.That(imported.Creator, Is.EqualTo("author 1")); + assertImportedOnce(imported, imported2); + }); - var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("name 1", "author 1"), "skin1.oSK")); + #endregion - Assert.That(imported2.Hash, Is.EqualTo(imported.Hash)); - } - finally - { - host.Exit(); - } - } - } + #region Cases where imports should be uniquely imported [Test] - public async Task TestSameMetadataNameDifferentFolderName() + public Task TestImportTwiceWithSameMetadataButDifferentFilename() => runSkinTest(async osu => { - 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")); + var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("test skin", "skinner"), "skin2.osk")); - var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("name 1", "author 1", false), "my custom skin 1")); - Assert.That(imported.Name, Is.EqualTo("name 1 [my custom skin 1]")); - Assert.That(imported.Creator, Is.EqualTo("author 1")); + assertImportedBoth(imported, imported2); + }); - var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("name 1", "author 1", false), "my custom skin 2")); - Assert.That(imported2.Name, Is.EqualTo("name 1 [my custom skin 2]")); - Assert.That(imported2.Creator, Is.EqualTo("author 1")); + [Test] + public Task TestImportTwiceWithNoMetadataDifferentDownloadFilename() => 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 imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk(string.Empty, string.Empty), "download2.osk")); - Assert.That(imported2.Hash, Is.Not.EqualTo(imported.Hash)); - } - finally - { - host.Exit(); - } - } + assertImportedBoth(imported, imported2); + }); + + [Test] + public Task TestImportTwiceWithSameFilenameDifferentMetadata() => runSkinTest(async osu => + { + var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("test skin v2", "skinner"), "skin.osk")); + var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("test skin v2.1", "skinner"), "skin.osk")); + + assertImportedBoth(imported, imported2); + assertCorrectMetadata(imported, "test skin v2 [skin]", "skinner"); + assertCorrectMetadata(imported2, "test skin v2.1 [skin]", "skinner"); + }); + + [Test] + public Task TestSameMetadataNameDifferentFolderName() => runSkinTest(async osu => + { + var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("name 1", "author 1"), "my custom skin 1")); + var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("name 1", "author 1"), "my custom skin 2")); + + assertImportedBoth(imported, imported2); + assertCorrectMetadata(imported, "name 1 [my custom skin 2]", "author 1"); + assertCorrectMetadata(imported, "name 1 [my custom skin 2]", "author 1"); + }); + + #endregion + + private void assertCorrectMetadata(SkinInfo imported, string name, string creator) + { + Assert.That(imported.Name, Is.EqualTo(name)); + Assert.That(imported.Creator, Is.EqualTo(creator)); } - private MemoryStream createOsk(string name, string author, bool makeUnique = true) + private void assertImportedBoth(SkinInfo imported, SkinInfo imported2) + { + Assert.That(imported2.ID, Is.Not.EqualTo(imported.ID)); + Assert.That(imported2.Hash, Is.Not.EqualTo(imported.Hash)); + Assert.That(imported2.Files.Select(f => f.FileInfoID), Is.Not.EquivalentTo(imported.Files.Select(f => f.FileInfoID))); + } + + private void assertImportedOnce(SkinInfo imported, SkinInfo imported2) + { + Assert.That(imported2.ID, Is.EqualTo(imported.ID)); + Assert.That(imported2.Hash, Is.EqualTo(imported.Hash)); + Assert.That(imported2.Files.Select(f => f.FileInfoID), Is.EquivalentTo(imported.Files.Select(f => f.FileInfoID))); + } + + private MemoryStream createOsk(string name, string author, bool makeUnique = false) { var zipStream = new MemoryStream(); using var zip = ZipArchive.Create(); @@ -218,6 +169,22 @@ namespace osu.Game.Tests.Skins.IO return stream; } + private async Task runSkinTest(Func action, [CallerMemberName] string callingMethodName = @"") + { + using (HeadlessGameHost host = new CleanRunHeadlessGameHost(callingMethodName)) + { + try + { + var osu = LoadOsuIntoHost(host); + await action(osu); + } + finally + { + host.Exit(); + } + } + } + private async Task loadSkinIntoOsu(OsuGameBase osu, ArchiveReader archive = null) { var skinManager = osu.Dependencies.Get(); From a95c754fd3d0e867d7910fe3f2a746a10261329b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 20 Oct 2021 16:49:58 +0900 Subject: [PATCH 06/68] Fix multiple issues and make metadata fallback process more logical --- osu.Game/Skinning/SkinManager.cs | 49 +++++++++++++++----------------- 1 file changed, 23 insertions(+), 26 deletions(-) diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index 9c7ea28cc4..452c081a41 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -140,26 +140,19 @@ namespace osu.Game.Skinning // 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. // LegacySkin will parse the skin.ini and populate `Skin.Configuration` during construction above. - bool hasSkinIni = !string.IsNullOrEmpty(instance.Configuration.SkinInfo.Name); - - if (hasSkinIni) - { - item.Name = instance.Configuration.SkinInfo.Name; - item.Creator = instance.Configuration.SkinInfo.Creator; - } - - item.Creator ??= unknown_creator_string; + string skinIniSourcedName = instance.Configuration.SkinInfo.Name; + string archiveName = item.Name.Replace(".osk", "", StringComparison.OrdinalIgnoreCase); bool isImport = reader != null; if (isImport) { + item.Name = !string.IsNullOrEmpty(skinIniSourcedName) ? skinIniSourcedName : archiveName; + item.Creator = instance.Configuration.SkinInfo.Creator ?? 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. - - string archiveName = item.Name.Replace(".osk", "", StringComparison.OrdinalIgnoreCase); - if (archiveName != item.Name) item.Name = $"{item.Name} [{archiveName}]"; } @@ -167,26 +160,26 @@ namespace osu.Game.Skinning // 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 (instance.Configuration.SkinInfo.Name != item.Name) - updateSkinIniMetadata(item, hasSkinIni); + if (skinIniSourcedName != item.Name) + updateSkinIniMetadata(item); return base.ComputeHash(item, reader); } - private void updateSkinIniMetadata(SkinInfo item, bool hasSkinIni) + private void updateSkinIniMetadata(SkinInfo item) { string nameLine = $"Name: {item.Name}"; - string authorLine = $"Author: {item.Name}"; + string authorLine = $"Author: {item.Creator}"; - if (hasSkinIni) + var existingFile = item.Files.SingleOrDefault(f => f.Filename == "skin.ini"); + + if (existingFile != null) { List outputLines = new List(); bool addedName = false; bool addedAuthor = false; - var existingFile = item.Files.First(f => f.Filename == "skin.ini"); - using (var stream = Files.Storage.GetStream(existingFile.FileInfo.StoragePath)) using (var sr = new StreamReader(stream)) { @@ -220,10 +213,12 @@ namespace osu.Game.Skinning } using (Stream stream = new MemoryStream()) - using (var sw = new StreamWriter(stream, Encoding.UTF8, 1024, true)) { - foreach (string line in outputLines) - sw.WriteLine(line); + using (var sw = new StreamWriter(stream, Encoding.UTF8, 1024, true)) + { + foreach (string line in outputLines) + sw.WriteLine(line); + } ReplaceFile(item, existingFile, stream); } @@ -231,11 +226,13 @@ namespace osu.Game.Skinning 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); + using (var sw = new StreamWriter(stream, Encoding.UTF8, 1024, true)) + { + sw.WriteLine("[General]"); + sw.WriteLine(nameLine); + sw.WriteLine(authorLine); + } AddFile(item, stream, "skin.ini"); } From 602303e9470225f1a57f647fb648331a1150a10e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 20 Oct 2021 17:03:30 +0900 Subject: [PATCH 07/68] Add test coverage for `skin.ini` contents --- osu.Game.Tests/Skins/IO/ImportSkinTest.cs | 103 ++++++++++++++++------ osu.Game/Skinning/SkinManager.cs | 4 + 2 files changed, 80 insertions(+), 27 deletions(-) diff --git a/osu.Game.Tests/Skins/IO/ImportSkinTest.cs b/osu.Game.Tests/Skins/IO/ImportSkinTest.cs index 8449b0bbd3..67c5be0c2a 100644 --- a/osu.Game.Tests/Skins/IO/ImportSkinTest.cs +++ b/osu.Game.Tests/Skins/IO/ImportSkinTest.cs @@ -9,6 +9,7 @@ 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; @@ -22,21 +23,36 @@ namespace osu.Game.Tests.Skins.IO [Test] public Task TestSingleImportDifferentFilename() => runSkinTest(async osu => { - var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("test skin", "skinner"), "skin.osk")); + var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("test skin", "skinner"), "skin.osk")); // When the import filename doesn't match, it should be appended (and update the skin.ini). - Assert.That(imported.Name, Is.EqualTo("test skin [skin]")); - Assert.That(imported.Creator, Is.EqualTo("skinner")); + assertCorrectMetadata(imported, "test skin [skin]", "skinner", osu); }); [Test] public Task TestSingleImportMatchingFilename() => runSkinTest(async osu => { - var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("test skin", "skinner"), "test skin.osk")); + var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("test skin", "skinner"), "test skin.osk")); // When the import filename matches it shouldn't be appended. - Assert.That(imported.Name, Is.EqualTo("test skin")); - Assert.That(imported.Creator, Is.EqualTo("skinner")); + assertCorrectMetadata(imported, "test skin", "skinner", osu); + }); + + [Test] + public Task TestSingleImportNoIniFile() => runSkinTest(async osu => + { + var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithNonIniFile(), "test skin.osk")); + + // When the import filename matches it shouldn't be appended. + assertCorrectMetadata(imported, "test skin", "Unknown", osu); + }); + + [Test] + public Task TestEmptyImportFails() => runSkinTest(osu => + { + Assert.ThrowsAsync(() => loadSkinIntoOsu(osu, new ZipArchiveReader(createEmptyOsk(), "test skin.osk"))); + + return Task.CompletedTask; }); #endregion @@ -46,8 +62,8 @@ namespace osu.Game.Tests.Skins.IO [Test] public Task TestImportTwiceWithSameMetadataAndFilename() => runSkinTest(async osu => { - var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("test skin", "skinner"), "skin.osk")); - var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("test skin", "skinner"), "skin.osk")); + var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("test skin", "skinner"), "skin.osk")); + var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("test skin", "skinner"), "skin.osk")); assertImportedOnce(imported, imported2); }); @@ -56,8 +72,8 @@ namespace osu.Game.Tests.Skins.IO 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 imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk(string.Empty, string.Empty), "download.osk")); + var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni(string.Empty, string.Empty), "download.osk")); + var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni(string.Empty, string.Empty), "download.osk")); assertImportedOnce(imported, imported2); }); @@ -65,14 +81,24 @@ namespace osu.Game.Tests.Skins.IO [Test] public Task TestImportUpperCasedOskArchive() => runSkinTest(async osu => { - var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("name 1", "author 1"), "name 1.OsK")); - assertCorrectMetadata(imported, "name 1", "author 1"); + var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("name 1", "author 1"), "name 1.OsK")); + assertCorrectMetadata(imported, "name 1", "author 1", osu); - var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("name 1", "author 1"), "name 1.oSK")); + var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("name 1", "author 1"), "name 1.oSK")); assertImportedOnce(imported, imported2); }); + [Test] + public Task TestSameMetadataNameSameFolderName() => runSkinTest(async osu => + { + var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("name 1", "author 1"), "my custom skin 1")); + var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("name 1", "author 1"), "my custom skin 1")); + + assertImportedOnce(imported, imported2); + assertCorrectMetadata(imported, "name 1 [my custom skin 1]", "author 1", osu); + }); + #endregion #region Cases where imports should be uniquely imported @@ -80,8 +106,8 @@ namespace osu.Game.Tests.Skins.IO [Test] public Task TestImportTwiceWithSameMetadataButDifferentFilename() => runSkinTest(async osu => { - var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("test skin", "skinner"), "skin.osk")); - var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("test skin", "skinner"), "skin2.osk")); + var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("test skin", "skinner"), "skin.osk")); + var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("test skin", "skinner"), "skin2.osk")); assertImportedBoth(imported, imported2); }); @@ -90,8 +116,8 @@ namespace osu.Game.Tests.Skins.IO public Task TestImportTwiceWithNoMetadataDifferentDownloadFilename() => 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 imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk(string.Empty, string.Empty), "download2.osk")); + var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni(string.Empty, string.Empty), "download.osk")); + var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni(string.Empty, string.Empty), "download2.osk")); assertImportedBoth(imported, imported2); }); @@ -99,31 +125,37 @@ namespace osu.Game.Tests.Skins.IO [Test] public Task TestImportTwiceWithSameFilenameDifferentMetadata() => runSkinTest(async osu => { - var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("test skin v2", "skinner"), "skin.osk")); - var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("test skin v2.1", "skinner"), "skin.osk")); + var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("test skin v2", "skinner"), "skin.osk")); + var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("test skin v2.1", "skinner"), "skin.osk")); assertImportedBoth(imported, imported2); - assertCorrectMetadata(imported, "test skin v2 [skin]", "skinner"); - assertCorrectMetadata(imported2, "test skin v2.1 [skin]", "skinner"); + assertCorrectMetadata(imported, "test skin v2 [skin]", "skinner", osu); + assertCorrectMetadata(imported2, "test skin v2.1 [skin]", "skinner", osu); }); [Test] public Task TestSameMetadataNameDifferentFolderName() => runSkinTest(async osu => { - var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("name 1", "author 1"), "my custom skin 1")); - var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOsk("name 1", "author 1"), "my custom skin 2")); + var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("name 1", "author 1"), "my custom skin 1")); + var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("name 1", "author 1"), "my custom skin 2")); assertImportedBoth(imported, imported2); - assertCorrectMetadata(imported, "name 1 [my custom skin 2]", "author 1"); - assertCorrectMetadata(imported, "name 1 [my custom skin 2]", "author 1"); + assertCorrectMetadata(imported, "name 1 [my custom skin 1]", "author 1", osu); + assertCorrectMetadata(imported2, "name 1 [my custom skin 2]", "author 1", osu); }); #endregion - private void assertCorrectMetadata(SkinInfo imported, string name, string creator) + private void assertCorrectMetadata(SkinInfo imported, string name, string creator, OsuGameBase osu) { Assert.That(imported.Name, Is.EqualTo(name)); Assert.That(imported.Creator, Is.EqualTo(creator)); + + // for extra safety let's reconstruct the skin, reading from the skin.ini. + var instance = imported.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 void assertImportedBoth(SkinInfo imported, SkinInfo imported2) @@ -140,7 +172,24 @@ namespace osu.Game.Tests.Skins.IO Assert.That(imported2.Files.Select(f => f.FileInfoID), Is.EquivalentTo(imported.Files.Select(f => f.FileInfoID))); } - private MemoryStream createOsk(string name, string author, bool makeUnique = false) + 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(); diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index 452c081a41..213c92c103 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -135,6 +135,10 @@ namespace osu.Game.Skinning protected override string ComputeHash(SkinInfo item, ArchiveReader reader = null) { + // we will be adding a hashable file below, but this is only useful if there are any other files in the skin. + if (item.Files.Count == 0) + throw new InvalidOperationException("Attempted to hash an archive with no files"); + var instance = GetSkin(item); // 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. From 5f53dd80213c50eccb324c668109429584f94ec6 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 20 Oct 2021 17:12:44 +0900 Subject: [PATCH 08/68] Rename test variable for legibility --- osu.Game.Tests/Skins/IO/ImportSkinTest.cs | 98 +++++++++++------------ 1 file changed, 49 insertions(+), 49 deletions(-) diff --git a/osu.Game.Tests/Skins/IO/ImportSkinTest.cs b/osu.Game.Tests/Skins/IO/ImportSkinTest.cs index 67c5be0c2a..21e4948972 100644 --- a/osu.Game.Tests/Skins/IO/ImportSkinTest.cs +++ b/osu.Game.Tests/Skins/IO/ImportSkinTest.cs @@ -23,28 +23,28 @@ namespace osu.Game.Tests.Skins.IO [Test] public Task TestSingleImportDifferentFilename() => runSkinTest(async osu => { - var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("test skin", "skinner"), "skin.osk")); + var import1 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("test skin", "skinner"), "skin.osk")); // When the import filename doesn't match, it should be appended (and update the skin.ini). - assertCorrectMetadata(imported, "test skin [skin]", "skinner", osu); + assertCorrectMetadata(import1, "test skin [skin]", "skinner", osu); }); [Test] public Task TestSingleImportMatchingFilename() => runSkinTest(async osu => { - var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("test skin", "skinner"), "test skin.osk")); + var import1 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("test skin", "skinner"), "test skin.osk")); // When the import filename matches it shouldn't be appended. - assertCorrectMetadata(imported, "test skin", "skinner", osu); + assertCorrectMetadata(import1, "test skin", "skinner", osu); }); [Test] public Task TestSingleImportNoIniFile() => runSkinTest(async osu => { - var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithNonIniFile(), "test skin.osk")); + var import1 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithNonIniFile(), "test skin.osk")); // When the import filename matches it shouldn't be appended. - assertCorrectMetadata(imported, "test skin", "Unknown", osu); + assertCorrectMetadata(import1, "test skin", "Unknown", osu); }); [Test] @@ -62,114 +62,114 @@ namespace osu.Game.Tests.Skins.IO [Test] public Task TestImportTwiceWithSameMetadataAndFilename() => runSkinTest(async osu => { - var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("test skin", "skinner"), "skin.osk")); - var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("test skin", "skinner"), "skin.osk")); + 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(imported, imported2); + 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(createOskWithIni(string.Empty, string.Empty), "download.osk")); - var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni(string.Empty, string.Empty), "download.osk")); + 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), "download.osk")); - assertImportedOnce(imported, imported2); + assertImportedOnce(import1, import2); }); [Test] public Task TestImportUpperCasedOskArchive() => runSkinTest(async osu => { - var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("name 1", "author 1"), "name 1.OsK")); - assertCorrectMetadata(imported, "name 1", "author 1", osu); + var import1 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("name 1", "author 1"), "name 1.OsK")); + assertCorrectMetadata(import1, "name 1", "author 1", osu); - var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("name 1", "author 1"), "name 1.oSK")); + var import2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("name 1", "author 1"), "name 1.oSK")); - assertImportedOnce(imported, imported2); + assertImportedOnce(import1, import2); }); [Test] public Task TestSameMetadataNameSameFolderName() => runSkinTest(async osu => { - var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("name 1", "author 1"), "my custom skin 1")); - var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("name 1", "author 1"), "my custom skin 1")); + 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")); - assertImportedOnce(imported, imported2); - assertCorrectMetadata(imported, "name 1 [my custom skin 1]", "author 1", osu); + assertImportedOnce(import1, import2); + assertCorrectMetadata(import1, "name 1 [my custom skin 1]", "author 1", osu); }); #endregion - #region Cases where imports should be uniquely imported + #region Cases where imports should be uniquely import1 [Test] public Task TestImportTwiceWithSameMetadataButDifferentFilename() => runSkinTest(async osu => { - var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("test skin", "skinner"), "skin.osk")); - var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("test skin", "skinner"), "skin2.osk")); + 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")); - assertImportedBoth(imported, imported2); + assertImportedBoth(import1, import2); }); [Test] public Task TestImportTwiceWithNoMetadataDifferentDownloadFilename() => 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(createOskWithIni(string.Empty, string.Empty), "download.osk")); - var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni(string.Empty, string.Empty), "download2.osk")); + 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(imported, imported2); + assertImportedBoth(import1, import2); }); [Test] public Task TestImportTwiceWithSameFilenameDifferentMetadata() => runSkinTest(async osu => { - var imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("test skin v2", "skinner"), "skin.osk")); - var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("test skin v2.1", "skinner"), "skin.osk")); + 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(imported, imported2); - assertCorrectMetadata(imported, "test skin v2 [skin]", "skinner", osu); - assertCorrectMetadata(imported2, "test skin v2.1 [skin]", "skinner", osu); + 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 imported = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("name 1", "author 1"), "my custom skin 1")); - var imported2 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createOskWithIni("name 1", "author 1"), "my custom skin 2")); + 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(imported, imported2); - assertCorrectMetadata(imported, "name 1 [my custom skin 1]", "author 1", osu); - assertCorrectMetadata(imported2, "name 1 [my custom skin 2]", "author 1", osu); + 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 imported, string name, string creator, OsuGameBase osu) + private void assertCorrectMetadata(SkinInfo import1, string name, string creator, OsuGameBase osu) { - Assert.That(imported.Name, Is.EqualTo(name)); - Assert.That(imported.Creator, Is.EqualTo(creator)); + 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 = imported.CreateInstance((IStorageResourceProvider)osu.Dependencies.Get(typeof(SkinManager))); + 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 void assertImportedBoth(SkinInfo imported, SkinInfo imported2) + private void assertImportedBoth(SkinInfo import1, SkinInfo import2) { - Assert.That(imported2.ID, Is.Not.EqualTo(imported.ID)); - Assert.That(imported2.Hash, Is.Not.EqualTo(imported.Hash)); - Assert.That(imported2.Files.Select(f => f.FileInfoID), Is.Not.EquivalentTo(imported.Files.Select(f => f.FileInfoID))); + 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 imported, SkinInfo imported2) + private void assertImportedOnce(SkinInfo import1, SkinInfo import2) { - Assert.That(imported2.ID, Is.EqualTo(imported.ID)); - Assert.That(imported2.Hash, Is.EqualTo(imported.Hash)); - Assert.That(imported2.Files.Select(f => f.FileInfoID), Is.EquivalentTo(imported.Files.Select(f => f.FileInfoID))); + 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() From 9e6e41d7c06c09e671c3274ef7f56a75cfd09f1e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 20 Oct 2021 17:33:36 +0900 Subject: [PATCH 09/68] Add migration to reset and repopulate existing skin hashes --- osu.Game/Database/ArchiveModelManager.cs | 11 +++++---- .../20211020081609_ResetSkinHashes.cs | 23 +++++++++++++++++++ osu.Game/Skinning/SkinManager.cs | 22 ++++++++++++++++++ 3 files changed, 52 insertions(+), 4 deletions(-) create mode 100644 osu.Game/Migrations/20211020081609_ResetSkinHashes.cs diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index 5b07bfe9a5..63a5da79a7 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -32,7 +32,7 @@ namespace osu.Game.Database /// The associated file join type. public abstract class ArchiveModelManager : IModelManager, IModelFileManager where TModel : class, IHasFiles, IHasPrimaryKey, ISoftDelete - where TFileModel : class, INamedFileInfo, new() + where TFileModel : class, INamedFileInfo, IHasPrimaryKey, new() { private const int import_queue_request_concurrency = 1; @@ -520,9 +520,12 @@ namespace osu.Game.Database { Files.Dereference(file.FileInfo); - // 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().Remove(file); + 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().Remove(file); + } } model.Files.Remove(file); diff --git a/osu.Game/Migrations/20211020081609_ResetSkinHashes.cs b/osu.Game/Migrations/20211020081609_ResetSkinHashes.cs new file mode 100644 index 0000000000..6d53c019ec --- /dev/null +++ b/osu.Game/Migrations/20211020081609_ResetSkinHashes.cs @@ -0,0 +1,23 @@ +// Copyright (c) ppy Pty Ltd . 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) + { + } + } +} diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index 213c92c103..0b611bfe8f 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -18,6 +18,7 @@ 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; @@ -84,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"; From 68c01fc20442312cd09a27c29194c2fb3774be35 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 20 Oct 2021 17:34:09 +0900 Subject: [PATCH 10/68] Fix infinite loop on default skin (it can't have a `skin.ini`) --- osu.Game/Skinning/SkinManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index 0b611bfe8f..afa411a15b 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -186,7 +186,7 @@ namespace osu.Game.Skinning // 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) + if (instance is LegacySkin && skinIniSourcedName != item.Name) updateSkinIniMetadata(item); return base.ComputeHash(item, reader); From 59b7210efaaf976301deefc9fdbd3c2d756f675f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 21 Oct 2021 13:35:26 +0900 Subject: [PATCH 11/68] Revert disallowing imports with no files While it is logical that we want this, from a testing perspective this is a bit of a nightmare to fix. Let's revisit at a later point in time. --- osu.Game.Tests/Skins/IO/ImportSkinTest.cs | 7 ++-- osu.Game/Database/ArchiveModelManager.cs | 40 ++++++++++++++--------- osu.Game/Skinning/SkinManager.cs | 4 --- 3 files changed, 28 insertions(+), 23 deletions(-) diff --git a/osu.Game.Tests/Skins/IO/ImportSkinTest.cs b/osu.Game.Tests/Skins/IO/ImportSkinTest.cs index 21e4948972..20bf42345f 100644 --- a/osu.Game.Tests/Skins/IO/ImportSkinTest.cs +++ b/osu.Game.Tests/Skins/IO/ImportSkinTest.cs @@ -48,11 +48,12 @@ namespace osu.Game.Tests.Skins.IO }); [Test] - public Task TestEmptyImportFails() => runSkinTest(osu => + public Task TestEmptyImportImportsWithFilename() => runSkinTest(async osu => { - Assert.ThrowsAsync(() => loadSkinIntoOsu(osu, new ZipArchiveReader(createEmptyOsk(), "test skin.osk"))); + var import1 = await loadSkinIntoOsu(osu, new ZipArchiveReader(createEmptyOsk(), "test skin.osk")); - return Task.CompletedTask; + // When the import filename matches it shouldn't be appended. + assertCorrectMetadata(import1, "test skin", "Unknown", osu); }); #endregion diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index 63a5da79a7..0bc4c40459 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -322,22 +322,22 @@ namespace osu.Game.Database .OrderBy(f => f.Filename) .ToArray(); - if (hashableFiles.Length == 0) - throw new InvalidOperationException("Attempted to hash an archive with no files"); - - // for now, concatenate all hashable files in the set to create a unique hash. - MemoryStream hashable = new MemoryStream(); - - foreach (TFileModel file in hashableFiles) + if (hashableFiles.Length > 0) { - using (Stream s = Files.Store.GetStream(file.FileInfo.StoragePath)) - s.CopyTo(hashable); + // for now, concatenate all hashable files in the set to create a unique hash. + MemoryStream hashable = new MemoryStream(); + + foreach (TFileModel file in hashableFiles) + { + using (Stream s = Files.Store.GetStream(file.FileInfo.StoragePath)) + s.CopyTo(hashable); + } + + if (hashable.Length > 0) + return hashable.ComputeSHA2Hash(); } - if (hashable.Length > 0) - return hashable.ComputeSHA2Hash(); - - return item.Hash; + return generateFallbackHash(); } /// @@ -707,10 +707,10 @@ namespace osu.Game.Database s.CopyTo(hashable); } - if (hashable.Length == 0) - throw new InvalidOperationException("Attempted to hash an archive with no files"); + if (hashable.Length > 0) + return hashable.ComputeSHA2Hash(); - return hashable.ComputeSHA2Hash(); + return generateFallbackHash(); } /// @@ -923,6 +923,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()) diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index afa411a15b..304e48e854 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -157,10 +157,6 @@ namespace osu.Game.Skinning protected override string ComputeHash(SkinInfo item, ArchiveReader reader = null) { - // we will be adding a hashable file below, but this is only useful if there are any other files in the skin. - if (item.Files.Count == 0) - throw new InvalidOperationException("Attempted to hash an archive with no files"); - var instance = GetSkin(item); // 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. From a5088cac27c2d6955165efd45a86b68ed7952128 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 21 Oct 2021 13:36:04 +0900 Subject: [PATCH 12/68] Fix default metadata propagation when no files are present --- osu.Game/Skinning/SkinInfo.cs | 6 +++--- osu.Game/Skinning/SkinManager.cs | 5 +++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/osu.Game/Skinning/SkinInfo.cs b/osu.Game/Skinning/SkinInfo.cs index 2bf8668ec6..3b34e23d57 100644 --- a/osu.Game/Skinning/SkinInfo.cs +++ b/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) diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index 304e48e854..fffdf19976 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -149,7 +149,7 @@ 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"; @@ -163,6 +163,7 @@ namespace osu.Game.Skinning // LegacySkin 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 = reader != null; @@ -170,7 +171,7 @@ namespace osu.Game.Skinning if (isImport) { item.Name = !string.IsNullOrEmpty(skinIniSourcedName) ? skinIniSourcedName : archiveName; - item.Creator = instance.Configuration.SkinInfo.Creator ?? unknown_creator_string; + 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. From 08971ff8f27e1489354f8ee596f89ce186bd6565 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 22 Oct 2021 11:03:28 +0900 Subject: [PATCH 13/68] Fix typo in region spedc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bartłomiej Dach --- osu.Game.Tests/Skins/IO/ImportSkinTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Skins/IO/ImportSkinTest.cs b/osu.Game.Tests/Skins/IO/ImportSkinTest.cs index 20bf42345f..b62fb8bd87 100644 --- a/osu.Game.Tests/Skins/IO/ImportSkinTest.cs +++ b/osu.Game.Tests/Skins/IO/ImportSkinTest.cs @@ -102,7 +102,7 @@ namespace osu.Game.Tests.Skins.IO #endregion - #region Cases where imports should be uniquely import1 + #region Cases where imports should be uniquely imported [Test] public Task TestImportTwiceWithSameMetadataButDifferentFilename() => runSkinTest(async osu => From e5b73f25cd40cadf90884fb8f2dc160d6a764ad8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 22 Oct 2021 12:56:05 +0900 Subject: [PATCH 14/68] Ensure newly created `skin.ini` files are set to latest version --- osu.Game/Skinning/SkinManager.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index fffdf19976..16ed25eeb1 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -255,6 +255,7 @@ namespace osu.Game.Skinning sw.WriteLine("[General]"); sw.WriteLine(nameLine); sw.WriteLine(authorLine); + sw.WriteLine("Version: latest"); } AddFile(item, stream, "skin.ini"); From 9cdc1ba59219ecd31df04c235def14567f4fbee4 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 22 Oct 2021 13:43:45 +0900 Subject: [PATCH 15/68] Fix default legacy skin not being able to read from stored `skin.ini` --- osu.Game/Skinning/DefaultLegacySkin.cs | 9 +++++- osu.Game/Skinning/LegacySkin.cs | 39 ++++++++++++++++---------- 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/osu.Game/Skinning/DefaultLegacySkin.cs b/osu.Game/Skinning/DefaultLegacySkin.cs index 16ac17546d..cd6dbd9ddd 100644 --- a/osu.Game/Skinning/DefaultLegacySkin.cs +++ b/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(resources.Resources, "Skins/Legacy"), resources, string.Empty) + : base( + skin, + new NamespacedResourceStore(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(skin, resources.Files).GetStream("skin.ini") + ) { Configuration.CustomColours["SliderBall"] = new Color4(2, 170, 255, 255); Configuration.CustomComboColours = new List diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index b09620411b..5ab3ef84db 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -69,28 +69,37 @@ namespace osu.Game.Skinning /// Access to raw game resources. /// The user-facing filename of the configuration file to be parsed. Can accept an .osu or skin.ini file. protected LegacySkin(SkinInfo skin, [CanBeNull] IResourceStore storage, [CanBeNull] IStorageResourceProvider resources, string configurationFilename) + : this(skin, storage, resources, storage?.GetStream(configurationFilename)) + { + } + + /// + /// Construct a new legacy skin instance. + /// + /// The model for this skin. + /// A storage for looking up files within this skin using user-facing filenames. + /// Access to raw game resources. + /// An optional stream containing the contents of a skin.ini file. + protected LegacySkin(SkinInfo skin, [CanBeNull] IResourceStore storage, [CanBeNull] IStorageResourceProvider resources, [CanBeNull] Stream configurationStream) : base(skin, resources) { - using (var stream = storage?.GetStream(configurationFilename)) + if (configurationStream != null) { - if (stream != null) + using (LineBufferedReader reader = new LineBufferedReader(configurationStream, true)) + Configuration = new LegacySkinDecoder().Decode(reader); + + configurationStream.Seek(0, SeekOrigin.Begin); + + using (LineBufferedReader reader = new LineBufferedReader(configurationStream)) { - using (LineBufferedReader reader = new LineBufferedReader(stream, true)) - Configuration = new LegacySkinDecoder().Decode(reader); + var maniaList = new LegacyManiaSkinDecoder().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; - } + foreach (var config in maniaList) + maniaConfigurations[config.Keys] = config; } - else - Configuration = new LegacySkinConfiguration(); } + else + Configuration = new LegacySkinConfiguration(); if (storage != null) { From 82d0a6515fbe39536dd86122aede7caec25badd8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 22 Oct 2021 14:08:12 +0900 Subject: [PATCH 16/68] Fix incorrect conditional for checking whether import (would fail for default skin) --- osu.Game/Skinning/SkinManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index 16ed25eeb1..707afade09 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -166,7 +166,7 @@ namespace osu.Game.Skinning string skinIniSourcedCreator = instance.Configuration.SkinInfo.Creator; string archiveName = item.Name.Replace(".osk", "", StringComparison.OrdinalIgnoreCase); - bool isImport = reader != null; + bool isImport = item.ID == 0; if (isImport) { From 93482414d6d69fff3e02a0083afb1fb55244db23 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 22 Oct 2021 14:41:59 +0900 Subject: [PATCH 17/68] Remove `SkinConfiguration` subclasses and allow configuration parsing for all skin types --- .../Legacy/CatchLegacySkinTransformer.cs | 2 +- .../Skinning/Legacy/LegacyStageBackground.cs | 2 +- .../Legacy/ManiaLegacySkinTransformer.cs | 2 +- .../Skinning/Legacy/LegacyMainCirclePiece.cs | 3 +- .../Skinning/Legacy/LegacyInputDrum.cs | 2 +- .../Gameplay/TestSceneHitObjectSamples.cs | 2 +- osu.Game.Tests/Skins/LegacySkinDecoderTest.cs | 2 +- .../Skins/TestSceneSkinConfigurationLookup.cs | 8 +-- .../Objects/Legacy/ConvertHitObjectParser.cs | 2 +- osu.Game/Skinning/DefaultSkin.cs | 1 - osu.Game/Skinning/DefaultSkinConfiguration.cs | 12 ---- osu.Game/Skinning/LegacyBeatmapSkin.cs | 2 +- osu.Game/Skinning/LegacySkin.cs | 55 ++++++++----------- osu.Game/Skinning/LegacySkinConfiguration.cs | 28 ---------- osu.Game/Skinning/LegacySkinDecoder.cs | 8 +-- osu.Game/Skinning/LegacySkinExtensions.cs | 2 +- osu.Game/Skinning/LegacySkinTransformer.cs | 2 +- osu.Game/Skinning/Skin.cs | 31 ++++++++++- osu.Game/Skinning/SkinConfiguration.cs | 20 +++++++ osu.Game/Skinning/SkinManager.cs | 2 +- 20 files changed, 92 insertions(+), 96 deletions(-) delete mode 100644 osu.Game/Skinning/DefaultSkinConfiguration.cs delete mode 100644 osu.Game/Skinning/LegacySkinConfiguration.cs diff --git a/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs b/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs index 10fc4e78b2..335d78388b 100644 --- a/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs +++ b/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs @@ -66,7 +66,7 @@ namespace osu.Game.Rulesets.Catch.Skinning.Legacy return null; case CatchSkinComponents.Catcher: - var version = GetConfig(LegacySkinConfiguration.LegacySetting.Version)?.Value ?? 1; + var version = GetConfig(SkinConfiguration.LegacySetting.Version)?.Value ?? 1; if (version < 2.3m) { diff --git a/osu.Game.Rulesets.Mania/Skinning/Legacy/LegacyStageBackground.cs b/osu.Game.Rulesets.Mania/Skinning/Legacy/LegacyStageBackground.cs index fec3e9493e..952fc7ddd6 100644 --- a/osu.Game.Rulesets.Mania/Skinning/Legacy/LegacyStageBackground.cs +++ b/osu.Game.Rulesets.Mania/Skinning/Legacy/LegacyStageBackground.cs @@ -98,7 +98,7 @@ namespace osu.Game.Rulesets.Mania.Skinning.Legacy float rightLineWidth = skin.GetManiaSkinConfig(LegacyManiaSkinConfigurationLookups.RightLineWidth, columnIndex)?.Value ?? 1; bool hasLeftLine = leftLineWidth > 0; - bool hasRightLine = rightLineWidth > 0 && skin.GetConfig(LegacySkinConfiguration.LegacySetting.Version)?.Value >= 2.4m + bool hasRightLine = rightLineWidth > 0 && skin.GetConfig(SkinConfiguration.LegacySetting.Version)?.Value >= 2.4m || isLastColumn; Color4 lineColour = skin.GetManiaSkinConfig(LegacyManiaSkinConfigurationLookups.ColumnLineColour, columnIndex)?.Value ?? Color4.White; diff --git a/osu.Game.Rulesets.Mania/Skinning/Legacy/ManiaLegacySkinTransformer.cs b/osu.Game.Rulesets.Mania/Skinning/Legacy/ManiaLegacySkinTransformer.cs index 814a737034..0de9a098bc 100644 --- a/osu.Game.Rulesets.Mania/Skinning/Legacy/ManiaLegacySkinTransformer.cs +++ b/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(() => GetConfig(LegacySkinConfiguration.LegacySetting.Version) != null); + isLegacySkin = new Lazy(() => GetConfig(SkinConfiguration.LegacySetting.Version) != null); hasKeyTexture = new Lazy(() => { var keyImage = this.GetManiaSkinConfig(LegacyManiaSkinConfigurationLookups.KeyImage, 0)?.Value ?? "mania-key1"; diff --git a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyMainCirclePiece.cs b/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyMainCirclePiece.cs index d1c9b1bf92..0e0f4f4291 100644 --- a/osu.Game.Rulesets.Osu/Skinning/Legacy/LegacyMainCirclePiece.cs +++ b/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) { - var legacyVersion = skin.GetConfig(LegacySetting.Version)?.Value; + var legacyVersion = skin.GetConfig(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. diff --git a/osu.Game.Rulesets.Taiko/Skinning/Legacy/LegacyInputDrum.cs b/osu.Game.Rulesets.Taiko/Skinning/Legacy/LegacyInputDrum.cs index 86be40dea8..43c5c07f80 100644 --- a/osu.Game.Rulesets.Taiko/Skinning/Legacy/LegacyInputDrum.cs +++ b/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.Version)?.Value >= 2.1m) + if (skin.GetConfig(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; diff --git a/osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs b/osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs index fc420e22a1..153d5b8e36 100644 --- a/osu.Game.Tests/Gameplay/TestSceneHitObjectSamples.cs +++ b/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 { diff --git a/osu.Game.Tests/Skins/LegacySkinDecoderTest.cs b/osu.Game.Tests/Skins/LegacySkinDecoderTest.cs index dcb866c99f..cfc140ce39 100644 --- a/osu.Game.Tests/Skins/LegacySkinDecoderTest.cs +++ b/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] diff --git a/osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs b/osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs index aadabec100..870d6d8f57 100644 --- a/osu.Game.Tests/Skins/TestSceneSkinConfigurationLookup.cs +++ b/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.Version)?.Value == 2.3m); + AddAssert("Check legacy version lookup", () => requester.GetConfig(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.Version)?.Value == 2.3m); + AddAssert("Check legacy version lookup", () => requester.GetConfig(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.Version)?.Value == LegacySkinConfiguration.LATEST_VERSION); + () => requester.GetConfig(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.Version)?.Value == 1.0m); + AddAssert("Check legacy version lookup", () => requester.GetConfig(SkinConfiguration.LegacySetting.Version)?.Value == 1.0m); } public enum LookupType diff --git a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs index 0942a7264d..d902918e83 100644 --- a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs +++ b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs @@ -481,7 +481,7 @@ namespace osu.Game.Rulesets.Objects.Legacy /// /// /// Layered hit samples are automatically added in all modes (except osu!mania), but can be disabled - /// using the skin config option. + /// using the skin config option. /// public readonly bool IsLayered; diff --git a/osu.Game/Skinning/DefaultSkin.cs b/osu.Game/Skinning/DefaultSkin.cs index 8e03bddb4d..d972ce3af6 100644 --- a/osu.Game/Skinning/DefaultSkin.cs +++ b/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; diff --git a/osu.Game/Skinning/DefaultSkinConfiguration.cs b/osu.Game/Skinning/DefaultSkinConfiguration.cs deleted file mode 100644 index 5842ee82ee..0000000000 --- a/osu.Game/Skinning/DefaultSkinConfiguration.cs +++ /dev/null @@ -1,12 +0,0 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. -// See the LICENCE file in the repository root for full licence text. - -namespace osu.Game.Skinning -{ - /// - /// A skin configuration pre-populated with sane defaults. - /// - public class DefaultSkinConfiguration : SkinConfiguration - { - } -} diff --git a/osu.Game/Skinning/LegacyBeatmapSkin.cs b/osu.Game/Skinning/LegacyBeatmapSkin.cs index 2093182dcc..8720a55076 100644 --- a/osu.Game/Skinning/LegacyBeatmapSkin.cs +++ b/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) diff --git a/osu.Game/Skinning/LegacySkin.cs b/osu.Game/Skinning/LegacySkin.cs index 5ab3ef84db..6facb6fafd 100644 --- a/osu.Game/Skinning/LegacySkin.cs +++ b/osu.Game/Skinning/LegacySkin.cs @@ -47,12 +47,6 @@ namespace osu.Game.Skinning /// protected virtual bool UseCustomSampleBanks => false; - public new LegacySkinConfiguration Configuration - { - get => base.Configuration as LegacySkinConfiguration; - set => base.Configuration = value; - } - private readonly Dictionary maniaConfigurations = new Dictionary(); [UsedImplicitly(ImplicitUseKindFlags.InstantiatedWithFixedConstructorSignature)] @@ -81,26 +75,8 @@ namespace osu.Game.Skinning /// Access to raw game resources. /// An optional stream containing the contents of a skin.ini file. protected LegacySkin(SkinInfo skin, [CanBeNull] IResourceStore storage, [CanBeNull] IStorageResourceProvider resources, [CanBeNull] Stream configurationStream) - : base(skin, resources) + : base(skin, resources, configurationStream) { - if (configurationStream != null) - { - using (LineBufferedReader reader = new LineBufferedReader(configurationStream, true)) - Configuration = new LegacySkinDecoder().Decode(reader); - - configurationStream.Seek(0, SeekOrigin.Begin); - - using (LineBufferedReader reader = new LineBufferedReader(configurationStream)) - { - var maniaList = new LegacyManiaSkinDecoder().Decode(reader); - - foreach (var config in maniaList) - maniaConfigurations[config.Keys] = config; - } - } - else - Configuration = new LegacySkinConfiguration(); - if (storage != null) { var samples = resources?.AudioManager?.GetSampleStore(storage); @@ -119,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 GetConfig(TLookup lookup) { switch (lookup) @@ -155,7 +146,7 @@ namespace osu.Game.Skinning break; - case LegacySkinConfiguration.LegacySetting legacy: + case SkinConfiguration.LegacySetting legacy: return legacySettingLookup(legacy); default: @@ -198,7 +189,7 @@ namespace osu.Game.Skinning case LegacyManiaSkinConfigurationLookups.ExplosionScale: Debug.Assert(maniaLookup.TargetColumn != null); - if (GetConfig(LegacySkinConfiguration.LegacySetting.Version)?.Value < 2.5m) + if (GetConfig(SkinConfiguration.LegacySetting.Version)?.Value < 2.5m) return SkinUtils.As(new Bindable(1)); if (existing.ExplosionWidth[maniaLookup.TargetColumn.Value] != 0) @@ -245,7 +236,7 @@ namespace osu.Game.Skinning case LegacyManiaSkinConfigurationLookups.HoldNoteLightScale: Debug.Assert(maniaLookup.TargetColumn != null); - if (GetConfig(LegacySkinConfiguration.LegacySetting.Version)?.Value < 2.5m) + if (GetConfig(SkinConfiguration.LegacySetting.Version)?.Value < 2.5m) return SkinUtils.As(new Bindable(1)); if (existing.HoldNoteLightWidth[maniaLookup.TargetColumn.Value] != 0) @@ -318,15 +309,15 @@ namespace osu.Game.Skinning => source.ImageLookups.TryGetValue(lookup, out var image) ? new Bindable(image) : null; [CanBeNull] - private IBindable legacySettingLookup(LegacySkinConfiguration.LegacySetting legacySetting) + private IBindable legacySettingLookup(SkinConfiguration.LegacySetting legacySetting) { switch (legacySetting) { - case LegacySkinConfiguration.LegacySetting.Version: - return SkinUtils.As(new Bindable(Configuration.LegacyVersion ?? LegacySkinConfiguration.LATEST_VERSION)); + case SkinConfiguration.LegacySetting.Version: + return SkinUtils.As(new Bindable(Configuration.LegacyVersion ?? SkinConfiguration.LATEST_VERSION)); default: - return genericLookup(legacySetting); + return genericLookup(legacySetting); } } diff --git a/osu.Game/Skinning/LegacySkinConfiguration.cs b/osu.Game/Skinning/LegacySkinConfiguration.cs deleted file mode 100644 index 20d1da8aaa..0000000000 --- a/osu.Game/Skinning/LegacySkinConfiguration.cs +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright (c) ppy Pty Ltd . 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; - - /// - /// Legacy version of this skin. - /// - public decimal? LegacyVersion { get; internal set; } - - public enum LegacySetting - { - Version, - ComboPrefix, - ComboOverlap, - ScorePrefix, - ScoreOverlap, - HitCirclePrefix, - HitCircleOverlap, - AnimationFramerate, - LayeredHitSounds - } - } -} diff --git a/osu.Game/Skinning/LegacySkinDecoder.cs b/osu.Game/Skinning/LegacySkinDecoder.cs index 2700f84815..dd8a9dedb2 100644 --- a/osu.Game/Skinning/LegacySkinDecoder.cs +++ b/osu.Game/Skinning/LegacySkinDecoder.cs @@ -6,14 +6,14 @@ using osu.Game.Beatmaps.Formats; namespace osu.Game.Skinning { - public class LegacySkinDecoder : LegacyDecoder + public class LegacySkinDecoder : LegacyDecoder { 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 var 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; diff --git a/osu.Game/Skinning/LegacySkinExtensions.cs b/osu.Game/Skinning/LegacySkinExtensions.cs index fd1f905868..479afabb00 100644 --- a/osu.Game/Skinning/LegacySkinExtensions.cs +++ b/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 { diff --git a/osu.Game/Skinning/LegacySkinTransformer.cs b/osu.Game/Skinning/LegacySkinTransformer.cs index 92b7a04dee..97084f34e0 100644 --- a/osu.Game/Skinning/LegacySkinTransformer.cs +++ b/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 { diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index 92441f40da..d5f41f01a4 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/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 DrawableComponentInfo => drawableComponentInfo; @@ -36,9 +39,17 @@ namespace osu.Game.Skinning public abstract IBindable GetConfig(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) + 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 +84,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() + { + var path = SkinInfo.Files.SingleOrDefault(f => f.Filename == "skin.ini")?.FileInfo.StoragePath; + + if (string.IsNullOrEmpty(path)) + return null; + + return resources?.Files.GetStream(path); + } + /// /// Remove all stored customisations for the provided target. /// diff --git a/osu.Game/Skinning/SkinConfiguration.cs b/osu.Game/Skinning/SkinConfiguration.cs index a18144246f..f71f6811e8 100644 --- a/osu.Game/Skinning/SkinConfiguration.cs +++ b/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; + /// /// Whether to allow as a fallback list for when no combo colours are provided. /// internal bool AllowDefaultComboColoursFallback = true; + /// + /// Legacy version of this skin. + /// + public decimal? LegacyVersion { get; internal set; } + + public enum LegacySetting + { + Version, + ComboPrefix, + ComboOverlap, + ScorePrefix, + ScoreOverlap, + HitCirclePrefix, + HitCircleOverlap, + AnimationFramerate, + LayeredHitSounds + } + public static List DefaultComboColours { get; } = new List { new Color4(255, 192, 0, 255), diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index 707afade09..177dbaf28f 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -183,7 +183,7 @@ namespace osu.Game.Skinning // 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 (instance is LegacySkin && skinIniSourcedName != item.Name) + if (skinIniSourcedName != item.Name) updateSkinIniMetadata(item); return base.ComputeHash(item, reader); From eef9949a0a928f3d10df97375119ccc1fbdbbf90 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 22 Oct 2021 14:48:20 +0900 Subject: [PATCH 18/68] Remove unnecessary branching around EF logic --- osu.Game/Database/ArchiveModelManager.cs | 48 +++++++----------------- 1 file changed, 14 insertions(+), 34 deletions(-) diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index 0bc4c40459..ad5bc6d420 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -511,29 +511,21 @@ namespace osu.Game.Database /// The existing file to be deleted. public void DeleteFile(TModel model, TFileModel file) { - if (model.ID > 0) + using (var usage = ContextFactory.GetForWrite()) { - using (var usage = ContextFactory.GetForWrite()) + // Dereference the existing file info, since the file model will be removed. + if (file.FileInfo != null) { - // Dereference the existing file info, since the file model will be removed. - if (file.FileInfo != null) + Files.Dereference(file.FileInfo); + + if (file.ID > 0) { - 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().Remove(file); - } + // 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().Remove(file); } - - model.Files.Remove(file); } - } - else - { - Files.Dereference(file.FileInfo); + model.Files.Remove(file); } } @@ -546,29 +538,17 @@ namespace osu.Game.Database /// The filename for the new file. public void AddFile(TModel model, Stream contents, string filename) { - if (model.ID > 0) + using (ContextFactory.GetForWrite()) { - using (ContextFactory.GetForWrite()) - { - model.Files.Add(new TFileModel - { - Filename = filename, - FileInfo = Files.Add(contents) - }); - - Update(model); - } - } - else - { - // This function may be called during the import process. - // Should not exist like this once we have switched to realm. model.Files.Add(new TFileModel { Filename = filename, FileInfo = Files.Add(contents) }); } + + if (model.ID > 0) + Update(model); } /// From 4440b9ca11b6c5e8add43c24a4546d0b8256bd01 Mon Sep 17 00:00:00 2001 From: goodtrailer Date: Sat, 23 Oct 2021 01:59:07 -0700 Subject: [PATCH 19/68] Change IHasRepeats.NodeSamples to IList from List --- osu.Game.Rulesets.Catch/Objects/JuiceStream.cs | 2 +- .../ManiaBeatmapSampleConversionTest.cs | 2 +- .../Beatmaps/Patterns/Legacy/DistanceObjectPatternGenerator.cs | 2 +- osu.Game.Rulesets.Mania/Objects/HoldNote.cs | 2 +- osu.Game.Rulesets.Osu/Objects/Slider.cs | 2 +- osu.Game.Rulesets.Taiko/Beatmaps/TaikoBeatmapConverter.cs | 2 +- .../Rulesets/Objects/Legacy/Catch/ConvertHitObjectParser.cs | 2 +- osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs | 2 +- osu.Game/Rulesets/Objects/Legacy/ConvertSlider.cs | 2 +- .../Rulesets/Objects/Legacy/Mania/ConvertHitObjectParser.cs | 3 ++- osu.Game/Rulesets/Objects/Legacy/Osu/ConvertHitObjectParser.cs | 2 +- .../Rulesets/Objects/Legacy/Taiko/ConvertHitObjectParser.cs | 2 +- osu.Game/Rulesets/Objects/Types/IHasRepeats.cs | 2 +- 13 files changed, 14 insertions(+), 13 deletions(-) diff --git a/osu.Game.Rulesets.Catch/Objects/JuiceStream.cs b/osu.Game.Rulesets.Catch/Objects/JuiceStream.cs index 6d5a960f06..282afb6343 100644 --- a/osu.Game.Rulesets.Catch/Objects/JuiceStream.cs +++ b/osu.Game.Rulesets.Catch/Objects/JuiceStream.cs @@ -145,7 +145,7 @@ namespace osu.Game.Rulesets.Catch.Objects public double Distance => Path.Distance; - public List> NodeSamples { get; set; } = new List>(); + public IList> NodeSamples { get; set; } = new List>(); public double? LegacyLastTickOffset { get; set; } } diff --git a/osu.Game.Rulesets.Mania.Tests/ManiaBeatmapSampleConversionTest.cs b/osu.Game.Rulesets.Mania.Tests/ManiaBeatmapSampleConversionTest.cs index c8feb4ae24..9c690f360a 100644 --- a/osu.Game.Rulesets.Mania.Tests/ManiaBeatmapSampleConversionTest.cs +++ b/osu.Game.Rulesets.Mania.Tests/ManiaBeatmapSampleConversionTest.cs @@ -38,7 +38,7 @@ namespace osu.Game.Rulesets.Mania.Tests private IList getSampleNames(IList hitSampleInfo) => hitSampleInfo.Select(sample => sample.LookupNames.First()).ToList(); - private IList> getNodeSampleNames(List> hitSampleInfo) + private IList> getNodeSampleNames(IList> hitSampleInfo) => hitSampleInfo?.Select(getSampleNames) .ToList(); diff --git a/osu.Game.Rulesets.Mania/Beatmaps/Patterns/Legacy/DistanceObjectPatternGenerator.cs b/osu.Game.Rulesets.Mania/Beatmaps/Patterns/Legacy/DistanceObjectPatternGenerator.cs index 1ed045f7e0..8f18521063 100644 --- a/osu.Game.Rulesets.Mania/Beatmaps/Patterns/Legacy/DistanceObjectPatternGenerator.cs +++ b/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 . /// /// The time to retrieve node samples at. - private List> nodeSamplesAt(int time) + private IList> nodeSamplesAt(int time) { if (!(HitObject is IHasPathWithRepeats curveData)) return null; diff --git a/osu.Game.Rulesets.Mania/Objects/HoldNote.cs b/osu.Game.Rulesets.Mania/Objects/HoldNote.cs index c1937af7e4..db0d3e2c5a 100644 --- a/osu.Game.Rulesets.Mania/Objects/HoldNote.cs +++ b/osu.Game.Rulesets.Mania/Objects/HoldNote.cs @@ -67,7 +67,7 @@ namespace osu.Game.Rulesets.Mania.Objects } } - public List> NodeSamples { get; set; } + public IList> NodeSamples { get; set; } /// /// The head note of the hold. diff --git a/osu.Game.Rulesets.Osu/Objects/Slider.cs b/osu.Game.Rulesets.Osu/Objects/Slider.cs index 07d03ee1eb..a0c503a0b0 100644 --- a/osu.Game.Rulesets.Osu/Objects/Slider.cs +++ b/osu.Game.Rulesets.Osu/Objects/Slider.cs @@ -79,7 +79,7 @@ namespace osu.Game.Rulesets.Osu.Objects /// internal float LazyTravelDistance; - public List> NodeSamples { get; set; } = new List>(); + public IList> NodeSamples { get; set; } = new List>(); [JsonIgnore] public IList TailSamples { get; private set; } diff --git a/osu.Game.Rulesets.Taiko/Beatmaps/TaikoBeatmapConverter.cs b/osu.Game.Rulesets.Taiko/Beatmaps/TaikoBeatmapConverter.cs index 32aad6c36a..0c5719f29c 100644 --- a/osu.Game.Rulesets.Taiko/Beatmaps/TaikoBeatmapConverter.cs +++ b/osu.Game.Rulesets.Taiko/Beatmaps/TaikoBeatmapConverter.cs @@ -81,7 +81,7 @@ namespace osu.Game.Rulesets.Taiko.Beatmaps { if (shouldConvertSliderToHits(obj, beatmap, distanceData, out var taikoDuration, out var tickSpacing)) { - List> allSamples = obj is IHasPathWithRepeats curveData ? curveData.NodeSamples : new List>(new[] { samples }); + IList> allSamples = obj is IHasPathWithRepeats curveData ? curveData.NodeSamples : new List>(new[] { samples }); int i = 0; diff --git a/osu.Game/Rulesets/Objects/Legacy/Catch/ConvertHitObjectParser.cs b/osu.Game/Rulesets/Objects/Legacy/Catch/ConvertHitObjectParser.cs index c29179f749..2beb6bdbf2 100644 --- a/osu.Game/Rulesets/Objects/Legacy/Catch/ConvertHitObjectParser.cs +++ b/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> nodeSamples) + IList> nodeSamples) { newCombo |= forceNewCombo; comboOffset += extraComboOffset; diff --git a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs index 0942a7264d..24022b8588 100644 --- a/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs +++ b/osu.Game/Rulesets/Objects/Legacy/ConvertHitObjectParser.cs @@ -408,7 +408,7 @@ namespace osu.Game.Rulesets.Objects.Legacy /// The samples to be played when the slider nodes are hit. This includes the head and tail of the slider. /// The hit object. protected abstract HitObject CreateSlider(Vector2 position, bool newCombo, int comboOffset, PathControlPoint[] controlPoints, double? length, int repeatCount, - List> nodeSamples); + IList> nodeSamples); /// /// Creates a legacy Spinner-type hit object. diff --git a/osu.Game/Rulesets/Objects/Legacy/ConvertSlider.cs b/osu.Game/Rulesets/Objects/Legacy/ConvertSlider.cs index ad191f7ff5..9ff92fcc75 100644 --- a/osu.Game/Rulesets/Objects/Legacy/ConvertSlider.cs +++ b/osu.Game/Rulesets/Objects/Legacy/ConvertSlider.cs @@ -24,7 +24,7 @@ namespace osu.Game.Rulesets.Objects.Legacy public double Distance => Path.Distance; - public List> NodeSamples { get; set; } + public IList> NodeSamples { get; set; } public int RepeatCount { get; set; } [JsonIgnore] diff --git a/osu.Game/Rulesets/Objects/Legacy/Mania/ConvertHitObjectParser.cs b/osu.Game/Rulesets/Objects/Legacy/Mania/ConvertHitObjectParser.cs index bc64518f40..0eb6caf8ee 100644 --- a/osu.Game/Rulesets/Objects/Legacy/Mania/ConvertHitObjectParser.cs +++ b/osu.Game/Rulesets/Objects/Legacy/Mania/ConvertHitObjectParser.cs @@ -4,6 +4,7 @@ using osuTK; using osu.Game.Audio; using System.Collections.Generic; +using osu.Framework.Bindables; namespace osu.Game.Rulesets.Objects.Legacy.Mania { @@ -26,7 +27,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> nodeSamples) + IList> nodeSamples) { return new ConvertSlider { diff --git a/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertHitObjectParser.cs b/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertHitObjectParser.cs index 75ecab0b8f..cb98721be5 100644 --- a/osu.Game/Rulesets/Objects/Legacy/Osu/ConvertHitObjectParser.cs +++ b/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> nodeSamples) + IList> nodeSamples) { newCombo |= forceNewCombo; comboOffset += extraComboOffset; diff --git a/osu.Game/Rulesets/Objects/Legacy/Taiko/ConvertHitObjectParser.cs b/osu.Game/Rulesets/Objects/Legacy/Taiko/ConvertHitObjectParser.cs index 13e3e84c6a..1eafc4e68b 100644 --- a/osu.Game/Rulesets/Objects/Legacy/Taiko/ConvertHitObjectParser.cs +++ b/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> nodeSamples) + IList> nodeSamples) { return new ConvertSlider { diff --git a/osu.Game/Rulesets/Objects/Types/IHasRepeats.cs b/osu.Game/Rulesets/Objects/Types/IHasRepeats.cs index 674e2aee88..2a4215b960 100644 --- a/osu.Game/Rulesets/Objects/Types/IHasRepeats.cs +++ b/osu.Game/Rulesets/Objects/Types/IHasRepeats.cs @@ -25,7 +25,7 @@ namespace osu.Game.Rulesets.Objects.Types /// n-1: The last repeat.
/// n: The last node. ///
- List> NodeSamples { get; } + IList> NodeSamples { get; } } public static class HasRepeatsExtensions From 0affe7b79d338e808f001476af5b2ae5cee89293 Mon Sep 17 00:00:00 2001 From: goodtrailer Date: Sat, 23 Oct 2021 02:25:20 -0700 Subject: [PATCH 20/68] Remove unnecessary using --- osu.Game/Rulesets/Objects/Legacy/Mania/ConvertHitObjectParser.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game/Rulesets/Objects/Legacy/Mania/ConvertHitObjectParser.cs b/osu.Game/Rulesets/Objects/Legacy/Mania/ConvertHitObjectParser.cs index 0eb6caf8ee..386eb8d3ee 100644 --- a/osu.Game/Rulesets/Objects/Legacy/Mania/ConvertHitObjectParser.cs +++ b/osu.Game/Rulesets/Objects/Legacy/Mania/ConvertHitObjectParser.cs @@ -4,7 +4,6 @@ using osuTK; using osu.Game.Audio; using System.Collections.Generic; -using osu.Framework.Bindables; namespace osu.Game.Rulesets.Objects.Legacy.Mania { From 051cc2cd92972c20027e4ef4dd4973f23cc7ac69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Sun, 24 Oct 2021 12:47:17 +0200 Subject: [PATCH 21/68] Update reference to configuration population process in comment --- osu.Game/Skinning/SkinManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index 177dbaf28f..860d09e011 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -161,7 +161,7 @@ namespace osu.Game.Skinning // 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. - // LegacySkin will parse the skin.ini and populate `Skin.Configuration` during construction above. + // `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); From 5303cae0444064e8696a8db979273ca6730155ee Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 24 Oct 2021 23:43:37 +0900 Subject: [PATCH 22/68] Add mention of stream close logic --- osu.Game/Skinning/Skin.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game/Skinning/Skin.cs b/osu.Game/Skinning/Skin.cs index d5f41f01a4..823a5d5133 100644 --- a/osu.Game/Skinning/Skin.cs +++ b/osu.Game/Skinning/Skin.cs @@ -47,6 +47,7 @@ namespace osu.Game.Skinning configurationStream ??= getConfigurationStream(); if (configurationStream != null) + // stream will be closed after use by LineBufferedReader. ParseConfigurationStream(configurationStream); else Configuration = new SkinConfiguration(); From 26cf5370c35bf977388eaf651503d783746bb34b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 24 Oct 2021 23:48:46 +0900 Subject: [PATCH 23/68] Remove unused `reader` parameter --- osu.Game/Database/ArchiveModelManager.cs | 4 ++-- osu.Game/Skinning/SkinManager.cs | 4 ++-- osu.Game/Tests/Visual/EditorTestScene.cs | 3 +-- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index ad5bc6d420..00e1f03b92 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -315,7 +315,7 @@ namespace osu.Game.Database /// /// In the case of no matching files, a hash will be generated from the passed archive's . /// - protected virtual string ComputeHash(TModel item, ArchiveReader reader = null) + protected virtual string ComputeHash(TModel item) { var hashableFiles = item.Files .Where(f => HashableFileTypes.Any(ext => f.Filename.EndsWith(ext, StringComparison.OrdinalIgnoreCase))) @@ -397,7 +397,7 @@ namespace osu.Game.Database LogForModel(item, @"Beginning import..."); item.Files = archive != null ? createFileInfos(archive, Files) : new List(); - item.Hash = ComputeHash(item, archive); + item.Hash = ComputeHash(item); await Populate(item, archive, cancellationToken).ConfigureAwait(false); diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs index 860d09e011..2187d2d875 100644 --- a/osu.Game/Skinning/SkinManager.cs +++ b/osu.Game/Skinning/SkinManager.cs @@ -155,7 +155,7 @@ namespace osu.Game.Skinning protected override bool HasCustomHashFunction => true; - protected override string ComputeHash(SkinInfo item, ArchiveReader reader = null) + protected override string ComputeHash(SkinInfo item) { var instance = GetSkin(item); @@ -186,7 +186,7 @@ namespace osu.Game.Skinning if (skinIniSourcedName != item.Name) updateSkinIniMetadata(item); - return base.ComputeHash(item, reader); + return base.ComputeHash(item); } private void updateSkinIniMetadata(SkinInfo item) diff --git a/osu.Game/Tests/Visual/EditorTestScene.cs b/osu.Game/Tests/Visual/EditorTestScene.cs index 798b0d01ee..4b02306d33 100644 --- a/osu.Game/Tests/Visual/EditorTestScene.cs +++ b/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; } From 708d40931b8009d3c036df32ad0487ab43abc6f1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 26 Oct 2021 16:36:54 +0900 Subject: [PATCH 24/68] Add back remaining attempt display on playlist room screen --- .../Playlists/PlaylistsRoomSubScreen.cs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsRoomSubScreen.cs b/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsRoomSubScreen.cs index d5e423a438..5f2e444165 100644 --- a/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsRoomSubScreen.cs +++ b/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsRoomSubScreen.cs @@ -153,6 +153,22 @@ namespace osu.Game.Screens.OnlinePlay.Playlists }, }, new Drawable[] + { + new FillFlowContainer + { + RelativeSizeAxes = Axes.X, + AutoSizeAxes = Axes.Y, + Alpha = Room.MaxAttempts.Value != null ? 1 : 0, + Margin = new MarginPadding { Bottom = 10 }, + Direction = FillDirection.Vertical, + Children = new Drawable[] + { + new OverlinedHeader("Progress"), + new RoomLocalUserInfo(), + } + }, + }, + new Drawable[] { new OverlinedHeader("Leaderboard") }, @@ -162,6 +178,7 @@ namespace osu.Game.Screens.OnlinePlay.Playlists }, RowDimensions = new[] { + new Dimension(GridSizeMode.AutoSize), new Dimension(GridSizeMode.AutoSize), new Dimension(GridSizeMode.AutoSize), new Dimension(), From bffd9162ba15055260021053a2bcfc5b23f13fd2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 26 Oct 2021 16:40:45 +0900 Subject: [PATCH 25/68] Tint attempt count red when no attempts remain --- osu.Game/Screens/OnlinePlay/Components/RoomLocalUserInfo.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/osu.Game/Screens/OnlinePlay/Components/RoomLocalUserInfo.cs b/osu.Game/Screens/OnlinePlay/Components/RoomLocalUserInfo.cs index 1fcf7f2277..3bad6cb183 100644 --- a/osu.Game/Screens/OnlinePlay/Components/RoomLocalUserInfo.cs +++ b/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 From 1c578f285accf3f17168708ba5c6aa1fd402f6ec Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 26 Oct 2021 16:54:55 +0900 Subject: [PATCH 26/68] Disable playlist start button when attempts have been exhausted --- .../Playlists/PlaylistsReadyButton.cs | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsReadyButton.cs b/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsReadyButton.cs index 9ac1fe1722..ea85a8bcc1 100644 --- a/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsReadyButton.cs +++ b/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsReadyButton.cs @@ -32,11 +32,32 @@ 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; } + + 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; } } From 42d8d4fd380bcdc2fdd4a7b96178b7f4a551386f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 26 Oct 2021 16:55:24 +0900 Subject: [PATCH 27/68] Add tooltips to start button to explain why it's not clickable --- .../OnlinePlay/Components/ReadyButton.cs | 18 +++++++++++-- .../Playlists/PlaylistsReadyButton.cs | 25 +++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Components/ReadyButton.cs b/osu.Game/Screens/OnlinePlay/Components/ReadyButton.cs index 8f85608b29..5a4a0a0fba 100644 --- a/osu.Game/Screens/OnlinePlay/Components/ReadyButton.cs +++ b/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"; + } + } } } diff --git a/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsReadyButton.cs b/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsReadyButton.cs index ea85a8bcc1..24f112ef0c 100644 --- a/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsReadyButton.cs +++ b/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 endDate { get; set; } + [Resolved(typeof(Room), nameof(Room.MaxAttempts))] + private Bindable maxAttempts { get; set; } + + [Resolved(typeof(Room), nameof(Room.UserScore))] + private Bindable userScore { get; set; } + [Resolved] private IBindable gameBeatmap { get; set; } @@ -56,6 +64,23 @@ namespace osu.Game.Screens.OnlinePlay.Playlists 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; From c15bfb2cf44d569062961a7ed59aee738b7684bc Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 26 Oct 2021 18:13:28 +0900 Subject: [PATCH 28/68] Handle bindable changes to fix new playlist creation not showing correct details --- .../OnlinePlay/Playlists/PlaylistsRoomSubScreen.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsRoomSubScreen.cs b/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsRoomSubScreen.cs index 5f2e444165..6d2a426e70 100644 --- a/osu.Game/Screens/OnlinePlay/Playlists/PlaylistsRoomSubScreen.cs +++ b/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 @@ -154,11 +158,11 @@ namespace osu.Game.Screens.OnlinePlay.Playlists }, new Drawable[] { - new FillFlowContainer + progressSection = new FillFlowContainer { RelativeSizeAxes = Axes.X, AutoSizeAxes = Axes.Y, - Alpha = Room.MaxAttempts.Value != null ? 1 : 0, + Alpha = 0, Margin = new MarginPadding { Bottom = 10 }, Direction = FillDirection.Vertical, Children = new Drawable[] From 2bc242001217920324cbddb9bc462e005a2632f0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 27 Oct 2021 13:54:20 +0900 Subject: [PATCH 29/68] Rename test scene It's not testing the sub-screen. It's testing creating playlists. --- ...nePlaylistsRoomSubScreen.cs => TestScenePlaylistCreation.cs} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename osu.Game.Tests/Visual/Playlists/{TestScenePlaylistsRoomSubScreen.cs => TestScenePlaylistCreation.cs} (98%) diff --git a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomSubScreen.cs b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistCreation.cs similarity index 98% rename from osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomSubScreen.cs rename to osu.Game.Tests/Visual/Playlists/TestScenePlaylistCreation.cs index deb9a22184..300a164644 100644 --- a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomSubScreen.cs +++ b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistCreation.cs @@ -22,7 +22,7 @@ using osuTK.Input; namespace osu.Game.Tests.Visual.Playlists { - public class TestScenePlaylistsRoomSubScreen : OnlinePlayTestScene + public class TestScenePlaylistCreation : OnlinePlayTestScene { private BeatmapManager manager; private RulesetStore rulesets; From c4a253cc75d5ece03ff3d44eb582c198877319f5 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 27 Oct 2021 13:55:14 +0900 Subject: [PATCH 30/68] Replace local instantiation of `TestBeatmap` with helper method --- .../Visual/Playlists/TestScenePlaylistCreation.cs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistCreation.cs b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistCreation.cs index 300a164644..a0b51610ec 100644 --- a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistCreation.cs +++ b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistCreation.cs @@ -16,7 +16,6 @@ using osu.Game.Rulesets; using osu.Game.Rulesets.Osu; using osu.Game.Screens.OnlinePlay.Playlists; using osu.Game.Screens.Play; -using osu.Game.Tests.Beatmaps; using osu.Game.Tests.Visual.OnlinePlay; using osuTK.Input; @@ -40,7 +39,7 @@ namespace osu.Game.Tests.Visual.Playlists public void SetupSteps() { AddStep("set room", () => SelectedRoom.Value = new Room()); - AddStep("ensure has beatmap", () => manager.Import(new TestBeatmap(new OsuRuleset().RulesetInfo).BeatmapInfo.BeatmapSet).Wait()); + AddStep("ensure has beatmap", () => manager.Import(CreateBeatmap(new OsuRuleset().RulesetInfo).BeatmapInfo.BeatmapSet).Wait()); AddStep("load match", () => LoadScreen(match = new TestPlaylistsRoomSubScreen(SelectedRoom.Value))); AddUntilStep("wait for load", () => match.IsCurrentScreen()); } @@ -57,7 +56,7 @@ namespace osu.Game.Tests.Visual.Playlists SelectedRoom.Value.EndDate.Value = DateTimeOffset.Now.AddMinutes(5); SelectedRoom.Value.Playlist.Add(new PlaylistItem { - Beatmap = { Value = new TestBeatmap(new OsuRuleset().RulesetInfo).BeatmapInfo }, + Beatmap = { Value = CreateBeatmap(new OsuRuleset().RulesetInfo).BeatmapInfo }, Ruleset = { Value = new OsuRuleset().RulesetInfo } }); }); @@ -75,7 +74,7 @@ namespace osu.Game.Tests.Visual.Playlists SelectedRoom.Value.Host.Value = API.LocalUser.Value; SelectedRoom.Value.Playlist.Add(new PlaylistItem { - Beatmap = { Value = new TestBeatmap(new OsuRuleset().RulesetInfo).BeatmapInfo }, + Beatmap = { Value = CreateBeatmap(new OsuRuleset().RulesetInfo).BeatmapInfo }, Ruleset = { Value = new OsuRuleset().RulesetInfo } }); }); @@ -94,13 +93,13 @@ namespace osu.Game.Tests.Visual.Playlists public void TestBeatmapUpdatedOnReImport() { BeatmapSetInfo importedSet = null; - TestBeatmap beatmap = null; + IBeatmap beatmap = null; // this step is required to make sure the further imports actually get online IDs. // all the playlist logic relies on online ID matching. AddStep("remove all matching online IDs", () => { - beatmap = new TestBeatmap(new OsuRuleset().RulesetInfo); + beatmap = CreateBeatmap(new OsuRuleset().RulesetInfo); var existing = manager.QueryBeatmapSets(s => s.OnlineBeatmapSetID == beatmap.BeatmapInfo.BeatmapSet.OnlineBeatmapSetID).ToList(); @@ -139,7 +138,7 @@ namespace osu.Game.Tests.Visual.Playlists AddAssert("match has altered beatmap", () => match.Beatmap.Value.Beatmap.Difficulty.CircleSize == 1); - AddStep("re-import original beatmap", () => manager.Import(new TestBeatmap(new OsuRuleset().RulesetInfo).BeatmapInfo.BeatmapSet).Wait()); + AddStep("re-import original beatmap", () => manager.Import(CreateBeatmap(new OsuRuleset().RulesetInfo).BeatmapInfo.BeatmapSet).Wait()); AddAssert("match has original beatmap", () => match.Beatmap.Value.Beatmap.Difficulty.CircleSize != 1); } From 84bb3a962b9def275ce92d29cb386b16913308a0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 27 Oct 2021 14:11:07 +0900 Subject: [PATCH 31/68] Centralise room "creation" --- .../Playlists/TestScenePlaylistCreation.cs | 46 +++++++++++++------ 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistCreation.cs b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistCreation.cs index a0b51610ec..9ee0fb4db5 100644 --- a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistCreation.cs +++ b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistCreation.cs @@ -47,18 +47,20 @@ namespace osu.Game.Tests.Visual.Playlists [Test] public void TestLoadSimpleMatch() { - AddStep("set room properties", () => + setupRoom(room => { - SelectedRoom.Value.RoomID.Value = 1; - SelectedRoom.Value.Name.Value = "my awesome room"; - SelectedRoom.Value.Host.Value = API.LocalUser.Value; - SelectedRoom.Value.RecentParticipants.Add(SelectedRoom.Value.Host.Value); - SelectedRoom.Value.EndDate.Value = DateTimeOffset.Now.AddMinutes(5); - SelectedRoom.Value.Playlist.Add(new PlaylistItem + room.RoomID.Value = 1; // forces room creation. + room.Name.Value = "my awesome room"; + room.Host.Value = API.LocalUser.Value; + room.RecentParticipants.Add(room.Host.Value); + room.EndDate.Value = DateTimeOffset.Now.AddMinutes(5); + room.Playlist.Add(new PlaylistItem { Beatmap = { Value = CreateBeatmap(new OsuRuleset().RulesetInfo).BeatmapInfo }, Ruleset = { Value = new OsuRuleset().RulesetInfo } }); + + OnlinePlayDependencies.RoomManager.CreateRoom(SelectedRoom.Value); }); AddStep("start match", () => match.ChildrenOfType().First().TriggerClick()); @@ -68,15 +70,17 @@ namespace osu.Game.Tests.Visual.Playlists [Test] public void TestPlaylistItemSelectedOnCreate() { - AddStep("set room properties", () => + setupRoom(room => { - SelectedRoom.Value.Name.Value = "my awesome room"; - SelectedRoom.Value.Host.Value = API.LocalUser.Value; - SelectedRoom.Value.Playlist.Add(new PlaylistItem + room.Name.Value = "my awesome room"; + room.Host.Value = API.LocalUser.Value; + room.Playlist.Add(new PlaylistItem { Beatmap = { Value = CreateBeatmap(new OsuRuleset().RulesetInfo).BeatmapInfo }, Ruleset = { Value = new OsuRuleset().RulesetInfo } }); + + OnlinePlayDependencies.RoomManager.CreateRoom(SelectedRoom.Value); }); AddStep("move mouse to create button", () => @@ -119,11 +123,11 @@ namespace osu.Game.Tests.Visual.Playlists importedSet = manager.Import(beatmap.BeatmapInfo.BeatmapSet).Result.Value; }); - AddStep("load room", () => + setupRoom(room => { - SelectedRoom.Value.Name.Value = "my awesome room"; - SelectedRoom.Value.Host.Value = API.LocalUser.Value; - SelectedRoom.Value.Playlist.Add(new PlaylistItem + room.Name.Value = "my awesome room"; + room.Host.Value = API.LocalUser.Value; + room.Playlist.Add(new PlaylistItem { Beatmap = { Value = importedSet.Beatmaps[0] }, Ruleset = { Value = new OsuRuleset().RulesetInfo } @@ -143,6 +147,18 @@ namespace osu.Game.Tests.Visual.Playlists AddAssert("match has original beatmap", () => match.Beatmap.Value.Beatmap.Difficulty.CircleSize != 1); } + private void setupRoom(Action room) + { + AddStep("setup room", () => + { + room(SelectedRoom.Value); + + // if this isn't done the test will crash when a poll kicks in. + // probably not correct, but works for now. + OnlinePlayDependencies.RoomManager.CreateRoom(SelectedRoom.Value); + }); + } + private class TestPlaylistsRoomSubScreen : PlaylistsRoomSubScreen { public new Bindable SelectedItem => base.SelectedItem; From 58237f6fee5b63e2d955389efe47377297f1c898 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 27 Oct 2021 14:26:37 +0900 Subject: [PATCH 32/68] Fix weird import flows --- .../Playlists/TestScenePlaylistCreation.cs | 43 +++++++++---------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistCreation.cs b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistCreation.cs index 9ee0fb4db5..c4e6f41f27 100644 --- a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistCreation.cs +++ b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistCreation.cs @@ -11,6 +11,7 @@ using osu.Framework.Platform; using osu.Framework.Screens; using osu.Framework.Testing; using osu.Game.Beatmaps; +using osu.Game.Database; using osu.Game.Online.Rooms; using osu.Game.Rulesets; using osu.Game.Rulesets.Osu; @@ -28,6 +29,8 @@ namespace osu.Game.Tests.Visual.Playlists private TestPlaylistsRoomSubScreen match; + private ILive importedBeatmap; + [BackgroundDependencyLoader] private void load(GameHost host, AudioManager audio) { @@ -39,7 +42,9 @@ namespace osu.Game.Tests.Visual.Playlists public void SetupSteps() { AddStep("set room", () => SelectedRoom.Value = new Room()); - AddStep("ensure has beatmap", () => manager.Import(CreateBeatmap(new OsuRuleset().RulesetInfo).BeatmapInfo.BeatmapSet).Wait()); + + importBeatmap(); + AddStep("load match", () => LoadScreen(match = new TestPlaylistsRoomSubScreen(SelectedRoom.Value))); AddUntilStep("wait for load", () => match.IsCurrentScreen()); } @@ -47,7 +52,7 @@ namespace osu.Game.Tests.Visual.Playlists [Test] public void TestLoadSimpleMatch() { - setupRoom(room => + setupAndCreateRoom(room => { room.RoomID.Value = 1; // forces room creation. room.Name.Value = "my awesome room"; @@ -56,11 +61,9 @@ namespace osu.Game.Tests.Visual.Playlists room.EndDate.Value = DateTimeOffset.Now.AddMinutes(5); room.Playlist.Add(new PlaylistItem { - Beatmap = { Value = CreateBeatmap(new OsuRuleset().RulesetInfo).BeatmapInfo }, + Beatmap = { Value = importedBeatmap.Value.Beatmaps.First() }, Ruleset = { Value = new OsuRuleset().RulesetInfo } }); - - OnlinePlayDependencies.RoomManager.CreateRoom(SelectedRoom.Value); }); AddStep("start match", () => match.ChildrenOfType().First().TriggerClick()); @@ -70,17 +73,15 @@ namespace osu.Game.Tests.Visual.Playlists [Test] public void TestPlaylistItemSelectedOnCreate() { - setupRoom(room => + setupAndCreateRoom(room => { room.Name.Value = "my awesome room"; room.Host.Value = API.LocalUser.Value; room.Playlist.Add(new PlaylistItem { - Beatmap = { Value = CreateBeatmap(new OsuRuleset().RulesetInfo).BeatmapInfo }, + Beatmap = { Value = importedBeatmap.Value.Beatmaps.First() }, Ruleset = { Value = new OsuRuleset().RulesetInfo } }); - - OnlinePlayDependencies.RoomManager.CreateRoom(SelectedRoom.Value); }); AddStep("move mouse to create button", () => @@ -97,15 +98,12 @@ namespace osu.Game.Tests.Visual.Playlists public void TestBeatmapUpdatedOnReImport() { BeatmapSetInfo importedSet = null; - IBeatmap beatmap = null; // this step is required to make sure the further imports actually get online IDs. // all the playlist logic relies on online ID matching. AddStep("remove all matching online IDs", () => { - beatmap = CreateBeatmap(new OsuRuleset().RulesetInfo); - - var existing = manager.QueryBeatmapSets(s => s.OnlineBeatmapSetID == beatmap.BeatmapInfo.BeatmapSet.OnlineBeatmapSetID).ToList(); + var existing = manager.QueryBeatmapSets(s => s.OnlineBeatmapSetID == importedBeatmap.Value.OnlineBeatmapSetID).ToList(); foreach (var s in existing) { @@ -118,12 +116,14 @@ namespace osu.Game.Tests.Visual.Playlists AddStep("import altered beatmap", () => { + IBeatmap beatmap = CreateBeatmap(new OsuRuleset().RulesetInfo); + beatmap.BeatmapInfo.BaseDifficulty.CircleSize = 1; importedSet = manager.Import(beatmap.BeatmapInfo.BeatmapSet).Result.Value; }); - setupRoom(room => + setupAndCreateRoom(room => { room.Name.Value = "my awesome room"; room.Host.Value = API.LocalUser.Value; @@ -134,20 +134,14 @@ namespace osu.Game.Tests.Visual.Playlists }); }); - AddStep("create room", () => - { - InputManager.MoveMouseTo(match.ChildrenOfType().Single()); - InputManager.Click(MouseButton.Left); - }); - AddAssert("match has altered beatmap", () => match.Beatmap.Value.Beatmap.Difficulty.CircleSize == 1); - AddStep("re-import original beatmap", () => manager.Import(CreateBeatmap(new OsuRuleset().RulesetInfo).BeatmapInfo.BeatmapSet).Wait()); + importBeatmap(); AddAssert("match has original beatmap", () => match.Beatmap.Value.Beatmap.Difficulty.CircleSize != 1); } - private void setupRoom(Action room) + private void setupAndCreateRoom(Action room) { AddStep("setup room", () => { @@ -159,6 +153,11 @@ namespace osu.Game.Tests.Visual.Playlists }); } + private void importBeatmap() + { + AddStep("import beatmap", () => importedBeatmap = manager.Import(CreateBeatmap(new OsuRuleset().RulesetInfo).BeatmapInfo.BeatmapSet).Result); + } + private class TestPlaylistsRoomSubScreen : PlaylistsRoomSubScreen { public new Bindable SelectedItem => base.SelectedItem; From 85b21174dd26cb55bc7598bf3d903bf6fd54d297 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Wed, 27 Oct 2021 14:49:50 +0900 Subject: [PATCH 33/68] Fix online play test request handling --- .../Visual/OnlinePlay/OnlinePlayTestScene.cs | 20 +++++++++++++++++-- .../TestRequestHandlingRoomManager.cs | 10 ---------- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/osu.Game/Tests/Visual/OnlinePlay/OnlinePlayTestScene.cs b/osu.Game/Tests/Visual/OnlinePlay/OnlinePlayTestScene.cs index 8716646074..d782160ee5 100644 --- a/osu.Game/Tests/Visual/OnlinePlay/OnlinePlayTestScene.cs +++ b/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,15 @@ namespace osu.Game.Tests.Visual.OnlinePlay ///
protected OnlinePlayTestSceneDependencies OnlinePlayDependencies => dependencies?.OnlinePlayDependencies; - private DelegatedDependencyContainer dependencies; - protected override Container Content => content; + + [Resolved] + private OsuGameBase game { get; set; } + private readonly Container content; private readonly Container drawableDependenciesContainer; + private DelegatedDependencyContainer dependencies; + private TestRoomRequestsHandler requestsHandler; protected OnlinePlayTestScene() { @@ -57,6 +62,17 @@ namespace osu.Game.Tests.Visual.OnlinePlay drawableDependenciesContainer.AddRange(OnlinePlayDependencies.DrawableComponents); }); + public override void SetUpSteps() + { + base.SetUpSteps(); + + AddStep("setup API", () => + { + requestsHandler = new TestRoomRequestsHandler(); + ((DummyAPIAccess)API).HandleRequest = request => requestsHandler.HandleRequest(request, API.LocalUser.Value, game); + }); + } + /// /// Creates the room dependencies. Called every . /// diff --git a/osu.Game/Tests/Visual/OnlinePlay/TestRequestHandlingRoomManager.cs b/osu.Game/Tests/Visual/OnlinePlay/TestRequestHandlingRoomManager.cs index d88fd68b20..ef0ceafd02 100644 --- a/osu.Game/Tests/Visual/OnlinePlay/TestRequestHandlingRoomManager.cs +++ b/osu.Game/Tests/Visual/OnlinePlay/TestRequestHandlingRoomManager.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; @@ -21,14 +19,6 @@ namespace osu.Game.Tests.Visual.OnlinePlay 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 onSuccess = null, Action onError = null) { JoinRoomRequested?.Invoke(room, password); From 676070946c38f1bfa79ce77b902a747f332c5bc6 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Wed, 27 Oct 2021 15:25:26 +0900 Subject: [PATCH 34/68] Fix missed base.SetUpSteps() --- .../Multiplayer/TestSceneMultiplayerGameplayLeaderboard.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerGameplayLeaderboard.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerGameplayLeaderboard.cs index eff107faee..10633f4a4f 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerGameplayLeaderboard.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerGameplayLeaderboard.cs @@ -43,6 +43,8 @@ namespace osu.Game.Tests.Visual.Multiplayer [SetUpSteps] public override void SetUpSteps() { + base.SetUpSteps(); + AddStep("set local user", () => ((DummyAPIAccess)API).LocalUser.Value = LookupCache.GetUserAsync(1).Result); AddStep("create leaderboard", () => From f2683639242e18f474749df7cdaaf2182337bc9f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 27 Oct 2021 15:20:01 +0900 Subject: [PATCH 35/68] Update `UpdateableBeatmapBackgroundSprite` to accept `IBeatmapInfo` --- .../UpdateableBeatmapBackgroundSprite.cs | 19 ++++++++++--------- .../OnlinePlay/DrawableRoomPlaylistItem.cs | 9 +++++++-- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/osu.Game/Beatmaps/Drawables/UpdateableBeatmapBackgroundSprite.cs b/osu.Game/Beatmaps/Drawables/UpdateableBeatmapBackgroundSprite.cs index 8943ad350e..636c568bd0 100644 --- a/osu.Game/Beatmaps/Drawables/UpdateableBeatmapBackgroundSprite.cs +++ b/osu.Game/Beatmaps/Drawables/UpdateableBeatmapBackgroundSprite.cs @@ -12,9 +12,9 @@ namespace osu.Game.Beatmaps.Drawables /// /// Display a beatmap background from a local source, but fallback to online source if not available. /// - public class UpdateableBeatmapBackgroundSprite : ModelBackedDrawable + public class UpdateableBeatmapBackgroundSprite : ModelBackedDrawable { - public readonly Bindable Beatmap = new Bindable(); + public readonly Bindable Beatmap = new Bindable(); protected override double LoadDelay => 500; @@ -39,7 +39,7 @@ namespace osu.Game.Beatmaps.Drawables protected override double TransformDuration => 400; - protected override Drawable CreateDrawable(BeatmapInfo model) + protected override Drawable CreateDrawable(IBeatmapInfo model) { var drawable = getDrawableForModel(model); drawable.RelativeSizeAxes = Axes.Both; @@ -50,15 +50,16 @@ namespace osu.Game.Beatmaps.Drawables return drawable; } - private Drawable getDrawableForModel(BeatmapInfo model) + private Drawable getDrawableForModel(IBeatmapInfo model) { // prefer online cover where available. - if (model?.BeatmapSet?.OnlineInfo != null) - return new OnlineBeatmapSetCover(model.BeatmapSet, beatmapSetCoverType); + if (model?.BeatmapSet is IBeatmapSetOnlineInfo online) + return new OnlineBeatmapSetCover(online, beatmapSetCoverType); - return model?.ID > 0 - ? new BeatmapBackgroundSprite(beatmaps.GetWorkingBeatmap(model)) - : new BeatmapBackgroundSprite(beatmaps.DefaultBeatmap); + if (model is BeatmapInfo localModel) + return new BeatmapBackgroundSprite(beatmaps.GetWorkingBeatmap(localModel)); + + return new BeatmapBackgroundSprite(beatmaps.DefaultBeatmap); } } } diff --git a/osu.Game/Screens/OnlinePlay/DrawableRoomPlaylistItem.cs b/osu.Game/Screens/OnlinePlay/DrawableRoomPlaylistItem.cs index 585b024623..264d49849c 100644 --- a/osu.Game/Screens/OnlinePlay/DrawableRoomPlaylistItem.cs +++ b/osu.Game/Screens/OnlinePlay/DrawableRoomPlaylistItem.cs @@ -333,13 +333,14 @@ namespace osu.Game.Screens.OnlinePlay public PanelBackground() { + UpdateableBeatmapBackgroundSprite backgroundSprite; + InternalChildren = new Drawable[] { - new UpdateableBeatmapBackgroundSprite + backgroundSprite = new UpdateableBeatmapBackgroundSprite { RelativeSizeAxes = Axes.Both, FillMode = FillMode.Fill, - Beatmap = { BindTarget = Beatmap } }, new FillFlowContainer { @@ -374,6 +375,10 @@ namespace osu.Game.Screens.OnlinePlay } } }; + + // manual binding required as playlists don't expose IBeatmapInfo currently. + // may be removed in the future if this changes. + Beatmap.BindValueChanged(beatmap => backgroundSprite.Beatmap.Value = beatmap.NewValue); } } } From d19580cf6023e336b26c354c64edc63c7326a8a6 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 25 Oct 2021 13:47:12 +0900 Subject: [PATCH 36/68] Fix incorrectly changed difficulty count in recently updated test --- osu.Game.Tests/Visual/Online/TestSceneBeatmapListingOverlay.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Online/TestSceneBeatmapListingOverlay.cs b/osu.Game.Tests/Visual/Online/TestSceneBeatmapListingOverlay.cs index 7042f1e4fe..c7a065fdd7 100644 --- a/osu.Game.Tests/Visual/Online/TestSceneBeatmapListingOverlay.cs +++ b/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().Any(d => d.IsPresent)); From 40d963fc8a115631bab1c7f0d1ba3ced61a0b456 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 26 Oct 2021 12:44:14 +0900 Subject: [PATCH 37/68] Allow setting of `APIBeatmap.Length` (and don't serialise twice) --- osu.Game/Online/API/Requests/Responses/APIBeatmap.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/osu.Game/Online/API/Requests/Responses/APIBeatmap.cs b/osu.Game/Online/API/Requests/Responses/APIBeatmap.cs index 0945ad30b4..e65dca752b 100644 --- a/osu.Game/Online/API/Requests/Responses/APIBeatmap.cs +++ b/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; } From db6343c44caf3d3f46ba9acfd3dc05c9e913dd17 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 27 Oct 2021 17:09:51 +0900 Subject: [PATCH 38/68] Correctly redirect `BPM` from API to interface --- osu.Game/Online/API/Requests/Responses/APIBeatmapSet.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Online/API/Requests/Responses/APIBeatmapSet.cs b/osu.Game/Online/API/Requests/Responses/APIBeatmapSet.cs index 83f04fb5f2..d8efa20b39 100644 --- a/osu.Game/Online/API/Requests/Responses/APIBeatmapSet.cs +++ b/osu.Game/Online/API/Requests/Responses/APIBeatmapSet.cs @@ -164,7 +164,7 @@ namespace osu.Game.Online.API.Requests.Responses IEnumerable IBeatmapSetInfo.Files => throw new NotImplementedException(); double IBeatmapSetInfo.MaxStarDifficulty => throw new NotImplementedException(); double IBeatmapSetInfo.MaxLength => throw new NotImplementedException(); - double IBeatmapSetInfo.MaxBPM => throw new NotImplementedException(); + double IBeatmapSetInfo.MaxBPM => BPM; #endregion } From 31306163003dc0ea62e9165a9987b9196af1407d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 27 Oct 2021 17:22:23 +0900 Subject: [PATCH 39/68] Update tournament components to use `APIBeatmap` directly --- .../TestSceneTournamentBeatmapPanel.cs | 6 ++--- .../TestSceneTournamentModDisplay.cs | 13 +++++------ .../NonVisual/LadderInfoSerialisationTest.cs | 4 ++-- .../Screens/TestSceneMapPoolScreen.cs | 2 +- .../TournamentTestScene.cs | 22 +++++++++---------- osu.Game.Tournament/Components/SongBar.cs | 22 +++++++++---------- .../Components/TournamentBeatmapPanel.cs | 7 +++--- osu.Game.Tournament/IPC/FileBasedIPC.cs | 6 ++--- osu.Game.Tournament/IPC/MatchIPCInfo.cs | 4 ++-- osu.Game.Tournament/Models/RoundBeatmap.cs | 4 ++-- osu.Game.Tournament/Models/SeedingBeatmap.cs | 4 ++-- .../Screens/BeatmapInfoScreen.cs | 4 ++-- .../Screens/Editors/RoundEditorScreen.cs | 6 ++--- .../Screens/Editors/SeedingEditorScreen.cs | 6 ++--- .../Screens/MapPool/MapPoolScreen.cs | 10 ++++----- osu.Game.Tournament/TournamentGameBase.cs | 4 ++-- 16 files changed, 59 insertions(+), 65 deletions(-) diff --git a/osu.Game.Tournament.Tests/Components/TestSceneTournamentBeatmapPanel.cs b/osu.Game.Tournament.Tests/Components/TestSceneTournamentBeatmapPanel.cs index f9c553cb3f..8139387a96 100644 --- a/osu.Game.Tournament.Tests/Components/TestSceneTournamentBeatmapPanel.cs +++ b/osu.Game.Tournament.Tests/Components/TestSceneTournamentBeatmapPanel.cs @@ -3,7 +3,6 @@ using osu.Framework.Allocation; using osu.Framework.Graphics; -using osu.Game.Beatmaps; using osu.Game.Online.API; using osu.Game.Online.API.Requests; using osu.Game.Online.API.Requests.Responses; @@ -23,14 +22,13 @@ namespace osu.Game.Tournament.Tests.Components [BackgroundDependencyLoader] private void load() { - var req = new GetBeatmapRequest(new BeatmapInfo { OnlineBeatmapID = 1091460 }); + var req = new GetBeatmapRequest(new APIBeatmap { OnlineID = 1091460 }); req.Success += success; api.Queue(req); } - private void success(APIBeatmap apiBeatmap) + private void success(APIBeatmap beatmap) { - var beatmap = apiBeatmap.ToBeatmapInfo(rulesets); Add(new TournamentBeatmapPanel(beatmap) { Anchor = Anchor.Centre, diff --git a/osu.Game.Tournament.Tests/Components/TestSceneTournamentModDisplay.cs b/osu.Game.Tournament.Tests/Components/TestSceneTournamentModDisplay.cs index 27eb55a9fb..3c22bdca03 100644 --- a/osu.Game.Tournament.Tests/Components/TestSceneTournamentModDisplay.cs +++ b/osu.Game.Tournament.Tests/Components/TestSceneTournamentModDisplay.cs @@ -4,12 +4,12 @@ using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; -using osu.Game.Beatmaps; using osu.Game.Online.API; using osu.Game.Online.API.Requests; using osu.Game.Online.API.Requests.Responses; using osu.Game.Rulesets; using osu.Game.Tournament.Components; +using osuTK; namespace osu.Game.Tournament.Tests.Components { @@ -23,12 +23,10 @@ namespace osu.Game.Tournament.Tests.Components private FillFlowContainer fillFlow; - private BeatmapInfo beatmapInfo; - [BackgroundDependencyLoader] private void load() { - var req = new GetBeatmapRequest(new BeatmapInfo { OnlineBeatmapID = 490154 }); + var req = new GetBeatmapRequest(new APIBeatmap { OnlineID = 490154 }); req.Success += success; api.Queue(req); @@ -38,18 +36,17 @@ namespace osu.Game.Tournament.Tests.Components Anchor = Anchor.Centre, Origin = Anchor.Centre, Direction = FillDirection.Full, - Spacing = new osuTK.Vector2(10) + Spacing = new Vector2(10) }); } - private void success(APIBeatmap apiBeatmap) + private void success(APIBeatmap beatmap) { - beatmapInfo = apiBeatmap.ToBeatmapInfo(rulesets); var mods = rulesets.GetRuleset(Ladder.Ruleset.Value.ID ?? 0).CreateInstance().AllMods; foreach (var mod in mods) { - fillFlow.Add(new TournamentBeatmapPanel(beatmapInfo, mod.Acronym) + fillFlow.Add(new TournamentBeatmapPanel(beatmap, mod.Acronym) { Anchor = Anchor.Centre, Origin = Anchor.Centre diff --git a/osu.Game.Tournament.Tests/NonVisual/LadderInfoSerialisationTest.cs b/osu.Game.Tournament.Tests/NonVisual/LadderInfoSerialisationTest.cs index 13cbcd3caf..2071512f60 100644 --- a/osu.Game.Tournament.Tests/NonVisual/LadderInfoSerialisationTest.cs +++ b/osu.Game.Tournament.Tests/NonVisual/LadderInfoSerialisationTest.cs @@ -44,8 +44,8 @@ namespace osu.Game.Tournament.Tests.NonVisual { Beatmaps = { - new RoundBeatmap { BeatmapInfo = TournamentTestScene.CreateSampleBeatmapInfo() }, - new RoundBeatmap { BeatmapInfo = TournamentTestScene.CreateSampleBeatmapInfo() }, + new RoundBeatmap { BeatmapInfo = TournamentTestScene.CreateSampleBeatmap() }, + new RoundBeatmap { BeatmapInfo = TournamentTestScene.CreateSampleBeatmap() }, } } }, diff --git a/osu.Game.Tournament.Tests/Screens/TestSceneMapPoolScreen.cs b/osu.Game.Tournament.Tests/Screens/TestSceneMapPoolScreen.cs index f4032fdd54..41fbf44179 100644 --- a/osu.Game.Tournament.Tests/Screens/TestSceneMapPoolScreen.cs +++ b/osu.Game.Tournament.Tests/Screens/TestSceneMapPoolScreen.cs @@ -132,7 +132,7 @@ namespace osu.Game.Tournament.Tests.Screens { Ladder.CurrentMatch.Value.Round.Value.Beatmaps.Add(new RoundBeatmap { - BeatmapInfo = CreateSampleBeatmapInfo(), + BeatmapInfo = CreateSampleBeatmap(), Mods = mods }); } diff --git a/osu.Game.Tournament.Tests/TournamentTestScene.cs b/osu.Game.Tournament.Tests/TournamentTestScene.cs index ce9fd91ff1..4f057286b6 100644 --- a/osu.Game.Tournament.Tests/TournamentTestScene.cs +++ b/osu.Game.Tournament.Tests/TournamentTestScene.cs @@ -7,7 +7,6 @@ using osu.Framework.Allocation; using osu.Framework.Platform; using osu.Framework.Testing; using osu.Framework.Utils; -using osu.Game.Beatmaps; using osu.Game.Online.API.Requests.Responses; using osu.Game.Rulesets; using osu.Game.Tests.Visual; @@ -74,19 +73,19 @@ namespace osu.Game.Tournament.Tests { new SeedingBeatmap { - BeatmapInfo = CreateSampleBeatmapInfo(), + BeatmapInfo = CreateSampleBeatmap(), Score = 12345672, Seed = { Value = 24 }, }, new SeedingBeatmap { - BeatmapInfo = CreateSampleBeatmapInfo(), + BeatmapInfo = CreateSampleBeatmap(), Score = 1234567, Seed = { Value = 12 }, }, new SeedingBeatmap { - BeatmapInfo = CreateSampleBeatmapInfo(), + BeatmapInfo = CreateSampleBeatmap(), Score = 1234567, Seed = { Value = 16 }, } @@ -100,19 +99,19 @@ namespace osu.Game.Tournament.Tests { new SeedingBeatmap { - BeatmapInfo = CreateSampleBeatmapInfo(), + BeatmapInfo = CreateSampleBeatmap(), Score = 234567, Seed = { Value = 3 }, }, new SeedingBeatmap { - BeatmapInfo = CreateSampleBeatmapInfo(), + BeatmapInfo = CreateSampleBeatmap(), Score = 234567, Seed = { Value = 6 }, }, new SeedingBeatmap { - BeatmapInfo = CreateSampleBeatmapInfo(), + BeatmapInfo = CreateSampleBeatmap(), Score = 234567, Seed = { Value = 12 }, } @@ -152,16 +151,15 @@ namespace osu.Game.Tournament.Tests } }; - public static BeatmapInfo CreateSampleBeatmapInfo() => - new BeatmapInfo + public static APIBeatmap CreateSampleBeatmap() => + new APIBeatmap { - Metadata = new BeatmapMetadata + BeatmapSet = new APIBeatmapSet { Title = "Test Title", Artist = "Test Artist", - ID = RNG.Next(0, 1000000) }, - OnlineInfo = new APIBeatmap(), + OnlineID = RNG.Next(0, 1000000), }; protected override ITestSceneTestRunner CreateRunner() => new TournamentTestSceneTestRunner(); diff --git a/osu.Game.Tournament/Components/SongBar.cs b/osu.Game.Tournament/Components/SongBar.cs index 60be7dec91..bdcb6f8ded 100644 --- a/osu.Game.Tournament/Components/SongBar.cs +++ b/osu.Game.Tournament/Components/SongBar.cs @@ -12,6 +12,7 @@ using osu.Game.Beatmaps; using osu.Game.Beatmaps.Legacy; using osu.Game.Extensions; using osu.Game.Graphics; +using osu.Game.Online.API.Requests.Responses; using osu.Game.Rulesets; using osu.Game.Screens.Menu; using osuTK; @@ -21,16 +22,15 @@ namespace osu.Game.Tournament.Components { public class SongBar : CompositeDrawable { - private BeatmapInfo beatmapInfo; + private APIBeatmap beatmapInfo; public const float HEIGHT = 145 / 2f; [Resolved] private IBindable ruleset { get; set; } - public BeatmapInfo BeatmapInfo + public APIBeatmap BeatmapInfo { - get => beatmapInfo; set { if (beatmapInfo == value) @@ -101,12 +101,12 @@ namespace osu.Game.Tournament.Components return; } - double bpm = beatmapInfo.BeatmapSet.OnlineInfo.BPM; + double bpm = beatmapInfo.BPM; // TODO: check this works. double length = beatmapInfo.Length; string hardRockExtra = ""; string srExtra = ""; - float ar = beatmapInfo.BaseDifficulty.ApproachRate; + float ar = beatmapInfo.Difficulty.ApproachRate; if ((mods & LegacyMods.HardRock) > 0) { @@ -132,9 +132,9 @@ namespace osu.Game.Tournament.Components default: stats = new (string heading, string content)[] { - ("CS", $"{beatmapInfo.BaseDifficulty.CircleSize:0.#}{hardRockExtra}"), + ("CS", $"{beatmapInfo.Difficulty.CircleSize:0.#}{hardRockExtra}"), ("AR", $"{ar:0.#}{hardRockExtra}"), - ("OD", $"{beatmapInfo.BaseDifficulty.OverallDifficulty:0.#}{hardRockExtra}"), + ("OD", $"{beatmapInfo.Difficulty.OverallDifficulty:0.#}{hardRockExtra}"), }; break; @@ -142,15 +142,15 @@ namespace osu.Game.Tournament.Components case 3: stats = new (string heading, string content)[] { - ("OD", $"{beatmapInfo.BaseDifficulty.OverallDifficulty:0.#}{hardRockExtra}"), - ("HP", $"{beatmapInfo.BaseDifficulty.DrainRate:0.#}{hardRockExtra}") + ("OD", $"{beatmapInfo.Difficulty.OverallDifficulty:0.#}{hardRockExtra}"), + ("HP", $"{beatmapInfo.Difficulty.DrainRate:0.#}{hardRockExtra}") }; break; case 2: stats = new (string heading, string content)[] { - ("CS", $"{beatmapInfo.BaseDifficulty.CircleSize:0.#}{hardRockExtra}"), + ("CS", $"{beatmapInfo.Difficulty.CircleSize:0.#}{hardRockExtra}"), ("AR", $"{ar:0.#}"), }; break; @@ -186,7 +186,7 @@ namespace osu.Game.Tournament.Components Children = new Drawable[] { new DiffPiece(stats), - new DiffPiece(("Star Rating", $"{beatmapInfo.StarDifficulty:0.#}{srExtra}")) + new DiffPiece(("Star Rating", $"{beatmapInfo.StarRating:0.#}{srExtra}")) } }, new FillFlowContainer diff --git a/osu.Game.Tournament/Components/TournamentBeatmapPanel.cs b/osu.Game.Tournament/Components/TournamentBeatmapPanel.cs index be29566e07..39fc1d0c0a 100644 --- a/osu.Game.Tournament/Components/TournamentBeatmapPanel.cs +++ b/osu.Game.Tournament/Components/TournamentBeatmapPanel.cs @@ -13,6 +13,7 @@ using osu.Framework.Graphics.Textures; using osu.Game.Beatmaps; using osu.Game.Beatmaps.Drawables; using osu.Game.Graphics; +using osu.Game.Online.API.Requests.Responses; using osu.Game.Tournament.Models; using osuTK.Graphics; @@ -20,7 +21,7 @@ namespace osu.Game.Tournament.Components { public class TournamentBeatmapPanel : CompositeDrawable { - public readonly IBeatmapInfo BeatmapInfo; + public readonly APIBeatmap BeatmapInfo; private readonly string mod; @@ -32,7 +33,7 @@ namespace osu.Game.Tournament.Components private readonly Bindable currentMatch = new Bindable(); private Box flash; - public TournamentBeatmapPanel(IBeatmapInfo beatmapInfo, string mod = null) + public TournamentBeatmapPanel(APIBeatmap beatmapInfo, string mod = null) { if (beatmapInfo == null) throw new ArgumentNullException(nameof(beatmapInfo)); @@ -62,7 +63,7 @@ namespace osu.Game.Tournament.Components { RelativeSizeAxes = Axes.Both, Colour = OsuColour.Gray(0.5f), - BeatmapSet = BeatmapInfo.BeatmapSet as IBeatmapSetOnlineInfo, + BeatmapSet = BeatmapInfo.BeatmapSet, }, new FillFlowContainer { diff --git a/osu.Game.Tournament/IPC/FileBasedIPC.cs b/osu.Game.Tournament/IPC/FileBasedIPC.cs index 6dde265bd6..15aa9dd588 100644 --- a/osu.Game.Tournament/IPC/FileBasedIPC.cs +++ b/osu.Game.Tournament/IPC/FileBasedIPC.cs @@ -11,10 +11,10 @@ using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Logging; using osu.Framework.Platform; using osu.Framework.Threading; -using osu.Game.Beatmaps; using osu.Game.Beatmaps.Legacy; using osu.Game.Online.API; using osu.Game.Online.API.Requests; +using osu.Game.Online.API.Requests.Responses; using osu.Game.Rulesets; using osu.Game.Tournament.Models; @@ -93,8 +93,8 @@ namespace osu.Game.Tournament.IPC Beatmap.Value = existing.BeatmapInfo; else { - beatmapLookupRequest = new GetBeatmapRequest(new BeatmapInfo { OnlineBeatmapID = beatmapId }); - beatmapLookupRequest.Success += b => Beatmap.Value = b.ToBeatmapInfo(Rulesets); + beatmapLookupRequest = new GetBeatmapRequest(new APIBeatmap { OnlineID = beatmapId }); + beatmapLookupRequest.Success += b => Beatmap.Value = b; API.Queue(beatmapLookupRequest); } } diff --git a/osu.Game.Tournament/IPC/MatchIPCInfo.cs b/osu.Game.Tournament/IPC/MatchIPCInfo.cs index 701258c6c7..fa7079b824 100644 --- a/osu.Game.Tournament/IPC/MatchIPCInfo.cs +++ b/osu.Game.Tournament/IPC/MatchIPCInfo.cs @@ -3,14 +3,14 @@ using osu.Framework.Bindables; using osu.Framework.Graphics; -using osu.Game.Beatmaps; using osu.Game.Beatmaps.Legacy; +using osu.Game.Online.API.Requests.Responses; namespace osu.Game.Tournament.IPC { public class MatchIPCInfo : Component { - public Bindable Beatmap { get; } = new Bindable(); + public Bindable Beatmap { get; } = new Bindable(); public Bindable Mods { get; } = new Bindable(); public Bindable State { get; } = new Bindable(); public Bindable ChatChannel { get; } = new Bindable(); diff --git a/osu.Game.Tournament/Models/RoundBeatmap.cs b/osu.Game.Tournament/Models/RoundBeatmap.cs index 5d43d0ca66..8f1968236e 100644 --- a/osu.Game.Tournament/Models/RoundBeatmap.cs +++ b/osu.Game.Tournament/Models/RoundBeatmap.cs @@ -1,7 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using osu.Game.Beatmaps; +using osu.Game.Online.API.Requests.Responses; namespace osu.Game.Tournament.Models { @@ -10,6 +10,6 @@ namespace osu.Game.Tournament.Models public int ID; public string Mods; - public BeatmapInfo BeatmapInfo; + public APIBeatmap BeatmapInfo; } } diff --git a/osu.Game.Tournament/Models/SeedingBeatmap.cs b/osu.Game.Tournament/Models/SeedingBeatmap.cs index 2cd6fa7188..37a8a92546 100644 --- a/osu.Game.Tournament/Models/SeedingBeatmap.cs +++ b/osu.Game.Tournament/Models/SeedingBeatmap.cs @@ -2,7 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using osu.Framework.Bindables; -using osu.Game.Beatmaps; +using osu.Game.Online.API.Requests.Responses; namespace osu.Game.Tournament.Models { @@ -10,7 +10,7 @@ namespace osu.Game.Tournament.Models { public int ID; - public BeatmapInfo BeatmapInfo; + public APIBeatmap BeatmapInfo; public long Score; diff --git a/osu.Game.Tournament/Screens/BeatmapInfoScreen.cs b/osu.Game.Tournament/Screens/BeatmapInfoScreen.cs index b94b164116..50324d9c83 100644 --- a/osu.Game.Tournament/Screens/BeatmapInfoScreen.cs +++ b/osu.Game.Tournament/Screens/BeatmapInfoScreen.cs @@ -4,8 +4,8 @@ using osu.Framework.Allocation; using osu.Framework.Bindables; using osu.Framework.Graphics; -using osu.Game.Beatmaps; using osu.Game.Beatmaps.Legacy; +using osu.Game.Online.API.Requests.Responses; using osu.Game.Tournament.Components; using osu.Game.Tournament.IPC; @@ -37,7 +37,7 @@ namespace osu.Game.Tournament.Screens SongBar.Mods = mods.NewValue; } - private void beatmapChanged(ValueChangedEvent beatmap) + private void beatmapChanged(ValueChangedEvent beatmap) { SongBar.FadeInFromZero(300, Easing.OutQuint); SongBar.BeatmapInfo = beatmap.NewValue; diff --git a/osu.Game.Tournament/Screens/Editors/RoundEditorScreen.cs b/osu.Game.Tournament/Screens/Editors/RoundEditorScreen.cs index 1d8c4e7476..2f77209d57 100644 --- a/osu.Game.Tournament/Screens/Editors/RoundEditorScreen.cs +++ b/osu.Game.Tournament/Screens/Editors/RoundEditorScreen.cs @@ -7,10 +7,10 @@ using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Shapes; -using osu.Game.Beatmaps; using osu.Game.Graphics; using osu.Game.Online.API; using osu.Game.Online.API.Requests; +using osu.Game.Online.API.Requests.Responses; using osu.Game.Overlays.Settings; using osu.Game.Rulesets; using osu.Game.Tournament.Components; @@ -234,11 +234,11 @@ namespace osu.Game.Tournament.Screens.Editors return; } - var req = new GetBeatmapRequest(new BeatmapInfo { OnlineBeatmapID = Model.ID }); + var req = new GetBeatmapRequest(new APIBeatmap { OnlineID = Model.ID }); req.Success += res => { - Model.BeatmapInfo = res.ToBeatmapInfo(rulesets); + Model.BeatmapInfo = res; updatePanel(); }; diff --git a/osu.Game.Tournament/Screens/Editors/SeedingEditorScreen.cs b/osu.Game.Tournament/Screens/Editors/SeedingEditorScreen.cs index d5b55823a5..c17939e040 100644 --- a/osu.Game.Tournament/Screens/Editors/SeedingEditorScreen.cs +++ b/osu.Game.Tournament/Screens/Editors/SeedingEditorScreen.cs @@ -7,10 +7,10 @@ using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Shapes; -using osu.Game.Beatmaps; using osu.Game.Graphics; using osu.Game.Online.API; using osu.Game.Online.API.Requests; +using osu.Game.Online.API.Requests.Responses; using osu.Game.Overlays.Settings; using osu.Game.Rulesets; using osu.Game.Tournament.Components; @@ -242,11 +242,11 @@ namespace osu.Game.Tournament.Screens.Editors return; } - var req = new GetBeatmapRequest(new BeatmapInfo { OnlineBeatmapID = Model.ID }); + var req = new GetBeatmapRequest(new APIBeatmap { OnlineID = Model.ID }); req.Success += res => { - Model.BeatmapInfo = res.ToBeatmapInfo(rulesets); + Model.BeatmapInfo = res; updatePanel(); }; diff --git a/osu.Game.Tournament/Screens/MapPool/MapPoolScreen.cs b/osu.Game.Tournament/Screens/MapPool/MapPoolScreen.cs index 5f6546c303..a2432d7b5e 100644 --- a/osu.Game.Tournament/Screens/MapPool/MapPoolScreen.cs +++ b/osu.Game.Tournament/Screens/MapPool/MapPoolScreen.cs @@ -8,8 +8,8 @@ using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Input.Events; using osu.Framework.Threading; -using osu.Game.Beatmaps; using osu.Game.Graphics.UserInterface; +using osu.Game.Online.API.Requests.Responses; using osu.Game.Tournament.Components; using osu.Game.Tournament.IPC; using osu.Game.Tournament.Models; @@ -105,14 +105,14 @@ namespace osu.Game.Tournament.Screens.MapPool ipc.Beatmap.BindValueChanged(beatmapChanged); } - private void beatmapChanged(ValueChangedEvent beatmap) + private void beatmapChanged(ValueChangedEvent beatmap) { if (CurrentMatch.Value == null || CurrentMatch.Value.PicksBans.Count(p => p.Type == ChoiceType.Ban) < 2) return; // if bans have already been placed, beatmap changes result in a selection being made autoamtically - if (beatmap.NewValue.OnlineBeatmapID != null) - addForBeatmap(beatmap.NewValue.OnlineBeatmapID.Value); + if (beatmap.NewValue.OnlineID > 0) + addForBeatmap(beatmap.NewValue.OnlineID); } private void setMode(TeamColour colour, ChoiceType choiceType) @@ -179,7 +179,7 @@ namespace osu.Game.Tournament.Screens.MapPool if (CurrentMatch.Value == null) return; - if (CurrentMatch.Value.Round.Value.Beatmaps.All(b => b.BeatmapInfo.OnlineBeatmapID != beatmapId)) + if (CurrentMatch.Value.Round.Value.Beatmaps.All(b => b.BeatmapInfo.OnlineID != beatmapId)) // don't attempt to add if the beatmap isn't in our pool return; diff --git a/osu.Game.Tournament/TournamentGameBase.cs b/osu.Game.Tournament/TournamentGameBase.cs index 978be720df..5f7c75f9a7 100644 --- a/osu.Game.Tournament/TournamentGameBase.cs +++ b/osu.Game.Tournament/TournamentGameBase.cs @@ -190,7 +190,7 @@ namespace osu.Game.Tournament { var req = new GetBeatmapRequest(new BeatmapInfo { OnlineBeatmapID = b.ID }); API.Perform(req); - b.BeatmapInfo = req.Response?.ToBeatmapInfo(RulesetStore); + b.BeatmapInfo = req.Response; addedInfo = true; } @@ -211,7 +211,7 @@ namespace osu.Game.Tournament { var req = new GetBeatmapRequest(new BeatmapInfo { OnlineBeatmapID = b.ID }); req.Perform(API); - b.BeatmapInfo = req.Response?.ToBeatmapInfo(RulesetStore); + b.BeatmapInfo = req.Response; addedInfo = true; } From 35168948fe871b2e32813e50a72119f36fd5f2a8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 27 Oct 2021 17:45:46 +0900 Subject: [PATCH 40/68] Fix `PopulateUser` not populating immediately when `immediate` is specified (due to scheduling) --- osu.Game.Tournament/TournamentGameBase.cs | 37 +++++++++++++++-------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/osu.Game.Tournament/TournamentGameBase.cs b/osu.Game.Tournament/TournamentGameBase.cs index 5f7c75f9a7..fd31c325b4 100644 --- a/osu.Game.Tournament/TournamentGameBase.cs +++ b/osu.Game.Tournament/TournamentGameBase.cs @@ -226,8 +226,30 @@ namespace osu.Game.Tournament { var req = new GetUserRequest(user.Id, Ruleset.Value); - req.Success += res => + if (immediate) { + API.Perform(req); + populate(); + } + else + { + req.Success += res => { populate(); }; + req.Failure += _ => + { + user.Id = 1; + failure?.Invoke(); + }; + + API.Queue(req); + } + + void populate() + { + var res = req.Response; + + if (res == null) + return; + user.Id = res.Id; user.Username = res.Username; @@ -236,18 +258,7 @@ namespace osu.Game.Tournament user.Cover = res.Cover; success?.Invoke(); - }; - - req.Failure += _ => - { - user.Id = 1; - failure?.Invoke(); - }; - - if (immediate) - API.Perform(req); - else - API.Queue(req); + } } protected override void LoadComplete() From 5448b94039b024884fec0a3efbfbab32cdc11217 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 27 Oct 2021 18:20:10 +0900 Subject: [PATCH 41/68] Change repopulation rules to cover the fact that the serisalisation model has changed This also adds a progress display to show how repopulation is going. It can take a while on a large bracket. --- osu.Game.Tournament/TournamentGameBase.cs | 125 +++++++++++++--------- 1 file changed, 72 insertions(+), 53 deletions(-) diff --git a/osu.Game.Tournament/TournamentGameBase.cs b/osu.Game.Tournament/TournamentGameBase.cs index fd31c325b4..5748315418 100644 --- a/osu.Game.Tournament/TournamentGameBase.cs +++ b/osu.Game.Tournament/TournamentGameBase.cs @@ -7,12 +7,14 @@ using System.Linq; using System.Threading.Tasks; using Newtonsoft.Json; using osu.Framework.Allocation; +using osu.Framework.Graphics; using osu.Framework.Graphics.Textures; using osu.Framework.Input; using osu.Framework.IO.Stores; using osu.Framework.Platform; -using osu.Game.Beatmaps; +using osu.Game.Graphics; using osu.Game.Online.API.Requests; +using osu.Game.Online.API.Requests.Responses; using osu.Game.Tournament.IO; using osu.Game.Tournament.IPC; using osu.Game.Tournament.Models; @@ -39,9 +41,18 @@ namespace osu.Game.Tournament return dependencies = new DependencyContainer(base.CreateChildDependencies(parent)); } + private TournamentSpriteText initialisationText; + [BackgroundDependencyLoader] private void load(Storage baseStorage) { + AddInternal(initialisationText = new TournamentSpriteText + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Font = OsuFont.Torus.With(size: 32), + }); + Resources.AddStore(new DllResourceStore(typeof(TournamentGameBase).Assembly)); dependencies.CacheAs(storage = new TournamentStorage(baseStorage)); @@ -123,7 +134,8 @@ namespace osu.Game.Tournament } addedInfo |= addPlayers(); - addedInfo |= addBeatmaps(); + addedInfo |= addRoundBeatmaps(); + addedInfo |= addSeedingBeatmaps(); if (addedInfo) SaveChanges(); @@ -145,6 +157,8 @@ namespace osu.Game.Tournament Add(ipc); taskCompletionSource.SetResult(true); + + initialisationText.Expire(); }); } @@ -153,75 +167,80 @@ namespace osu.Game.Tournament /// private bool addPlayers() { - bool addedInfo = false; + var playersRequiringPopulation = ladder.Teams + .SelectMany(t => t.Players) + .Where(p => string.IsNullOrEmpty(p.Username) + || p.Statistics?.GlobalRank == null + || p.Statistics?.CountryRank == null).ToList(); - foreach (var t in ladder.Teams) + if (playersRequiringPopulation.Count == 0) + return false; + + for (int i = 0; i < playersRequiringPopulation.Count; i++) { - foreach (var p in t.Players) - { - if (string.IsNullOrEmpty(p.Username) - || p.Statistics?.GlobalRank == null - || p.Statistics?.CountryRank == null) - { - PopulateUser(p, immediate: true); - addedInfo = true; - } - } + var p = playersRequiringPopulation[i]; + PopulateUser(p, immediate: true); + updateLoadProgressMessage($"Populating user stats ({i} / {playersRequiringPopulation.Count})"); } - return addedInfo; + return true; } /// /// Add missing beatmap info based on beatmap IDs /// - private bool addBeatmaps() + private bool addRoundBeatmaps() { - bool addedInfo = false; + var beatmapsRequiringPopulation = ladder.Rounds + .SelectMany(r => r.Beatmaps) + .Where(b => string.IsNullOrEmpty(b.BeatmapInfo?.BeatmapSet?.Title) && b.ID > 0).ToList(); - foreach (var r in ladder.Rounds) + if (beatmapsRequiringPopulation.Count == 0) + return false; + + for (int i = 0; i < beatmapsRequiringPopulation.Count; i++) { - foreach (var b in r.Beatmaps.ToList()) - { - if (b.BeatmapInfo != null) - continue; + var b = beatmapsRequiringPopulation[i]; - if (b.ID > 0) - { - var req = new GetBeatmapRequest(new BeatmapInfo { OnlineBeatmapID = b.ID }); - API.Perform(req); - b.BeatmapInfo = req.Response; + var req = new GetBeatmapRequest(new APIBeatmap { OnlineID = b.ID }); + API.Perform(req); + b.BeatmapInfo = req.Response ?? new APIBeatmap(); - addedInfo = true; - } - - if (b.BeatmapInfo == null) - // if online population couldn't be performed, ensure we don't leave a null value behind - r.Beatmaps.Remove(b); - } + updateLoadProgressMessage($"Populating round beatmaps ({i} / {beatmapsRequiringPopulation.Count})"); } - foreach (var t in ladder.Teams) - { - foreach (var s in t.SeedingResults) - { - foreach (var b in s.Beatmaps) - { - if (b.BeatmapInfo == null && b.ID > 0) - { - var req = new GetBeatmapRequest(new BeatmapInfo { OnlineBeatmapID = b.ID }); - req.Perform(API); - b.BeatmapInfo = req.Response; - - addedInfo = true; - } - } - } - } - - return addedInfo; + return true; } + /// + /// Add missing beatmap info based on beatmap IDs + /// + private bool addSeedingBeatmaps() + { + var beatmapsRequiringPopulation = ladder.Teams + .SelectMany(r => r.SeedingResults) + .SelectMany(r => r.Beatmaps) + .Where(b => string.IsNullOrEmpty(b.BeatmapInfo?.BeatmapSet?.Title) && b.ID > 0).ToList(); + + if (beatmapsRequiringPopulation.Count == 0) + return false; + + for (int i = 0; i < beatmapsRequiringPopulation.Count; i++) + { + var b = beatmapsRequiringPopulation[i]; + + var req = new GetBeatmapRequest(new APIBeatmap { OnlineID = b.ID }); + API.Perform(req); + b.BeatmapInfo = req.Response ?? new APIBeatmap(); + + updateLoadProgressMessage($"Populating seeding beatmaps ({i} / {beatmapsRequiringPopulation.Count})"); + } + + return true; + } + + private void updateLoadProgressMessage(string s) => Schedule(() => initialisationText.Text = s); + public void PopulateUser(User user, Action success = null, Action failure = null, bool immediate = false) { var req = new GetUserRequest(user.Id, Ruleset.Value); From f1499641f3d12789dfaf185b4aa2ad29352052e8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 27 Oct 2021 18:25:23 +0900 Subject: [PATCH 42/68] Rename all usages of `APIBeatmap` to `beatmap` in tournament namespace --- .../NonVisual/LadderInfoSerialisationTest.cs | 4 +-- .../Screens/TestSceneMapPoolScreen.cs | 2 +- .../TournamentTestScene.cs | 12 ++++---- osu.Game.Tournament/Components/SongBar.cs | 30 +++++++++---------- .../Components/TournamentBeatmapPanel.cs | 18 +++++------ osu.Game.Tournament/IPC/FileBasedIPC.cs | 4 +-- osu.Game.Tournament/Models/RoundBeatmap.cs | 4 ++- osu.Game.Tournament/Models/SeedingBeatmap.cs | 4 ++- .../Screens/BeatmapInfoScreen.cs | 2 +- .../Screens/Editors/RoundEditorScreen.cs | 12 ++++---- .../Screens/Editors/SeedingEditorScreen.cs | 12 ++++---- .../Screens/MapPool/MapPoolScreen.cs | 10 +++---- .../Screens/TeamIntro/SeedingScreen.cs | 4 +-- osu.Game.Tournament/TournamentGameBase.cs | 8 ++--- 14 files changed, 65 insertions(+), 61 deletions(-) diff --git a/osu.Game.Tournament.Tests/NonVisual/LadderInfoSerialisationTest.cs b/osu.Game.Tournament.Tests/NonVisual/LadderInfoSerialisationTest.cs index 2071512f60..8bdf909af3 100644 --- a/osu.Game.Tournament.Tests/NonVisual/LadderInfoSerialisationTest.cs +++ b/osu.Game.Tournament.Tests/NonVisual/LadderInfoSerialisationTest.cs @@ -44,8 +44,8 @@ namespace osu.Game.Tournament.Tests.NonVisual { Beatmaps = { - new RoundBeatmap { BeatmapInfo = TournamentTestScene.CreateSampleBeatmap() }, - new RoundBeatmap { BeatmapInfo = TournamentTestScene.CreateSampleBeatmap() }, + new RoundBeatmap { Beatmap = TournamentTestScene.CreateSampleBeatmap() }, + new RoundBeatmap { Beatmap = TournamentTestScene.CreateSampleBeatmap() }, } } }, diff --git a/osu.Game.Tournament.Tests/Screens/TestSceneMapPoolScreen.cs b/osu.Game.Tournament.Tests/Screens/TestSceneMapPoolScreen.cs index 41fbf44179..f732c5582b 100644 --- a/osu.Game.Tournament.Tests/Screens/TestSceneMapPoolScreen.cs +++ b/osu.Game.Tournament.Tests/Screens/TestSceneMapPoolScreen.cs @@ -132,7 +132,7 @@ namespace osu.Game.Tournament.Tests.Screens { Ladder.CurrentMatch.Value.Round.Value.Beatmaps.Add(new RoundBeatmap { - BeatmapInfo = CreateSampleBeatmap(), + Beatmap = CreateSampleBeatmap(), Mods = mods }); } diff --git a/osu.Game.Tournament.Tests/TournamentTestScene.cs b/osu.Game.Tournament.Tests/TournamentTestScene.cs index 4f057286b6..81741a43a9 100644 --- a/osu.Game.Tournament.Tests/TournamentTestScene.cs +++ b/osu.Game.Tournament.Tests/TournamentTestScene.cs @@ -73,19 +73,19 @@ namespace osu.Game.Tournament.Tests { new SeedingBeatmap { - BeatmapInfo = CreateSampleBeatmap(), + Beatmap = CreateSampleBeatmap(), Score = 12345672, Seed = { Value = 24 }, }, new SeedingBeatmap { - BeatmapInfo = CreateSampleBeatmap(), + Beatmap = CreateSampleBeatmap(), Score = 1234567, Seed = { Value = 12 }, }, new SeedingBeatmap { - BeatmapInfo = CreateSampleBeatmap(), + Beatmap = CreateSampleBeatmap(), Score = 1234567, Seed = { Value = 16 }, } @@ -99,19 +99,19 @@ namespace osu.Game.Tournament.Tests { new SeedingBeatmap { - BeatmapInfo = CreateSampleBeatmap(), + Beatmap = CreateSampleBeatmap(), Score = 234567, Seed = { Value = 3 }, }, new SeedingBeatmap { - BeatmapInfo = CreateSampleBeatmap(), + Beatmap = CreateSampleBeatmap(), Score = 234567, Seed = { Value = 6 }, }, new SeedingBeatmap { - BeatmapInfo = CreateSampleBeatmap(), + Beatmap = CreateSampleBeatmap(), Score = 234567, Seed = { Value = 12 }, } diff --git a/osu.Game.Tournament/Components/SongBar.cs b/osu.Game.Tournament/Components/SongBar.cs index bdcb6f8ded..9af73a2abb 100644 --- a/osu.Game.Tournament/Components/SongBar.cs +++ b/osu.Game.Tournament/Components/SongBar.cs @@ -22,21 +22,21 @@ namespace osu.Game.Tournament.Components { public class SongBar : CompositeDrawable { - private APIBeatmap beatmapInfo; + private APIBeatmap beatmap; public const float HEIGHT = 145 / 2f; [Resolved] private IBindable ruleset { get; set; } - public APIBeatmap BeatmapInfo + public APIBeatmap Beatmap { set { - if (beatmapInfo == value) + if (beatmap == value) return; - beatmapInfo = value; + beatmap = value; update(); } } @@ -95,18 +95,18 @@ namespace osu.Game.Tournament.Components private void update() { - if (beatmapInfo == null) + if (beatmap == null) { flow.Clear(); return; } - double bpm = beatmapInfo.BPM; // TODO: check this works. - double length = beatmapInfo.Length; + double bpm = beatmap.BPM; // TODO: check this works. + double length = beatmap.Length; string hardRockExtra = ""; string srExtra = ""; - float ar = beatmapInfo.Difficulty.ApproachRate; + float ar = beatmap.Difficulty.ApproachRate; if ((mods & LegacyMods.HardRock) > 0) { @@ -132,9 +132,9 @@ namespace osu.Game.Tournament.Components default: stats = new (string heading, string content)[] { - ("CS", $"{beatmapInfo.Difficulty.CircleSize:0.#}{hardRockExtra}"), + ("CS", $"{beatmap.Difficulty.CircleSize:0.#}{hardRockExtra}"), ("AR", $"{ar:0.#}{hardRockExtra}"), - ("OD", $"{beatmapInfo.Difficulty.OverallDifficulty:0.#}{hardRockExtra}"), + ("OD", $"{beatmap.Difficulty.OverallDifficulty:0.#}{hardRockExtra}"), }; break; @@ -142,15 +142,15 @@ namespace osu.Game.Tournament.Components case 3: stats = new (string heading, string content)[] { - ("OD", $"{beatmapInfo.Difficulty.OverallDifficulty:0.#}{hardRockExtra}"), - ("HP", $"{beatmapInfo.Difficulty.DrainRate:0.#}{hardRockExtra}") + ("OD", $"{beatmap.Difficulty.OverallDifficulty:0.#}{hardRockExtra}"), + ("HP", $"{beatmap.Difficulty.DrainRate:0.#}{hardRockExtra}") }; break; case 2: stats = new (string heading, string content)[] { - ("CS", $"{beatmapInfo.Difficulty.CircleSize:0.#}{hardRockExtra}"), + ("CS", $"{beatmap.Difficulty.CircleSize:0.#}{hardRockExtra}"), ("AR", $"{ar:0.#}"), }; break; @@ -186,7 +186,7 @@ namespace osu.Game.Tournament.Components Children = new Drawable[] { new DiffPiece(stats), - new DiffPiece(("Star Rating", $"{beatmapInfo.StarRating:0.#}{srExtra}")) + new DiffPiece(("Star Rating", $"{beatmap.StarRating:0.#}{srExtra}")) } }, new FillFlowContainer @@ -229,7 +229,7 @@ namespace osu.Game.Tournament.Components } } }, - new TournamentBeatmapPanel(beatmapInfo) + new TournamentBeatmapPanel(beatmap) { RelativeSizeAxes = Axes.X, Width = 0.5f, diff --git a/osu.Game.Tournament/Components/TournamentBeatmapPanel.cs b/osu.Game.Tournament/Components/TournamentBeatmapPanel.cs index 39fc1d0c0a..328b9278d5 100644 --- a/osu.Game.Tournament/Components/TournamentBeatmapPanel.cs +++ b/osu.Game.Tournament/Components/TournamentBeatmapPanel.cs @@ -21,7 +21,7 @@ namespace osu.Game.Tournament.Components { public class TournamentBeatmapPanel : CompositeDrawable { - public readonly APIBeatmap BeatmapInfo; + public readonly APIBeatmap Beatmap; private readonly string mod; @@ -33,11 +33,11 @@ namespace osu.Game.Tournament.Components private readonly Bindable currentMatch = new Bindable(); private Box flash; - public TournamentBeatmapPanel(APIBeatmap beatmapInfo, string mod = null) + public TournamentBeatmapPanel(APIBeatmap beatmap, string mod = null) { - if (beatmapInfo == null) throw new ArgumentNullException(nameof(beatmapInfo)); + if (beatmap == null) throw new ArgumentNullException(nameof(beatmap)); - BeatmapInfo = beatmapInfo; + Beatmap = beatmap; this.mod = mod; Width = 400; @@ -63,7 +63,7 @@ namespace osu.Game.Tournament.Components { RelativeSizeAxes = Axes.Both, Colour = OsuColour.Gray(0.5f), - BeatmapSet = BeatmapInfo.BeatmapSet, + BeatmapSet = Beatmap.BeatmapSet, }, new FillFlowContainer { @@ -76,7 +76,7 @@ namespace osu.Game.Tournament.Components { new TournamentSpriteText { - Text = BeatmapInfo.GetDisplayTitleRomanisable(false), + Text = Beatmap.GetDisplayTitleRomanisable(false), Font = OsuFont.Torus.With(weight: FontWeight.Bold), }, new FillFlowContainer @@ -93,7 +93,7 @@ namespace osu.Game.Tournament.Components }, new TournamentSpriteText { - Text = BeatmapInfo.Metadata?.Author, + Text = Beatmap.Metadata?.Author, Padding = new MarginPadding { Right = 20 }, Font = OsuFont.Torus.With(weight: FontWeight.Bold, size: 14) }, @@ -105,7 +105,7 @@ namespace osu.Game.Tournament.Components }, new TournamentSpriteText { - Text = BeatmapInfo.DifficultyName, + Text = Beatmap.DifficultyName, Font = OsuFont.Torus.With(weight: FontWeight.Bold, size: 14) }, } @@ -149,7 +149,7 @@ namespace osu.Game.Tournament.Components private void updateState() { - var found = currentMatch.Value.PicksBans.FirstOrDefault(p => p.BeatmapID == BeatmapInfo.OnlineID); + var found = currentMatch.Value.PicksBans.FirstOrDefault(p => p.BeatmapID == Beatmap.OnlineID); bool doFlash = found != choice; choice = found; diff --git a/osu.Game.Tournament/IPC/FileBasedIPC.cs b/osu.Game.Tournament/IPC/FileBasedIPC.cs index 15aa9dd588..a57f9fd691 100644 --- a/osu.Game.Tournament/IPC/FileBasedIPC.cs +++ b/osu.Game.Tournament/IPC/FileBasedIPC.cs @@ -87,10 +87,10 @@ namespace osu.Game.Tournament.IPC lastBeatmapId = beatmapId; - var existing = ladder.CurrentMatch.Value?.Round.Value?.Beatmaps.FirstOrDefault(b => b.ID == beatmapId && b.BeatmapInfo != null); + var existing = ladder.CurrentMatch.Value?.Round.Value?.Beatmaps.FirstOrDefault(b => b.ID == beatmapId && b.Beatmap != null); if (existing != null) - Beatmap.Value = existing.BeatmapInfo; + Beatmap.Value = existing.Beatmap; else { beatmapLookupRequest = new GetBeatmapRequest(new APIBeatmap { OnlineID = beatmapId }); diff --git a/osu.Game.Tournament/Models/RoundBeatmap.cs b/osu.Game.Tournament/Models/RoundBeatmap.cs index 8f1968236e..2fd79546f1 100644 --- a/osu.Game.Tournament/Models/RoundBeatmap.cs +++ b/osu.Game.Tournament/Models/RoundBeatmap.cs @@ -1,6 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using Newtonsoft.Json; using osu.Game.Online.API.Requests.Responses; namespace osu.Game.Tournament.Models @@ -10,6 +11,7 @@ namespace osu.Game.Tournament.Models public int ID; public string Mods; - public APIBeatmap BeatmapInfo; + [JsonProperty("BeatmapInfo")] + public APIBeatmap Beatmap; } } diff --git a/osu.Game.Tournament/Models/SeedingBeatmap.cs b/osu.Game.Tournament/Models/SeedingBeatmap.cs index 37a8a92546..26f3016ac8 100644 --- a/osu.Game.Tournament/Models/SeedingBeatmap.cs +++ b/osu.Game.Tournament/Models/SeedingBeatmap.cs @@ -1,6 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using Newtonsoft.Json; using osu.Framework.Bindables; using osu.Game.Online.API.Requests.Responses; @@ -10,7 +11,8 @@ namespace osu.Game.Tournament.Models { public int ID; - public APIBeatmap BeatmapInfo; + [JsonProperty("BeatmapInfo")] + public APIBeatmap Beatmap; public long Score; diff --git a/osu.Game.Tournament/Screens/BeatmapInfoScreen.cs b/osu.Game.Tournament/Screens/BeatmapInfoScreen.cs index 50324d9c83..77b816d24c 100644 --- a/osu.Game.Tournament/Screens/BeatmapInfoScreen.cs +++ b/osu.Game.Tournament/Screens/BeatmapInfoScreen.cs @@ -40,7 +40,7 @@ namespace osu.Game.Tournament.Screens private void beatmapChanged(ValueChangedEvent beatmap) { SongBar.FadeInFromZero(300, Easing.OutQuint); - SongBar.BeatmapInfo = beatmap.NewValue; + SongBar.Beatmap = beatmap.NewValue; } } } diff --git a/osu.Game.Tournament/Screens/Editors/RoundEditorScreen.cs b/osu.Game.Tournament/Screens/Editors/RoundEditorScreen.cs index 2f77209d57..f6bc607447 100644 --- a/osu.Game.Tournament/Screens/Editors/RoundEditorScreen.cs +++ b/osu.Game.Tournament/Screens/Editors/RoundEditorScreen.cs @@ -226,9 +226,9 @@ namespace osu.Game.Tournament.Screens.Editors Model.ID = id.NewValue ?? 0; if (id.NewValue != id.OldValue) - Model.BeatmapInfo = null; + Model.Beatmap = null; - if (Model.BeatmapInfo != null) + if (Model.Beatmap != null) { updatePanel(); return; @@ -238,13 +238,13 @@ namespace osu.Game.Tournament.Screens.Editors req.Success += res => { - Model.BeatmapInfo = res; + Model.Beatmap = res; updatePanel(); }; req.Failure += _ => { - Model.BeatmapInfo = null; + Model.Beatmap = null; updatePanel(); }; @@ -259,9 +259,9 @@ namespace osu.Game.Tournament.Screens.Editors { drawableContainer.Clear(); - if (Model.BeatmapInfo != null) + if (Model.Beatmap != null) { - drawableContainer.Child = new TournamentBeatmapPanel(Model.BeatmapInfo, Model.Mods) + drawableContainer.Child = new TournamentBeatmapPanel(Model.Beatmap, Model.Mods) { Anchor = Anchor.CentreLeft, Origin = Anchor.CentreLeft, diff --git a/osu.Game.Tournament/Screens/Editors/SeedingEditorScreen.cs b/osu.Game.Tournament/Screens/Editors/SeedingEditorScreen.cs index c17939e040..9abf1d3adb 100644 --- a/osu.Game.Tournament/Screens/Editors/SeedingEditorScreen.cs +++ b/osu.Game.Tournament/Screens/Editors/SeedingEditorScreen.cs @@ -234,9 +234,9 @@ namespace osu.Game.Tournament.Screens.Editors Model.ID = id.NewValue ?? 0; if (id.NewValue != id.OldValue) - Model.BeatmapInfo = null; + Model.Beatmap = null; - if (Model.BeatmapInfo != null) + if (Model.Beatmap != null) { updatePanel(); return; @@ -246,13 +246,13 @@ namespace osu.Game.Tournament.Screens.Editors req.Success += res => { - Model.BeatmapInfo = res; + Model.Beatmap = res; updatePanel(); }; req.Failure += _ => { - Model.BeatmapInfo = null; + Model.Beatmap = null; updatePanel(); }; @@ -267,9 +267,9 @@ namespace osu.Game.Tournament.Screens.Editors { drawableContainer.Clear(); - if (Model.BeatmapInfo != null) + if (Model.Beatmap != null) { - drawableContainer.Child = new TournamentBeatmapPanel(Model.BeatmapInfo, result.Mod.Value) + drawableContainer.Child = new TournamentBeatmapPanel(Model.Beatmap, result.Mod.Value) { Anchor = Anchor.CentreLeft, Origin = Anchor.CentreLeft, diff --git a/osu.Game.Tournament/Screens/MapPool/MapPoolScreen.cs b/osu.Game.Tournament/Screens/MapPool/MapPoolScreen.cs index a2432d7b5e..3ae007f955 100644 --- a/osu.Game.Tournament/Screens/MapPool/MapPoolScreen.cs +++ b/osu.Game.Tournament/Screens/MapPool/MapPoolScreen.cs @@ -147,11 +147,11 @@ namespace osu.Game.Tournament.Screens.MapPool if (map != null) { - if (e.Button == MouseButton.Left && map.BeatmapInfo.OnlineID > 0) - addForBeatmap(map.BeatmapInfo.OnlineID); + if (e.Button == MouseButton.Left && map.Beatmap.OnlineID > 0) + addForBeatmap(map.Beatmap.OnlineID); else { - var existing = CurrentMatch.Value.PicksBans.FirstOrDefault(p => p.BeatmapID == map.BeatmapInfo.OnlineID); + var existing = CurrentMatch.Value.PicksBans.FirstOrDefault(p => p.BeatmapID == map.Beatmap.OnlineID); if (existing != null) { @@ -179,7 +179,7 @@ namespace osu.Game.Tournament.Screens.MapPool if (CurrentMatch.Value == null) return; - if (CurrentMatch.Value.Round.Value.Beatmaps.All(b => b.BeatmapInfo.OnlineID != beatmapId)) + if (CurrentMatch.Value.Round.Value.Beatmaps.All(b => b.Beatmap.OnlineID != beatmapId)) // don't attempt to add if the beatmap isn't in our pool return; @@ -245,7 +245,7 @@ namespace osu.Game.Tournament.Screens.MapPool flowCount = 1; } - currentFlow.Add(new TournamentBeatmapPanel(b.BeatmapInfo, b.Mods) + currentFlow.Add(new TournamentBeatmapPanel(b.Beatmap, b.Mods) { Anchor = Anchor.TopCentre, Origin = Anchor.TopCentre, diff --git a/osu.Game.Tournament/Screens/TeamIntro/SeedingScreen.cs b/osu.Game.Tournament/Screens/TeamIntro/SeedingScreen.cs index 3a0bd232b0..d34a8583b9 100644 --- a/osu.Game.Tournament/Screens/TeamIntro/SeedingScreen.cs +++ b/osu.Game.Tournament/Screens/TeamIntro/SeedingScreen.cs @@ -141,9 +141,9 @@ namespace osu.Game.Tournament.Screens.TeamIntro Spacing = new Vector2(5), Children = new Drawable[] { - new TournamentSpriteText { Text = beatmap.BeatmapInfo.Metadata.Title, Colour = TournamentGame.TEXT_COLOUR, }, + new TournamentSpriteText { Text = beatmap.Beatmap.Metadata.Title, Colour = TournamentGame.TEXT_COLOUR, }, new TournamentSpriteText { Text = "by", Colour = TournamentGame.TEXT_COLOUR, Font = OsuFont.Torus.With(weight: FontWeight.Regular) }, - new TournamentSpriteText { Text = beatmap.BeatmapInfo.Metadata.Artist, Colour = TournamentGame.TEXT_COLOUR, Font = OsuFont.Torus.With(weight: FontWeight.Regular) }, + new TournamentSpriteText { Text = beatmap.Beatmap.Metadata.Artist, Colour = TournamentGame.TEXT_COLOUR, Font = OsuFont.Torus.With(weight: FontWeight.Regular) }, } }, new FillFlowContainer diff --git a/osu.Game.Tournament/TournamentGameBase.cs b/osu.Game.Tournament/TournamentGameBase.cs index 5748315418..4551192583 100644 --- a/osu.Game.Tournament/TournamentGameBase.cs +++ b/osu.Game.Tournament/TournamentGameBase.cs @@ -193,7 +193,7 @@ namespace osu.Game.Tournament { var beatmapsRequiringPopulation = ladder.Rounds .SelectMany(r => r.Beatmaps) - .Where(b => string.IsNullOrEmpty(b.BeatmapInfo?.BeatmapSet?.Title) && b.ID > 0).ToList(); + .Where(b => string.IsNullOrEmpty(b.Beatmap?.BeatmapSet?.Title) && b.ID > 0).ToList(); if (beatmapsRequiringPopulation.Count == 0) return false; @@ -204,7 +204,7 @@ namespace osu.Game.Tournament var req = new GetBeatmapRequest(new APIBeatmap { OnlineID = b.ID }); API.Perform(req); - b.BeatmapInfo = req.Response ?? new APIBeatmap(); + b.Beatmap = req.Response ?? new APIBeatmap(); updateLoadProgressMessage($"Populating round beatmaps ({i} / {beatmapsRequiringPopulation.Count})"); } @@ -220,7 +220,7 @@ namespace osu.Game.Tournament var beatmapsRequiringPopulation = ladder.Teams .SelectMany(r => r.SeedingResults) .SelectMany(r => r.Beatmaps) - .Where(b => string.IsNullOrEmpty(b.BeatmapInfo?.BeatmapSet?.Title) && b.ID > 0).ToList(); + .Where(b => string.IsNullOrEmpty(b.Beatmap?.BeatmapSet?.Title) && b.ID > 0).ToList(); if (beatmapsRequiringPopulation.Count == 0) return false; @@ -231,7 +231,7 @@ namespace osu.Game.Tournament var req = new GetBeatmapRequest(new APIBeatmap { OnlineID = b.ID }); API.Perform(req); - b.BeatmapInfo = req.Response ?? new APIBeatmap(); + b.Beatmap = req.Response ?? new APIBeatmap(); updateLoadProgressMessage($"Populating seeding beatmaps ({i} / {beatmapsRequiringPopulation.Count})"); } From f3dba49aae9794e22336c18020c16f8c1d3b0b0d Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Wed, 27 Oct 2021 16:10:22 +0900 Subject: [PATCH 43/68] Rename room managers --- .../TestSceneLoungeRoomsContainer.cs | 2 +- .../Visual/Multiplayer/TestSceneMultiplayer.cs | 7 ++++--- .../TestSceneMultiplayerLoungeSubScreen.cs | 2 +- .../Visual/Multiplayer/TestSceneTeamVersus.cs | 4 ++-- .../Navigation/TestSceneScreenNavigation.cs | 4 ++-- .../TestScenePlaylistsLoungeSubScreen.cs | 2 +- .../IMultiplayerTestSceneDependencies.cs | 2 +- .../Visual/Multiplayer/MultiplayerTestScene.cs | 12 +++++------- .../MultiplayerTestSceneDependencies.cs | 4 ++-- .../Multiplayer/TestMultiplayerClient.cs | 4 ++-- ...anager.cs => TestMultiplayerRoomManager.cs} | 18 ++++++------------ .../Visual/OnlinePlay/OnlinePlayTestScene.cs | 8 +------- .../OnlinePlayTestSceneDependencies.cs | 5 ++++- ...ndlingRoomManager.cs => TestRoomManager.cs} | 2 +- 14 files changed, 33 insertions(+), 43 deletions(-) rename osu.Game/Tests/Visual/Multiplayer/{TestRequestHandlingMultiplayerRoomManager.cs => TestMultiplayerRoomManager.cs} (56%) rename osu.Game/Tests/Visual/OnlinePlay/{TestRequestHandlingRoomManager.cs => TestRoomManager.cs} (97%) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneLoungeRoomsContainer.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneLoungeRoomsContainer.cs index 1bdf3c2750..c3d5f7ec23 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneLoungeRoomsContainer.cs +++ b/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; diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs index f4a72dd7e7..bb4603da69 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs @@ -4,6 +4,7 @@ using System; using System.Diagnostics; using System.Linq; +using Microsoft.VisualStudio.TestPlatform.CommunicationUtilities; using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Audio; @@ -48,7 +49,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(); @@ -624,9 +625,9 @@ namespace osu.Game.Tests.Visual.Multiplayer 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(); } } } diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerLoungeSubScreen.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerLoungeSubScreen.cs index b7da31a2b5..de3df754a2 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerLoungeSubScreen.cs +++ b/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; diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs index 80217a7726..7b34150610 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs @@ -184,9 +184,9 @@ namespace osu.Game.Tests.Visual.Multiplayer 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(); } } } diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs index ce437e7299..87223a3eca 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs @@ -461,10 +461,10 @@ namespace osu.Game.Tests.Visual.Navigation public TestMultiplayer() { - Client = new TestMultiplayerClient((TestRequestHandlingMultiplayerRoomManager)RoomManager); + Client = new TestMultiplayerClient((TestMultiplayerRoomManager)RoomManager); } - protected override RoomManager CreateRoomManager() => new TestRequestHandlingMultiplayerRoomManager(); + protected override RoomManager CreateRoomManager() => new TestMultiplayerRoomManager(); } } } diff --git a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsLoungeSubScreen.cs b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsLoungeSubScreen.cs index 5c248163d7..9ba0da1911 100644 --- a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsLoungeSubScreen.cs +++ b/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; diff --git a/osu.Game/Tests/Visual/Multiplayer/IMultiplayerTestSceneDependencies.cs b/osu.Game/Tests/Visual/Multiplayer/IMultiplayerTestSceneDependencies.cs index 3362ebbbd6..204c189591 100644 --- a/osu.Game/Tests/Visual/Multiplayer/IMultiplayerTestSceneDependencies.cs +++ b/osu.Game/Tests/Visual/Multiplayer/IMultiplayerTestSceneDependencies.cs @@ -22,7 +22,7 @@ namespace osu.Game.Tests.Visual.Multiplayer /// /// The cached . /// - new TestRequestHandlingMultiplayerRoomManager RoomManager { get; } + new TestMultiplayerRoomManager RoomManager { get; } /// /// The cached . diff --git a/osu.Game/Tests/Visual/Multiplayer/MultiplayerTestScene.cs b/osu.Game/Tests/Visual/Multiplayer/MultiplayerTestScene.cs index f259784170..c628541825 100644 --- a/osu.Game/Tests/Visual/Multiplayer/MultiplayerTestScene.cs +++ b/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) - { - var room = CreateRoom(); - - RoomManager.CreateRoom(room); - SelectedRoom.Value = room; - } + SelectedRoom.Value = CreateRoom(); }); protected virtual Room CreateRoom() @@ -64,7 +59,10 @@ 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(); diff --git a/osu.Game/Tests/Visual/Multiplayer/MultiplayerTestSceneDependencies.cs b/osu.Game/Tests/Visual/Multiplayer/MultiplayerTestSceneDependencies.cs index 2e13fb6a56..a2b0b066a7 100644 --- a/osu.Game/Tests/Visual/Multiplayer/MultiplayerTestSceneDependencies.cs +++ b/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); } - protected override IRoomManager CreateRoomManager() => new TestRequestHandlingMultiplayerRoomManager(); + protected override IRoomManager CreateRoomManager() => new TestMultiplayerRoomManager(); protected virtual TestSpectatorClient CreateSpectatorClient() => new TestSpectatorClient(); } diff --git a/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs b/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs index 5e4e5942d9..cd0f070d73 100644 --- a/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs +++ b/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; } diff --git a/osu.Game/Tests/Visual/Multiplayer/TestRequestHandlingMultiplayerRoomManager.cs b/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerRoomManager.cs similarity index 56% rename from osu.Game/Tests/Visual/Multiplayer/TestRequestHandlingMultiplayerRoomManager.cs rename to osu.Game/Tests/Visual/Multiplayer/TestMultiplayerRoomManager.cs index 5de518990a..fef2a0a16d 100644 --- a/osu.Game/Tests/Visual/Multiplayer/TestRequestHandlingMultiplayerRoomManager.cs +++ b/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 { /// - /// A for use in multiplayer test scenes, backed by a . + /// A for use in multiplayer test scenes. /// Should generally not be used by itself outside of a . /// - public class TestRequestHandlingMultiplayerRoomManager : MultiplayerRoomManager + public class TestMultiplayerRoomManager : MultiplayerRoomManager { - public IReadOnlyList ServerSideRooms => handler.ServerSideRooms; + [Resolved] + private TestRoomRequestsHandler requestsHandler { get; set; } - 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 IReadOnlyList ServerSideRooms => requestsHandler.ServerSideRooms; /// /// Adds a room to a local "server-side" list that's returned when a is fired. /// /// The room. - public void AddServerSideRoom(Room room) => handler.AddServerSideRoom(room); + public void AddServerSideRoom(Room room) => requestsHandler.AddServerSideRoom(room); } } diff --git a/osu.Game/Tests/Visual/OnlinePlay/OnlinePlayTestScene.cs b/osu.Game/Tests/Visual/OnlinePlay/OnlinePlayTestScene.cs index d782160ee5..4771ace1c4 100644 --- a/osu.Game/Tests/Visual/OnlinePlay/OnlinePlayTestScene.cs +++ b/osu.Game/Tests/Visual/OnlinePlay/OnlinePlayTestScene.cs @@ -36,7 +36,6 @@ namespace osu.Game.Tests.Visual.OnlinePlay private readonly Container content; private readonly Container drawableDependenciesContainer; private DelegatedDependencyContainer dependencies; - private TestRoomRequestsHandler requestsHandler; protected OnlinePlayTestScene() { @@ -65,12 +64,7 @@ namespace osu.Game.Tests.Visual.OnlinePlay public override void SetUpSteps() { base.SetUpSteps(); - - AddStep("setup API", () => - { - requestsHandler = new TestRoomRequestsHandler(); - ((DummyAPIAccess)API).HandleRequest = request => requestsHandler.HandleRequest(request, API.LocalUser.Value, game); - }); + AddStep("setup API", () => ((DummyAPIAccess)API).HandleRequest = request => OnlinePlayDependencies.RequestsHandler.HandleRequest(request, API.LocalUser.Value, game)); } /// diff --git a/osu.Game/Tests/Visual/OnlinePlay/OnlinePlayTestSceneDependencies.cs b/osu.Game/Tests/Visual/OnlinePlay/OnlinePlayTestSceneDependencies.cs index defc971eef..9e5264fb12 100644 --- a/osu.Game/Tests/Visual/OnlinePlay/OnlinePlayTestSceneDependencies.cs +++ b/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; } /// /// All cached dependencies which are also 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(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(); } } diff --git a/osu.Game/Tests/Visual/OnlinePlay/TestRequestHandlingRoomManager.cs b/osu.Game/Tests/Visual/OnlinePlay/TestRoomManager.cs similarity index 97% rename from osu.Game/Tests/Visual/OnlinePlay/TestRequestHandlingRoomManager.cs rename to osu.Game/Tests/Visual/OnlinePlay/TestRoomManager.cs index ef0ceafd02..5fe3dc8406 100644 --- a/osu.Game/Tests/Visual/OnlinePlay/TestRequestHandlingRoomManager.cs +++ b/osu.Game/Tests/Visual/OnlinePlay/TestRoomManager.cs @@ -13,7 +13,7 @@ namespace osu.Game.Tests.Visual.OnlinePlay /// /// A very simple for use in online play test scenes. /// - public class TestRequestHandlingRoomManager : RoomManager + public class TestRoomManager : RoomManager { public Action JoinRoomRequested; From 48f280440c696925653ed36a6794e83f404cf637 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Wed, 27 Oct 2021 19:36:43 +0900 Subject: [PATCH 44/68] Fix incorrect clearing of room --- .../Visual/Multiplayer/TestSceneMultiplayerMatchFooter.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchFooter.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchFooter.cs index 44a8d7b439..6536ef2ca1 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerMatchFooter.cs +++ b/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, From a87d8d0359af262408e097c6e3da406436be2cc8 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Wed, 27 Oct 2021 20:47:39 +0900 Subject: [PATCH 45/68] Fix dependency missing in a few TestScenes Hopefully these are rather temporary cases until a better solution is found for these dependency-loading screens. --- .../Visual/Multiplayer/TestSceneMultiplayer.cs | 11 +++++++++++ .../Visual/Multiplayer/TestSceneTeamVersus.cs | 11 +++++++++++ 2 files changed, 22 insertions(+) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs index bb4603da69..a106f3ea95 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs @@ -18,6 +18,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; @@ -34,6 +35,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; @@ -617,10 +619,19 @@ 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 diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs index 7b34150610..9bb19d4286 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs +++ b/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,10 +178,19 @@ 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 From 72bb72a559dda6edfb923a1d89fcc8a83e9a9621 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Wed, 27 Oct 2021 20:56:03 +0900 Subject: [PATCH 46/68] Fix a case of missed base.SetUpSteps() --- .../TestSceneMultiplayerGameplayLeaderboardTeams.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerGameplayLeaderboardTeams.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerGameplayLeaderboardTeams.cs index 32114fa500..3d48ddc7ca 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerGameplayLeaderboardTeams.cs +++ b/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", () => From 7245baba691b96dd2c86c9abdebb2b3b7139c61d Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 28 Oct 2021 00:13:25 +0900 Subject: [PATCH 47/68] Remove `IBeatmapSetOnlineInfo` implementation from `BeatmapSetInfo` to avoid null cases --- .../TestSceneUpdateableBeatmapSetCover.cs | 12 ++++++------ .../Components/TournamentBeatmapPanel.cs | 2 +- osu.Game/Beatmaps/BeatmapSetInfo.cs | 2 +- .../Drawables/UpdateableBeatmapBackgroundSprite.cs | 5 +++++ .../Drawables/UpdateableOnlineBeatmapSetCover.cs | 2 +- .../BeatmapListing/BeatmapListingSearchControl.cs | 2 +- .../Overlays/BeatmapListing/Panels/BeatmapPanel.cs | 2 +- .../Overlays/BeatmapSet/BeatmapSetHeaderContent.cs | 2 +- .../Overlays/Dashboard/Home/DashboardBeatmapPanel.cs | 2 +- .../Sections/Historical/DrawableMostPlayedBeatmap.cs | 2 +- 10 files changed, 19 insertions(+), 14 deletions(-) diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneUpdateableBeatmapSetCover.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneUpdateableBeatmapSetCover.cs index 3ac3002713..f67f6258cc 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneUpdateableBeatmapSetCover.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneUpdateableBeatmapSetCover.cs @@ -25,7 +25,7 @@ namespace osu.Game.Tests.Visual.UserInterface { AddStep("setup cover", () => Child = new UpdateableOnlineBeatmapSetCover(coverType) { - BeatmapSet = CreateBeatmap(Ruleset.Value).BeatmapInfo.BeatmapSet, + OnlineInfo = CreateBeatmap(Ruleset.Value).BeatmapInfo.BeatmapSet.OnlineInfo, RelativeSizeAxes = Axes.Both, Masking = true, }); @@ -68,7 +68,7 @@ namespace osu.Game.Tests.Visual.UserInterface var cover = new UpdateableOnlineBeatmapSetCover(coverType) { - BeatmapSet = setInfo, + OnlineInfo = setInfo.OnlineInfo, Height = 100, Masking = true, }; @@ -99,12 +99,12 @@ namespace osu.Game.Tests.Visual.UserInterface AddStep("setup cover", () => Child = updateableCover = new TestUpdateableOnlineBeatmapSetCover { - BeatmapSet = CreateBeatmap(Ruleset.Value).BeatmapInfo.BeatmapSet, + OnlineInfo = CreateBeatmap(Ruleset.Value).BeatmapInfo.BeatmapSet.OnlineInfo, RelativeSizeAxes = Axes.Both, Masking = true, }); - AddStep("change model", () => updateableCover.BeatmapSet = null); + AddStep("change model", () => updateableCover.OnlineInfo = null); AddWaitStep("wait some", 5); AddAssert("no cover added", () => !updateableCover.ChildrenOfType().Any()); } @@ -117,7 +117,7 @@ namespace osu.Game.Tests.Visual.UserInterface AddStep("setup cover", () => Child = updateableCover = new TestUpdateableOnlineBeatmapSetCover(0) { - BeatmapSet = createBeatmapWithCover("https://assets.ppy.sh/beatmaps/1189904/covers/cover.jpg"), + OnlineInfo = createBeatmapWithCover("https://assets.ppy.sh/beatmaps/1189904/covers/cover.jpg").OnlineInfo, RelativeSizeAxes = Axes.Both, Masking = true, Alpha = 0.4f @@ -128,7 +128,7 @@ namespace osu.Game.Tests.Visual.UserInterface AddUntilStep("wait for fade complete", () => initialCover.Alpha == 1); AddStep("switch beatmap", - () => updateableCover.BeatmapSet = createBeatmapWithCover("https://assets.ppy.sh/beatmaps/1079428/covers/cover.jpg")); + () => updateableCover.OnlineInfo = createBeatmapWithCover("https://assets.ppy.sh/beatmaps/1079428/covers/cover.jpg").OnlineInfo); AddUntilStep("new cover loaded", () => updateableCover.ChildrenOfType().Except(new[] { initialCover }).Any()); } diff --git a/osu.Game.Tournament/Components/TournamentBeatmapPanel.cs b/osu.Game.Tournament/Components/TournamentBeatmapPanel.cs index be29566e07..70544f3a3f 100644 --- a/osu.Game.Tournament/Components/TournamentBeatmapPanel.cs +++ b/osu.Game.Tournament/Components/TournamentBeatmapPanel.cs @@ -62,7 +62,7 @@ namespace osu.Game.Tournament.Components { RelativeSizeAxes = Axes.Both, Colour = OsuColour.Gray(0.5f), - BeatmapSet = BeatmapInfo.BeatmapSet as IBeatmapSetOnlineInfo, + OnlineInfo = BeatmapInfo.BeatmapSet as IBeatmapSetOnlineInfo, }, new FillFlowContainer { diff --git a/osu.Game/Beatmaps/BeatmapSetInfo.cs b/osu.Game/Beatmaps/BeatmapSetInfo.cs index ae32ad000e..6ed617898f 100644 --- a/osu.Game/Beatmaps/BeatmapSetInfo.cs +++ b/osu.Game/Beatmaps/BeatmapSetInfo.cs @@ -14,7 +14,7 @@ using osu.Game.Online.API.Requests.Responses; namespace osu.Game.Beatmaps { [ExcludeFromDynamicCompile] - public class BeatmapSetInfo : IHasPrimaryKey, IHasFiles, ISoftDelete, IEquatable, IBeatmapSetInfo, IBeatmapSetOnlineInfo + public class BeatmapSetInfo : IHasPrimaryKey, IHasFiles, ISoftDelete, IEquatable, IBeatmapSetInfo { public int ID { get; set; } diff --git a/osu.Game/Beatmaps/Drawables/UpdateableBeatmapBackgroundSprite.cs b/osu.Game/Beatmaps/Drawables/UpdateableBeatmapBackgroundSprite.cs index 636c568bd0..4100fe9586 100644 --- a/osu.Game/Beatmaps/Drawables/UpdateableBeatmapBackgroundSprite.cs +++ b/osu.Game/Beatmaps/Drawables/UpdateableBeatmapBackgroundSprite.cs @@ -57,7 +57,12 @@ namespace osu.Game.Beatmaps.Drawables return new OnlineBeatmapSetCover(online, beatmapSetCoverType); if (model is BeatmapInfo localModel) + { + if (localModel.BeatmapSet?.OnlineInfo != null) + return new OnlineBeatmapSetCover(localModel.BeatmapSet.OnlineInfo, beatmapSetCoverType); + return new BeatmapBackgroundSprite(beatmaps.GetWorkingBeatmap(localModel)); + } return new BeatmapBackgroundSprite(beatmaps.DefaultBeatmap); } diff --git a/osu.Game/Beatmaps/Drawables/UpdateableOnlineBeatmapSetCover.cs b/osu.Game/Beatmaps/Drawables/UpdateableOnlineBeatmapSetCover.cs index 73f87beb58..4a6a1b888e 100644 --- a/osu.Game/Beatmaps/Drawables/UpdateableOnlineBeatmapSetCover.cs +++ b/osu.Game/Beatmaps/Drawables/UpdateableOnlineBeatmapSetCover.cs @@ -13,7 +13,7 @@ namespace osu.Game.Beatmaps.Drawables { private readonly BeatmapSetCoverType coverType; - public IBeatmapSetOnlineInfo BeatmapSet + public IBeatmapSetOnlineInfo OnlineInfo { get => Model; set => Model = value; diff --git a/osu.Game/Overlays/BeatmapListing/BeatmapListingSearchControl.cs b/osu.Game/Overlays/BeatmapListing/BeatmapListingSearchControl.cs index da2dcfebdf..776a8e73b0 100644 --- a/osu.Game/Overlays/BeatmapListing/BeatmapListingSearchControl.cs +++ b/osu.Game/Overlays/BeatmapListing/BeatmapListingSearchControl.cs @@ -59,7 +59,7 @@ namespace osu.Game.Overlays.BeatmapListing return; } - beatmapCover.BeatmapSet = value; + beatmapCover.OnlineInfo = value.OnlineInfo; beatmapCover.FadeTo(0.1f, 200, Easing.OutQuint); } } diff --git a/osu.Game/Overlays/BeatmapListing/Panels/BeatmapPanel.cs b/osu.Game/Overlays/BeatmapListing/Panels/BeatmapPanel.cs index 779f3860f2..c7fa98f159 100644 --- a/osu.Game/Overlays/BeatmapListing/Panels/BeatmapPanel.cs +++ b/osu.Game/Overlays/BeatmapListing/Panels/BeatmapPanel.cs @@ -163,7 +163,7 @@ namespace osu.Game.Overlays.BeatmapListing.Panels protected Drawable CreateBackground() => new UpdateableOnlineBeatmapSetCover { RelativeSizeAxes = Axes.Both, - BeatmapSet = SetInfo, + OnlineInfo = SetInfo.OnlineInfo, }; public class Statistic : FillFlowContainer diff --git a/osu.Game/Overlays/BeatmapSet/BeatmapSetHeaderContent.cs b/osu.Game/Overlays/BeatmapSet/BeatmapSetHeaderContent.cs index 6f85846720..4c94e95383 100644 --- a/osu.Game/Overlays/BeatmapSet/BeatmapSetHeaderContent.cs +++ b/osu.Game/Overlays/BeatmapSet/BeatmapSetHeaderContent.cs @@ -227,7 +227,7 @@ namespace osu.Game.Overlays.BeatmapSet BeatmapSet.BindValueChanged(setInfo => { Picker.BeatmapSet = rulesetSelector.BeatmapSet = author.BeatmapSet = beatmapAvailability.BeatmapSet = Details.BeatmapSet = setInfo.NewValue; - cover.BeatmapSet = setInfo.NewValue; + cover.OnlineInfo = setInfo.NewValue?.OnlineInfo; if (setInfo.NewValue == null) { diff --git a/osu.Game/Overlays/Dashboard/Home/DashboardBeatmapPanel.cs b/osu.Game/Overlays/Dashboard/Home/DashboardBeatmapPanel.cs index edc737d8fe..50186def37 100644 --- a/osu.Game/Overlays/Dashboard/Home/DashboardBeatmapPanel.cs +++ b/osu.Game/Overlays/Dashboard/Home/DashboardBeatmapPanel.cs @@ -82,7 +82,7 @@ namespace osu.Game.Overlays.Dashboard.Home RelativeSizeAxes = Axes.Both, Anchor = Anchor.Centre, Origin = Anchor.Centre, - BeatmapSet = SetInfo + OnlineInfo = SetInfo.OnlineInfo } }, new Container diff --git a/osu.Game/Overlays/Profile/Sections/Historical/DrawableMostPlayedBeatmap.cs b/osu.Game/Overlays/Profile/Sections/Historical/DrawableMostPlayedBeatmap.cs index c4c8bfb84f..ac4299ae49 100644 --- a/osu.Game/Overlays/Profile/Sections/Historical/DrawableMostPlayedBeatmap.cs +++ b/osu.Game/Overlays/Profile/Sections/Historical/DrawableMostPlayedBeatmap.cs @@ -46,7 +46,7 @@ namespace osu.Game.Overlays.Profile.Sections.Historical { RelativeSizeAxes = Axes.Y, Width = cover_width, - BeatmapSet = mostPlayed.BeatmapSet, + OnlineInfo = mostPlayed.BeatmapSet, }, new Container { From cd04ca12400e641b08c3e06f99832dd39c3c21d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 27 Oct 2021 20:35:19 +0200 Subject: [PATCH 48/68] Remove unused using statement --- osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs index a106f3ea95..ebb348c3a2 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs @@ -4,7 +4,6 @@ using System; using System.Diagnostics; using System.Linq; -using Microsoft.VisualStudio.TestPlatform.CommunicationUtilities; using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Audio; From 798349243fabb373d290303d618809ab779607da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 27 Oct 2021 20:38:52 +0200 Subject: [PATCH 49/68] Cache test request handler in screen navigation test --- .../Visual/Navigation/TestSceneScreenNavigation.cs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs index 87223a3eca..1df0680b69 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs +++ b/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,9 +461,19 @@ namespace osu.Game.Tests.Visual.Navigation [Cached(typeof(MultiplayerClient))] public readonly TestMultiplayerClient Client; + [Cached] + public readonly TestRoomRequestsHandler RequestsHandler; + public TestMultiplayer() { Client = new TestMultiplayerClient((TestMultiplayerRoomManager)RoomManager); + RequestsHandler = new TestRoomRequestsHandler(); + } + + [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(); From 65a7b6776e72000e8784c60216d207f5a5c774e2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 28 Oct 2021 05:50:22 +0900 Subject: [PATCH 50/68] Add note regarding `OnlineInfo` being temporary --- osu.Game/Beatmaps/BeatmapSetInfo.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game/Beatmaps/BeatmapSetInfo.cs b/osu.Game/Beatmaps/BeatmapSetInfo.cs index 6ed617898f..79cc8b70fb 100644 --- a/osu.Game/Beatmaps/BeatmapSetInfo.cs +++ b/osu.Game/Beatmaps/BeatmapSetInfo.cs @@ -35,6 +35,7 @@ namespace osu.Game.Beatmaps [NotNull] public List Files { get; set; } = new List(); + // This field is temporary and only used by `APIBeatmapSet.ToBeatmapSet` (soon to be removed) and tests (to be updated to provide APIBeatmapSet instead). [NotMapped] public APIBeatmapSet OnlineInfo { get; set; } From 3905ecd18b37abc9b988359f0891742d7fed6ca4 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 28 Oct 2021 13:09:03 +0900 Subject: [PATCH 51/68] Use retrieval of bindable and tidy up code slightly --- osu.Game/OsuGame.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs index 25ce62b787..f9e080a93c 100644 --- a/osu.Game/OsuGame.cs +++ b/osu.Game/OsuGame.cs @@ -158,6 +158,8 @@ namespace osu.Game private Bindable configRuleset; + private Bindable uiScale; + private Bindable configSkin; private readonly string[] args; @@ -219,6 +221,7 @@ namespace osu.Game // bind config int to database RulesetInfo configRuleset = LocalConfig.GetBindable(OsuSetting.Ruleset); + uiScale = LocalConfig.GetBindable(OsuSetting.UIScale); var preferredRuleset = RulesetStore.GetRuleset(configRuleset.Value); @@ -1022,26 +1025,23 @@ namespace osu.Game public override bool OnPressed(KeyBindingPressEvent e) { + const float adjustment_increment = 0.05f; + switch (e.Action) { case PlatformAction.ZoomIn: - adjustUIScaling(1); + uiScale.Value += adjustment_increment; return true; case PlatformAction.ZoomOut: - adjustUIScaling(-1); + uiScale.Value -= adjustment_increment; return true; case PlatformAction.ZoomDefault: - LocalConfig.GetBindable(OsuSetting.UIScale).SetDefault(); + uiScale.SetDefault(); return true; } - void adjustUIScaling(int amount) - { - LocalConfig.SetValue(OsuSetting.UIScale, LocalConfig.Get(OsuSetting.UIScale) + (0.05f * amount)); - } - return base.OnPressed(e); } From 1213c5553a64944df87eeae67e2b2a20a6dcc02c Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Thu, 28 Oct 2021 13:16:50 +0900 Subject: [PATCH 52/68] Remove forgotten attribute --- .../Multiplayer/TestSceneMultiplayerGameplayLeaderboard.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerGameplayLeaderboard.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerGameplayLeaderboard.cs index 10633f4a4f..832998d5d3 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerGameplayLeaderboard.cs +++ b/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,7 +39,6 @@ namespace osu.Game.Tests.Visual.Multiplayer Dependencies.Cache(config = new OsuConfigManager(LocalStorage)); } - [SetUpSteps] public override void SetUpSteps() { base.SetUpSteps(); From 0e293471c2b7ee4fb49b6baa4b19697291c67a59 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Thu, 28 Oct 2021 14:29:49 +0900 Subject: [PATCH 53/68] Clean up multiplayer-screen tests by removing intermediate screen --- .../Multiplayer/TestSceneMultiplayer.cs | 78 ++++------------- .../Visual/Multiplayer/TestSceneTeamVersus.cs | 66 ++------------- .../Navigation/TestSceneScreenNavigation.cs | 36 +------- .../Visual/TestMultiplayerScreenStack.cs | 84 +++++++++++++++++++ .../MultiplayerTestSceneDependencies.cs | 2 +- .../Multiplayer/TestMultiplayerRoomManager.cs | 9 +- .../OnlinePlayTestSceneDependencies.cs | 4 +- 7 files changed, 119 insertions(+), 160 deletions(-) create mode 100644 osu.Game.Tests/Visual/TestMultiplayerScreenStack.cs diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs index ebb348c3a2..11caf9f498 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayer.cs @@ -17,15 +17,12 @@ 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; using osu.Game.Rulesets; using osu.Game.Rulesets.Osu; using osu.Game.Rulesets.Osu.Mods; -using osu.Game.Screens; -using osu.Game.Screens.OnlinePlay.Components; using osu.Game.Screens.OnlinePlay.Lounge; using osu.Game.Screens.OnlinePlay.Lounge.Components; using osu.Game.Screens.OnlinePlay.Match; @@ -33,8 +30,8 @@ using osu.Game.Screens.OnlinePlay.Multiplayer; using osu.Game.Screens.OnlinePlay.Multiplayer.Match; using osu.Game.Screens.Play; using osu.Game.Screens.Ranking; +using osu.Game.Screens.Spectate; using osu.Game.Tests.Resources; -using osu.Game.Tests.Visual.OnlinePlay; using osu.Game.Users; using osuTK.Input; @@ -46,11 +43,10 @@ namespace osu.Game.Tests.Visual.Multiplayer private RulesetStore rulesets; private BeatmapSetInfo importedSet; - private DependenciesScreen dependenciesScreen; - private TestMultiplayer multiplayerScreen; - private TestMultiplayerClient client; + private TestMultiplayerScreenStack multiplayerScreenStack; - private TestMultiplayerRoomManager roomManager => multiplayerScreen.RoomManager; + private TestMultiplayerClient client => multiplayerScreenStack.Client; + private TestMultiplayerRoomManager roomManager => multiplayerScreenStack.RoomManager; [Cached(typeof(UserLookupCache))] private UserLookupCache lookupCache = new TestUserLookupCache(); @@ -72,22 +68,8 @@ namespace osu.Game.Tests.Visual.Multiplayer importedSet = beatmaps.GetAllUsableBeatmapSetsEnumerable(IncludedDetails.All).First(); }); - AddStep("create multiplayer screen", () => multiplayerScreen = new TestMultiplayer()); - - AddStep("load dependencies", () => - { - client = new TestMultiplayerClient(roomManager); - - // The screen gets suspended so it stops receiving updates. - Child = client; - - LoadScreen(dependenciesScreen = new DependenciesScreen(client)); - }); - - AddUntilStep("wait for dependencies to load", () => dependenciesScreen.IsLoaded); - - AddStep("load multiplayer", () => LoadScreen(multiplayerScreen)); - AddUntilStep("wait for multiplayer to load", () => multiplayerScreen.IsLoaded); + AddStep("load multiplayer", () => LoadScreen(multiplayerScreenStack = new TestMultiplayerScreenStack())); + AddUntilStep("wait for multiplayer to load", () => multiplayerScreenStack.IsLoaded); AddUntilStep("wait for lounge to load", () => this.ChildrenOfType().FirstOrDefault()?.IsLoaded == true); } @@ -443,7 +425,7 @@ namespace osu.Game.Tests.Visual.Multiplayer AddStep("start match externally", () => client.StartMatch()); - AddAssert("play not started", () => multiplayerScreen.IsCurrentScreen()); + AddAssert("play not started", () => multiplayerScreenStack.IsCurrentScreen()); } [Test] @@ -487,7 +469,7 @@ namespace osu.Game.Tests.Visual.Multiplayer importedSet = beatmaps.GetAllUsableBeatmapSetsEnumerable(IncludedDetails.All).First(); }); - AddUntilStep("play started", () => !multiplayerScreen.IsCurrentScreen()); + AddUntilStep("play started", () => multiplayerScreenStack.CurrentScreen is SpectatorScreen); } [Test] @@ -529,16 +511,16 @@ namespace osu.Game.Tests.Visual.Multiplayer AddStep("open mod overlay", () => this.ChildrenOfType().Single().TriggerClick()); - AddStep("invoke on back button", () => multiplayerScreen.OnBackButton()); + AddStep("invoke on back button", () => multiplayerScreenStack.OnBackButton()); AddAssert("mod overlay is hidden", () => this.ChildrenOfType().Single().State.Value == Visibility.Hidden); AddAssert("dialog overlay is hidden", () => DialogOverlay.State.Value == Visibility.Hidden); - testLeave("back button", () => multiplayerScreen.OnBackButton()); + testLeave("back button", () => multiplayerScreenStack.OnBackButton()); // mimics home button and OS window close - testLeave("forced exit", () => multiplayerScreen.Exit()); + testLeave("forced exit", () => multiplayerScreenStack.Exit()); void testLeave(string actionName, Action action) { @@ -579,7 +561,7 @@ namespace osu.Game.Tests.Visual.Multiplayer AddStep("click start button", () => InputManager.Click(MouseButton.Left)); - AddUntilStep("wait for player", () => Stack.CurrentScreen is Player); + AddUntilStep("wait for player", () => multiplayerScreenStack.CurrentScreen is Player); // Gameplay runs in real-time, so we need to incrementally check if gameplay has finished in order to not time out. for (double i = 1000; i < TestResources.QUICK_BEATMAP_LENGTH; i += 1000) @@ -588,15 +570,15 @@ namespace osu.Game.Tests.Visual.Multiplayer AddUntilStep($"wait for time > {i}", () => this.ChildrenOfType().SingleOrDefault()?.GameplayClock.CurrentTime > time); } - AddUntilStep("wait for results", () => Stack.CurrentScreen is ResultsScreen); + AddUntilStep("wait for results", () => multiplayerScreenStack.CurrentScreen is ResultsScreen); } private MultiplayerReadyButton readyButton => this.ChildrenOfType().Single(); private void createRoom(Func room) { - AddUntilStep("wait for lounge", () => multiplayerScreen.ChildrenOfType().SingleOrDefault()?.IsLoaded == true); - AddStep("open room", () => multiplayerScreen.ChildrenOfType().Single().Open(room())); + AddUntilStep("wait for lounge", () => multiplayerScreenStack.ChildrenOfType().SingleOrDefault()?.IsLoaded == true); + AddStep("open room", () => multiplayerScreenStack.ChildrenOfType().Single().Open(room())); AddUntilStep("wait for room open", () => this.ChildrenOfType().FirstOrDefault()?.IsLoaded == true); AddWaitStep("wait for transition", 2); @@ -609,35 +591,5 @@ namespace osu.Game.Tests.Visual.Multiplayer AddUntilStep("wait for join", () => client.Room != null); } - - /// - /// Used for the sole purpose of adding as a resolvable dependency. - /// - private class DependenciesScreen : OsuScreen - { - [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 TestMultiplayerRoomManager RoomManager { get; private set; } - - protected override RoomManager CreateRoomManager() => RoomManager = new TestMultiplayerRoomManager(); - } } } diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs index 9bb19d4286..ad92886bab 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs @@ -6,25 +6,19 @@ using System.Linq; using NUnit.Framework; using osu.Framework.Allocation; using osu.Framework.Audio; -using osu.Framework.Graphics; 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; using osu.Game.Rulesets; using osu.Game.Rulesets.Osu; -using osu.Game.Screens; -using osu.Game.Screens.OnlinePlay.Components; using osu.Game.Screens.OnlinePlay.Lounge; 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 @@ -35,9 +29,9 @@ namespace osu.Game.Tests.Visual.Multiplayer private RulesetStore rulesets; private BeatmapSetInfo importedSet; - private DependenciesScreen dependenciesScreen; - private TestMultiplayer multiplayerScreen; - private TestMultiplayerClient client; + private TestMultiplayerScreenStack multiplayerScreenStack; + + private TestMultiplayerClient client => multiplayerScreenStack.Client; [Cached(typeof(UserLookupCache))] private UserLookupCache lookupCache = new TestUserLookupCache(); @@ -59,24 +53,8 @@ namespace osu.Game.Tests.Visual.Multiplayer importedSet = beatmaps.GetAllUsableBeatmapSetsEnumerable(IncludedDetails.All).First(); }); - AddStep("create multiplayer screen", () => multiplayerScreen = new TestMultiplayer()); - - AddStep("load dependencies", () => - { - client = new TestMultiplayerClient(multiplayerScreen.RoomManager); - - // The screen gets suspended so it stops receiving updates. - Child = client; - - LoadScreen(dependenciesScreen = new DependenciesScreen(client)); - }); - - AddUntilStep("wait for dependencies screen", () => Stack.CurrentScreen is DependenciesScreen); - AddUntilStep("wait for dependencies to start load", () => dependenciesScreen.LoadState > LoadState.NotLoaded); - AddUntilStep("wait for dependencies to load", () => dependenciesScreen.IsLoaded); - - AddStep("load multiplayer", () => LoadScreen(multiplayerScreen)); - AddUntilStep("wait for multiplayer to load", () => multiplayerScreen.IsLoaded); + AddStep("load multiplayer", () => LoadScreen(multiplayerScreenStack = new TestMultiplayerScreenStack())); + AddUntilStep("wait for multiplayer to load", () => multiplayerScreenStack.IsLoaded); AddUntilStep("wait for lounge to load", () => this.ChildrenOfType().FirstOrDefault()?.IsLoaded == true); } @@ -122,7 +100,7 @@ namespace osu.Game.Tests.Visual.Multiplayer AddStep("press button", () => { - InputManager.MoveMouseTo(multiplayerScreen.ChildrenOfType().First()); + InputManager.MoveMouseTo(multiplayerScreenStack.ChildrenOfType().First()); InputManager.Click(MouseButton.Left); }); AddAssert("user on team 1", () => (client.Room?.Users.FirstOrDefault()?.MatchState as TeamVersusUserState)?.TeamID == 1); @@ -156,7 +134,7 @@ namespace osu.Game.Tests.Visual.Multiplayer private void createRoom(Func room) { - AddStep("open room", () => multiplayerScreen.ChildrenOfType().Single().Open(room())); + AddStep("open room", () => multiplayerScreenStack.ChildrenOfType().Single().Open(room())); AddUntilStep("wait for room open", () => this.ChildrenOfType().FirstOrDefault()?.IsLoaded == true); AddWaitStep("wait for transition", 2); @@ -169,35 +147,5 @@ namespace osu.Game.Tests.Visual.Multiplayer AddUntilStep("wait for join", () => client.Room != null); } - - /// - /// Used for the sole purpose of adding as a resolvable dependency. - /// - private class DependenciesScreen : OsuScreen - { - [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 TestMultiplayerRoomManager RoomManager { get; private set; } - - protected override RoomManager CreateRoomManager() => RoomManager = new TestMultiplayerRoomManager(); - } } } diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs index 1df0680b69..5d4594c415 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs @@ -9,23 +9,18 @@ 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; using osu.Game.Overlays.Toolbar; using osu.Game.Rulesets.Mods; using osu.Game.Rulesets.Osu.Mods; using osu.Game.Screens.Menu; -using osu.Game.Screens.OnlinePlay.Components; using osu.Game.Screens.OnlinePlay.Lounge; using osu.Game.Screens.Play; using osu.Game.Screens.Ranking; 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; @@ -335,12 +330,12 @@ namespace osu.Game.Tests.Visual.Navigation [Test] public void TestPushMatchSubScreenAndPressBackButtonImmediately() { - TestMultiplayer multiplayer = null; + TestMultiplayerScreenStack multiplayerScreenStack = null; - PushAndConfirm(() => multiplayer = new TestMultiplayer()); + PushAndConfirm(() => multiplayerScreenStack = new TestMultiplayerScreenStack()); - AddUntilStep("wait for lounge", () => multiplayer.ChildrenOfType().SingleOrDefault()?.IsLoaded == true); - AddStep("open room", () => multiplayer.ChildrenOfType().Single().Open()); + AddUntilStep("wait for lounge", () => multiplayerScreenStack.ChildrenOfType().SingleOrDefault()?.IsLoaded == true); + AddStep("open room", () => multiplayerScreenStack.ChildrenOfType().Single().Open()); AddStep("press back button", () => Game.ChildrenOfType().First().Action()); AddWaitStep("wait two frames", 2); } @@ -455,28 +450,5 @@ namespace osu.Game.Tests.Visual.Navigation protected override bool DisplayStableImportPrompt => false; } - - private class TestMultiplayer : Screens.OnlinePlay.Multiplayer.Multiplayer - { - [Cached(typeof(MultiplayerClient))] - public readonly TestMultiplayerClient Client; - - [Cached] - public readonly TestRoomRequestsHandler RequestsHandler; - - public TestMultiplayer() - { - Client = new TestMultiplayerClient((TestMultiplayerRoomManager)RoomManager); - RequestsHandler = new TestRoomRequestsHandler(); - } - - [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(); - } } } diff --git a/osu.Game.Tests/Visual/TestMultiplayerScreenStack.cs b/osu.Game.Tests/Visual/TestMultiplayerScreenStack.cs new file mode 100644 index 0000000000..6b3657df44 --- /dev/null +++ b/osu.Game.Tests/Visual/TestMultiplayerScreenStack.cs @@ -0,0 +1,84 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Threading; +using osu.Framework.Allocation; +using osu.Framework.Graphics; +using osu.Framework.Screens; +using osu.Game.Online.API; +using osu.Game.Online.Multiplayer; +using osu.Game.Screens; +using osu.Game.Screens.OnlinePlay.Components; +using osu.Game.Tests.Visual.Multiplayer; +using osu.Game.Tests.Visual.OnlinePlay; + +namespace osu.Game.Tests.Visual +{ + /// + /// An loadable into s via , + /// which provides dependencies for and loads an isolated screen. + ///

+ /// This screen: + /// + /// Provides a to be resolved as a dependency in the screen, + /// which is typically a part of . + /// Rebinds the to handle requests via a . + /// Provides a for the screen. + /// + ///

+ ///
+ public class TestMultiplayerScreenStack : OsuScreen + { + public Screens.OnlinePlay.Multiplayer.Multiplayer MultiplayerScreen => multiplayerScreen; + + public TestMultiplayerRoomManager RoomManager => multiplayerScreen.RoomManager; + + public IScreen CurrentScreen => screenStack.CurrentScreen; + + public new bool IsLoaded => base.IsLoaded && MultiplayerScreen.IsLoaded; + + [Cached(typeof(MultiplayerClient))] + public readonly TestMultiplayerClient Client; + + private readonly OsuScreenStack screenStack; + private readonly TestMultiplayer multiplayerScreen; + + public TestMultiplayerScreenStack() + { + multiplayerScreen = new TestMultiplayer(); + + InternalChildren = new Drawable[] + { + Client = new TestMultiplayerClient(RoomManager), + screenStack = new OsuScreenStack { RelativeSizeAxes = Axes.Both } + }; + + screenStack.Push(multiplayerScreen); + } + + [BackgroundDependencyLoader] + private void load(IAPIProvider api, OsuGameBase game) + { + ((DummyAPIAccess)api).HandleRequest = request => multiplayerScreen.RequestsHandler.HandleRequest(request, api.LocalUser.Value, game); + } + + public override bool OnBackButton() => multiplayerScreen.OnBackButton(); + + public override bool OnExiting(IScreen next) + { + if (screenStack.CurrentScreen == null) + return base.OnExiting(next); + + screenStack.Exit(); + return true; + } + + private class TestMultiplayer : Screens.OnlinePlay.Multiplayer.Multiplayer + { + public new TestMultiplayerRoomManager RoomManager { get; private set; } + public TestRoomRequestsHandler RequestsHandler { get; private set; } + + protected override RoomManager CreateRoomManager() => RoomManager = new TestMultiplayerRoomManager(RequestsHandler = new TestRoomRequestsHandler()); + } + } +} diff --git a/osu.Game/Tests/Visual/Multiplayer/MultiplayerTestSceneDependencies.cs b/osu.Game/Tests/Visual/Multiplayer/MultiplayerTestSceneDependencies.cs index a2b0b066a7..ed349a7103 100644 --- a/osu.Game/Tests/Visual/Multiplayer/MultiplayerTestSceneDependencies.cs +++ b/osu.Game/Tests/Visual/Multiplayer/MultiplayerTestSceneDependencies.cs @@ -31,7 +31,7 @@ namespace osu.Game.Tests.Visual.Multiplayer CacheAs(SpectatorClient); } - protected override IRoomManager CreateRoomManager() => new TestMultiplayerRoomManager(); + protected override IRoomManager CreateRoomManager() => new TestMultiplayerRoomManager(RequestsHandler); protected virtual TestSpectatorClient CreateSpectatorClient() => new TestSpectatorClient(); } diff --git a/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerRoomManager.cs b/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerRoomManager.cs index fef2a0a16d..4129d190be 100644 --- a/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerRoomManager.cs +++ b/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerRoomManager.cs @@ -2,7 +2,6 @@ // See the LICENCE file in the repository root for full licence text. using System.Collections.Generic; -using osu.Framework.Allocation; using osu.Game.Online.Rooms; using osu.Game.Screens.OnlinePlay.Components; using osu.Game.Screens.OnlinePlay.Multiplayer; @@ -16,8 +15,12 @@ namespace osu.Game.Tests.Visual.Multiplayer ///
public class TestMultiplayerRoomManager : MultiplayerRoomManager { - [Resolved] - private TestRoomRequestsHandler requestsHandler { get; set; } + private readonly TestRoomRequestsHandler requestsHandler; + + public TestMultiplayerRoomManager(TestRoomRequestsHandler requestsHandler) + { + this.requestsHandler = requestsHandler; + } public IReadOnlyList ServerSideRooms => requestsHandler.ServerSideRooms; diff --git a/osu.Game/Tests/Visual/OnlinePlay/OnlinePlayTestSceneDependencies.cs b/osu.Game/Tests/Visual/OnlinePlay/OnlinePlayTestSceneDependencies.cs index 9e5264fb12..24c4ff79d4 100644 --- a/osu.Game/Tests/Visual/OnlinePlay/OnlinePlayTestSceneDependencies.cs +++ b/osu.Game/Tests/Visual/OnlinePlay/OnlinePlayTestSceneDependencies.cs @@ -34,10 +34,10 @@ namespace osu.Game.Tests.Visual.OnlinePlay public OnlinePlayTestSceneDependencies() { SelectedRoom = new Bindable(); - RoomManager = CreateRoomManager(); + RequestsHandler = new TestRoomRequestsHandler(); OngoingOperationTracker = new OngoingOperationTracker(); AvailabilityTracker = new OnlinePlayBeatmapAvailabilityTracker(); - RequestsHandler = new TestRoomRequestsHandler(); + RoomManager = CreateRoomManager(); dependencies = new DependencyContainer(new CachedModelDependencyContainer(null) { Model = { BindTarget = SelectedRoom } }); From 8bc414e7254781de0271389be14e73e19497f0fc Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Thu, 28 Oct 2021 14:34:27 +0900 Subject: [PATCH 54/68] Upgrade nvika to 1.0.3 --- .config/dotnet-tools.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.config/dotnet-tools.json b/.config/dotnet-tools.json index 007e4341b8..1b06aa4d17 100644 --- a/.config/dotnet-tools.json +++ b/.config/dotnet-tools.json @@ -15,7 +15,7 @@ ] }, "smoogipoo.nvika": { - "version": "1.0.1", + "version": "1.0.3", "commands": [ "nvika" ] @@ -33,4 +33,4 @@ ] } } -} +} \ No newline at end of file From f3d208fecf32d42cb78d77417cc1123916a8d21b Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 28 Oct 2021 14:48:36 +0900 Subject: [PATCH 55/68] Remove TODO (tested to work correctly) --- osu.Game.Tournament/Components/SongBar.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tournament/Components/SongBar.cs b/osu.Game.Tournament/Components/SongBar.cs index 9af73a2abb..a74b88c592 100644 --- a/osu.Game.Tournament/Components/SongBar.cs +++ b/osu.Game.Tournament/Components/SongBar.cs @@ -101,7 +101,7 @@ namespace osu.Game.Tournament.Components return; } - double bpm = beatmap.BPM; // TODO: check this works. + double bpm = beatmap.BPM; double length = beatmap.Length; string hardRockExtra = ""; string srExtra = ""; From ef26b0ba8aba42209ddbae7c2de2ef8cd6f2334e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 28 Oct 2021 15:00:30 +0900 Subject: [PATCH 56/68] Fix serlialisation failure during ladder saving causing all existing file content to be deleted --- osu.Game.Tournament/TournamentGameBase.cs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/osu.Game.Tournament/TournamentGameBase.cs b/osu.Game.Tournament/TournamentGameBase.cs index 978be720df..22cc07ad8d 100644 --- a/osu.Game.Tournament/TournamentGameBase.cs +++ b/osu.Game.Tournament/TournamentGameBase.cs @@ -269,17 +269,19 @@ namespace osu.Game.Tournament ladder.Matches.Where(p => p.LosersProgression.Value != null).Select(p => new TournamentProgression(p.ID, p.LosersProgression.Value.ID, true))) .ToList(); + string serialisedLadder = JsonConvert.SerializeObject(ladder, + new JsonSerializerSettings + { + Formatting = Formatting.Indented, + NullValueHandling = NullValueHandling.Ignore, + DefaultValueHandling = DefaultValueHandling.Ignore, + Converters = new JsonConverter[] { new JsonPointConverter() } + }); + using (var stream = storage.GetStream(bracket_filename, FileAccess.Write, FileMode.Create)) using (var sw = new StreamWriter(stream)) { - sw.Write(JsonConvert.SerializeObject(ladder, - new JsonSerializerSettings - { - Formatting = Formatting.Indented, - NullValueHandling = NullValueHandling.Ignore, - DefaultValueHandling = DefaultValueHandling.Ignore, - Converters = new JsonConverter[] { new JsonPointConverter() } - })); + sw.Write(serialisedLadder); } } From a6669a3892744152b7c4628303325263e45499e1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 28 Oct 2021 15:04:09 +0900 Subject: [PATCH 57/68] Add mention of why the serialisation happens outside stream usage --- osu.Game.Tournament/TournamentGameBase.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Game.Tournament/TournamentGameBase.cs b/osu.Game.Tournament/TournamentGameBase.cs index 22cc07ad8d..0e8646d15c 100644 --- a/osu.Game.Tournament/TournamentGameBase.cs +++ b/osu.Game.Tournament/TournamentGameBase.cs @@ -269,6 +269,7 @@ namespace osu.Game.Tournament ladder.Matches.Where(p => p.LosersProgression.Value != null).Select(p => new TournamentProgression(p.ID, p.LosersProgression.Value.ID, true))) .ToList(); + // Serialise before opening stream for writing, so if there's a failure it will leave the file in the previous state. string serialisedLadder = JsonConvert.SerializeObject(ladder, new JsonSerializerSettings { @@ -280,9 +281,7 @@ namespace osu.Game.Tournament using (var stream = storage.GetStream(bracket_filename, FileAccess.Write, FileMode.Create)) using (var sw = new StreamWriter(stream)) - { sw.Write(serialisedLadder); - } } protected override UserInputManager CreateUserInputManager() => new TournamentInputManager(); From 56e3a42abebff9d036810f08d71025de2547edf1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 28 Oct 2021 15:14:29 +0900 Subject: [PATCH 58/68] Remove unused using statement --- osu.Game.Tests/Visual/TestMultiplayerScreenStack.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game.Tests/Visual/TestMultiplayerScreenStack.cs b/osu.Game.Tests/Visual/TestMultiplayerScreenStack.cs index 6b3657df44..7f1171db1f 100644 --- a/osu.Game.Tests/Visual/TestMultiplayerScreenStack.cs +++ b/osu.Game.Tests/Visual/TestMultiplayerScreenStack.cs @@ -1,7 +1,6 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using System.Threading; using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Screens; From 313f78cbfbe1638dfd2827560c584282850e9e8f Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Thu, 28 Oct 2021 15:24:44 +0900 Subject: [PATCH 59/68] Fix non-null inspection --- osu.Game.Tournament/Components/TournamentBeatmapPanel.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Tournament/Components/TournamentBeatmapPanel.cs b/osu.Game.Tournament/Components/TournamentBeatmapPanel.cs index 328b9278d5..b81a45c1d7 100644 --- a/osu.Game.Tournament/Components/TournamentBeatmapPanel.cs +++ b/osu.Game.Tournament/Components/TournamentBeatmapPanel.cs @@ -93,7 +93,7 @@ namespace osu.Game.Tournament.Components }, new TournamentSpriteText { - Text = Beatmap.Metadata?.Author, + Text = Beatmap.Metadata.Author, Padding = new MarginPadding { Right = 20 }, Font = OsuFont.Torus.With(weight: FontWeight.Bold, size: 14) }, From b0b27a47d5ba5bd2240cb0ff0175ebce04f00cec Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Thu, 28 Oct 2021 15:28:16 +0900 Subject: [PATCH 60/68] Rename test scene --- ...enePlaylistCreation.cs => TestScenePlaylistsRoomCreation.cs} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename osu.Game.Tests/Visual/Playlists/{TestScenePlaylistCreation.cs => TestScenePlaylistsRoomCreation.cs} (98%) diff --git a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistCreation.cs b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomCreation.cs similarity index 98% rename from osu.Game.Tests/Visual/Playlists/TestScenePlaylistCreation.cs rename to osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomCreation.cs index c4e6f41f27..1f43d254b6 100644 --- a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistCreation.cs +++ b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomCreation.cs @@ -22,7 +22,7 @@ using osuTK.Input; namespace osu.Game.Tests.Visual.Playlists { - public class TestScenePlaylistCreation : OnlinePlayTestScene + public class TestScenePlaylistsRoomCreation : OnlinePlayTestScene { private BeatmapManager manager; private RulesetStore rulesets; From e2c08fb2e5443e40667253bbe7aee6db93ca4153 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Thu, 28 Oct 2021 15:41:59 +0900 Subject: [PATCH 61/68] Create room by clicking button --- .../TestScenePlaylistsRoomCreation.cs | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomCreation.cs b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomCreation.cs index 1f43d254b6..21ab5c761c 100644 --- a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomCreation.cs +++ b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomCreation.cs @@ -54,7 +54,6 @@ namespace osu.Game.Tests.Visual.Playlists { setupAndCreateRoom(room => { - room.RoomID.Value = 1; // forces room creation. room.Name.Value = "my awesome room"; room.Host.Value = API.LocalUser.Value; room.RecentParticipants.Add(room.Host.Value); @@ -84,13 +83,6 @@ namespace osu.Game.Tests.Visual.Playlists }); }); - AddStep("move mouse to create button", () => - { - InputManager.MoveMouseTo(this.ChildrenOfType().Single()); - }); - - AddStep("click", () => InputManager.Click(MouseButton.Left)); - AddAssert("first playlist item selected", () => match.SelectedItem.Value == SelectedRoom.Value.Playlist[0]); } @@ -143,13 +135,12 @@ namespace osu.Game.Tests.Visual.Playlists private void setupAndCreateRoom(Action room) { - AddStep("setup room", () => - { - room(SelectedRoom.Value); + AddStep("setup room", () => room(SelectedRoom.Value)); - // if this isn't done the test will crash when a poll kicks in. - // probably not correct, but works for now. - OnlinePlayDependencies.RoomManager.CreateRoom(SelectedRoom.Value); + AddStep("click create button", () => + { + InputManager.MoveMouseTo(this.ChildrenOfType().Single()); + InputManager.Click(MouseButton.Left); }); } From dce19cf7e47bfaa66619523a8ed9e871c0fe49f8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 28 Oct 2021 15:42:06 +0900 Subject: [PATCH 62/68] Add visual test for `SettingsNumberBox` usage in settings source context --- osu.Game.Tests/Visual/Settings/TestSceneSettingsSource.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/osu.Game.Tests/Visual/Settings/TestSceneSettingsSource.cs b/osu.Game.Tests/Visual/Settings/TestSceneSettingsSource.cs index d530e1f796..fc81d9792e 100644 --- a/osu.Game.Tests/Visual/Settings/TestSceneSettingsSource.cs +++ b/osu.Game.Tests/Visual/Settings/TestSceneSettingsSource.cs @@ -6,6 +6,7 @@ using osu.Framework.Bindables; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Game.Configuration; +using osu.Game.Overlays.Settings; using osuTK; namespace osu.Game.Tests.Visual.Settings @@ -58,6 +59,13 @@ namespace osu.Game.Tests.Visual.Settings Default = string.Empty, Value = "Sample text" }; + + [SettingSource("Sample number textbox", "Textbox number entry", SettingControlType = typeof(SettingsNumberBox))] + public Bindable IntTextboxBindable { get; } = new Bindable + { + Default = null, + Value = null + }; } private enum TestEnum From 1e1f8c472adc57f9612b41d8cdcba47b55e2efa6 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 28 Oct 2021 16:34:12 +0900 Subject: [PATCH 63/68] Add icons to various progress notification states --- .../TestSceneNotificationOverlay.cs | 40 +++++++++++++- .../Notifications/ProgressNotification.cs | 54 +++++++++++++++++-- 2 files changed, 88 insertions(+), 6 deletions(-) diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneNotificationOverlay.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneNotificationOverlay.cs index d0f6f3fe47..a2aa0499d2 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneNotificationOverlay.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneNotificationOverlay.cs @@ -41,6 +41,44 @@ namespace osu.Game.Tests.Visual.UserInterface notificationOverlay.UnreadCount.ValueChanged += count => { displayedCount.Text = $"displayed count: {count.NewValue}"; }; }); + [Test] + public void TestCompleteProgress() + { + ProgressNotification notification = null; + AddStep("add progress notification", () => + { + notification = new ProgressNotification + { + Text = @"Uploading to BSS...", + CompletionText = "Uploaded to BSS!", + }; + notificationOverlay.Post(notification); + progressingNotifications.Add(notification); + }); + + AddUntilStep("wait completion", () => notification.State == ProgressNotificationState.Completed); + } + + [Test] + public void TestCancelProgress() + { + ProgressNotification notification = null; + AddStep("add progress notification", () => + { + notification = new ProgressNotification + { + Text = @"Uploading to BSS...", + CompletionText = "Uploaded to BSS!", + }; + notificationOverlay.Post(notification); + progressingNotifications.Add(notification); + }); + + AddWaitStep("wait 3", 3); + + AddStep("cancel notification", () => notification.State = ProgressNotificationState.Cancelled); + } + [Test] public void TestBasicFlow() { @@ -138,7 +176,7 @@ namespace osu.Game.Tests.Visual.UserInterface foreach (var n in progressingNotifications.FindAll(n => n.State == ProgressNotificationState.Active)) { if (n.Progress < 1) - n.Progress += (float)(Time.Elapsed / 400) * RNG.NextSingle(); + n.Progress += (float)(Time.Elapsed / 2000); else n.State = ProgressNotificationState.Completed; } diff --git a/osu.Game/Overlays/Notifications/ProgressNotification.cs b/osu.Game/Overlays/Notifications/ProgressNotification.cs index f8cd31f193..b27e15dd2c 100644 --- a/osu.Game/Overlays/Notifications/ProgressNotification.cs +++ b/osu.Game/Overlays/Notifications/ProgressNotification.cs @@ -4,11 +4,15 @@ using System; using System.Threading; using osu.Framework.Allocation; +using osu.Framework.Extensions.Color4Extensions; using osu.Framework.Graphics; +using osu.Framework.Graphics.Colour; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Shapes; +using osu.Framework.Graphics.Sprites; using osu.Game.Graphics; using osu.Game.Graphics.Containers; +using osu.Game.Graphics.UserInterface; using osuTK; using osuTK.Graphics; @@ -16,6 +20,8 @@ namespace osu.Game.Overlays.Notifications { public class ProgressNotification : Notification, IHasCompletionTarget { + private const float loading_spinner_size = 22; + public string Text { set => Schedule(() => textDrawable.Text = value); @@ -65,29 +71,53 @@ namespace osu.Game.Overlays.Notifications private void updateState() { + const double colour_fade_duration = 200; + switch (state) { case ProgressNotificationState.Queued: Light.Colour = colourQueued; Light.Pulsate = false; progressBar.Active = false; + + iconBackground.FadeColour(ColourInfo.GradientVertical(colourQueued, colourQueued.Lighten(0.5f)), colour_fade_duration); + loadingSpinner.Show(); break; case ProgressNotificationState.Active: Light.Colour = colourActive; Light.Pulsate = true; progressBar.Active = true; + + iconBackground.FadeColour(ColourInfo.GradientVertical(colourActive, colourActive.Lighten(0.5f)), colour_fade_duration); + loadingSpinner.Show(); break; case ProgressNotificationState.Cancelled: cancellationTokenSource.Cancel(); + iconBackground.FadeColour(ColourInfo.GradientVertical(Color4.Gray, Color4.Gray.Lighten(0.5f)), colour_fade_duration); + loadingSpinner.Hide(); + + var icon = new SpriteIcon + { + Icon = FontAwesome.Solid.Ban, + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Size = new Vector2(loading_spinner_size), + }; + + IconContent.Add(icon); + + icon.FadeInFromZero(200, Easing.OutQuint); + Light.Colour = colourCancelled; Light.Pulsate = false; progressBar.Active = false; break; case ProgressNotificationState.Completed: + loadingSpinner.Hide(); NotificationContent.MoveToY(-DrawSize.Y / 2, 200, Easing.OutQuint); this.FadeOut(200).Finally(d => Completed()); break; @@ -115,15 +145,13 @@ namespace osu.Game.Overlays.Notifications private Color4 colourActive; private Color4 colourCancelled; + private Box iconBackground; + private LoadingSpinner loadingSpinner; + private readonly TextFlowContainer textDrawable; public ProgressNotification() { - IconContent.Add(new Box - { - RelativeSizeAxes = Axes.Both, - }); - Content.Add(textDrawable = new OsuTextFlowContainer { Colour = OsuColour.Gray(128), @@ -138,6 +166,9 @@ namespace osu.Game.Overlays.Notifications RelativeSizeAxes = Axes.X, }); + // make some extra space for the progress bar. + IconContent.Margin = new MarginPadding { Bottom = 5 }; + State = ProgressNotificationState.Queued; // don't close on click by default. @@ -150,6 +181,19 @@ namespace osu.Game.Overlays.Notifications colourQueued = colours.YellowDark; colourActive = colours.Blue; colourCancelled = colours.Red; + + IconContent.AddRange(new Drawable[] + { + iconBackground = new Box + { + RelativeSizeAxes = Axes.Both, + Colour = Color4.White, + }, + loadingSpinner = new LoadingSpinner + { + Size = new Vector2(loading_spinner_size), + } + }); } public override void Close() From bacaa9d20152c77c978abf1e593165e1ade84120 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Thu, 28 Oct 2021 16:48:17 +0900 Subject: [PATCH 64/68] Fix playlists test crash --- osu.Game/Tests/Visual/OnlinePlay/OnlinePlayTestScene.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/osu.Game/Tests/Visual/OnlinePlay/OnlinePlayTestScene.cs b/osu.Game/Tests/Visual/OnlinePlay/OnlinePlayTestScene.cs index 4771ace1c4..430aae72f8 100644 --- a/osu.Game/Tests/Visual/OnlinePlay/OnlinePlayTestScene.cs +++ b/osu.Game/Tests/Visual/OnlinePlay/OnlinePlayTestScene.cs @@ -64,7 +64,12 @@ namespace osu.Game.Tests.Visual.OnlinePlay public override void SetUpSteps() { base.SetUpSteps(); - AddStep("setup API", () => ((DummyAPIAccess)API).HandleRequest = request => OnlinePlayDependencies.RequestsHandler.HandleRequest(request, API.LocalUser.Value, game)); + + AddStep("setup API", () => + { + var handler = OnlinePlayDependencies.RequestsHandler; + ((DummyAPIAccess)API).HandleRequest = request => handler.HandleRequest(request, API.LocalUser.Value, game); + }); } /// From b096bf6cd1c8857964e7c884c88665646ea0441f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 28 Oct 2021 16:56:29 +0900 Subject: [PATCH 65/68] Ensure all test beatmaps have unique OnlineIDs to avoid import conflicts The order of operation of imports means that the already-imported instances are considered most correct (they have usually been populated using an online source, although that is skipped in tests), so on two consecutive test imports in a test scene, the second will lose its online IDs completely. This aims to ensure that all test beatmaps will have online IDs, which is the general expected behaviour. --- .../Visual/Playlists/TestScenePlaylistsRoomCreation.cs | 5 +---- osu.Game/Tests/Beatmaps/TestBeatmap.cs | 6 ++++++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomCreation.cs b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomCreation.cs index 21ab5c761c..e9825f25d5 100644 --- a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomCreation.cs +++ b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomCreation.cs @@ -144,10 +144,7 @@ namespace osu.Game.Tests.Visual.Playlists }); } - private void importBeatmap() - { - AddStep("import beatmap", () => importedBeatmap = manager.Import(CreateBeatmap(new OsuRuleset().RulesetInfo).BeatmapInfo.BeatmapSet).Result); - } + private void importBeatmap() => AddStep("import beatmap", () => importedBeatmap = manager.Import(CreateBeatmap(new OsuRuleset().RulesetInfo).BeatmapInfo.BeatmapSet).Result); private class TestPlaylistsRoomSubScreen : PlaylistsRoomSubScreen { diff --git a/osu.Game/Tests/Beatmaps/TestBeatmap.cs b/osu.Game/Tests/Beatmaps/TestBeatmap.cs index 15b72ce6e3..caf83973c4 100644 --- a/osu.Game/Tests/Beatmaps/TestBeatmap.cs +++ b/osu.Game/Tests/Beatmaps/TestBeatmap.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.IO; using System.Text; +using System.Threading; using osu.Framework.Extensions; using osu.Game.Beatmaps; using osu.Game.IO; @@ -16,6 +17,9 @@ namespace osu.Game.Tests.Beatmaps { public class TestBeatmap : Beatmap { + private static int onlineSetID; + private static int onlineBeatmapID; + public TestBeatmap(RulesetInfo ruleset, bool withHitObjects = true) { var baseBeatmap = CreateBeatmap(); @@ -31,8 +35,10 @@ namespace osu.Game.Tests.Beatmaps BeatmapInfo.RulesetID = ruleset.ID ?? 0; BeatmapInfo.BeatmapSet.Metadata = BeatmapInfo.Metadata; BeatmapInfo.BeatmapSet.Beatmaps = new List { BeatmapInfo }; + BeatmapInfo.BeatmapSet.OnlineBeatmapSetID = Interlocked.Increment(ref onlineSetID); BeatmapInfo.Length = 75000; BeatmapInfo.OnlineInfo = new APIBeatmap(); + BeatmapInfo.OnlineBeatmapID = Interlocked.Increment(ref onlineBeatmapID); BeatmapInfo.BeatmapSet.OnlineInfo = new APIBeatmapSet { Status = BeatmapSetOnlineStatus.Ranked, From 8076e22e70b7e498a1a9fdc1351ca1e71fbf05ef Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 28 Oct 2021 17:10:04 +0900 Subject: [PATCH 66/68] Replace weird import handling --- .../TestSceneDrawableRoomPlaylist.cs | 8 +++++--- .../TestScenePlaylistsRoomCreation.cs | 19 ++++--------------- 2 files changed, 9 insertions(+), 18 deletions(-) diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs index 93bdbb79f4..f44b0c9716 100644 --- a/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs +++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneDrawableRoomPlaylist.cs @@ -38,8 +38,6 @@ namespace osu.Game.Tests.Visual.Multiplayer { Dependencies.Cache(rulesets = new RulesetStore(ContextFactory)); Dependencies.Cache(manager = new BeatmapManager(LocalStorage, ContextFactory, rulesets, null, audio, Resources, host, Beatmap.Default)); - - manager.Import(new TestBeatmap(new OsuRuleset().RulesetInfo).BeatmapInfo.BeatmapSet).Wait(); } [Test] @@ -204,7 +202,11 @@ namespace osu.Game.Tests.Visual.Multiplayer [Test] public void TestDownloadButtonHiddenWhenBeatmapExists() { - createPlaylist(new TestBeatmap(new OsuRuleset().RulesetInfo).BeatmapInfo); + var beatmap = new TestBeatmap(new OsuRuleset().RulesetInfo).BeatmapInfo; + + AddStep("import beatmap", () => manager.Import(beatmap.BeatmapSet).Wait()); + + createPlaylist(beatmap); assertDownloadButtonVisible(false); diff --git a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomCreation.cs b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomCreation.cs index e9825f25d5..cec86fa97e 100644 --- a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomCreation.cs +++ b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomCreation.cs @@ -91,27 +91,16 @@ namespace osu.Game.Tests.Visual.Playlists { BeatmapSetInfo importedSet = null; - // this step is required to make sure the further imports actually get online IDs. - // all the playlist logic relies on online ID matching. - AddStep("remove all matching online IDs", () => - { - var existing = manager.QueryBeatmapSets(s => s.OnlineBeatmapSetID == importedBeatmap.Value.OnlineBeatmapSetID).ToList(); - - foreach (var s in existing) - { - s.OnlineBeatmapSetID = null; - foreach (var b in s.Beatmaps) - b.OnlineBeatmapID = null; - manager.Update(s); - } - }); - AddStep("import altered beatmap", () => { IBeatmap beatmap = CreateBeatmap(new OsuRuleset().RulesetInfo); beatmap.BeatmapInfo.BaseDifficulty.CircleSize = 1; + // intentionally increment online IDs to clash with import below. + beatmap.BeatmapInfo.OnlineBeatmapID++; + beatmap.BeatmapInfo.BeatmapSet.OnlineBeatmapSetID++; + importedSet = manager.Import(beatmap.BeatmapInfo.BeatmapSet).Result.Value; }); From c0848486d69474513975c640052aaff464ca3b38 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 28 Oct 2021 17:10:12 +0900 Subject: [PATCH 67/68] Add missing test coverage for attempt count display --- .../TestScenePlaylistsRoomCreation.cs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomCreation.cs b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomCreation.cs index cec86fa97e..3f7409c5ba 100644 --- a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomCreation.cs +++ b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomCreation.cs @@ -15,6 +15,7 @@ using osu.Game.Database; using osu.Game.Online.Rooms; using osu.Game.Rulesets; using osu.Game.Rulesets.Osu; +using osu.Game.Screens.OnlinePlay.Components; using osu.Game.Screens.OnlinePlay.Playlists; using osu.Game.Screens.Play; using osu.Game.Tests.Visual.OnlinePlay; @@ -65,10 +66,33 @@ namespace osu.Game.Tests.Visual.Playlists }); }); + AddUntilStep("Progress details are hidden", () => match.ChildrenOfType().FirstOrDefault()?.Parent.Alpha == 0); + AddStep("start match", () => match.ChildrenOfType().First().TriggerClick()); AddUntilStep("player loader loaded", () => Stack.CurrentScreen is PlayerLoader); } + [Test] + public void TestAttemptLimitedMatch() + { + setupAndCreateRoom(room => + { + room.RoomID.Value = 1; // forces room creation. + room.Name.Value = "my awesome room"; + room.MaxAttempts.Value = 5; + room.Host.Value = API.LocalUser.Value; + room.RecentParticipants.Add(room.Host.Value); + room.EndDate.Value = DateTimeOffset.Now.AddMinutes(5); + room.Playlist.Add(new PlaylistItem + { + Beatmap = { Value = importedBeatmap.Value.Beatmaps.First() }, + Ruleset = { Value = new OsuRuleset().RulesetInfo } + }); + }); + + AddUntilStep("Progress details are visible", () => match.ChildrenOfType().FirstOrDefault()?.Parent.Alpha == 1); + } + [Test] public void TestPlaylistItemSelectedOnCreate() { From c96b07c62528a7f44c6f7537f67692ee2b65d486 Mon Sep 17 00:00:00 2001 From: smoogipoo Date: Thu, 28 Oct 2021 18:24:02 +0900 Subject: [PATCH 68/68] Remove unnecessary/wrong code --- .../Visual/Playlists/TestScenePlaylistsRoomCreation.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomCreation.cs b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomCreation.cs index 3f7409c5ba..cda7e95a46 100644 --- a/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomCreation.cs +++ b/osu.Game.Tests/Visual/Playlists/TestScenePlaylistsRoomCreation.cs @@ -77,7 +77,6 @@ namespace osu.Game.Tests.Visual.Playlists { setupAndCreateRoom(room => { - room.RoomID.Value = 1; // forces room creation. room.Name.Value = "my awesome room"; room.MaxAttempts.Value = 5; room.Host.Value = API.LocalUser.Value;