1
0
mirror of https://github.com/ppy/osu.git synced 2025-01-28 09:02:58 +08:00

Merge pull request #15204 from peppy/fix-skin-layout-editor-crash

Fix saving layouts on some stable imported skins failing
This commit is contained in:
Dan Balasescu 2021-10-28 11:55:17 +09:00 committed by GitHub
commit ba05664f3e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
26 changed files with 467 additions and 283 deletions

View File

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

View File

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

View File

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

View File

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

View File

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

View File

@ -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
{

View File

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

View File

@ -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]

View File

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

View File

@ -32,7 +32,7 @@ namespace osu.Game.Database
/// <typeparam name="TFileModel">The associated file join type.</typeparam>
public abstract class ArchiveModelManager<TModel, TFileModel> : IModelManager<TModel>, IModelFileManager<TModel, TFileModel>
where TModel : class, IHasFiles<TFileModel>, IHasPrimaryKey, ISoftDelete
where TFileModel : class, INamedFileInfo, new()
where TFileModel : class, INamedFileInfo, IHasPrimaryKey, new()
{
private const int import_queue_request_concurrency = 1;
@ -315,25 +315,29 @@ namespace osu.Game.Database
/// <remarks>
/// In the case of no matching files, a hash will be generated from the passed archive's <see cref="ArchiveReader.Name"/>.
/// </remarks>
protected virtual string ComputeHash(TModel item, ArchiveReader reader = null)
protected virtual string ComputeHash(TModel item)
{
if (reader != null)
// 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();
// 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))
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();
}
/// <summary>
@ -393,7 +397,7 @@ namespace osu.Game.Database
LogForModel(item, @"Beginning import...");
item.Files = archive != null ? createFileInfos(archive, Files) : new List<TFileModel>();
item.Hash = ComputeHash(item, archive);
item.Hash = ComputeHash(item);
await Populate(item, archive, cancellationToken).ConfigureAwait(false);
@ -516,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<TFileModel>().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<TFileModel>().Remove(file);
}
}
model.Files.Remove(file);
@ -540,9 +547,10 @@ namespace osu.Game.Database
Filename = filename,
FileInfo = Files.Add(contents)
});
Update(model);
}
if (model.ID > 0)
Update(model);
}
/// <summary>
@ -684,7 +692,7 @@ namespace osu.Game.Database
if (hashable.Length > 0)
return hashable.ComputeSHA2Hash();
return reader.Name.ComputeSHA2Hash();
return generateFallbackHash();
}
/// <summary>
@ -897,6 +905,14 @@ namespace osu.Game.Database
#endregion
private static string generateFallbackHash()
{
// if a hash could no be generated from file content, presume a unique / new import.
// therefore, let's use a guaranteed unique hash.
// this doesn't follow the SHA2 hashing schema intentionally, so such entries on the data store can be identified.
return Guid.NewGuid().ToString();
}
private string getValidFilename(string filename)
{
foreach (char c in Path.GetInvalidFileNameChars())

View File

@ -0,0 +1,23 @@
// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Migrations;
using osu.Game.Database;
namespace osu.Game.Migrations
{
[DbContext(typeof(OsuDbContext))]
[Migration("20211020081609_ResetSkinHashes")]
public partial class ResetSkinHashes : Migration
{
protected override void Up(MigrationBuilder migrationBuilder)
{
migrationBuilder.Sql($"UPDATE SkinInfo SET Hash = null");
}
protected override void Down(MigrationBuilder migrationBuilder)
{
}
}
}

View File

@ -481,7 +481,7 @@ namespace osu.Game.Rulesets.Objects.Legacy
/// </summary>
/// <remarks>
/// Layered hit samples are automatically added in all modes (except osu!mania), but can be disabled
/// using the <see cref="LegacySkinConfiguration.LegacySetting.LayeredHitSounds"/> skin config option.
/// using the <see cref="SkinConfiguration.LegacySetting.LayeredHitSounds"/> skin config option.
/// </remarks>
public readonly bool IsLayered;

View File

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

View File

@ -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;

View File

@ -1,12 +0,0 @@
// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.
namespace osu.Game.Skinning
{
/// <summary>
/// A skin configuration pre-populated with sane defaults.
/// </summary>
public class DefaultSkinConfiguration : SkinConfiguration
{
}
}

View File

@ -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)

View File

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

View File

@ -1,28 +0,0 @@
// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.
namespace osu.Game.Skinning
{
public class LegacySkinConfiguration : SkinConfiguration
{
public const decimal LATEST_VERSION = 2.7m;
/// <summary>
/// Legacy version of this skin.
/// </summary>
public decimal? LegacyVersion { get; internal set; }
public enum LegacySetting
{
Version,
ComboPrefix,
ComboOverlap,
ScorePrefix,
ScoreOverlap,
HitCirclePrefix,
HitCircleOverlap,
AnimationFramerate,
LayeredHitSounds
}
}
}

View File

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

View File

@ -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
{

View File

@ -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
{

View File

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

View File

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

View File

@ -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)

View File

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

View File

@ -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;
}