From 59abb59ee862cc83145e7af2efb6f2e17dfcb407 Mon Sep 17 00:00:00 2001 From: Krzysztof Gutkowski Date: Thu, 17 Aug 2023 00:49:48 +0200 Subject: [PATCH 1/8] Set correct date added value when importing stable beatmapsets --- .../Database/LegacyBeatmapImporterTest.cs | 31 ++++++++++++++- osu.Game/Beatmaps/BeatmapImporter.cs | 38 ++++++++++++++++++- .../Archives/LegacyDirectoryArchiveReader.cs | 4 +- 3 files changed, 70 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tests/Database/LegacyBeatmapImporterTest.cs b/osu.Game.Tests/Database/LegacyBeatmapImporterTest.cs index b237556d11..638ad94472 100644 --- a/osu.Game.Tests/Database/LegacyBeatmapImporterTest.cs +++ b/osu.Game.Tests/Database/LegacyBeatmapImporterTest.cs @@ -1,19 +1,22 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. +using System; using System.Collections.Generic; using System.IO; using System.Linq; using NUnit.Framework; using osu.Framework.Platform; using osu.Framework.Testing; +using osu.Game.Beatmaps; using osu.Game.Database; using osu.Game.IO; +using osu.Game.Tests.Resources; namespace osu.Game.Tests.Database { [TestFixture] - public class LegacyBeatmapImporterTest + public class LegacyBeatmapImporterTest : RealmTest { private readonly TestLegacyBeatmapImporter importer = new TestLegacyBeatmapImporter(); @@ -60,6 +63,32 @@ namespace osu.Game.Tests.Database } } + [Test] + public void TestStableDateAddedApplied() + { + RunTestWithRealmAsync(async (realm, storage) => + { + using (HeadlessGameHost host = new CleanRunHeadlessGameHost()) + using (var tmpStorage = new TemporaryNativeStorage("stable-songs-folder")) + { + var stableStorage = new StableStorage(tmpStorage.GetFullPath(""), host); + var songsStorage = stableStorage.GetStorageForDirectory(StableStorage.STABLE_DEFAULT_SONGS_PATH); + + System.IO.Compression.ZipFile.ExtractToDirectory(TestResources.GetQuickTestBeatmapForImport(), songsStorage.GetFullPath("renatus")); + + string[] beatmaps = Directory.GetFiles(songsStorage.GetFullPath("renatus"), "*.osu", SearchOption.TopDirectoryOnly); + + File.SetLastWriteTimeUtc(beatmaps[beatmaps.Length / 2], new DateTime(2000, 1, 1, 12, 0, 0)); + + await new LegacyBeatmapImporter(new BeatmapImporter(storage, realm)).ImportFromStableAsync(stableStorage); + + var set = realm.Realm.All(); + + Assert.AreEqual(new DateTimeOffset(new DateTime(2000, 1, 1, 12, 0, 0, DateTimeKind.Utc)), set.First().DateAdded); + } + }); + } + private class TestLegacyBeatmapImporter : LegacyBeatmapImporter { public TestLegacyBeatmapImporter() diff --git a/osu.Game/Beatmaps/BeatmapImporter.cs b/osu.Game/Beatmaps/BeatmapImporter.cs index c840b4fa94..6c3f393069 100644 --- a/osu.Game/Beatmaps/BeatmapImporter.cs +++ b/osu.Game/Beatmaps/BeatmapImporter.cs @@ -302,14 +302,50 @@ namespace osu.Game.Beatmaps beatmap = Decoder.GetDecoder(stream).Decode(stream); } + var dateAdded = DateTimeOffset.UtcNow; + + // Apply proper date added for a beatmapset when importing from stable. + // Stable tracks said date using the filesystem last modified date on the .osu file. + if (reader is LegacyDirectoryArchiveReader legacyReader) + { + dateAdded = determineDateAdded(legacyReader); + } + return new BeatmapSetInfo { OnlineID = beatmap.BeatmapInfo.BeatmapSet?.OnlineID ?? -1, // Metadata = beatmap.Metadata, - DateAdded = DateTimeOffset.UtcNow + DateAdded = dateAdded }; } + /// + /// Used for beatmapsets in legacy (stable) storage. + /// Determine the date a given beatmapset has been added to the game. + /// The specific date is determined based on the oldest `.osu` file existing + /// in the beatmapset directory. + /// + /// + /// + private DateTimeOffset determineDateAdded(LegacyDirectoryArchiveReader reader) + { + var beatmaps = reader.Filenames.Where(f => f.EndsWith(".osu", StringComparison.OrdinalIgnoreCase)); + + var dateAdded = File.GetLastWriteTimeUtc(reader.GetPath(beatmaps.First())); + + foreach (string beatmapName in beatmaps) + { + var currentDateAdded = File.GetLastWriteTimeUtc(reader.GetPath(beatmapName)); + + if (currentDateAdded < dateAdded) + { + dateAdded = currentDateAdded; + } + } + + return new DateTimeOffset(dateAdded); + } + /// /// Create all required s for the provided archive. /// diff --git a/osu.Game/IO/Archives/LegacyDirectoryArchiveReader.cs b/osu.Game/IO/Archives/LegacyDirectoryArchiveReader.cs index dfae58aed7..8a576e33d8 100644 --- a/osu.Game/IO/Archives/LegacyDirectoryArchiveReader.cs +++ b/osu.Game/IO/Archives/LegacyDirectoryArchiveReader.cs @@ -21,7 +21,9 @@ namespace osu.Game.IO.Archives this.path = Path.GetFullPath(path); } - public override Stream GetStream(string name) => File.OpenRead(Path.Combine(path, name)); + public override Stream GetStream(string name) => File.OpenRead(GetPath(name)); + + public string GetPath(string name) => Path.Combine(path, name); public override void Dispose() { From 046cc62db2b3c58096a3a9697c4f6a3ed76a232f Mon Sep 17 00:00:00 2001 From: Krzysztof Gutkowski Date: Thu, 17 Aug 2023 01:11:09 +0200 Subject: [PATCH 2/8] Cleanup tests --- osu.Game.Tests/Database/LegacyBeatmapImporterTest.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Database/LegacyBeatmapImporterTest.cs b/osu.Game.Tests/Database/LegacyBeatmapImporterTest.cs index 638ad94472..d6056fa71b 100644 --- a/osu.Game.Tests/Database/LegacyBeatmapImporterTest.cs +++ b/osu.Game.Tests/Database/LegacyBeatmapImporterTest.cs @@ -82,9 +82,10 @@ namespace osu.Game.Tests.Database await new LegacyBeatmapImporter(new BeatmapImporter(storage, realm)).ImportFromStableAsync(stableStorage); - var set = realm.Realm.All(); + var beatmapset = realm.Realm.All().First(); - Assert.AreEqual(new DateTimeOffset(new DateTime(2000, 1, 1, 12, 0, 0, DateTimeKind.Utc)), set.First().DateAdded); + Assert.NotNull(beatmapset); + Assert.AreEqual(new DateTimeOffset(new DateTime(2000, 1, 1, 12, 0, 0, DateTimeKind.Utc)), beatmapset.DateAdded); } }); } From 360f9750e1c3311928c57198d77c82b02ae4cfde Mon Sep 17 00:00:00 2001 From: OliBomby Date: Thu, 17 Aug 2023 16:30:18 +0200 Subject: [PATCH 3/8] Allow selecting empty control point groups --- osu.Game/Screens/Edit/Timing/ControlPointList.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/osu.Game/Screens/Edit/Timing/ControlPointList.cs b/osu.Game/Screens/Edit/Timing/ControlPointList.cs index 555c36aac0..64e2d5c1a8 100644 --- a/osu.Game/Screens/Edit/Timing/ControlPointList.cs +++ b/osu.Game/Screens/Edit/Timing/ControlPointList.cs @@ -147,6 +147,10 @@ namespace osu.Game.Screens.Edit.Timing trackedType = null; else { + // If the selected group has no control points, clear the tracked type. + // Otherwise the user will be unable to select a group with no control points. + if (selectedGroup.Value.ControlPoints.Count == 0) + trackedType = null; // If the selected group only has one control point, update the tracking type. if (selectedGroup.Value.ControlPoints.Count == 1) trackedType = selectedGroup.Value?.ControlPoints.Single().GetType(); From 9023059bc0cbbbe7caaa6e7fe8dd7be913f478e8 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 18 Aug 2023 17:27:09 +0900 Subject: [PATCH 4/8] Convert to switch statement --- .../Screens/Edit/Timing/ControlPointList.cs | 30 ++++++++++++------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/osu.Game/Screens/Edit/Timing/ControlPointList.cs b/osu.Game/Screens/Edit/Timing/ControlPointList.cs index 64e2d5c1a8..22e37b9efb 100644 --- a/osu.Game/Screens/Edit/Timing/ControlPointList.cs +++ b/osu.Game/Screens/Edit/Timing/ControlPointList.cs @@ -147,17 +147,25 @@ namespace osu.Game.Screens.Edit.Timing trackedType = null; else { - // If the selected group has no control points, clear the tracked type. - // Otherwise the user will be unable to select a group with no control points. - if (selectedGroup.Value.ControlPoints.Count == 0) - trackedType = null; - // If the selected group only has one control point, update the tracking type. - if (selectedGroup.Value.ControlPoints.Count == 1) - trackedType = selectedGroup.Value?.ControlPoints.Single().GetType(); - // If the selected group has more than one control point, choose the first as the tracking type - // if we don't already have a singular tracked type. - else if (trackedType == null) - trackedType = selectedGroup.Value?.ControlPoints.FirstOrDefault()?.GetType(); + switch (selectedGroup.Value.ControlPoints.Count) + { + // If the selected group has no control points, clear the tracked type. + // Otherwise the user will be unable to select a group with no control points. + case 0: + trackedType = null; + break; + + // If the selected group only has one control point, update the tracking type. + case 1: + trackedType = selectedGroup.Value?.ControlPoints.Single().GetType(); + break; + + // If the selected group has more than one control point, choose the first as the tracking type + // if we don't already have a singular tracked type. + default: + trackedType ??= selectedGroup.Value?.ControlPoints.FirstOrDefault()?.GetType(); + break; + } } if (trackedType != null) From eb2460d18014d3a14d0b2001be69b1bc778cfd5e Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 18 Aug 2023 17:40:00 +0900 Subject: [PATCH 5/8] Remove dead metadata transfer code --- osu.Game/Beatmaps/BeatmapImporter.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game/Beatmaps/BeatmapImporter.cs b/osu.Game/Beatmaps/BeatmapImporter.cs index 6c3f393069..21a694b3ba 100644 --- a/osu.Game/Beatmaps/BeatmapImporter.cs +++ b/osu.Game/Beatmaps/BeatmapImporter.cs @@ -314,7 +314,6 @@ namespace osu.Game.Beatmaps return new BeatmapSetInfo { OnlineID = beatmap.BeatmapInfo.BeatmapSet?.OnlineID ?? -1, - // Metadata = beatmap.Metadata, DateAdded = dateAdded }; } From 864f1bdb3e6781dd8c947f517d830610c5a376b4 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 18 Aug 2023 17:54:39 +0900 Subject: [PATCH 6/8] Move population of import time to the `Populate` method Feels like a better place to be doing this. I think we want to keep any kind of potentially expensive work in the `Populate` method. --- osu.Game/Beatmaps/BeatmapImporter.cs | 40 +++++++++++----------------- 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapImporter.cs b/osu.Game/Beatmaps/BeatmapImporter.cs index 21a694b3ba..7e82516bed 100644 --- a/osu.Game/Beatmaps/BeatmapImporter.cs +++ b/osu.Game/Beatmaps/BeatmapImporter.cs @@ -152,6 +152,8 @@ namespace osu.Game.Beatmaps if (archive != null) beatmapSet.Beatmaps.AddRange(createBeatmapDifficulties(beatmapSet, realm)); + beatmapSet.DateAdded = getDateAdded(archive); + foreach (BeatmapInfo b in beatmapSet.Beatmaps) { b.BeatmapSet = beatmapSet; @@ -302,47 +304,37 @@ namespace osu.Game.Beatmaps beatmap = Decoder.GetDecoder(stream).Decode(stream); } - var dateAdded = DateTimeOffset.UtcNow; - - // Apply proper date added for a beatmapset when importing from stable. - // Stable tracks said date using the filesystem last modified date on the .osu file. - if (reader is LegacyDirectoryArchiveReader legacyReader) - { - dateAdded = determineDateAdded(legacyReader); - } - return new BeatmapSetInfo { OnlineID = beatmap.BeatmapInfo.BeatmapSet?.OnlineID ?? -1, - DateAdded = dateAdded }; } /// - /// Used for beatmapsets in legacy (stable) storage. /// Determine the date a given beatmapset has been added to the game. - /// The specific date is determined based on the oldest `.osu` file existing - /// in the beatmapset directory. + /// For legacy imports, we can use the oldest file write time for any `.osu` file in the directory. + /// For any other import types, use "now". /// - /// - /// - private DateTimeOffset determineDateAdded(LegacyDirectoryArchiveReader reader) + private DateTimeOffset getDateAdded(ArchiveReader? reader) { - var beatmaps = reader.Filenames.Where(f => f.EndsWith(".osu", StringComparison.OrdinalIgnoreCase)); + DateTimeOffset dateAdded = DateTimeOffset.UtcNow; - var dateAdded = File.GetLastWriteTimeUtc(reader.GetPath(beatmaps.First())); - - foreach (string beatmapName in beatmaps) + if (reader is LegacyDirectoryArchiveReader legacyReader) { - var currentDateAdded = File.GetLastWriteTimeUtc(reader.GetPath(beatmapName)); + var beatmaps = reader.Filenames.Where(f => f.EndsWith(".osu", StringComparison.OrdinalIgnoreCase)); - if (currentDateAdded < dateAdded) + dateAdded = File.GetLastWriteTimeUtc(legacyReader.GetPath(beatmaps.First())); + + foreach (string beatmapName in beatmaps) { - dateAdded = currentDateAdded; + var currentDateAdded = File.GetLastWriteTimeUtc(legacyReader.GetPath(beatmapName)); + + if (currentDateAdded < dateAdded) + dateAdded = currentDateAdded; } } - return new DateTimeOffset(dateAdded); + return dateAdded; } /// From 5e0b89a1a862c20a938dc3b7d1e63be87e717ff9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 18 Aug 2023 17:56:43 +0900 Subject: [PATCH 7/8] Rename `GetPath` to `GetFullPath` to better match expectations --- osu.Game/Beatmaps/BeatmapImporter.cs | 4 ++-- osu.Game/IO/Archives/LegacyDirectoryArchiveReader.cs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapImporter.cs b/osu.Game/Beatmaps/BeatmapImporter.cs index 7e82516bed..14719da1bc 100644 --- a/osu.Game/Beatmaps/BeatmapImporter.cs +++ b/osu.Game/Beatmaps/BeatmapImporter.cs @@ -323,11 +323,11 @@ namespace osu.Game.Beatmaps { var beatmaps = reader.Filenames.Where(f => f.EndsWith(".osu", StringComparison.OrdinalIgnoreCase)); - dateAdded = File.GetLastWriteTimeUtc(legacyReader.GetPath(beatmaps.First())); + dateAdded = File.GetLastWriteTimeUtc(legacyReader.GetFullPath(beatmaps.First())); foreach (string beatmapName in beatmaps) { - var currentDateAdded = File.GetLastWriteTimeUtc(legacyReader.GetPath(beatmapName)); + var currentDateAdded = File.GetLastWriteTimeUtc(legacyReader.GetFullPath(beatmapName)); if (currentDateAdded < dateAdded) dateAdded = currentDateAdded; diff --git a/osu.Game/IO/Archives/LegacyDirectoryArchiveReader.cs b/osu.Game/IO/Archives/LegacyDirectoryArchiveReader.cs index 8a576e33d8..1503705022 100644 --- a/osu.Game/IO/Archives/LegacyDirectoryArchiveReader.cs +++ b/osu.Game/IO/Archives/LegacyDirectoryArchiveReader.cs @@ -21,9 +21,9 @@ namespace osu.Game.IO.Archives this.path = Path.GetFullPath(path); } - public override Stream GetStream(string name) => File.OpenRead(GetPath(name)); + public override Stream GetStream(string name) => File.OpenRead(GetFullPath(name)); - public string GetPath(string name) => Path.Combine(path, name); + public string GetFullPath(string filename) => Path.Combine(path, filename); public override void Dispose() { From 80a143ae85cd2e2b53213c891c6521b8457cd7a9 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 18 Aug 2023 18:00:33 +0900 Subject: [PATCH 8/8] Tidy up test a bit more --- osu.Game.Tests/Database/LegacyBeatmapImporterTest.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/osu.Game.Tests/Database/LegacyBeatmapImporterTest.cs b/osu.Game.Tests/Database/LegacyBeatmapImporterTest.cs index d6056fa71b..0144c0bf97 100644 --- a/osu.Game.Tests/Database/LegacyBeatmapImporterTest.cs +++ b/osu.Game.Tests/Database/LegacyBeatmapImporterTest.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.IO; +using System.IO.Compression; using System.Linq; using NUnit.Framework; using osu.Framework.Platform; @@ -74,7 +75,7 @@ namespace osu.Game.Tests.Database var stableStorage = new StableStorage(tmpStorage.GetFullPath(""), host); var songsStorage = stableStorage.GetStorageForDirectory(StableStorage.STABLE_DEFAULT_SONGS_PATH); - System.IO.Compression.ZipFile.ExtractToDirectory(TestResources.GetQuickTestBeatmapForImport(), songsStorage.GetFullPath("renatus")); + ZipFile.ExtractToDirectory(TestResources.GetQuickTestBeatmapForImport(), songsStorage.GetFullPath("renatus")); string[] beatmaps = Directory.GetFiles(songsStorage.GetFullPath("renatus"), "*.osu", SearchOption.TopDirectoryOnly); @@ -82,10 +83,10 @@ namespace osu.Game.Tests.Database await new LegacyBeatmapImporter(new BeatmapImporter(storage, realm)).ImportFromStableAsync(stableStorage); - var beatmapset = realm.Realm.All().First(); + var importedSet = realm.Realm.All().Single(); - Assert.NotNull(beatmapset); - Assert.AreEqual(new DateTimeOffset(new DateTime(2000, 1, 1, 12, 0, 0, DateTimeKind.Utc)), beatmapset.DateAdded); + Assert.NotNull(importedSet); + Assert.AreEqual(new DateTimeOffset(new DateTime(2000, 1, 1, 12, 0, 0, DateTimeKind.Utc)), importedSet.DateAdded); } }); }