Introduce new version of spectator API with explicit score token passing#523
Draft
bdach wants to merge 9 commits into
Draft
Introduce new version of spectator API with explicit score token passing#523bdach wants to merge 9 commits into
bdach wants to merge 9 commits into
Conversation
One continuing sticking point in attempts to make online flows more reliable is the serialising factor of spectator server in one part of submission, namely replay saving. Up until now, the active score token ID was part of `SpectatorClientState`, and relied on as ambient server-side state to make `SendFrameData()` and `EndPlaying()` calls work. However, only being able to store *one* token ID means that the client cannot move on to start a new play without concluding the last one, which is a problem, as the user essentially cannot start a new play until they're sure the old one concluded. This PR attempts to improve this bottleneck by allowing clients to send spectator data out-of-order. The general rules are as follows: - The user can have up to `MAX_STARTED_SCORES = 5` scores started at any given time; their tokens are stored to the state. - All methods now explicitly accept a score token. `null` score tokens are generally ignored for the replay pathways, instead only updating the `SpectatorClientState`. - Clients are now allowed to interleave `SendFrameDataV2()` and `EndPlayingV2()` calls from multiple scores. In cases of those calls, the ambient `SpectatorClientState` is only updated if the score token supplied is `null` or equal to the latest non-`null` score token stored to the state.
Most of the data that was sent was redundant; the only real thing that needs to be sent across is `SpectatedUserState`. This helps client-side as there is no longer any need to keep around full states from non-latest started scores.
The client relies on this server-side to make reconnection work: https://github.com/ppy/osu/blob/57dcc3e95afbd437c0b1d637fc90695caf39b980/osu.Game/Online/Spectator/OnlineSpectatorClient.cs#L75
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
RFC.
Related:
Compatibility matrix:
One continuing sticking point in attempts to make online flows more reliable is the serialising factor of spectator server in one part of submission, namely replay saving.
Up until now, the active score token ID was part of
SpectatorClientState, and relied on as ambient server-side state to makeSendFrameData()andEndPlaying()calls work.However, only being able to store one token ID means that the client cannot move on to start a new play without concluding the last one, which is a problem, as the user essentially cannot start a new play until they're sure the old one concluded. This is a large pain point for users with unreliable network connections.
This PR attempts to improve this bottleneck by allowing clients to send spectator data out-of-order. The general rules are as follows:
MAX_STARTED_SCORES = 5scores started at any given time; their tokens are stored to the state. (Value of constant up for negotiation.)nullscore tokens are generally ignored for the replay code paths, instead only updating theSpectatorClientState.BeginPlayingV2()call will always overwrite properties onSpectatorClientStatethat are notScoreTokens.SendFrameDataV2()andEndPlayingV2()calls from multiple scores. In cases of those calls, the ambientSpectatorClientStateis only updated if the score token supplied isnullor equal to the latest non-nullscore token stored to the state.Listening to suggestions with respect to collection of metrics (should the total number of scores started be tracked to a counter?)
Footnotes
Old client continues to use the old version of the API which did not functionally change and is not aware of the existence of the new API. Spectating and replay upload operational. ↩ ↩2
New client will attempt to use the new version of the API which does not exist on old server. Complete failure of spectator and replay upload flows for all users affected. ↩
New client uses the new version of the API which exists on new server. Spectating and replay upload operational. ↩