Add skin de-duplication

osu.Game/Beatmaps/BeatmapManager.cs

@@ -129,18 +129,8 @@ namespace osu.Game.Beatmaps
                 beatmaps.ForEach(b => b.OnlineBeatmapID = null);
         }
 
-        protected override BeatmapSetInfo CheckForExisting(BeatmapSetInfo model)
+        protected override BeatmapSetInfo CheckForExisting(BeatmapSetInfo model) =>
-        {
+            beatmaps.ConsumableItems.FirstOrDefault(b => b.Hash == model.Hash);
-            // check if this beatmap has already been imported and exit early if so
-            var existingHashMatch = beatmaps.ConsumableItems.FirstOrDefault(b => b.Hash == model.Hash);
-            if (existingHashMatch != null)
-            {
-                Undelete(existingHashMatch);
-                return existingHashMatch;
-            }
-
-            return null;
-        }
 
         /// <summary>
         /// Downloads a beatmap.

osu.Game/Database/ArchiveModelManager.cs

@@ -235,6 +235,7 @@ namespace osu.Game.Database
 
                         if (existing != null)
                         {
+                            Undelete(existing);
                             Logger.Log($"Found existing {typeof(TModel)} for {item} (ID {existing.ID}). Skipping import.", LoggingTarget.Database);
                             return existing;
                         }
@@ -471,6 +472,11 @@ namespace osu.Game.Database
         {
         }
 
+        /// <summary>
+        /// Check whether an existing model already exists for a new import item.
+        /// </summary>
+        /// <param name="model">The new model proposed for import. Note that <see cref="Populate"/> has not yet been run on this model.</param>
+        /// <returns>An existing model which matches the criteria to skip importing, else null.</returns>
         protected virtual TModel CheckForExisting(TModel model) => null;
 
         private DbSet<TModel> queryModel() => ContextFactory.Get().Set<TModel>();

osu.Game/Database/IHasFiles.cs

@@ -14,5 +14,7 @@ namespace osu.Game.Database
 
     {
         List<TFile> Files { get; set; }
+
+        string Hash { get; set; }
     }
 }

osu.Game/Skinning/SkinInfo.cs

@@ -15,16 +15,20 @@ namespace osu.Game.Skinning
 
         public string Name { get; set; }
 
+        public string Hash { get; set; }
+
         public string Creator { get; set; }
 
         public List<SkinFileInfo> Files { get; set; }
 
         public bool DeletePending { get; set; }
 
+        public string FullName => $"\"{Name}\" by {Creator}";
+
         public static SkinInfo Default { get; } = new SkinInfo { Name = "osu!lazer", Creator = "team osu!" };
 
         public bool Equals(SkinInfo other) => other != null && ID == other.ID;
 
-        public override string ToString() => $"\"{Name}\" by {Creator}";
+        public override string ToString() => FullName;
     }
 }

osu.Game/Skinning/SkinManager.cs

@@ -28,6 +28,28 @@ namespace osu.Game.Skinning
 
         protected override string ImportFromStablePath => "Skins";
 
+        public SkinManager(Storage storage, DatabaseContextFactory contextFactory, IIpcHost importHost, AudioManager audio)
+            : base(storage, contextFactory, new SkinStore(contextFactory, storage), importHost)
+        {
+            this.audio = audio;
+
+            ItemRemoved += removedInfo =>
+            {
+                // check the removed skin is not the current user choice. if it is, switch back to default.
+                if (removedInfo.ID == CurrentSkinInfo.Value.ID)
+                    CurrentSkinInfo.Value = SkinInfo.Default;
+            };
+
+            CurrentSkinInfo.ValueChanged += info => CurrentSkin.Value = getSkin(info);
+            CurrentSkin.ValueChanged += skin =>
+            {
+                if (skin.SkinInfo != CurrentSkinInfo.Value)
+                    throw new InvalidOperationException($"Setting {nameof(CurrentSkin)}'s value directly is not supported. Use {nameof(CurrentSkinInfo)} instead.");
+
+                SourceChanged?.Invoke();
+            };
+        }
+
         /// <summary>
         /// Returns a list of all usable <see cref="SkinInfo"/>s. Includes the special default skin plus all skins from <see cref="GetAllUserSkins"/>.
         /// </summary>
@@ -45,24 +67,16 @@ namespace osu.Game.Skinning
         /// <returns>A list of available <see cref="SkinInfo"/>.</returns>
         public List<SkinInfo> GetAllUserSkins() => ModelStore.ConsumableItems.Where(s => !s.DeletePending).ToList();
 
-        protected override SkinInfo CreateModel(ArchiveReader archive) => new SkinInfo
+        protected override SkinInfo CheckForExisting(SkinInfo model)
-        {
+            => ModelStore.ConsumableItems.FirstOrDefault(s => s.Name == model.Name && s.Creator == model.Creator);
-            Name = archive.Name
+
-        };
+        protected override SkinInfo CreateModel(ArchiveReader archive) => new SkinInfo { Name = archive.Name };
 
         protected override void Populate(SkinInfo model, ArchiveReader archive)
         {
             base.Populate(model, archive);
-            populate(model);
-        }
 
-        /// <summary>
+            Skin reference = getSkin(model);
-        /// Populate a <see cref="SkinInfo"/> from its <see cref="SkinConfiguration"/> (if possible).
-        /// </summary>
-        /// <param name="model"></param>
-        private void populate(SkinInfo model)
-        {
-            Skin reference = GetSkin(model);
             if (!string.IsNullOrEmpty(reference.Configuration.SkinInfo.Name))
             {
                 model.Name = reference.Configuration.SkinInfo.Name;
@@ -80,7 +94,7 @@ namespace osu.Game.Skinning
         /// </summary>
         /// <param name="skinInfo">The skin to lookup.</param>
         /// <returns>A <see cref="Skin"/> instance correlating to the provided <see cref="SkinInfo"/>.</returns>
-        public Skin GetSkin(SkinInfo skinInfo)
+        private Skin getSkin(SkinInfo skinInfo)
         {
             if (skinInfo == SkinInfo.Default)
                 return new DefaultSkin();
@@ -88,28 +102,6 @@ namespace osu.Game.Skinning
             return new LegacySkin(skinInfo, Files.Store, audio);
         }
 
-        public SkinManager(Storage storage, DatabaseContextFactory contextFactory, IIpcHost importHost, AudioManager audio)
-            : base(storage, contextFactory, new SkinStore(contextFactory, storage), importHost)
-        {
-            this.audio = audio;
-
-            ItemRemoved += removedInfo =>
-            {
-                // check the removed skin is not the current user choice. if it is, switch back to default.
-                if (removedInfo.ID == CurrentSkinInfo.Value.ID)
-                    CurrentSkinInfo.Value = SkinInfo.Default;
-            };
-
-            CurrentSkinInfo.ValueChanged += info => CurrentSkin.Value = GetSkin(info);
-            CurrentSkin.ValueChanged += skin =>
-            {
-                if (skin.SkinInfo != CurrentSkinInfo.Value)
-                    throw new InvalidOperationException($"Setting {nameof(CurrentSkin)}'s value directly is not supported. Use {nameof(CurrentSkinInfo)} instead.");
-
-                SourceChanged?.Invoke();
-            };
-        }
-
         /// <summary>
         /// Perform a lookup query on available <see cref="SkinInfo"/>s.
         /// </summary>