From cf1da1dd18bc7d9e3824acae4c94b53f45eebd4a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 7 Jul 2022 22:38:54 +0900 Subject: [PATCH] Fix skins potentially being duplicated on batch import Resolves https://github.com/ppy/osu/discussions/19024#discussioncomment-3099200 --- osu.Game/Database/RealmArchiveModelImporter.cs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/osu.Game/Database/RealmArchiveModelImporter.cs b/osu.Game/Database/RealmArchiveModelImporter.cs index 76f6db1384..ce3d0652b3 100644 --- a/osu.Game/Database/RealmArchiveModelImporter.cs +++ b/osu.Game/Database/RealmArchiveModelImporter.cs @@ -258,15 +258,13 @@ namespace osu.Game.Database { cancellationToken.ThrowIfCancellationRequested(); - bool checkedExisting = false; - TModel? existing = null; + TModel? existing; if (batchImport && archive != null) { // this is a fast bail condition to improve large import performance. item.Hash = computeHashFast(archive); - checkedExisting = true; existing = CheckForExisting(item, realm); if (existing != null) @@ -311,8 +309,12 @@ namespace osu.Game.Database // TODO: we may want to run this outside of the transaction. Populate(item, archive, realm, cancellationToken); - if (!checkedExisting) - existing = CheckForExisting(item, realm); + // Populate() may have adjusted file content (see SkinImporter.updateSkinIniMetadata), so regardless of whether a fast check was done earlier, let's + // check for existing items a second time. + // + // If this is ever a performance issue, the fast-check hash can be compared and trigger a skip of this second check if it still matches. + // I don't think it is a huge deal doing a second indexed check, though. + existing = CheckForExisting(item, realm); if (existing != null) {