From fd3f4a9e7b30560c6270845af4a77ae1ed8073ec Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 29 Apr 2024 22:26:44 +0800 Subject: [PATCH 1/5] Preserve storyboard events when saving a beatmap in the editor Until we have full encoding support for storyboards, this stop-gap measure ensures that storyboards don't just disappear from existence. --- .../Beatmaps/Formats/LegacyBeatmapEncoderTest.cs | 15 +++++++++++++++ osu.Game/Beatmaps/Beatmap.cs | 2 ++ osu.Game/Beatmaps/BeatmapConverter.cs | 1 + osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs | 14 ++++++++++++++ osu.Game/Beatmaps/Formats/LegacyBeatmapEncoder.cs | 3 +++ osu.Game/Beatmaps/IBeatmap.cs | 6 ++++++ .../Rulesets/Difficulty/DifficultyCalculator.cs | 2 ++ osu.Game/Screens/Edit/EditorBeatmap.cs | 2 ++ osu.Game/Tests/Beatmaps/TestBeatmap.cs | 1 + 9 files changed, 46 insertions(+) diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs index e847b61fbe..ef30f020ce 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs @@ -37,6 +37,21 @@ namespace osu.Game.Tests.Beatmaps.Formats private static IEnumerable allBeatmaps = beatmaps_resource_store.GetAvailableResources().Where(res => res.EndsWith(".osu", StringComparison.Ordinal)); + [Test] + public void TestUnsupportedStoryboardEvents() + { + const string name = "Resources/storyboard_only_video.osu"; + + var decoded = decodeFromLegacy(beatmaps_resource_store.GetStream(name), name); + var decodedAfterEncode = decodeFromLegacy(encodeToLegacy(decoded), name); + + Assert.That(decoded.beatmap.UnhandledEventLines.Count, Is.EqualTo(1)); + Assert.That(decoded.beatmap.UnhandledEventLines.Single(), Is.EqualTo("Video,0,\"video.avi\"")); + + Assert.That(decodedAfterEncode.beatmap.UnhandledEventLines.Count, Is.EqualTo(1)); + Assert.That(decodedAfterEncode.beatmap.UnhandledEventLines.Single(), Is.EqualTo("Video,0,\"video.avi\"")); + } + [TestCaseSource(nameof(allBeatmaps))] public void TestEncodeDecodeStability(string name) { diff --git a/osu.Game/Beatmaps/Beatmap.cs b/osu.Game/Beatmaps/Beatmap.cs index 6db9febf36..ae77e4adcf 100644 --- a/osu.Game/Beatmaps/Beatmap.cs +++ b/osu.Game/Beatmaps/Beatmap.cs @@ -63,6 +63,8 @@ namespace osu.Game.Beatmaps public List Breaks { get; set; } = new List(); + public List UnhandledEventLines { get; set; } = new List(); + [JsonIgnore] public double TotalBreakTime => Breaks.Sum(b => b.Duration); diff --git a/osu.Game/Beatmaps/BeatmapConverter.cs b/osu.Game/Beatmaps/BeatmapConverter.cs index c7c244bf0e..b68c80d4b3 100644 --- a/osu.Game/Beatmaps/BeatmapConverter.cs +++ b/osu.Game/Beatmaps/BeatmapConverter.cs @@ -66,6 +66,7 @@ namespace osu.Game.Beatmaps beatmap.ControlPointInfo = original.ControlPointInfo; beatmap.HitObjects = convertHitObjects(original.HitObjects, original, cancellationToken).OrderBy(s => s.StartTime).ToList(); beatmap.Breaks = original.Breaks; + beatmap.UnhandledEventLines = original.UnhandledEventLines; return beatmap; } diff --git a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs index 386dada328..7407c3590f 100644 --- a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs @@ -420,6 +420,10 @@ namespace osu.Game.Beatmaps.Formats if (!Enum.TryParse(split[0], out LegacyEventType type)) throw new InvalidDataException($@"Unknown event type: {split[0]}"); + // Until we have full storyboard encoder coverage, let's track any lines which aren't handled + // and store them to a temporary location such that they aren't lost on editor save / export. + bool lineSupportedByEncoder = false; + switch (type) { case LegacyEventType.Sprite: @@ -427,7 +431,11 @@ namespace osu.Game.Beatmaps.Formats // In some older beatmaps, it is not present and replaced by a storyboard-level background instead. // Allow the first sprite (by file order) to act as the background in such cases. if (string.IsNullOrEmpty(beatmap.BeatmapInfo.Metadata.BackgroundFile)) + { beatmap.BeatmapInfo.Metadata.BackgroundFile = CleanFilename(split[3]); + lineSupportedByEncoder = true; + } + break; case LegacyEventType.Video: @@ -439,12 +447,14 @@ namespace osu.Game.Beatmaps.Formats if (!OsuGameBase.VIDEO_EXTENSIONS.Contains(Path.GetExtension(filename).ToLowerInvariant())) { beatmap.BeatmapInfo.Metadata.BackgroundFile = filename; + lineSupportedByEncoder = true; } break; case LegacyEventType.Background: beatmap.BeatmapInfo.Metadata.BackgroundFile = CleanFilename(split[2]); + lineSupportedByEncoder = true; break; case LegacyEventType.Break: @@ -452,8 +462,12 @@ namespace osu.Game.Beatmaps.Formats double end = Math.Max(start, getOffsetTime(Parsing.ParseDouble(split[2]))); beatmap.Breaks.Add(new BreakPeriod(start, end)); + lineSupportedByEncoder = true; break; } + + if (!lineSupportedByEncoder) + beatmap.UnhandledEventLines.Add(line); } private void handleTimingPoint(string line) diff --git a/osu.Game/Beatmaps/Formats/LegacyBeatmapEncoder.cs b/osu.Game/Beatmaps/Formats/LegacyBeatmapEncoder.cs index 290d29090a..186b565c39 100644 --- a/osu.Game/Beatmaps/Formats/LegacyBeatmapEncoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyBeatmapEncoder.cs @@ -156,6 +156,9 @@ namespace osu.Game.Beatmaps.Formats foreach (var b in beatmap.Breaks) writer.WriteLine(FormattableString.Invariant($"{(int)LegacyEventType.Break},{b.StartTime},{b.EndTime}")); + + foreach (string l in beatmap.UnhandledEventLines) + writer.WriteLine(l); } private void handleControlPoints(TextWriter writer) diff --git a/osu.Game/Beatmaps/IBeatmap.cs b/osu.Game/Beatmaps/IBeatmap.cs index 6fe494ca0f..5cc38e5b84 100644 --- a/osu.Game/Beatmaps/IBeatmap.cs +++ b/osu.Game/Beatmaps/IBeatmap.cs @@ -42,6 +42,12 @@ namespace osu.Game.Beatmaps /// List Breaks { get; } + /// + /// All lines from the [Events] section which aren't handled in the encoding process yet. + /// These lines shoule be written out to the beatmap file on save or export. + /// + List UnhandledEventLines { get; } + /// /// Total amount of break time in the beatmap. /// diff --git a/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs b/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs index 1599dff8d9..d37cfc28b9 100644 --- a/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs +++ b/osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs @@ -330,6 +330,8 @@ namespace osu.Game.Rulesets.Difficulty } public List Breaks => baseBeatmap.Breaks; + public List UnhandledEventLines => baseBeatmap.UnhandledEventLines; + public double TotalBreakTime => baseBeatmap.TotalBreakTime; public IEnumerable GetStatistics() => baseBeatmap.GetStatistics(); public double GetMostCommonBeatLength() => baseBeatmap.GetMostCommonBeatLength(); diff --git a/osu.Game/Screens/Edit/EditorBeatmap.cs b/osu.Game/Screens/Edit/EditorBeatmap.cs index dc1fda13f4..7a3ea474fb 100644 --- a/osu.Game/Screens/Edit/EditorBeatmap.cs +++ b/osu.Game/Screens/Edit/EditorBeatmap.cs @@ -174,6 +174,8 @@ namespace osu.Game.Screens.Edit public List Breaks => PlayableBeatmap.Breaks; + public List UnhandledEventLines => PlayableBeatmap.UnhandledEventLines; + public double TotalBreakTime => PlayableBeatmap.TotalBreakTime; public IReadOnlyList HitObjects => PlayableBeatmap.HitObjects; diff --git a/osu.Game/Tests/Beatmaps/TestBeatmap.cs b/osu.Game/Tests/Beatmaps/TestBeatmap.cs index ff670e1232..de7bcfcfaa 100644 --- a/osu.Game/Tests/Beatmaps/TestBeatmap.cs +++ b/osu.Game/Tests/Beatmaps/TestBeatmap.cs @@ -27,6 +27,7 @@ namespace osu.Game.Tests.Beatmaps BeatmapInfo = baseBeatmap.BeatmapInfo; ControlPointInfo = baseBeatmap.ControlPointInfo; Breaks = baseBeatmap.Breaks; + UnhandledEventLines = baseBeatmap.UnhandledEventLines; if (withHitObjects) HitObjects = baseBeatmap.HitObjects; From 19897c4c074fbca635b9bdbee66f4fb6e368a126 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 30 Apr 2024 00:50:40 +0800 Subject: [PATCH 2/5] Add testing for actual presence of video after encode-decode --- .../Beatmaps/Formats/LegacyBeatmapEncoderTest.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs index ef30f020ce..b931896898 100644 --- a/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs +++ b/osu.Game.Tests/Beatmaps/Formats/LegacyBeatmapEncoderTest.cs @@ -25,6 +25,7 @@ using osu.Game.Rulesets.Osu; using osu.Game.Rulesets.Osu.Objects; using osu.Game.Rulesets.Taiko; using osu.Game.Skinning; +using osu.Game.Storyboards; using osu.Game.Tests.Resources; using osuTK; @@ -43,13 +44,14 @@ namespace osu.Game.Tests.Beatmaps.Formats const string name = "Resources/storyboard_only_video.osu"; var decoded = decodeFromLegacy(beatmaps_resource_store.GetStream(name), name); - var decodedAfterEncode = decodeFromLegacy(encodeToLegacy(decoded), name); - Assert.That(decoded.beatmap.UnhandledEventLines.Count, Is.EqualTo(1)); Assert.That(decoded.beatmap.UnhandledEventLines.Single(), Is.EqualTo("Video,0,\"video.avi\"")); - Assert.That(decodedAfterEncode.beatmap.UnhandledEventLines.Count, Is.EqualTo(1)); - Assert.That(decodedAfterEncode.beatmap.UnhandledEventLines.Single(), Is.EqualTo("Video,0,\"video.avi\"")); + var memoryStream = encodeToLegacy(decoded); + + var storyboard = new LegacyStoryboardDecoder().Decode(new LineBufferedReader(memoryStream)); + StoryboardLayer video = storyboard.Layers.Single(l => l.Name == "Video"); + Assert.That(video.Elements.Count, Is.EqualTo(1)); } [TestCaseSource(nameof(allBeatmaps))] From ba9f4e4baff591cc4b35787216edfa4d88ccabc1 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Tue, 30 Apr 2024 16:08:30 +0800 Subject: [PATCH 3/5] Don't skip lines in beatmap decoding Was added in cc76c58f5f250151bb85ad5efa3f6ce008f0cbb0 without any specific reasoning. Likely not required (and will fix some storyboard elements inside `.osu` files from not being correctly saved). --- osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs index 7407c3590f..84f3c0d82a 100644 --- a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs @@ -167,8 +167,6 @@ namespace osu.Game.Beatmaps.Formats beatmapInfo.SamplesMatchPlaybackRate = false; } - protected override bool ShouldSkipLine(string line) => base.ShouldSkipLine(line) || line.StartsWith(' ') || line.StartsWith('_'); - protected override void ParseLine(Beatmap beatmap, Section section, string line) { switch (section) From 44091b1f352273c36f7545bfe44eaaeb0ef19003 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 1 May 2024 17:33:03 +0800 Subject: [PATCH 4/5] Fix some lines still getting forgotten about --- .../Beatmaps/Formats/LegacyBeatmapDecoder.cs | 75 ++++++++++--------- 1 file changed, 38 insertions(+), 37 deletions(-) diff --git a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs index 84f3c0d82a..3ecc29bd02 100644 --- a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs @@ -415,53 +415,54 @@ namespace osu.Game.Beatmaps.Formats { string[] split = line.Split(','); - if (!Enum.TryParse(split[0], out LegacyEventType type)) - throw new InvalidDataException($@"Unknown event type: {split[0]}"); // Until we have full storyboard encoder coverage, let's track any lines which aren't handled // and store them to a temporary location such that they aren't lost on editor save / export. bool lineSupportedByEncoder = false; - switch (type) + if (Enum.TryParse(split[0], out LegacyEventType type)) { - case LegacyEventType.Sprite: - // Generally, the background is the first thing defined in a beatmap file. - // In some older beatmaps, it is not present and replaced by a storyboard-level background instead. - // Allow the first sprite (by file order) to act as the background in such cases. - if (string.IsNullOrEmpty(beatmap.BeatmapInfo.Metadata.BackgroundFile)) - { - beatmap.BeatmapInfo.Metadata.BackgroundFile = CleanFilename(split[3]); + switch (type) + { + case LegacyEventType.Sprite: + // Generally, the background is the first thing defined in a beatmap file. + // In some older beatmaps, it is not present and replaced by a storyboard-level background instead. + // Allow the first sprite (by file order) to act as the background in such cases. + if (string.IsNullOrEmpty(beatmap.BeatmapInfo.Metadata.BackgroundFile)) + { + beatmap.BeatmapInfo.Metadata.BackgroundFile = CleanFilename(split[3]); + lineSupportedByEncoder = true; + } + + break; + + case LegacyEventType.Video: + string filename = CleanFilename(split[2]); + + // Some very old beatmaps had incorrect type specifications for their backgrounds (ie. using 1 for VIDEO + // instead of 0 for BACKGROUND). To handle this gracefully, check the file extension against known supported + // video extensions and handle similar to a background if it doesn't match. + if (!OsuGameBase.VIDEO_EXTENSIONS.Contains(Path.GetExtension(filename).ToLowerInvariant())) + { + beatmap.BeatmapInfo.Metadata.BackgroundFile = filename; + lineSupportedByEncoder = true; + } + + break; + + case LegacyEventType.Background: + beatmap.BeatmapInfo.Metadata.BackgroundFile = CleanFilename(split[2]); lineSupportedByEncoder = true; - } + break; - break; + case LegacyEventType.Break: + double start = getOffsetTime(Parsing.ParseDouble(split[1])); + double end = Math.Max(start, getOffsetTime(Parsing.ParseDouble(split[2]))); - case LegacyEventType.Video: - string filename = CleanFilename(split[2]); - - // Some very old beatmaps had incorrect type specifications for their backgrounds (ie. using 1 for VIDEO - // instead of 0 for BACKGROUND). To handle this gracefully, check the file extension against known supported - // video extensions and handle similar to a background if it doesn't match. - if (!OsuGameBase.VIDEO_EXTENSIONS.Contains(Path.GetExtension(filename).ToLowerInvariant())) - { - beatmap.BeatmapInfo.Metadata.BackgroundFile = filename; + beatmap.Breaks.Add(new BreakPeriod(start, end)); lineSupportedByEncoder = true; - } - - break; - - case LegacyEventType.Background: - beatmap.BeatmapInfo.Metadata.BackgroundFile = CleanFilename(split[2]); - lineSupportedByEncoder = true; - break; - - case LegacyEventType.Break: - double start = getOffsetTime(Parsing.ParseDouble(split[1])); - double end = Math.Max(start, getOffsetTime(Parsing.ParseDouble(split[2]))); - - beatmap.Breaks.Add(new BreakPeriod(start, end)); - lineSupportedByEncoder = true; - break; + break; + } } if (!lineSupportedByEncoder) From f0eef329139f0dbe25fb0aa5ee534b33386f466f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 1 May 2024 15:21:39 +0200 Subject: [PATCH 5/5] Fix code quality inspection --- osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs index 3ecc29bd02..6fa78fa8e6 100644 --- a/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs +++ b/osu.Game/Beatmaps/Formats/LegacyBeatmapDecoder.cs @@ -415,7 +415,6 @@ namespace osu.Game.Beatmaps.Formats { string[] split = line.Split(','); - // Until we have full storyboard encoder coverage, let's track any lines which aren't handled // and store them to a temporary location such that they aren't lost on editor save / export. bool lineSupportedByEncoder = false;