search

ppy / osu

rhythm is just a *click* away!
https://osu.ppy.sh
MIT License
15.24k stars 2.27k forks source link

Maps with slashes in metadata are exported incorrectly #14758

Closed Wieku closed 2 years ago

Wieku commented 3 years ago

Describe the bug: When a map contains slash character anywhere in artist/title/difficulty metadata, after exporting it, it's unimportable in stable. It looks like slashes aren't sanitized so .osu files end up being in subfolders.

Example maps that fail to import after exporting in lazer: https://osu.ppy.sh/beatmapsets/423194#osu/914570 https://osu.ppy.sh/beatmapsets/855677#osu/1787848

Screenshots or videos showing encountered issue: image image

osu!lazer version: 2021.907.0-lazer

Logs: not needed

Youssef-Hammad commented 3 years ago

Can you tell me more about how you recreate the issue? I downloaded https://osu.ppy.sh/beatmapsets/423194#osu/914570, imported it into osu!lazer, exported it, imported the export in osu! and it loads fine. Tried that with the latest release of lazer and with master with this as the last commit https://github.com/Youssef-Hammad/osu/commit/90f1592d151229b0fa3f7bdc22662bea5ba554ce

bdach commented 3 years ago

@Youssef-Hammad please re-read the opening post. the reproduction scenario is described quite clearly.

the cause is the exportBeatmap() call re-saving the beatmap:

https://github.com/ppy/osu/blob/801bee7c47e280555261016ba82edc8743653fe9/osu.Game/Screens/Edit/Editor.cs#L704-L708

which ends up changing the filename of the .osu file and making it so that it contains slashes:

https://github.com/ppy/osu/blob/801bee7c47e280555261016ba82edc8743653fe9/osu.Game/Beatmaps/BeatmapManager.cs#L269-L270

as could be seen on the original (if renamed to .zip and previewed/extracted) stable looks to sanitize slashes from difficulty names when creating .osz archives. the way to proceed here is probably to mirror that.

Wieku commented 3 years ago

Additional note, looking at that snippet @bdach shared it seems that no Windows incompatible characters are sanitized, so maps containing \, <, >, |, ?, *, " and : will fail to import/unpack as well.

Youssef-Hammad commented 3 years ago

@Wieku Thanks for the note, I think I got it working for the / then I saw your comment, will add the rest of the characters and make a PR

peppy commented 2 years ago

This is a pretty important one, but I'm going to hold off fixing it until we've switched to the realm importer, to save fixing it potentially in two different implementations.