/ OpenTTD Public
Change: no longer use UDP when entering the lobby of a server #9114
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.
Motivation / Problem
With STUN in mind, it is a lot easier if we have a single protocol to talk between client and server. This was mostly true, except for a part in the lobby window.
Most likely to avoid code-duplication (random guess honestly), opening the lobby does two requests: one via UDP to get the latest game-info, and one via TCP to get the latest company-info. This PR sets out to change that both to TCP.
Accidentally, this solves a rare edge-case, where someone create a new company on a server, TCP reacts with the company-list, but the UDP packet is lost. In this case, the new company is not shown. Only until the UDP packet is received, it will be shown correctly. By using TCP here over UDP, we ensure this isn't possible.
The first commit moves a lot of code related to NetworkGameInfo to a single place, so adding GameInfo to TCP (next to UDP) doesn't cause code duplication.
The second commit adds a packet to the TCP to exchange GameInfo over TCP too.
Sadly, I wasn't a very smart cookie when I wrote the network protocol, and over TCP we do not handshake a protocol-version before we initiate conversation. This means that there are potential (future) issues with this PR. Currently this isn't an issue, but just to walk through the situations:
New client + new server:
Old client + new server / new client + old server:
-nto join a server bypasses lobby, and as such works fine too (well, it hits revision mismatch if that is the case, ofc)
Currently "Add Server" still uses UDP to query the server. A future PR will change this to also use TCP here (only for manual servers and manual refresh). This does mean that adding older servers will result in a protocol error, as older server don't know this new packet. In such future PR, the client will pick up on this and show a nice "Server is too old" text.
It would help to backport this to 1.11. It would mean that when 1.12 releases, the 1.11.N can be added manually and show the game info (and that the version is incompatible). This would mean for most of our servers there isn't a real problem, and it is only an issue if people add really old servers manually.
This is also the reason that this future PR is not included in this PR.
Additionally, the last two commits from this PR don't have to be backported. It makes use of this new TCP packet; without it nothing breaks, and means there is no functional difference after backporting. But it is better to play it safe, so do not backport the last two commits please.
Additionally, if this is backported, so should #9129, as it fixes packet order.
None, this should be fully backports compatible. The GUI prevents opening the lobby for old servers, and UDP is used for anything before that.
Checklist for review
Some things are not automated, and forgotten often. This list is a reminder for the reviewers.