Feature: play easier together with friends from behind home routers #9017

TrueBrain · 2021-04-11T12:45:05Z

Integrated #9234. Closes #9234.

Test binaries can be found here: https://www.openttd.org/downloads/openttd-branches/pr9017/latest.html
See below how to help out testing.

~~If you build yourself, make sure to name your branch "pr9017". Otherwise your revision will not match anyone using the binaries above.~~

!!NOTE!! Currently the Game Coordinator is not working with this PR, while I rebuild this PR from the ground up. See https://gist.github.com/TrueBrain/75c016cdb90efa18cb79f3f6bae9af25 for details.

Motivation / Problem

Currently, to host a network server you really need to know a few things about routers and networking. This is way to advanced for the average Joe, and this becomes very clear when looking at Steam Discussions.
For Rich Presence (Discord, Steam, ..) we need "invite codes" so people can join each others game.

So, can we do this? I wrote a gist to explain the process of getting to this PR: https://gist.github.com/TrueBrain/f5685dc0fdbd012aac48ea2a36158198

But in case you are not into reading, let me summarize:

Every invite-only / public server creates a persistent TCP connection to a Game Coordinator (replacing the master-server)
Every server gets assigned an unique "join key" (called an "invite code" in the UI)
Server listing returns only the "join key" of a server
A client can join any server based on this "join key" (without even knowing the IP of the server), either via listing or directly inputting it
The Game Coordinator will try to connect the server and client. It tries 3 methods: direct connect, STUN, and if all fails, TURN (see gists for details)

There are now three kind of servers:

Local servers: do not connect to the Game Coordinator. "Search LAN" does work for these servers, as do people with the public IP and if port-forwarding is setup correctly. Just the old "non-advertised" servers.
Invite-only servers: the owner of the server needs to give the invite-code to someone before he can join.
Public servers are the old advertised servers.

Description

See the individual commits and the gist above. Not much more to add.

Limitations

Server info is delayed by 30s (by design). You can refresh individual servers if you want up-to-date information.

Testing

If you want to help out testing, great!

Start the client by downloading it here: https://www.openttd.org/downloads/openttd-branches/pr9017/latest.html (or build it yourself, make sure you name your branch pr9017!)
Start a server, make it public or invite-only
Ask someone to join your server

Notice how you didn't have to setup firewalls, port-forwarding, etc?

Let me know if you find any problems, etc.

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

The bug fix is important enough to be backported? (label: 'backport requested')
This PR affects the save game format? (label 'savegame upgrade')
This PR affects the GS/AI API? (label 'needs review: Script API')
- ai_changelog.hpp, gs_changelog.hpp need updating.
- The compatibility wrappers (compat_*.nut) need updating.
This PR affects the NewGRF API? (label 'needs review: NewGRF')
- newgrf_debug_data.h may need updating.
- PR must be added to API tracker

nielsmh

I didn't read everything in detail, but this doesn't remove local network UDP broadcast query, right?

src/network/network_gui.cpp

TrueBrain · 2021-04-11T20:39:54Z

I didn't read everything in detail, but this doesn't remove local network UDP broadcast query, right?

Local broadcast will remain, yes.

James103 · 2021-04-25T14:19:44Z

Load test Game Controller if it can handle 2k servers and 5k clients

How exactly did you arrive at the load test goals (2,000 servers and 5,000 clients)?

TrueBrain · 2021-04-25T15:48:51Z

Load test Game Controller if it can handle 2k servers and 5k clients

How exactly did you arrive at the load test goals (2,000 servers and 5,000 clients)?

I was sure I wrote that down somewhere .. guess I did not :D

It is a worst-case estimate: on peak on Steam we had 3.2k people online. Say, with a good multiplayer, they all want to play with their friends, round it up, gives an upper value of ~5k.
Assuming most people will just play with their friends, means ~2.5 people per server, so 2k servers.

Infrastructure wise, being able to handle that, means I can sleep peaceful at night.

You think I am understating? Let me know :)

src/network/core/tcp_connect.cpp

src/network/core/address.h

src/network/network_coordinator.cpp

src/network/network_gamelist.cpp

src/network/network_gui.cpp

src/network/core/tcp_coordinator.cpp

src/network/network_coordinator.h

TrueBrain · 2021-05-09T13:38:35Z

This is pretty much done and ready for review. I am 100% sure there are bugs, so lets find them and fix them :)

The Game Coordinator is not yet running on AWS, and is not scalable yet. Otherwise it should do its job. So any client-facing bug you can find, please report that.

My suggestion is to get this PR code-reviewed and merged. After that I will work on the Game Coordinator, to make it production-ready. But that also gives time to test the client itself.

There is plenty of future-work to be done, but this can be found back here:
https://gist.github.com/TrueBrain/d430a5f0e7e4312b895278f092067495

src/network/core/address.cpp

src/network/network_gamelist.cpp

src/network/network.cpp

src/network/network_coordinator.cpp

src/network/core/address.cpp

src/network/core/tcp_stun.h

src/network/network_coordinator.cpp

rubidium42 · 2021-05-09T15:37:12Z

src/network/core/game_info.cpp

+					GRFConfig *c = new GRFConfig();
+					DeserializeGRFIdentifier(p, &c->ident);
+					std::string name = {};
+					if (newgrf_mode == GAME_INFO_NEWGRF_MODE_FULL) {


Do you know the average number of times the same NewGRF is used by different servers? Like NewGRF A is used by 10 servers, NewGRF B by 5, NewGRF C by 1, so the average is about 16/5 ~= 5. That could be used to influence choosing an even more efficient way.
You would then be creating a list of used NewGRFs at the top of the packet with the whole listing, and here you pass indices into that list. Those indices can probably be uint16 but stick with uint32 for now for an added safety buffer. At an average of 2 you would be saving about 16 + NewGRF name length bytes per NewGRF, with an average at 3 you would save about 32 + 2 * NewGRF name length, and so forth.

There are tons of more efficient ways honestly. I liked the suggestion to apply compression on such packets .. that also sound really useful, and maybe means we don't even need a trick like yours. But that needs more work and benchmarking.

For now, I went for a solution that was simple and elegant, rather than complicated and touching a lot of stuff. Future work could optimize this by a lot. But I rather not do that in this PR :) If you agree, ofc.

Btw, this last commit is purely here as otherwise after this PR we would have lost functionality we had before the PR.

glx22 · 2021-05-09T22:17:48Z

Did some testing with a dedicated server and a client.

connection type is "Behind NAT", while a build from master is correctly advertised on master server, so direct connection should work.
clicked "search internet", server is listed, its address is the invite code
clicked "search LAN", server is added with one of the local ipv4
clicked "search LAN" again, server is added with the other local ipv4
after a short time both local ipv4 are removed from the list, and I can see some redrawing issues for the server list too
joined the only left server as spectator, works as expected

I had an issue with a ":3979" server, but I can't reproduce it :(

Edit: I had the IPv6 firewall activated on the router, and it has only 2 modes on or off, no fine tuning. Disabled it to test and connection type became "Public". Build from master was advertising only the IPv4, as IPv6 was blocked by this "better than nothing" firewall.
So to resume:

if server is IPv6 only, connection type is "Behind NAT" if IPv6 firewall is on, "Public" otherwise
if server is IPv4 only, connection type is "Behind NAT" but should be "Public" because direct connection works (can advertise)
if server is IPv4/IPv6, connection type is "Behind NAT" if IPv6 firewall is on, "Public" otherwise

TrueBrain · 2021-05-10T08:28:32Z

* connection type is "Behind NAT", while a build from master is correctly advertised on master server, so direct connection should work.

Yeah, the GC only validates it for the IP you connect with (so IPv6 in your case; the IPv4 won't be detected atm). I will see what I can do about it :) EDIT: fixed!

* clicked "search internet", server is listed, its address is the invite code
* clicked "search LAN", server is added with one of the local ipv4
* clicked "search LAN" again, server is added with the other local ipv4
* after a short time both local ipv4 are removed from the list, and I can see some redrawing issues for the server list too

Yeah, fun bug. LAN servers were not marked as "do not expire", and as such when the server listing via GC came in, it noticed the LAN server was stale (didn't get an update), so it assumed it would be offline. And that removes it :) Fixed now :)

Tnx for testing, much appreciated :)

James103 · 2021-07-01T02:18:56Z

@TrueBrain You mentioned that you load tested the Game Controller and showed that it can handle at least 10k clients. Do you also plan on load testing the TURN server as well, in order to get a good feel for how many active clients and servers it can support so that the network can scale accordingly?

TrueBrain · 2021-07-01T12:27:23Z

@TrueBrain You mentioned that you load tested the Game Controller and showed that it can handle at least 10k clients. Do you also plan on load testing the TURN server as well, in order to get a good feel for how many active clients and servers it can support so that the network can scale accordingly?

Yeah, tested that in one go too! Also the STUN server btw. Seems to all work and seems rather stable. Python and asyncio have a very small overhead per connected client (few KB of RAM, that is it). And CPU is the one thing we aren't short in, so not a real concern. So that all seems to work fine during testing. Of course .. real world can be a different story (stalling connections, very slow ones, etc) .. but I think we fixed most of those issues already with the current master-server (the framework used is identical between the two).

The reason for the 10k btw is that my computer couldn't spawn more clients in a useful way :P

src/network/core/tcp_connect.cpp

src/network/network_coordinator.cpp

src/lang/english.txt

src/network/core/address.cpp

src/network/network_gui.cpp

…w dirty

…dress This allows future extensions to have different ways of referencing a server, instead of forcing to use IP:port.

The Master Server doesn't understand NetworkGameInfo v5, so it is of no use anyway.

This removes all UDP from the game except for a local broadcast to find LAN games.

In this mode you do register to the Game Coordinator, but your server will not show up in the public server listing. You can give your friends the invite code of the server with which they can join.

NewGRFs cannot be changed in multiplayer games, and consumes a lot of bytes. So only send it once to the Game Coordiantor on registration, and skip the field after that for any update. A new game will restart the registration, so any NewGRF change between games is picked up via the new registration. Clients that look at the NewGRFs a server has, want to know the names of the NewGRFs more than the IDs. So in addition to the ID and MD5, send the full name to the clients too. This solution is picked so the Game Coordinator isn't sending the names of all the NewGRFs to the clients constantly (which would make the server-listing very large), but still allow clinets to see the name of a specific server if he clicks the button.

TrueBrain · 2021-07-19T20:22:17Z

Slowly we have been merging parts of this PR into master. #9447 is the last piece. Once that has been merged, everything in this PR (and yes, I checked) has been merged into master too.

By splitting it up, there was a huge jump in quality of what has been delivered. It really was the right call to make everything smaller, although it took significantly more work to do so. Either way, the result is that the backend is running on AWS like the rest of our infra, and that the client/server are doing what they should be .. at least for as far as I could reasonably test.

After #9447 is merged, the testing is up to the public .. and I am sure they will find plenty of edge-cases for us to fix :D But we will tackle them if/when they come in.

For now, I am going to close this PR. Tnx all who contributed, and feel free to test the latest nightly for STUN .. and hopefully soon TURN will hit the nightly too :D

TrueBrain marked this pull request as draft April 11, 2021 12:45

nielsmh reviewed Apr 11, 2021

View reviewed changes

src/network/network_gui.cpp Outdated Show resolved Hide resolved

TrueBrain force-pushed the stun-me branch from 16066d3 to 035f726 Compare April 13, 2021 10:40

TrueBrain force-pushed the stun-me branch from 54b30cd to 8279e23 Compare April 20, 2021 14:05

TrueBrain changed the title ~~WIP: allow multiplayer without port forwarding (using STUN)~~ WIP: allow multiplayer without port forwarding May 5, 2021

TrueBrain force-pushed the stun-me branch 2 times, most recently from 63b5564 to 628266f Compare May 5, 2021 12:55

rubidium42 reviewed May 5, 2021

View reviewed changes

TrueBrain force-pushed the stun-me branch 12 times, most recently from 7fed847 to 09f08bc Compare May 9, 2021 13:32

TrueBrain marked this pull request as ready for review May 9, 2021 13:38

TrueBrain changed the title ~~WIP: allow multiplayer without port forwarding~~ Feature: multiplayer without port forwarding May 9, 2021

rubidium42 reviewed May 9, 2021

View reviewed changes

TrueBrain force-pushed the stun-me branch from 09f08bc to 18ec1a4 Compare May 9, 2021 17:23

TrueBrain mentioned this pull request May 10, 2021

Change: reworded many of the network errors during connect/listen #9230

Merged

glx22 mentioned this pull request Jun 29, 2021

Change: [strgen] 'warnings' for translations are now 'infos' unless invoked with -w or -t #9406

Merged

TrueBrain force-pushed the stun-me branch 2 times, most recently from 8246f40 to 1b60488 Compare June 30, 2021 13:47

rubidium42 reviewed Jul 1, 2021

View reviewed changes

TrueBrain force-pushed the stun-me branch from 1b60488 to 8d92161 Compare July 1, 2021 18:56

TrueBrain and others added 17 commits July 2, 2021 14:30

Codechange: use UpdateNetworkGameWindow() over manually marking windo…

3fc9e15

…w dirty

Codechange: allow Connect() to bind to a local address

37a358e

Codechange: track servers with a ServerAddress instead of a NetworkAd…

c7920da

…dress This allows future extensions to have different ways of referencing a server, instead of forcing to use IP:port.

Codechange: track version of network servers to prune once out-of-date

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

GPG key ID: 4AEE18F83AFDEB23
Expired

Verified
Learn about vigilant mode

dd2d662

Add: use Game Coordinator to annouce public servers

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

GPG key ID: 4AEE18F83AFDEB23
Expired

Verified
Learn about vigilant mode

e181729

Add: use Game Coordinator to get latest public server listing

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

f93d039

Add: allow connecting to server based on join-key

Unverified

This user has not yet uploaded their public signing key.

GPG key ID: A048098B57B02178

Learn about vigilant mode

c27698d

Add: support STUN for network connections

83694a5

Add: support TURN for network connections

d989953

Remove: old server announcement to Master Server

781adf1

The Master Server doesn't understand NetworkGameInfo v5, so it is of no use anyway.

Remove: old server listing via Master Server

849c00e

This removes all UDP from the game except for a local broadcast to find LAN games.

Remove: unused UDP protocol parts

41778a9

Cleanup: remove unused optional ctor parameter for NetworkGameList

2868560

Add: allow setting your server visibility to "invite-only"

8f98dc5

In this mode you do register to the Game Coordinator, but your server will not show up in the public server listing. You can give your friends the invite code of the server with which they can join.

Feature: raise the maximum NewGRF limit to 255

50f7492

Add: inform clients what game-script a server is running

31a766f

TrueBrain force-pushed the stun-me branch from 8d92161 to 31a766f Compare July 2, 2021 12:30

TrueBrain mentioned this pull request Jul 3, 2021

Replace master-server (UDP-based) with Game Coordinator (TCP-based) #9411

Merged

TrueBrain mentioned this pull request Jul 17, 2021

Feature: allow the use of TURN to connect client and server together #9447

Merged

TrueBrain closed this Jul 19, 2021

TrueBrain deleted the stun-me branch July 19, 2021 20:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: play easier together with friends from behind home routers #9017

Feature: play easier together with friends from behind home routers #9017

TrueBrain commented Apr 11, 2021 •

edited

Loading

nielsmh left a comment

TrueBrain commented Apr 11, 2021

James103 commented Apr 25, 2021

TrueBrain commented Apr 25, 2021

TrueBrain commented May 9, 2021

rubidium42 May 9, 2021

TrueBrain May 9, 2021

glx22 commented May 9, 2021 •

edited

Loading

TrueBrain commented May 10, 2021 •

edited

Loading

James103 commented Jul 1, 2021 •

edited

Loading

TrueBrain commented Jul 1, 2021

TrueBrain commented Jul 19, 2021

Feature: play easier together with friends from behind home routers #9017

Feature: play easier together with friends from behind home routers #9017

Conversation

TrueBrain commented Apr 11, 2021 • edited Loading

Motivation / Problem

Description

Limitations

Testing

Checklist for review

nielsmh left a comment

Choose a reason for hiding this comment

TrueBrain commented Apr 11, 2021

James103 commented Apr 25, 2021

TrueBrain commented Apr 25, 2021

TrueBrain commented May 9, 2021

rubidium42 May 9, 2021

Choose a reason for hiding this comment

TrueBrain May 9, 2021

Choose a reason for hiding this comment

glx22 commented May 9, 2021 • edited Loading

TrueBrain commented May 10, 2021 • edited Loading

James103 commented Jul 1, 2021 • edited Loading

TrueBrain commented Jul 1, 2021

TrueBrain commented Jul 19, 2021

TrueBrain commented Apr 11, 2021 •

edited

Loading

glx22 commented May 9, 2021 •

edited

Loading

TrueBrain commented May 10, 2021 •

edited

Loading

James103 commented Jul 1, 2021 •

edited

Loading