WIP: Always use STUN for hosting games using the lobby #7002

Draft
Dunedan wants to merge 1 commits from Dunedan/0ad:make-stun-mandatory into main
Member

This makes using STUN mandatory for games hosted using the multiplayer
lobby. The motivation for that is a reduction in complexity, because
right now if STUN is disabled we use a home-grown STUN-like logic, which
got implemented before Pyrogenesis got STUN support.

That home-grown logic relies on a custom ejabberd module (mod_ipstamp),
which inserts the external IP-address of a host in the response messages
when a host registers a game. Originally mod_ipstamp was also used to
inform all potential players of a hosts IP-address, however that has
already been removed to let hosts to only share their IP-address with
players actually joining their game.

Removing the home-grown logic and instead always relying on STUN removes
complexity in Pyrogenesis and the lobby server and also eases hosting
games for players, as they don't have to figure out anymore whether they
need to enable STUN or not.

These changes shouldn't negatively impact the ability of Pyrogenesis to
handle different types of NAT or broken networks. There is one
difference though: While the custom logic using mod_ipstamp utilized TCP
as transport protocol, the STUN implementation in Pyrogenesis currently
uses UDP. That doesn't allow hosts with UDP-connectivity issues to
resolve their external IP-address anymore, however without
UDP-connectivity they aren't able to successfully host games anyway, as
the actual game updates are transferred using UDP as well.

Continuation of D5311

This makes using STUN mandatory for games hosted using the multiplayer lobby. The motivation for that is a reduction in complexity, because right now if STUN is disabled we use a home-grown STUN-like logic, which got implemented before Pyrogenesis got STUN support. That home-grown logic relies on a custom ejabberd module (mod_ipstamp), which inserts the external IP-address of a host in the response messages when a host registers a game. Originally mod_ipstamp was also used to inform all potential players of a hosts IP-address, however that has already been removed to let hosts to only share their IP-address with players actually joining their game. Removing the home-grown logic and instead always relying on STUN removes complexity in Pyrogenesis and the lobby server and also eases hosting games for players, as they don't have to figure out anymore whether they need to enable STUN or not. These changes shouldn't negatively impact the ability of Pyrogenesis to handle different types of NAT or broken networks. There is one difference though: While the custom logic using mod_ipstamp utilized TCP as transport protocol, the STUN implementation in Pyrogenesis currently uses UDP. That doesn't allow hosts with UDP-connectivity issues to resolve their external IP-address anymore, however without UDP-connectivity they aren't able to successfully host games anyway, as the actual game updates are transferred using UDP as well. Continuation of [D5311](https://code.wildfiregames.com/D5311)
Dunedan added 1 commit 2024-08-30 17:59:02 +02:00
WIP: Always use STUN for hosting games using the lobby
All checks were successful
checkrefs / checkrefs (pull_request) Successful in 1m1s
pre-commit / build (pull_request) Successful in 1m13s
0ad-freebsd/pipeline/pr-main This commit looks good
0ad-windows/pipeline/pr-main This commit looks good
0ad-macos/pipeline/pr-main This commit looks good
0ad-linux/pipeline/pr-main This commit looks good
bb0a35204f
This makes using STUN mandatory for games hosted using the multiplayer
lobby. The motivation for that is a reduction in complexity, because
right now if STUN is disabled we use a home-grown STUN-like logic, which
got implemented before Pyrogenesis got STUN support.

That home-grown logic relies on a custom ejabberd module (mod_ipstamp),
which inserts the external IP-address of a host in the response messages
when a host registers a game. Originally mod_ipstamp was also used to
inform all potential players of a hosts IP-address, however that has
already been removed to let hosts to only share their IP-address with
players actually joining their game.

Removing the home-grown logic and instead always relying on STUN removes
complexity in Pyrogenesis and the lobby server and also eases hosting
games for players, as they don't have to figure out anymore whether they
need to enable STUN or not.

These changes shouldn't negatively impact the ability of Pyrogenesis to
handle different types of NAT or broken networks. There is one
difference though: While the custom logic using mod_ipstamp utilized TCP
as transport protocol, the STUN implementation in Pyrogenesis currently
uses UDP. That doesn't allow hosts with UDP-connectivity issues to
resolve their external IP-address anymore, however without
UDP-connectivity they aren't able to successfully host games anyway, as
the actual game updates are transferred using UDP as well.

TODO:
- remove lobby.stun.enabled config option
Stan reviewed 2024-08-31 11:02:58 +02:00
@ -229,3 +228,3 @@
CStr ip;
u16 port = 0;
if (g_XmppClient && m_UseSTUN)
if (g_XmppClient && !localNetwork)
Owner

Do you have a g_XmppClient in local network?

Do you have a g_XmppClient in local network?
Author
Member

Whether or not there is an XMPP client isn't related to the IP-addresses of game participants (what localNetwork seems to be about), but whether a game gets started through the lobby or not. So I believe this check should stay in there.

Whether or not there is an XMPP client isn't related to the IP-addresses of game participants (what `localNetwork` seems to be about), but whether a game gets started through the lobby or not. So I believe this check should stay in there.
Owner

I might be wrong but I believe that's a specific case where you want the local ip to be used in the lobby because you're playing with a relative and other people on the internet.

I might be wrong but I believe that's a specific case where you want the local ip to be used in the lobby because you're playing with a relative and other people on the internet.
Owner
https://mod.io/g/0ad/m/any-ip
Author
Member

I might be wrong but I believe that's a specific case where you want the local ip to be used in the lobby because you're playing with a relative and other people on the internet.

Just so we're on the same page: You did notice that there is another else if (g_XmppClient && localNetwork) clause after the mentioned statement, which is supposed to handle the local network case?

> I might be wrong but I believe that's a specific case where you want the local ip to be used in the lobby because you're playing with a relative and other people on the internet. Just so we're on the same page: You did notice that there is another `else if (g_XmppClient && localNetwork)` clause after the mentioned statement, which is supposed to handle the local network case?
Owner

Oh yeah just hoped to clarify what the local network variable was doing

Oh yeah just hoped to clarify what the local network variable was doing
All checks were successful
checkrefs / checkrefs (pull_request) Successful in 1m1s
Required
Details
pre-commit / build (pull_request) Successful in 1m13s
Required
Details
0ad-freebsd/pipeline/pr-main This commit looks good
0ad-windows/pipeline/pr-main This commit looks good
0ad-macos/pipeline/pr-main This commit looks good
0ad-linux/pipeline/pr-main This commit looks good
Required
Details
This pull request is marked as a work in progress.
This branch is out-of-date with the base branch

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u make-stun-mandatory:Dunedan-make-stun-mandatory
git checkout Dunedan-make-stun-mandatory
Sign in to join this conversation.
No reviewers
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: 0ad/0ad#7002
No description provided.