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

Client versions need to be checked on connect #52

Open
peppy opened this issue Feb 9, 2021 · 3 comments
Open

Client versions need to be checked on connect #52

peppy opened this issue Feb 9, 2021 · 3 comments
Assignees

Comments

@peppy
Copy link
Member

peppy commented Feb 9, 2021

We need a way to stop older clients from connecting. This is already possible on score token retrieval (via checks osu-web side) but we don't have a flow in place for this server.

See ppy/osu#11686 for an example of an issue caused by old versions connecting to the server.

@bdach
Copy link
Collaborator

bdach commented Feb 28, 2024

Rough sketch of server-side changes: https://github.com/ppy/osu-server-spectator/compare/master...bdach:osu-server-spectator:client-version-check-wip?expand=1

Needs further client-side consideration though.

@smoogipoo
Copy link
Contributor

Can you explain what client-side considerations are needed?

@bdach
Copy link
Collaborator

bdach commented Mar 4, 2024

Well the issue there is how the client is supposed to respond to an invalid version being detected by the spectator server. What the spectator server will do is obvious - it will drop the connections from an old client and/or not honor any requests made by it. But the client?

  • It can either do the same thing as it does with the concurrency limiter, i.e. just log the user out. This is the easy option, but it is also arguably anti-user (there's no reason why the user can't continue to play without submission etc.), and will arguably mess with our workflows a little bit in rare cases (sometimes you need to check something using real production data, and as such you will generally launch the game in debug but using production endpoints - and implementing this behaviour would either put a stop to that entirely or require another local modification to be feasible).

  • Or it can try and keep the user logged in but carefully don't let them do anything that touches multiplayer and spectator (which includes score submission too, by the way). This is the pro-user approach but also very annoying to happen since you basically create this half-logged in state and performing the proper messaging to convey this to the user is really finicky.

I'd have to change the disconnection flow used currently to convey the reason for disconnection, then handle that on the client side properly - which is also triply annoying because we have three hubs and every single one of them will receive a disconnect event near-simultaneously so I'd have to do some client-side debouncing etc etc.

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

No branches or pull requests

3 participants