1
0
mirror of https://github.com/ppy/osu.git synced 2024-12-14 11:42:56 +08:00

Add skin.ini write support to allow for more correct hashing

This commit is contained in:
Dean Herbert 2021-10-20 15:22:47 +09:00
parent 08f3bc3f44
commit 789c715f13
3 changed files with 162 additions and 46 deletions

View File

@ -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<SkinManager>().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<SkinManager>().GetAllUserSkins(true).Count, Is.EqualTo(1));
Assert.That(osu.Dependencies.Get<SkinManager>().GetAllUserSkins(true).Count, Is.EqualTo(2));
// 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));
// skin.ini will be rewritten and therefore not match.
Assert.That(imported.Files.First().FileInfoID, Is.Not.EqualTo(imported2.Files.First().FileInfoID));
}
finally
{

View File

@ -507,18 +507,26 @@ namespace osu.Game.Database
/// <param name="file">The existing file to be deleted.</param>
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<TFileModel>().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<TFileModel>().Remove(file);
}
model.Files.Remove(file);
}
}
else
{
Files.Dereference(file.FileInfo);
model.Files.Remove(file);
}
}
@ -531,15 +539,28 @@ namespace osu.Game.Database
/// <param name="filename">The filename for the new file.</param>
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);
}
}

View File

@ -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<string> outputLines = new List<string>();
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}]";
}
/// <summary>
/// Retrieve a <see cref="Skin"/> instance for the provided <see cref="SkinInfo"/>
/// </summary>