Skip to content
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

Adjust type for total score fields #158

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

nanaya
Copy link
Contributor

@nanaya nanaya commented Nov 27, 2024

Seems working. At least for the index schema creation, no idea if it fixes the query problem ("failed to create query: Value [3552068564] is out of range for an integer").

Resolves #157.

@peppy
Copy link
Member

peppy commented Nov 28, 2024

I think this would only need to apply for legacy_total_score.. but maybe we just want to limit the values at the indexer level instead, since the old osu_score_high tables don't support this either (and i hope they won't ever have to and we move forward with lazer in cases where this matters).

@nanaya
Copy link
Contributor Author

nanaya commented Nov 28, 2024

I didn't check the score closer but apparently the one I'm looking at was a cheater score?

I've updated the schema to just legacy score and added pr to limit total score to 2B on osu-web.

@peppy
Copy link
Member

peppy commented Nov 28, 2024

this will require an index rebuild, right? not sure if it's worth it when this has only happened once ever, like we probably want to guard this from reaching ES.

web-10 just doesn't allow scores this high, so it's likely a new component which is.

@nanaya
Copy link
Contributor Author

nanaya commented Nov 28, 2024

I don't know if it ever happened with legacy total score; the ones I saw are lazer scores. If there's a safety check somewhere to prevent legacy total score from hitting the limit then this change isn't needed yeah. I don't think clamping it for indexing makes sense though if the number is legitimate as it would potentially fuck up leaderboard.

It can still be merged and just never applied until next index rebuild anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Total score fields should be bigger
2 participants