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

allow custom Channel type to be wrapped in InjectionFactory#fromChannel #2756

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

wuangg
Copy link

@wuangg wuangg commented Jan 23, 2024

You can create your own custom Channel implementation by extending server socket channel classes that are available in Netty. In Minecraft server there are two transport APIs being used (NIO and Epoll), however extending two different server socket channel classes just to serve only a single purpose usually is not a good solution, as it is not worth effort to maintain two classes at the time.

Fortunately, in the network manager class there's a Channel field. For those plugins/Spigot forks want to do some tinkering with packet sending/receiving through Netty API, they can wrap the original Channel which was gotten from ChannelHandlerContext into their own implementation (which is exactly what ProtocolLib is doing).

The worst part is the way how ProtocolLib injects into Netty channel during a server connection initialization and therefore it overwrites the declared custom Channel implementation in the network manager. The main culprit is InjectionFactory#fromChannel because it takes the Channel straight from ChannelHandlerContext given by InjectionChannelInboundHandler as the wrapped channel for the channel injector.

This PR fixes the issue by looking up for existing Channel field inside an already found network manager and if the field has null value, use the Channel from ChannelHandlerContext as fallback.

@derklaro
Copy link
Contributor

I would really like to know a use-case for creating a custom channel wrapper... changing this basically trusts the implementation to pass channel attributes to the original channel coming from ctx.channel, which is not an ideal solution imo

@wuangg
Copy link
Author

wuangg commented Jan 24, 2024

I would really like to know a use-case for creating a custom channel wrapper

My use case here mostly tampering with methods that schedule the command to Netty event loop every time a network manager sends a packet. By creating a custom channel wrapper, it also wraps my custom event loop wrapper. The Netty channel proxy in ProtocolLib just simply overwrites my channel wrapper, rendering my code completely stop working.

which is not an ideal solution

Yes, this is not an ideal solution but, creating a new Spigot/Paper fork then extending 2 or 3 different existing server socket channel classes just to serve a simple use-case like mine seems like wasting too much effort, creating a new ProtocolLib fork then keep it up-to-date with upstream feels like it is already putting a strain on my already existing workload, so I think this approach here is the best. I opened this PR for a feature request for better compatibility between plugins/Spigot forks and for any person who has the same use-case as mine could benefit from this. The code change is relatively small and I don't think it is going to cause any harm at all.

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.

2 participants