ppy / osu-web

the browser-facing portion of osu!
https://osu.ppy.sh
GNU Affero General Public License v3.0
974 stars 380 forks source link

Pinned scores for unloved maps persist #11239

Closed KommToby closed 2 months ago

KommToby commented 4 months ago

Example Profile: https://osu.ppy.sh/users/10609949 Corresponding Score: https://osu.ppy.sh/scores/1637404790

Not sure if this will cause issues as said beatmap has been changed since, and is now qualified for rank.

bdach commented 4 months ago

Currently things are arguably correct, the score is preserved (as it should be, arguably), and is unranked / doesn't give pp.

The biggest problem that I see is that on rank, things happen to scores:

https://github.com/ppy/osu-web/blob/f417275e8ef125b8573fa53c15467e3f45ba2e3e/app/Models/Beatmapset.php#L640-L643

That says "remove" in both cases but the implementation is really "remove" for old tables and "unrank" for new, as far as I can tell:

https://github.com/ppy/osu-web/blob/f417275e8ef125b8573fa53c15467e3f45ba2e3e/app/Jobs/RemoveBeatmapsetBestScores.php#L55-L81 https://github.com/ppy/osu-web/blob/f417275e8ef125b8573fa53c15467e3f45ba2e3e/app/Jobs/RemoveBeatmapsetSoloScores.php#L69-L75

So I can imagine things breaking down on rank or after rank. Something like "we have this row for a legacy score that was preserved, but that score got actually deleted from the old tables, how is this possible", at which point manual (or semi-manual, via osu-queue-score-statistics commands like verify) corrective action may be taken to clean that up, at which point the pin will presumably break (I believe they're all migrated to solo_scores now?)

@peppy thoughts on how to handle?

peppy commented 2 months ago

Hmm, so the part which can break is the verify command? And basically we don't want to remove rows from the new table where they are not present in the old table?

Would a rational solution be to turn off the deletion flow in the verify command by default? Or at least check for pins first?

nanaya commented 2 months ago

There are only few areas which osu-web queries old score tables now

bdach commented 2 months ago

Or at least check for pins first?

This is probably the thing to do here yeah.