Skip to content

Fix disparate score token types being used as one type #185

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

Closed
wants to merge 1 commit into from

Conversation

bdach
Copy link
Collaborator

@bdach bdach commented Sep 1, 2023

Server-side counterpart to ppy/osu#24697. See description of said pull for rationale.

The replay upload flow and the user statistics update flow would rely on
the score token sent by client to spectator server in
`BeginPlaySession()`, in order to unmap the score token to a solo score
ID when processing finishes.

However, spectator server thus far has not been aware that there are
actually _two_ types of token - and to be fair, client gave it no
practical way to actually know this. In solo, the token ID comes from
`solo_score_tokens`, and in multiplayer, the token ID comes from
`multiplayer_score_links`. Spectator server has however been treating
both types as the solo score type. In particular, this could mean that
the replay recording flow would royally confuse things and unmap the
token to a score ID wrong, and as such, upload an entirely misattributed
replay.

To resolve, leverage the updated client contract which now passes a
`ScoreToken` with an explicit type rather than a `long` to do the right
thing for both types of token.
@bdach bdach self-assigned this Sep 5, 2023
bdach added a commit to bdach/osu-infrastructure that referenced this pull request Sep 5, 2023
This PR adds documentation about the multiplayer score submission flow
in the form of a sequence diagram. It was primarily created by taking
the existing solo score submission flow sequence diagram and adapting
it to the differences specific to the multi flow (different endpoints,
different tables, different ID schemes).

This diagram does not describe the current state of things. It describes
where things _should_ be after a sequence of some already-PRed changes
such as:

- ppy/osu-web#10437
- ppy/osu#24697
- ppy/osu-server-spectator#185

and some changes that are still being worked on (primarily client-side
at this point).

The main goal of the document is to inform the changes above. (And to be
something that I can easily refer to, because I keep looking up the same
information over and over right now.)
@peppy
Copy link
Member

peppy commented Oct 16, 2023

Closing for now to keep things clean. See ppy/osu-infrastructure#24 for rationale.

@peppy peppy closed this Oct 16, 2023
@bdach bdach deleted the fix-score-token-confusion branch March 25, 2024 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

2 participants