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

netplay: switch from miniupnpc to LibPlum #3802

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ManManson
Copy link
Member

@ManManson ManManson commented May 12, 2024

LibPlum provides the following advantages comparing to the old miniupnpc option:

  • It supports not only UPnP, but also more modern and widely adopted NAT-PMP and PCP protocols for automatic port mapping.
  • Even more lightweight than miniupnpc, has no dependencies, much cleaner API.
  • Can be made to work with IPv6 (mostly thanks to the PCP support).

The patch also renames UPnP configuration option to portMapping. The old config option value will be used as a fallback if there's already one existing in user config.

@ManManson ManManson force-pushed the libplum_integration branch 2 times, most recently from 00f35a5 to 6ce9021 Compare May 13, 2024 15:27
@ManManson
Copy link
Member Author

Updated the branch to get LibPlum via submodule instead of doing FetchContent_Declare/FetchContent_MakeAvailable.

The PR still needs some work, e.g.: before connecting to a lobby server, WZ would need to wait for libplum to finish discovery (with either success or failure, with timeout of 10 seconds on the WZ size), since the external port discovered by the libplum can be different from the internal port assigned initially by the WZ config. We need to communicate it to the lobby somehow.

Still, we can at least observe the CI status, especially given that the LibPlum submodule points to the paullouisageneau/libplum@2c76313, which contains all recent fixes for the CI/packaging.

LibPlum provides the following advantages comparing
to the old miniupnpc option:
* It supports not only UPnP, but also more modern and
  widely adopted NAT-PMP and PCP protocols for automatic
  port mapping.
* Even more lightweight than miniupnpc, has no dependencies,
  much cleaner API.
* Can be made to work with IPv6 (mostly thanks to the
  PCP support).

The patch also renames `UPnP` configuration option to `portMapping`.
The old config option value will be used as a fallback if there's
already one existing in user config.

Signed-off-by: Pavel Solodovnikov <pavel.al.solodovnikov@gmail.com>
This incorporates all recent fixes anlongside a few
more which fix some IPv6 related issues.

Signed-off-by: Pavel Solodovnikov <pavel.al.solodovnikov@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant