ProtocolLib

3M Downloads

ProtocolLib 5.0.0 Sync/Async - strange behavior - PacketAdapter dont get called?

Wolf2323 opened this issue ยท 14 comments

commented

Describe the bug
With ProtocolLib 4.8.0 everything worked fine. Since users of my plugin BetonQuest started to use ProtocolLib 5.0.0 things started to be strange.

We have something called ChatInterceptor. As we are a quest plugin, we intercept all CHAT packages during an NPC conversation, and then we send the chat after it. Now after the update, all chat messages are delayed by like 3-4 seconds.
I started to debug my code, as i did not know at that moment, that it is caused by the new ProtocolLib version, and i came up with a solution for my delay problem, by changing ProtocolLibrary.getProtocolManager().getAsynchronousManager().registerAsyncHandler(packetAdapter) to ProtocolLibrary.getProtocolManager().addPacketListener(packetAdapter).
But than the PacketAdapter was not called at all for packets, that was sent by an async context. So i started to sync some things, but things like the player chat messages are still async.

To Reproduce
I think there is no easy way to reproduce this. But with BetonQuest, ProtocolLib, Citizens, and an other plugin intercepting packets it could work. But this is not simple.

I guess it should also work with an example like in the wiki: https://github.com/dmulloy2/ProtocolLib/wiki/Packet-listeners-and-adapters

If you still want more information to reproduce, i can may provide a zip with a setup,.

Expected behavior
i think the best description could be the same behavior as in ProtocolLib 4.8.0 and before.

Screenshots

Version Info
dump-2022-04-11_23.17.16.txt

Additional context
You can view our original code here: https://github.com/BetonQuest/BetonQuest/blob/master/src/main/java/org/betonquest/betonquest/compatibility/protocollib/conversation/PacketInterceptor.java
And my WIP PR(that was my try) here: https://github.com/BetonQuest/BetonQuest/pull/1823/files

commented

Indeed there is something weird about packet listeners in 5.0.0-SNAPSHOT.
This is the code I used to intercept chat packets.

ProtocolLibrary.getProtocolManager().addPacketListener(new PacketAdapter(PacketAdapter.params().plugin(plugin).listenerPriority(ListenerPriority.MONITOR).types(PacketType.Play.Server.CHAT)) {
     @Override
     public void onPacketSending(PacketEvent event) {
          System.out.println("Hello World!");
     }
}

On 4.8.0 it works fine but on 5.0.0-SNAPSHOT, it seems to only be called for the first chat message sent, and then every other chat message just bypasses the listener afterwards.

commented

I hope this gets fixed so we can continue building amazing things around the plugin.

commented

This needs to be paid attention too. Huge cock block atm.

commented

@Wolf2323 do I understand this correctly, the issue only occurs when using async listener right? When you switch over to sync listeners, there are no problems at all?

commented

no, if i switch so sync, than for example chat messages from a player are not intercepted anymore at all.

If you want more assistant or something, you can also contact me on Discord and we can also talk in german, if that helps ;)

commented

I just need a picture of the problem so that when I start digging later I don't need to ask again. But that listeners are only called once is very suspicious, because I am very sure that mine are always called ๐Ÿค” . The PR that you linked which contained a temporary fix for the problem does seem to also only switch to sync listeners?

commented

That was only my first try. It seemed, that it worked, because before that try the complete chat was delayed by like 10 seconds, with that change the delay disappeared, but than the packages are not intercepted anymore.

commented

So basically, you're listening to an outbound chat packet, the listener gets called once but never again?

commented

But that listeners are only called once is very suspicious

I've set up a quick test plugin to demonstrate this. The listener seems to only fire "Hello World" for the first player chat message/async system messages. Synced system messages are still firing the listeners afterwards, but not chat messages or async system messages. I'm guessing it has something to do with the fact that it is a synced chat listener and the packets are coming in async.

(Tested on PaperMC 289 1.18.2)
Here's the pre-built testing jar: https://drive.google.com/file/d/1dFNCxXeXh1aJ1-drtSQMc1_xgR9GbWBc/view?usp=sharing
Here's the source:

package com.loohp.testing;

import com.comphenix.protocol.PacketType;
import com.comphenix.protocol.ProtocolLibrary;
import com.comphenix.protocol.events.ListenerPriority;
import com.comphenix.protocol.events.PacketAdapter;
import com.comphenix.protocol.events.PacketEvent;
import net.md_5.bungee.api.ChatColor;
import org.bukkit.event.Listener;
import org.bukkit.plugin.java.JavaPlugin;

public class Testing extends JavaPlugin implements Listener {

	public static Testing plugin;

	@Override
	public void onEnable() {
		plugin = this;
		getServer().getPluginManager().registerEvents(this, this);

		ProtocolLibrary.getProtocolManager().addPacketListener(new PacketAdapter(PacketAdapter.params().plugin(this).listenerPriority(ListenerPriority.MONITOR).types(PacketType.Play.Server.CHAT)) {
			@Override
			public void onPacketSending(PacketEvent event) {
				System.out.println("Hello World!");
			}
		});

		getServer().getConsoleSender().sendMessage(ChatColor.GREEN + "[Testing] Testing has been enabled!");
	}

}
commented

So we're talking about outbound packets which are only intercepted once, and then never again. And it's only chat or all listeners?

commented

We only use this with PacketType.Play.Server.CHAT so i can only confirm it for this

commented

So.... did a bit of digging and actually found the issue, fixed another one when you have async and sync listeners registered on the fly. Do you mind to test that out? https://github.com/derklaro/ProtocolLib/suites/6346677241/artifacts/228802991 is the direct download of the protocol build output (fix for interested people: derklaro@26c44fb)

Would appreciate some feedback!

commented

It seems to fix everything. Thank you very much, i appreciate your work!

commented

Yup, just tested it. Works great! Thanks for the fix!