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

Jump to bottom

Disconnecting clients affect other clients in the queue #8751

Closed
ldpl opened this issue Feb 26, 2021 · 5 comments · Fixed by #8755
Closed

Disconnecting clients affect other clients in the queue #8751

ldpl opened this issue Feb 26, 2021 · 5 comments · Fixed by #8755
Milestone
1.11.0

Comments

@ldpl
Copy link
Contributor

ldpl commented Feb 26, 2021 •
edited

Version of OpenTTD

master (af32675), 1.10.3, 1.9.3

Expected result

Client connects without issues regardless of what other clients do.

Actual result

Client loses connection

Steps to reproduce

  1. Start a network server. (larger map size and no indusries will make testing easier)
  2. Start two clients and select that server
  3. Join the server on both clients at about the same time
  4. Cancel first client while it is downloading the map
  5. Second client will fail to connect

There is also a variation of this bug where second client gets desync. Happens when first client connects with "new company" and leaves the server before the second one finishes downloading the map.

P.S. it also says 0 clients in front of you while waiting
@JGRennison
Copy link
Contributor

It appears that clients only transition out of STATUS_MAP_WAIT in the success path of ServerNetworkGameSocketHandler::SendMap of the client in the STATUS_MAP phase.
Also doing that check in the failure paths ought to make that go away.

The 0 clients thing just looks like an off by one error in ServerNetworkGameSocketHandler::SendWait(), probably the initial value should be 1.

@ldpl
Copy link
Contributor Author

ldpl commented Feb 26, 2021 •
edited

And desync happens because clients are not queued so by the time second client executes COMPANY_CTRL first client is already gone and company is not created. So it may actually need to be moved to a separate bug as it can likely cause issues not just for connecting players but in other cases as well (e.g. client lag).

@JGRennison
Copy link
Contributor

That seems like a misdesign of CmdCompanyCtrl really. It shouldn't be the job of clients to synchronously track the states of other clients.
Probably the client ID check could be just removed entirely. It looks like everything would be fine if ci is nullptr.

@TrueBrain
Copy link
Member

P.S. it also says 0 clients in front of you while waiting

Linguistic this is correct. There is nobody waiting in front of you :P

An additional bug: if the client in SEND_MAP is taking for-ever, everyone in the queue gets a notification that the server is not responding. There is no ping/pong while waiting, I guess.

@TrueBrain TrueBrain mentioned this issue Feb 27, 2021
Merged
@TrueBrain TrueBrain added this to the 1.11.0 milestone Feb 27, 2021
@ldpl
Copy link
Contributor Author

ldpl commented Feb 27, 2021

Suddenly remembered, this desync may actually be the one I was looking for in #8093

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.11.0
Development

Successfully merging a pull request may close this issue.

Fix: several bugs related to joining a network server TrueBrain/OpenTTD
3 participants
@ldpl @TrueBrain @JGRennison