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

Valve sockets networking #465

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

Conversation

maximegmd
Copy link

@maximegmd maximegmd commented Oct 27, 2021

This PR replaces websockets with GameNetworkingSockets by Valve.

A few caveats with this PR:

  • There is an issue with GameNetworkingSockets (see Increasing throughput of a connection ValveSoftware/GameNetworkingSockets#198) that is worked around by polling manually every frame, so until there is a fix for this, polling manually will have to do.
  • This requires the manual management of big packet fragmentation, it also involves copying packets around a lot, I am not knowledgeable enough in C# to know if and how this can be optimized.

I benchmarked the library and managed to get 8.5MB/s of throughput (hard limit of GNS, will PR an increase with Valve).

It's supposed to work fine on other platforms, untested.

@PhantomGamers
Copy link
Collaborator

I'm not sure if this is actually going to be viable because of the imports of overloaded functions but here is a ValveSocket.cs with the DllImports mostly converted to DynDllImports
ValveSockets.cs.txt

PhantomGamers and others added 4 commits October 28, 2021 13:32
- Move prebuilt GameNetworkingSockets libs to subfolder
- Change ValveSockets.csproj to SDK format
- Remove ReferenceAssemblies package reference from dep\Directory.Build.props as it's no longer needed
- Include `BepInEx.Core` instead of `MonoMod.Utils` to avoid extra download
Shows time it takes for OnAllPlayersSyncCompleted() to trigger from when a player begins joining a game when in debug mode.
@mmjr-x
Copy link
Contributor

mmjr-x commented Oct 29, 2021

@Yamashi Not sure if you want to do anything with it, however I was looking at the C# wrapper and it seems that it only supports the 1.2.x version of the native library (there is an issue confirming) while the current version of the native library is at 1.3.x . There have been lots of commits in the mean time so they might have fixed ValveSoftware/GameNetworkingSockets#198 in the mean time.

On an unrelated note, (this might also interest you @PhantomGamers) there has been some talk about exposing the P2P signaling stuff through the C# wrapper, you can find more about it here

@PhantomGamers
Copy link
Collaborator

That 1.3.0 compatibility issue seems to be only on Linux, which I'm not really worried about as DSP doesn't have a native Linux version anyway

@mmjr-x
Copy link
Contributor

mmjr-x commented Oct 29, 2021

@PhantomGamers perhaps I didn't formulate it clearly enough, however I am fairly sure the version of the native library that is included with this PR is the 1.2 variant.

My Reasoning being, the compiled dll's came from the NuGet version of the c# wrapper, which included it in this commit riina-eng/ValveSockets-CSharp@f693e2b on 27 March 2021, this predates the release of the 1.3 version of the native Lib (Released on 30 May 2021)

@PhantomGamers
Copy link
Collaborator

Ah, I see. Well, the good news is that according to that it sounds like we can use 1.30 with the wrapper in our case as least lol

@mmjr-x
Copy link
Contributor

mmjr-x commented Oct 29, 2021

@Yamashi or @PhantomGamers I compiled the 1.3.0 version from https://github.com/ValveSoftware/GameNetworkingSockets/releases via vcpkg and have the binaries here if one of you wants to try:
GameNetworkingSockets-v1.3.0-x64-win-binaries.zip

@PhantomGamers
Copy link
Collaborator

Has this been compiled with ICE support as well?

@mmjr-x
Copy link
Contributor

mmjr-x commented Oct 29, 2021

I did not change anything from the default config, so I guess not. Perhaps try this version first? if it works I will try to compile with that option

@PhantomGamers
Copy link
Collaborator

PhantomGamers commented Oct 29, 2021

Not only does it work, it's really fucking fast!

[Debug :NebulaMultiplayerMod] Joined in 00:00:05.6972498

Faster than telepathy! Woah

Co-Authored-By: mmjr-x <2429824+mmjr-x@users.noreply.github.com>
@PhantomGamers
Copy link
Collaborator

False alarm! I had loaded the wrong save because I was messing with a different one earlier, it's not actually faster >_<, but it does work lol.

@mmjr-x
Copy link
Contributor

mmjr-x commented Oct 29, 2021

Glad it works at least! Perhaps its worth doing a build of the latest master instead of the latest release, I will try to find the time for that tomorrow :)

@maximegmd
Copy link
Author

maximegmd commented Oct 29, 2021

In terms of performance it appears to be faster than WebSockets, not as fast as Telepathy but this is expected considering we are very conservative with the computing power we request for polling, this could be improved or we can just wait for the issue to be solved by Valve.

Possible improvements are in the following areas:

  • Reduce fragment packet overhead, they arrive in order, so the offset field is not necessary. We could also use a 7 bit fragment id to reduce the overhead from 5 bytes to 1 byte (the opcode that specifies if a packet is a fragment or not has 7 unused bits). Similarly we could remove the length field from subsequent fragment packets.
  • Move the NebuleConnection.Update() call to the polling thread to ensure the send buffer is never empty between frames.
  • Implement the ICE/STUN p2p functionality to bypass port forwarding.
  • Use Span instead of copying the byte buffer to a new byte buffer and just skipping a few bytes.
  • Implement a synchronized tick, this can then be used to pass accurate time points for interpolation, along with dropping unreliable packets that are older than what we have.
  • Add debug information to the UI (send/recv speed, jitter, drop rate)

@PhantomGamers
Copy link
Collaborator

Ignore the edit, I wasn't sure if it would actually let me tick the checkboxes on your comment haha

Use Span instead of copying the byte buffer to a new byte buffer and just skipping a few bytes.

Maybe ArraySegmentX would be useful for this as well, it's what Mirror uses

Also before this gets merged we should remove any logging that exposes IP addresses

@mmjr-x
Copy link
Contributor

mmjr-x commented Oct 30, 2021

I have compiled the latest master (this time through visual studio) with USE_STEAMWEBRTC (ICE Support):
GameNetworkingSockets-USE_STEAMWEBRTC-win-x64-Release-e4601df.zip

aesgcmtestvectors, GameNetworkingSockets.dll.manifest and libssl-1_1-x64.dll are problably not required (although I am not sure about libssl-1_1-x64.dll ) however I just included the complete build output. I build it in Release mode, however I can provide one with debug if requested :)

Yamashi and others added 5 commits October 31, 2021 16:43
- built from ValveSoftware/GameNetworkingSockets@5c793b9

- update valvesockets to support the changes made in GameNetworkingSockets

WARNING: I didn't implement the changes to `SteamAPI_ISteamNetworkingUtils_GetFirstConfigValue`, I instead removed it altogether as we weren't using it anyway.
@PhantomGamers
Copy link
Collaborator

Did some testing with this today and sadly it still seems to be borked with bigger factories. If the client tries to connect to a planet with a large factory it will just hang on requesting factory data, and if a client connects to an empty planet and then travels to a larger planet the planet will never load in (and humorously the player's mecha will be tied to that planet forever, and if they try to go further than 1000 meters away the camera will disconnect from the mecha).

Another note is that the size of the factory that this happens with varies from client to client. I was able to load in to a particular planet that another client was unable to. I didn't have the proper logging enabled to see what the size of the planets were at the time so this isn't super helpful but it's just something to note.

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.

5 participants