-
Notifications
You must be signed in to change notification settings - Fork 64
RealIP Fabric support #40
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
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.
First if all, thanks for this great contribution. We're always thankful for such wonderful contributions.
Before approving this PR, there are still some minor (arguably nit-picky) points which need adjustment.
Also, as a question: How is this implementation able to work over multiple versions (so others than 1.16)? Is the limit you've set artificial or necessary?
src/main/java/net/tcpshield/tcpshield/fabric/TCPShieldFabric.java
Outdated
Show resolved
Hide resolved
src/main/java/net/tcpshield/tcpshield/fabric/TCPShieldFabric.java
Outdated
Show resolved
Hide resolved
src/main/java/net/tcpshield/tcpshield/fabric/impl/FabricConfigImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/net/tcpshield/tcpshield/fabric/impl/FabricConfigImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/net/tcpshield/tcpshield/fabric/impl/FabricConfigImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/net/tcpshield/tcpshield/fabric/impl/FabricConfigImpl.java
Outdated
Show resolved
Hide resolved
Fabric does no version abstraction, so the most you can do is hope it just works:tm:. That being said, I tested the mod on 1.14.4, 1.15, and 1.16.5 (the only major versions Fabric supports outside snapshots), and it worked on all 3. This will only break if Mojang changes early networking code.
|
paulzhng
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.
Absolutely amazing work done here. Looking forward to include this in v2.5.
I took the freedom and updated the README.
This commit introduces Fabric support for the TCPShield RealIP plugin. We use Loom version 0.6 and Minecraft version 1.16.5 to support building with Java 8 alongside the other plugin loader implementations. Configuration loading is done with simple YAML parsing, as Fabric does not ship a default configuration library. For information on FabricPacket#setPacketHostname no-op, visit TCPShield#40 (comment).
Hello, I have arrived with Fabric support. This PR has been sanity-checked by at least 2 individuals (one of which was me), and both of us were able to successfully reproduce TCP Shield functionality on a dedicated server with the full DNS setup.
PR Overview
build.gradle:Fabric does not provide networking hooks as early as the handshake-phase, so I had to pull in Mixin. To use Mixin, I also had to pull in the full Minecraft dependency. I use the Fabric Loom plugin to achieve this (it is essentially a standard Fabric mod buildscript at this point).
Initialization
Similar to the startup listeners & initializers in the other modules, the Fabric folder now has an initializer named
TCPShieldFabric. This class is loaded by thefabric.mod.jsonfile and sets up the Fabric packet handler.Handshake Handling
To handle handshakes, I placed a mixin in
ServerHandshakeNetworkHandler#onHandshake, which calls the Fabric packet handler with the abstracted packet & player data.Implementation Notes
Config:
Fabric does not ship a standard configuration library, so I wrote a simple custom parser in
FabricConfigImpl.We have confirmed clients cannot connect to the server IP directly, while the domain setup with TCPShield works great. The server properly identities the clients IP. Finally, I have also confirmed the client does not have access to the server's true IP (but reports being connected to TCPShield instead):
If there are any changes you would like me to make, please let me know. Thank you in advance!