Support for adding custom packet adapter

[AbhigyaKrishna]

The use case scenario for this is like adding some other packet library based packet adapter. Eg: ProtocolLib

In my current situation, I have this setup:

implementation("com.github.megavexnetwork.scoreboard-library:scoreboard-library-implementation:${Dependencies.SCOREBOARD_LIBRARY}")
implementation("com.github.megavexnetwork.scoreboard-library:scoreboard-library-v1_20_R1:${Dependencies.SCOREBOARD_LIBRARY}")

The protocol version for the working server is 1_17_R1 and the adapter for 1.20 works fine until this version (I have already checked it). And, I am have a issue with NoPacketAdapterAvailableException.

This would not be the best way to implement this, but for my situation it would work as I can initialize it by:

var scoreboardLibrary = new ScoreboardLibraryImpl(plugin, net.megavex.scoreboardlibrary.implementation.packetAdapter.v1_20_R1.PacketAdapterImpl());

[vytskalt]

The packet adapters use NMS packages specific to the version, so I don't see why that would work. Did you try showing a sidebar or team when you tested if it works?

[AbhigyaKrishna]

Only thing that would break are the craftbukkit classes as there are no version string associated in the nms code of modern version.

I could probably implement a workaround (reflection) for craft classes in a new packet adapter module in my project. This would eliminate the hassle of implementing every protocol version in my project.

[AbhigyaKrishna]

There are no changes in the packets used in the implementation of packet adapter, through versions 1.17-1.20.

[vytskalt]

Oh, I wasn't aware that spigot mappings are no longer used in class names. I think I'll just replace the usage of the craftbukkit classes with reflection and make it automatically load that packet adapter for 1.17+

[AbhigyaKrishna]

Is there a issue merging this change? This just adds one constructor to the ScoreboardImpl class. I am also thinking of optimizing the packet adapter for my implementation by using bytebuffers to allocate a new instance of the packet classes instead of reflection and unsafe calls.

[vytskalt]

Is there a issue merging this change? This just adds one constructor to the ScoreboardImpl class. I am also thinking of optimizing the packet adapter for my implementation by using bytebuffers to allocate a new instance of the packet classes instead of reflection and unsafe calls.

Anything outside the api module is internal and shouldn't be used directly. For the bytebuffer optimisation, you could use the packetevents adapter as thats exactly what packetevents does

[AbhigyaKrishna]

Anything outside the api module is internal and shouldn't be used directly. For the bytebuffer optimisation, you could use the packetevents adapter as thats exactly what packetevents does

That is a good point! But I intend not to enforce another library unless necessary. We could probably expose the packet-adapter-base module in the api as it required anyways. And provide a builder pattern for some configuration for initializing the scoreboard library implementation.

[vytskalt]

Anything outside the api module is internal and shouldn't be used directly. For the bytebuffer optimisation, you could use the packetevents adapter as thats exactly what packetevents does

That is a good point! But intend not to enforce another library unless necessary. We could probably expose the packet-adapter-base module in the api as it required anyways. And provide a builder pattern for some configuration for initializing the scoreboard library implementation.

I don't see why users should create their own packet adapters. Are there performance issues you encountered? The packet adapters are being called from an async task regardless if you use the library from the main thread or not, so how the packets are created & sent shouldn't matter.

[AbhigyaKrishna]

I don't see why users should create their own packet adapters. Are there performance issues you encountered? The packet adapters are being called from an async task regardless if you use the library from the main thread or not, so how the packets are created & sent shouldn't matter.

They would obvious create their own packet adapter for the versions which are not supported natively from 1.9-1.19. And shading/forcing to use a packet library, like packetevents, just for one protocol version would be insane from my pov.

[AbhigyaKrishna]

If you would be changing the craft class import, its also fine by me.

[vytskalt]

I don't see why users should create their own packet adapters. Are there performance issues you encountered? The packet adapters are being called from an async task regardless if you use the library from the main thread or not, so how the packets are created & sent shouldn't matter.

They would obvious create their own packet adapter for the versions which are not supported natively from 1.9-1.19. And shading/forcing to use a packet library, like packetevents, just for one protocol version would be insane from my pov.

They should PR it so everyone could use it. I could also add a ProtocolLib adapter, since most servers have it anyways.

If you would be changing the craft class import, its also fine by me.

yes, I will do that

[AbhigyaKrishna]

You can close this pr.

[vytskalt]

@AbhigyaKrishna done in 2.0.0-RC11

[AbhigyaKrishna]

Thank you