Nobody really paid attention to the lobby window, and it completely
missed its purpose. Most people don't even wait for companies to
show up, but just hit "New Company".
This in turn means people create a lot of unneeded companies, while
they "just want to watch the game" or join another company.
Instead, "Join Game" now just joins the game as spectators.
Soon we will make "join game" join the game as spectator first,
so limiting the amount of spectators makes no sense anymore in
that context. Not sure it ever did make sense.
"my_client" wasn't always free'd when a game ended. "my_client"
keeps a reference inside the PT_NCLIENT pool. The rest of the
code assumes that when you are not in a game, it can freely
reset this pool.
In result: several ways to trigger a use-after-free.
TURN is a last resort, used only if all other methods failed.
TURN is a relay approach to connect client and server together, where
openttd.org (by default) is the middleman.
It is very unlikely either the client or server cannot connect to
the STUN server, as they are both already connected to the Game
Coordinator. But in the odd case it does fail, estabilishing the
connection fails without any further possibility to recover.
Before 8a2da49 the NewGRF names were synchronized using UDP packets, however
those have been removed. With this a new version of the GameInfo packet is
introduced that allows to specify the type of serialisation happens for
NewGRFs. Either only the GRF ID and checksum, or those two plus the name of
the NewGRF.
On this request for local servers will send the NewGRFs names.
The Game Coordinator will get the names on the first registration, and after
that only the GRF ID and checksum.
These were filled with "<Unknown>" (before 8a2da49) and later their name would get filled via UDP requests to the server. These UDP packets do not exist anymore, so they will always remain "<Unknown>".
Remove that logic and just use the generic translated error GRF UNKNOWN string instead.
This method doesn't require port-forwarding to be used, and works for
most common NAT routers in home setups. But, for sure it doesn't work
for all setups, and not everyone will be able to use this.
This removes the need to know a server IP to join it. Invite codes
are small (~7 characters) indentifiers for servers, which can be
exchanged with other players to join the servers.
Normally TCPConnecter will do a DNS resolving of the connection_string
and connect to it. But for SERVER_ADDRESS_INVITE_CODE this is different:
the Game Coordinator does the "resolving".
This means we need to allow TCPConnecter to not setup a connection
and allow it to be told when a connection has been setup by an external
(to TCPConnecter) part of the code. We do this by telling the (active)
socket for the connection.
This means the rest of the code doesn't need to know the TCPConnecter
is not doing a simple resolve+connect. The rest of the code only
cares the connection is established; not how it was established.
OTTD_COORDINATOR_CS for the game coordinator defaults to coordinator.openttd.org:3976
OTTD_CONTENT_SERVER_CS for the content server defaults to content.openttd.org:3978
OTTD_CONTENT_MIRROR_CS for the content mirror server defaults to binaries.openttd.org:80
* Codechange: [Network] split CloseSocket and CloseConnection more clearly
- CloseSocket now closes the actual OS socket.
- CloseConnection frees up the resources to just before CloseSocket.
- dtors call CloseSocket / CloseConnection where needed.
In the destructors of many of the network related classes Close() is called, just like the
top class in that hierarchy. However, due to virtual functions getting resolved statically
in the destructor it would always call the empty Close() of the top class.
Document the other cases where a virtual call is resolved statically.
It now follows very simple rules:
0 - Fatal, user should know about this
1 - Error, but we are recovering
2 - Warning, wrong but okay if you don't know
3 - Info, information you might care about
4 -
5 - Debug #1 - High level debug messages
6 - Debug #2 - Low level debug messages
7 - Trace information
The code mixed up "client has quit but we already told everyone"
with "client lost connection, handle this".
Split up those two signals:
- CLIENT_QUIT means we told everyone and the connection is now dead
- CONNECTION_LIST means we should tell everyone we lost a client
The most common case never needs access to it anymore. Make the
one exception to this explicit. This means the fact that we
store it is now an implementation detail.
Hostnames like "content.openttd.org" resolve into multiple IPv4 and IPv6.
It is possible that either of the IPs is not working, either due to
a poorly configured OS (having IPv6 but no valid route), broken network
paths, or a service that is temporary unavailable.
Instead of trying the IPs one by one, waiting for a 3s timeout between
each, be a bit more like browsers, and stack attempts on top of each
other with slight delays. This is called Happy Eyebells.
Initially, try the first IPv6 address. If within 250ms there is no
connection yet, try the first IPv4 address. 250ms later, try the
second IPv6 address, etc, till all addresses are tried.
If any connection is created, abort all the other (pending) connections
and use the one that is created. If all fail 3s after the last connect(),
trigger a timeout for all.
We now resolve the connection_string to a NetworkAddress in a much
later state. This means there are fewer places constructing a NetworkAddress.
The main benefit of this is in later PRs that introduce different types
of NetworkAddresses. Storing this in things like NetworkGameList is
rather complex, especially as NetworkAddress has to be mutable at all
times.
Additionally, the NetworkAddress is a complex object to store simple
information: how to connect to this server.
Split the updating in a "static" version that only needs to be called when a new map is loaded or some settings are changed, and a "dynamic" version that updates everything that changes regularly such as the current game date or the number of spectators.
The idea is that if you query an older server that does not support
this packet yet, the client receives an error. The assumption was
that on every "illegal packet" the connection would be closed. This
turns out to be false.
Now CLIENT_GAME_INFO aligns with the old PACKET_CLIENT_NEWGRFS_CHECKED,
which does a pre-check (which fails), and an error is sent back
and the connection is closed.
This is not a nice solution, but it is the best we got.
Currently we use default OS timeout for TCP connections, which
is around 30s. 99% of the users will never notice this, but there
are a few cases where this is an issue:
- If you have a broken IPv6 connection, using Content Service is
first tried over IPv6. Only after 30s it times out and tries
IPv4. Nobody is waiting for that 30s.
- Upcoming STUN support has several methods of establishing a
connection between client and server. This requires feedback
from connect() to know if any method worked (they have to be
tried one by one). With 30s, this would take a very long time.
What is good to mention, is that there is no good value here. Any
value will have edge-cases where the experience is suboptimal. But
with 3s we support most of the stable connections, and if it fails,
the user can just retry. On the other side of the spectrum, with 30s,
it means the user has no possibility to use the service. So worst case
we annoy a few users with them having the retry vs annoying a few
users which have no means of resolving the situation.
Strictly seen the comment is true, as it says 'e.g.', but it is
misleading. The server name is just that: the name of the server
as configured. No need to mention advertising.
When ever you saw this debug lines (which you never should), they
showed an empty address. It is also not very useful to have, as it
always points to a known server anyway.
Patric Stout
Rubidium
milek7
rubidium42
Milek7
Charles Pigott