-
Notifications
You must be signed in to change notification settings - Fork 385
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
Include user score position when indexing playlist item scores #11354
Conversation
ScoreTransformer::MULTIPLAYER_BASE_INCLUDES | ||
[ | ||
...ScoreTransformer::MULTIPLAYER_BASE_INCLUDES, | ||
'position', |
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.
I can maybe skip this one and hardcode a 1..50 position client-side if it helps. (Does mean we would require a client deploy for daily challenge to go live though.)
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.
yeah this will cause as many additional queries as there are scores
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.
Have force-pushed this out.
ppy/osu#29036 would be the client-side solution.
Needed for daily challenge (without this client can't show position numbers for the user score on the leaderboard).
0ec9c56
to
d77b27b
Compare
Only doing this client-side, because doing this server-side is expensive: ppy/osu-web#11354 (comment)
Needed for daily challenge (without this client can't show position numbers for the user score on the leaderboard).
A second review for performance implications is probably best-advised, this monkey just fix no number showing with hammer. I tried to ask once but got no reply.