Out/In bound protocol injection improvements#1524
Out/In bound protocol injection improvements#1524dmulloy2 merged 9 commits intodmulloy2:masterfrom derklaro:pr/improve-channel-injection
Conversation
|
Looks great! Thank you for your work on this! My thinking is to release 4.8.0 after merging in 1.18.2 support, bump the version to 5.0.0, then release this on jenkins for some testing. Metrics would imply that at least most people aren't using the bleeding edge builds: https://bstats.org/plugin/bukkit/ProtocolLib/1453 |
|
Sounds good 👍 |
|
And is it possible to use ReflectASM for better performance? |
|
Well, not really (I guess)... I can recheck if that is possible but from the code I see the library will not be able to override "trusted" final fields, as for example used in records. Furthermore, I don't like that the lib doesn't get any "real" updates, for example the asm library dependency is 5 years out of date... |
I guess we can bump it now? |
|
@dmulloy2 was the merge into master rather than dev intentional? |
|
yeah im just messing around with formatting and linting on dev |
|
ah i see 👍 |
|
Love you guys <3 |
|
Now we are just waiting for ViaVersion to do the same... But they might take much longer. |
|
Hi, is this already on latest dev builds? I'd love to do some testing |
|
Yes |
thanks |
| /** | ||
| * Retrieve the serialized client packet as it appears on the network stream. | ||
| */ | ||
| INTERCEPT_INPUT_BUFFER, |
There was a problem hiding this comment.
There was a problem hiding this comment.
There is no replacement for this. See my initial PR comment for details
This pull request introduces quite some changes to the way protocol lib handles and sends packets to the clients.
Current stuff which isn't done yet completely:
Some changes I made to the api:
InvocationTargetExceptionfromsendServerPacket. It requires the api user to catch these exceptions while they are never thrown by the underlying code and are arguably relicts from ancient days.SocketInjectoranymore (which provided access to a Socket which wrapped the incoming channel), mainly as that api was probably unused by most users and was already broken, depending which server software a user was using.But this pull request also includes some very nice changes:
write/writeAndFlushwill always be on the main thread (on the vanilla server at least).Just a few more notes I want to conclude this with:
I think this pull request resolves any Issue regarding performance and/or how we inject into the channel pipeline! Love to get some feedback on this from everybody who previously reported such issues 😀
I would no recommend anyone to use this in production as this change might still contain bugs