From 15b533f2a4dcf427b9ddfe5838e237427a7f0bbb Mon Sep 17 00:00:00 2001
From: Dean Herbert <pe@ppy.sh>
Date: Fri, 11 Sep 2020 15:06:10 +0900
Subject: [PATCH] Hash skins based on name, not skin.ini contents

It is feasible that a user may be changing the contents of skin.ini
without changing the skin name / author. Such changes should not create
a new skin if already imported.
---
 osu.Game/Database/ArchiveModelManager.cs | 10 +++++++---
 osu.Game/Skinning/SkinManager.cs         |  8 ++++++++
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs
index 49d7edd56c..e87ab8167a 100644
--- a/osu.Game/Database/ArchiveModelManager.cs
+++ b/osu.Game/Database/ArchiveModelManager.cs
@@ -253,6 +253,9 @@ namespace osu.Game.Database
         /// Generally should include all file types which determine the file's uniqueness.
         /// Large files should be avoided if possible.
         /// </summary>
+        /// <remarks>
+        /// This is only used by the default hash implementation. If <see cref="ComputeHash"/> is overridden, it will not be used.
+        /// </remarks>
         protected abstract string[] HashableFileTypes { get; }
 
         internal static void LogForModel(TModel model, string message, Exception e = null)
@@ -271,7 +274,7 @@ 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>
-        private string computeHash(TModel item, ArchiveReader reader = null)
+        protected virtual string ComputeHash(TModel item, ArchiveReader reader = null)
         {
             // for now, concatenate all .osu files in the set to create a unique hash.
             MemoryStream hashable = new MemoryStream();
@@ -318,10 +321,11 @@ namespace osu.Game.Database
                 LogForModel(item, "Beginning import...");
 
                 item.Files = archive != null ? createFileInfos(archive, Files) : new List<TFileModel>();
-                item.Hash = computeHash(item, archive);
 
                 await Populate(item, archive, cancellationToken);
 
+                item.Hash = ComputeHash(item, archive);
+
                 using (var write = ContextFactory.GetForWrite()) // used to share a context for full import. keep in mind this will block all writes.
                 {
                     try
@@ -437,7 +441,7 @@ namespace osu.Game.Database
         {
             using (ContextFactory.GetForWrite())
             {
-                item.Hash = computeHash(item);
+                item.Hash = ComputeHash(item);
                 ModelStore.Update(item);
             }
         }
diff --git a/osu.Game/Skinning/SkinManager.cs b/osu.Game/Skinning/SkinManager.cs
index e1f713882a..46130cbdd4 100644
--- a/osu.Game/Skinning/SkinManager.cs
+++ b/osu.Game/Skinning/SkinManager.cs
@@ -12,6 +12,7 @@ using Microsoft.EntityFrameworkCore;
 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;
@@ -86,6 +87,13 @@ namespace osu.Game.Skinning
 
         protected override SkinInfo CreateModel(ArchiveReader archive) => new SkinInfo { Name = archive.Name };
 
+        protected override string ComputeHash(SkinInfo item, ArchiveReader reader = null)
+        {
+            // this is the optimal way to hash legacy skins, but will need to be reconsidered when we move forward with skin implementation.
+            // likely, the skin should expose a real version (ie. the version of the skin, not the skin.ini version it's targeting).
+            return item.ToString().ComputeSHA2Hash();
+        }
+
         protected override async Task Populate(SkinInfo model, ArchiveReader archive, CancellationToken cancellationToken = default)
         {
             await base.Populate(model, archive, cancellationToken);