Merge pull request #28033 from peppy/preserve-storyboard

Preserve storyboard events when saving a beatmap in the editor

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;
 
@@ -37,6 +38,22 @@ namespace osu.Game.Tests.Beatmaps.Formats
 
         private static IEnumerable<string> 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);
+            Assert.That(decoded.beatmap.UnhandledEventLines.Count, Is.EqualTo(1));
+            Assert.That(decoded.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))]
         public void TestEncodeDecodeStability(string name)
         {

osu.Game/Beatmaps/Beatmap.cs

@@ -63,6 +63,8 @@ namespace osu.Game.Beatmaps
 
         public List<BreakPeriod> Breaks { get; set; } = new List<BreakPeriod>();
 
+        public List<string> UnhandledEventLines { get; set; } = new List<string>();
+
         [JsonIgnore]
         public double TotalBreakTime => Breaks.Sum(b => b.Duration);
 

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;
         }

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)
@@ -417,43 +415,57 @@ 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]);
-                    break;
+                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;
+                        }
 
-                case LegacyEventType.Video:
-                    string filename = CleanFilename(split[2]);
+                        break;
 
-                    // 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;
-                    }
+                    case LegacyEventType.Video:
+                        string filename = CleanFilename(split[2]);
 
-                    break;
+                        // 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;
+                        }
 
-                case LegacyEventType.Background:
-                    beatmap.BeatmapInfo.Metadata.BackgroundFile = CleanFilename(split[2]);
-                    break;
+                        break;
 
-                case LegacyEventType.Break:
-                    double start = getOffsetTime(Parsing.ParseDouble(split[1]));
-                    double end = Math.Max(start, getOffsetTime(Parsing.ParseDouble(split[2])));
+                    case LegacyEventType.Background:
+                        beatmap.BeatmapInfo.Metadata.BackgroundFile = CleanFilename(split[2]);
+                        lineSupportedByEncoder = true;
+                        break;
 
-                    beatmap.Breaks.Add(new BreakPeriod(start, end));
-                    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;
+                }
             }
+
+            if (!lineSupportedByEncoder)
+                beatmap.UnhandledEventLines.Add(line);
         }
 
         private void handleTimingPoint(string line)

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)

osu.Game/Beatmaps/IBeatmap.cs

@@ -42,6 +42,12 @@ namespace osu.Game.Beatmaps
         /// </summary>
         List<BreakPeriod> Breaks { get; }
 
+        /// <summary>
+        /// 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.
+        /// </summary>
+        List<string> UnhandledEventLines { get; }
+
         /// <summary>
         /// Total amount of break time in the beatmap.
         /// </summary>

osu.Game/Rulesets/Difficulty/DifficultyCalculator.cs

@@ -330,6 +330,8 @@ namespace osu.Game.Rulesets.Difficulty
             }
 
             public List<BreakPeriod> Breaks => baseBeatmap.Breaks;
+            public List<string> UnhandledEventLines => baseBeatmap.UnhandledEventLines;
+
             public double TotalBreakTime => baseBeatmap.TotalBreakTime;
             public IEnumerable<BeatmapStatistic> GetStatistics() => baseBeatmap.GetStatistics();
             public double GetMostCommonBeatLength() => baseBeatmap.GetMostCommonBeatLength();

osu.Game/Screens/Edit/EditorBeatmap.cs

@@ -174,6 +174,8 @@ namespace osu.Game.Screens.Edit
 
         public List<BreakPeriod> Breaks => PlayableBeatmap.Breaks;
 
+        public List<string> UnhandledEventLines => PlayableBeatmap.UnhandledEventLines;
+
         public double TotalBreakTime => PlayableBeatmap.TotalBreakTime;
 
         public IReadOnlyList<HitObject> HitObjects => PlayableBeatmap.HitObjects;

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;