Skip to content

Update the last played date of a beatmap when importing a replay by the local user #25303

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 4 commits into from
Oct 30, 2023

Conversation

peppy
Copy link
Member

@peppy peppy commented Oct 30, 2023

Improves the situation with #25296.


With this change, importing beatmaps scores from stable will now choose a more correct "Last Played" date for beatmaps based on the most recent score imported.

This also applies when importing one of your own scores at any point.

@peppy peppy added area:import dealing with importing of data from stable or file formats and removed area:scoring labels Oct 30, 2023
Comment on lines +188 to +189
// This needs to be run after user detail population to ensure we have a valid user id.
if (api.IsLoggedIn && api.LocalUser.Value.OnlineID == model.UserID && (model.BeatmapInfo.LastPlayed == null || model.Date > model.BeatmapInfo.LastPlayed))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess this is accepting that due to how the replay format works, username changes can potentially cause the score to be misattributed to the local user and thus also change the last played date?

I guess it's not a huge deal but I do wanna ask to make sure we want to keep using this piece of potentially-unreliable data for more stuff.

Copy link
Member Author

@peppy peppy Oct 30, 2023

Choose a reason for hiding this comment

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

There's an assumption the flow here will be replaced / get more reliable going forward. Username changes are an edge case that will affect 1 in 100,000

@pull-request-size pull-request-size bot added size/L and removed size/M labels Oct 30, 2023
@bdach bdach enabled auto-merge October 30, 2023 08:29
@bdach bdach disabled auto-merge October 30, 2023 09:12
@bdach bdach merged commit 2dc2469 into ppy:master Oct 30, 2023
@peppy peppy deleted the last-played-import-score-update branch November 1, 2023 03:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:import dealing with importing of data from stable or file formats size/L type:behavioural
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants