-
-
Notifications
You must be signed in to change notification settings - Fork 2
Hole punching implementation and improvements to networking including Steam P2P #24
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
Conversation
diamondpixel
commented
Dec 30, 2025
Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 5 to 6. - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](actions/upload-artifact@v5...v6) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
--- updated-dependencies: - dependency-name: Hamunii.BepInEx.AutoPlugin dependency-version: 2.1.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
- Add RequiresX capability properties to IEncryptedTransportClient - Server-side Steam clients now correctly skip UDP sequencing logic - Lazy-init managers only when needed (saves memory for Steam) - Remove HolePunch stubs (to be replaced with pre-processors)
…d lobby management system with Steam integration and some improvements to the system
|
By the way i may and may not, have included my Public IP accidentally on the commit..... |
Extremelyd1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've finished the review, there are a couple of comments left that I would like to see changed and one somewhat larger question about whether we need to contact an external STUN server.
Apart from that the code looks pretty good. I'll save functional testing until this branch is ready to be merged with main. We need to keep in mind to change the hostname (and maybe port) of the MMS when we do.
Extremelyd1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple instances of Buffer.BlockCopy instead of Array.Copy and a single comment still. I've commited some style, comment, and compiler warning supression fixes already.
SSMP/Networking/Transport/SteamP2P/SteamEncryptedTransportServer.cs
Outdated
Show resolved
Hide resolved
| /// <typeparam name="T">The type of the generic client packet data.</typeparam> | ||
| /// <returns>An instance of the packet data in the packet.</returns> | ||
| private T FindOrCreatePacketData<T>( | ||
| private T? FindOrCreatePacketData<T>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed this in earlier reviews, but you shouldn't have made the return type nullable here. The method either finds the packet data or creates a new instance of it. So in all cases the method returns a non-null instance of packet data.