-
Notifications
You must be signed in to change notification settings - Fork 133
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
feat: add score status recalculation to recalc.py #573
base: master
Are you sure you want to change the base?
Conversation
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Could you link documentation for this? Py docs still show I find it kinda unlikely python would make a breaking change for this, but perhaps? |
I was looking at https://docs.python.org/3/library/argparse.html, which seems to have been correct for me. |
description
attribute error & add score status recalculation to recalc.py
merging this can make fake data work. Am i correct? |
Fake data works without this. What you are trying to do is just not what you use fake data for. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
appreciate this is a super late review, but only one general issue i see with this. would be good if this could be addressed, i was reminded about this PR following the akatsuki-pp-py bump 😅
await ctx.database.execute( | ||
"UPDATE scores SET status = 1 " | ||
"WHERE map_md5 = :map_md5 AND userid = :userid AND mode = :mode AND status > 0", | ||
pair, | ||
) | ||
|
||
await ctx.database.execute( | ||
"UPDATE scores SET status = 2 WHERE id = :id", | ||
{"id": best["id"]}, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ideally this runs in a transaction, if an API request comes in inbetween these 2 queries or something then they're going to have no best on the map.
additionally, for no reason other than being picky, would be good to add a AND id != :best_id
on the first query just so that it doesn't get unnecessarily touched until the following query
Describe your changes
Added a third re-calculation that re-evaluates the status of all non-failed scores (
status>0
)The new re-calculation is important to perform when PP is being updated, but might also be desired to run isolated from PP re-calculation.
For all (map_md5, userid, mode) pairs it'll find the best score of the user (which is determined by PP in bpy) and sets the status to 2. The status of all other non-failed scores on that map are set to 1.
Discussing necessary before merge
The re-calculation of statuses is necessary since after the update of PP a score which was previously considered the best score by a user on a map might no longer be the best and replaced by a different, simply submitted (
status=1
) score.In order to account for this, we would need to not only re-calculate all
status=2
scores, but also allstatus=1
scores. This would introduce a significantly higher time consumption for running the script.For my 12-day old server, I currently have 54,346 scores with
status=2
and 73,475 scores withstatus>=1
, out of 305,761 total scores. That's an increase of around 35%, but will increase as time goes on as users will start playing the same maps more causing more submitted, but not best scores.Checklist