Your encoder makes sure it calls the "encoder" handler in the pipeline IMMEDIATELY after your encoder. Your encoder takes in NMS objects and spits out buffers since you make sure "encoder" is called right after your handler. Why? Other injectors want to get in between you and the "encoder". ViaVersion decided to wrap vanilla's encoder, which means I can't (easily) process bytes before via translates them.
I'd say the damage has been done, there are so many people maintaining on legacy versions of ProtocolLib, but if this could change, I'd encourage my users to update ProtocolLib, honestly...

For anyone curious, yes ViaVersion can actually do adjustments and could theoretically support older and newer versions of ProtocolLib if they fix their injector. Via just needs to make sure their encoder is before vanilla’s encoder in the pipeline, since the vanilla encoder converts nms objects to bytes, via will translate these bytes(which they can). Issue is they said they fixed this, but it will be pushed in the next major update. Anyway no worries, none of this affects us.

Thanks for the idea. I generally would agree that rewriting the way how protocol lib hooks into the channel pipeline is a good thing to do, as the code powering the injection is partly from 2013 (don't get me wrong, for 2013 that is very good 😂 ). However I personally would like to hear the opinion of @dmulloy2 on rewriting this part, as it will (99.9% sure) require a v5 of protocol lib as the rewrite will contain breaking changes...

This could however fix the described issue in #1415 as well, and speed up the whole thing up if we get the reflection calls in the pipeline to a bare minimum...

I hope dmulloy2 replies, seems like he's been inactive lately, perhaps busy (can happen)
This does contain breaking changes, and ViaVersion would also have to adjust. But it would be worth it.

I think there are two ways why this can break:

• someone relies on the way how protocollib injects into the pipeline and a change to that breaks that plugin
• we remove methods from the injector and replace a good chunk of the old code which some (for whatever reason) treats as api... 🤔

(I don't think that ViaVersion needs / must adapt to our change at all here, wrapping the vanilla encoder will stillt be a valid way to get and modify the packets send down the channel pipeline)

True actually, well ViaVersion seems to make sure it calls fireChannelRead in ProtocolLib's context for some reason.

The way I see it is, (since ViaVersion wraps the vanilla encoder & decoder) is ViaVersion did this as a reaction to ProtocolLib's injector? Or I might be completely wrong. It wouldn't hurt if they adjusted too.

The way I see it is, (since ViaVersion wraps the vanilla encoder & decoder) is ViaVersion did this as a reaction to ProtocolLib's injector? Or I might be completely wrong. It wouldn't hurt if they adjusted too.

That would be correct, I believe they do some special handling if PL is present as a reaction to how we've been doing things. Depending on how they do it, it may or may not be a breaking change. If it's something like checking for protocol_lib_encoder, we can just change the name and probably fine.

Thanks for the idea. I generally would agree that rewriting the way how protocol lib hooks into the channel pipeline is a good thing to do, as the code powering the injection is partly from 2013 (don't get me wrong, for 2013 that is very good 😂 ). However I personally would like to hear the opinion of @dmulloy2 on rewriting this part, as it will (99.9% sure) require a v5 of protocol lib as the rewrite will contain breaking changes...

This could however fix the described issue in #1415 as well, and speed up the whole thing up if we get the reflection calls in the pipeline to a bare minimum...

I'd be very interested to see what this would look like if you're up for it

I'm very very glad to see dmulloy2 agreeing to this!

I don't know if I'm up to do it all alone, but @derklaro I'd be more than happy to help out where possible. Please let me know.

Resolved