Describe the bug
You are probably (hopefully!) already aware of this, but since the 5.0 version builds of protocollib everything has been broken on my end. Packets sent using both protocollib's sendServerPacket and using NMS protocol sendPacket method are being dropped, resulting in a whole host of problems:

• Spawn packets arent being sent
• Entity metadata packets arent being sent
• Entity equipment packets arent being sent
• Mount packets arent being sent

I've tried to see if it can have anything to do with my own code, so I cancelled packet listeners/monitors for packets but the problem remains.

To Reproduce
Steps to reproduce the behavior:

1. Install BKCommonLib dev build https://ci.mg-dev.eu/job/BKCommonLib/1267/
2. Install Traincarts https://ci.mg-dev.eu/job/TrainCarts/1086/
3. Place a minecart
4. Try entering it. Nothing happens because the mount packet TC sends isnt being sent.

For a more extreme demo, also install TC-Coasters ( https://ci.mg-dev.eu/job/TC%20Coasters/247/ ) and try to place down multiple nodes. Entity metadata is stripped so youve got a bunch of floating bats handing around.

Expected behavior
Packets not being dropped

Screenshots

Version Info
https://paste.traincarts.net/nunoderiji.pl

Additional context
This is the protocollib handler logic of BKCL. If this matters (maybe Im using a legacy API that 5.0 broke and I need to use a different API, for example) let me know what I should do differently.

https://github.com/bergerhealer/BKCommonLib/blob/master/src/main/java/com/bergerkiller/bukkit/common/internal/network/ProtocolLibPacketHandler.java

This problem did not exist with the 4.8 release on spigot, so Im currently telling people to use that one. A lot of people have been rolling with the 5.0 dev builds and its been a bit of a support pain over at the discord...

This code triggered the situation with the piglin:

        PacketUtil.addPacketMonitor(this.plugin, this.monitor,
                PacketType.OUT_ENTITY_SPAWN,
                PacketType.OUT_ENTITY_SPAWN_LIVING,
                PacketType.OUT_ENTITY_SPAWN_NAMED,
                PacketType.OUT_ENTITY_DESTROY,
                PacketType.OUT_RESPAWN);

So looks like simply monitoring for the entity spawn packets (which involves no packet modification) is enough to trigger packets sent after to no longer be received by the client. The handlers use asynchronous mode.

Alternative explanation: monitoring such a packet causes the metadata packet to be received before the spawn packet. (order broken)

Given mounting is broken, and this logic heavily depends on this monitor, my guess is that the monitor isnt called either.

https://github.com/bergerhealer/BKCommonLib/blob/master/src/main/java/com/bergerkiller/bukkit/common/internal/CommonVehicleMountManager.java#L111

I can confirm something. I removed all listeners/monitors and now the issue with the piglin doesnt occur. Im going to work out what packet being listened to triggers this problem.

Should be fixed now, can you verify with https://github.com/derklaro/ProtocolLib/suites/5812286236/artifacts/194546127 please?

Interesting, I am only able to reproduce the issue if I'm enabling async packet listeners... Is that the case for you too?

Yes, Im using async packet listeners. Everything is async by design in my plugins as I dont want to cause any more overhead than strictly required. Let me try making that sync instead by removing the async listener option

EDIT*

Yup no problem when I force all listeners to be sync instead of async, so the issue lies here

Okay thanks for your detailed help, then let me take a look at that async code again ;)

Can confirm all my issues are fixed with that build. The piglin, the mounting and the weird broken spawning of the tc-coaster leashes/etc.

Hi,
no I am not aware that packets are being dropped when using with ProtocolLib v5. During testing spawning entities worked great, but as I am a bit busy today I will look into that tomorrow (same for your receive packet issue). Anyway just fyi, I scrolled through your code and there are two notes:

• TemporaryPlayer shouldn't be used, if you are listening to a PacketEvent just use PacketEvent#isPlayerTemporary instead ;)
• Regarding "send packet after current handled packet", you can define scheduled packets in the packet marker supplied to sendServerPacket, those will be scheduled after the current packet was processed.

Hope that helps a bit, sorry for your support inconvenience :/

Ah good, I'll change it to use the isPlayerTemporary() method instead of checking for that class. A bit more stable that way.

When I have a little time Ill look into the marker solution. QueuePacket is a bit of a mess and that sounds like a much cleaner approach, wasn't aware it existed :)

As for the bugs, there was also a chunk hole when I just spawned, so it's affecting not just my plugin it seems...

Note: MC 1.18.2

This is a blind guess, but in my experience the spawn packets work fine, its the packets sent shortly after the spawn packet that break. So entity metadata, equipment, etc. As such, you can trivially reproduce it by trying to spawn piglin with a spawn egg - they all spawn without any equipment on and without any weapons. Removing protocollib 5.0 and they do all spawn with armor/weapons again.

I just tested that on Paper 1.18.2 (Build 267) and it seems to be working for me 🤔

Yes, Im using async packet listeners. Everything is async by design in my plugins as I dont want to cause any more overhead than strictly required. Let me try making that sync instead by removing the async listener option

fyi: ASYNC does not mean that the listener isn't called from the main thread, it just indicates that the listener is thread safe

@derklaro Oh yeah thats fine, Im more worried about protocollib delaying a packet sent from another thread by dispatching it to the main thread first. Thats the main reason I avoid sync, as some packets I listen to are sent on other threads by plugins sometimes.

Out of curiosity, is there a mode thats truly async as well? So, only hooks the netty transport queue for example and runs exclusively on the netty thread? Out of laziness mostly thats how the fallback handler of mine does it.

Hm, not currently. But because of the rewrite of the injection, it should be very easy to add that 🤔