Skip to content

Commit d89d6ed

Browse files
author
Bartłomiej Dach
authored
Fix legacy beatmap export dropping background specification (ppy#37892)
- Closes ppy#37884 - Closes ppy#37890 Due to lack of population of `Storyboard.Beatmap` and `Storyboard.BeatmapInfo` post-decoding, `LegacyBeatmapExporter` would completely drop background specifications on exported beatmap packages. This affects both direct legacy export to file (`.osz`) as well as beatmap submission. I will not pretend that the API here is optimal but I do not see very easy opportunities to curtail misuse. Storyboards can be treated as either parts of a beatmap or standalone entities, and if a requirement is added to forcibly provide a beatmap and its info when encoding out a storyboard, I also foresee a requirement to bypass this later when design mode is implemented, which would be a return to square one. There is likely room for cleanup around `Storyboard` to maybe make this nicer (remove passing of both `Beatmap` and `BeatmapInfo` and just pass `Beatmap` instead, maybe shuffle some properties from `Beatmap` to `Storyboard` to remove the requirement of having to bolt the beatmap on to begin with). I leave voicing opinions on that, and how soon that should be done, to reviewers. My primary intent at this time is to hotfix a major issue in a released build. The external editing feature is not involved in this bug and any attempts to claim so are misdirections.
1 parent 840d37e commit d89d6ed

3 files changed

Lines changed: 26 additions & 1 deletion

File tree

osu.Game.Tests/Beatmaps/IO/LegacyBeatmapExporterTest.cs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,29 @@ public void TestFractionalObjectCoordinatesRounded()
122122
() => Is.EqualTo(384).Within(0.00001));
123123
}
124124

125+
[Test]
126+
public void TestBackgroundSpecificationPreserved()
127+
{
128+
IWorkingBeatmap beatmap = null!;
129+
MemoryStream outStream = null!;
130+
131+
// Ensure importer encoding is correct
132+
AddStep("import beatmap", () => beatmap = importBeatmapFromArchives(@"241526 Soleily - Renatus.osz"));
133+
AddAssert("beatmap background is correct", () => beatmap.BeatmapInfo.Metadata.BackgroundFile, () => Is.EqualTo("machinetop_background.jpg"));
134+
135+
// Ensure exporter legacy conversion is correct
136+
AddStep("export", () =>
137+
{
138+
outStream = new MemoryStream();
139+
140+
new LegacyBeatmapExporter(LocalStorage)
141+
.ExportToStream((BeatmapSetInfo)beatmap.BeatmapInfo.BeatmapSet!, outStream, null);
142+
});
143+
144+
AddStep("import beatmap again", () => beatmap = importBeatmapFromStream(outStream));
145+
AddAssert("beatmap background is still correct", () => beatmap.BeatmapInfo.Metadata.BackgroundFile, () => Is.EqualTo("machinetop_background.jpg"));
146+
}
147+
125148
[Test]
126149
public void TestExportStability()
127150
{

osu.Game/Database/LegacyBeatmapExporter.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,8 @@ public LegacyBeatmapExporter(Storage storage)
7474

7575
using var storyboardStreamReader = new LineBufferedReader(storyboardStream);
7676
var beatmapStoryboard = new LegacyStoryboardDecoder().Decode(storyboardStreamReader);
77+
beatmapStoryboard.Beatmap = beatmapContent;
78+
beatmapStoryboard.BeatmapInfo = beatmapInfo;
7779

7880
MutateBeatmap(model, playableBeatmap);
7981

osu.Game/Storyboards/Storyboard.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ public class Storyboard
1818
private readonly Dictionary<string, StoryboardLayer> layers = new Dictionary<string, StoryboardLayer>();
1919
public IEnumerable<StoryboardLayer> Layers => layers.Values;
2020

21-
public BeatmapInfo BeatmapInfo = new BeatmapInfo();
21+
public BeatmapInfo BeatmapInfo { get; set; } = new BeatmapInfo();
2222
public IBeatmap Beatmap { get; set; } = new Beatmap();
2323

2424
/// <summary>

0 commit comments

Comments
 (0)