Suggestion: Remove ProtocolLib dependency in favour of PDC

[Techirion]

I'd like to suggest ditching ProtocolLib and migrating to to PDC to store UUIDs. In terms of performance ProtocolLib itself is not too great, adding packet processing on top of that doesn't make it faster either...

So I was looking into this from another perspective, it would be possible to store data that is stored on signs in PDC? With that get rid of ProtocolLib dependency and whole processing thing?

I guess the alternative would be to migrate to PacketEvents, which appears to be faster and more regularly updated.

Or do nothing, it works in the current state and changing this would break existing signs...

[Lori3f6]

I've thought about this when I saw the PDC stuffs but it breaks things and need a lot of code changes.
And there is lack of ways to update all the signs all over the server at once, which may cause some problems.
It might be possible to update the signs automatically during chunk loading but that is also workload for the server to do compare to the package content filtering.
So unless there is a huge improvement if we change this, I may prefer to make it stay as it is, might not be perfect but works
Have you encountered any performance issue due to the packet content filtering which is using by now?

[Techirion]

From what I saw in our Spark profiles, performance gain would be 0.5 to 1% and another 1.5% to 2.5% if we can remove ProtocolLib itself. That's also depending on how many signs are there... by no means huge improvement, but we cut down and optimized everything else we could...

For signs itself ChunkLoadEvent would do the trick, tho it would probably be even better to do it on interact, this would mean some of the signs would be outdated, but it would also spread the load...

Ill submit PR if we decide to fork and add PacketEvents support...

[Lori3f6]

We may have a try. But do we need to introduce NMS for this due to the spigot/paper API don't provide packet manipulation methods if I am not mistaken

…

On Mon, Aug 19, 2024, 01:19 IAISI ***@***.***> wrote: From what I saw in our Spark profiles, performance gain would be 0.5 to 1% and another 1.5% to 2.5% if we can remove ProtocolLib itself. That's also depending on how many signs are there... by no means huge improvement, but we cut down and optimized everything else we could... For signs itself ChunkLoadEvent would do the trick, tho it would probably be even better to do it on interact, this would mean some of the signs would be outdated, but it would also spread the load... Ill submit PR if we decide to fork and add PacketEvent support... — Reply to this email directly, view it on GitHub <#26 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ANFR7OEFLWQIVRXE2MFJXETZSETYBAVCNFSM6AAAAABMWXSZE6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEOJVGQZTANRZGA> . You are receiving this because you commented.Message ID: ***@***.***>

[Techirion]

With PacketEvents I was referring to https://github.com/retrooper/packetevents

Implementation would be somewhat similar to current ProtocolLib implementation and would essentially work the same way.

[Lori3f6]

It might be better we make packetevents a soft dependency (as ProtocolLib) and automatically choose between these two as some servers may not able to completely remove ProtocolLib as other plugin may need to use it as a dependency, so they can keep using ProtocolLib with LockettePro without importing new dependencies

[FireInstall]

Our fork
You might want to add the support for offline server (aka uuid check toggable) back in. Removing ProtocolLib would also solve #19

Elsewise it might even be usefull for you to have a look into Padlock, where I redid almost the whole plugin.