Skip to content

Change lazer's valid filename method to match stable #33579

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 9, 2025

Conversation

peppy
Copy link
Member

@peppy peppy commented Jun 9, 2025

On revisiting the issue at hand, this honestly seems like the best way forward. It also addresses my concerns that with the method we were using, filenames could end up being half underscores.

The main reason for choosing to change the lazer end is that stable's difficulty update process is based on sending the beatmap's filename to the server. This means that if we use a proposed fix of checking online ID, it will still mean beatmaps cannot be updated on stable (for all users which have downloaded the beatmap) if a mapper updates once on lazer.

Implementation inspired by:

On revisiting the issue at hand, this honestly seems like the best way
forward. It also addresses my concerns that with the method we were
using, filenames could end up being half underscores.

The main reason for choosing to change the lazer end is that stable's
difficulty update process is based on sending the beatmap's filename to
the server. This means that if we use a proposed fix of checking online
ID, it will still mean beatmaps cannot be updated on stable (for all
users which have downloaded the beatmap) if a mapper updates once on
lazer.

Implementation inspired by:

- https://referencesource.microsoft.com/#mscorlib/system/io/path.cs,1144ad3c4eff3f24
- https://github.com/peppy/osu-stable-reference/blob/996648fba06baf4e7d2e0b248959399444017895/osu!/GameplayElements/Beatmaps/Beatmap.cs#L1575-L1590

Closes ppy#33060.
@peppy peppy requested a review from bdach June 9, 2025 10:44
@bdach
Copy link
Collaborator

bdach commented Jun 9, 2025

This means that if we use a proposed fix of checking online ID, it will still mean beatmaps cannot be updated on stable (for all users which have downloaded the beatmap) if a mapper updates once on lazer.

I'm not opposed to the direction but I don't understand this part.

@bdach
Copy link
Collaborator

bdach commented Jun 9, 2025

I'm just going to buy this diff wholesale because it fixes issues like #33421, which I can claim because @wuaht is the first of many people who actually produced coherent reproduction steps other than "I didn't do nothing".

The reason that this fixes such issues is that preserving the filename matters for lazer too, because if a beatmap made in lazer is downloaded to stable, then modified there, saved, and exported, all of the filenames get rewritten to stable's convention. Then, on re-importing that export back to lazer, lazer only checks the md5 and the filename of the map. The id is not checked because mappers have been known to do broken crap like reuse online IDs to fix submission failures which means the online ID cannot be trusted for anything.

Due to the above, changing the filename convention causes lazer to no longer associate the reimport with the online beatmap, therefore effectively making it a beatmap set with no status but actual beatmap ID inside, which is when all that "encountered id not assigned by server" crap starts and the rest follows.

Jesus wept.

@pull-request-size pull-request-size bot added size/M and removed size/S labels Jun 9, 2025
Copy link
Collaborator

@bdach bdach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've pushed an xmldoc rewording in 257fec8, please make sure you're ok with it.

@peppy
Copy link
Member Author

peppy commented Jun 9, 2025

Sorry about that, missed the xmldoc. Looks good 👍 .

@bdach bdach merged commit d7d2fc8 into ppy:master Jun 9, 2025
8 of 10 checks passed
@peppy peppy deleted the fix-beatmap-save-filename-discrepancy branch June 11, 2025 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants