Improve injector listener tracking and fix terminal packets breaking injection

[Ingrim4]

Description

This PR marks the initial phase of enhancements for the injector, addressing #2919. Additionally, it resolves an issue with inbound terminal packets like ClientIntentionPacket and ServerboundFinishConfigurationPacket. ProtocolLib is unable to retrieve the inbound PacketType.Protocol for these packets, a consequence of the PacketDecoder removing itself when a decoded packet is terminal. While indirectly related to #2933 and #2945, this PR distinguishes itself by implementing a (probably) accurate/robust solution for the underlying problem.

This PR primarily focuses on revamping the registration process for packet listeners. Instead of maintaining multiple lists of packet listeners per packet type and sets of packet types, we now primarily use two PacketTypeMultiMaps to track the registered packet listeners. I also reworked the collections used to track packet listeners, addressing several issues such as race conditions, performance regressions, and overall readability. The new implementation ensures thread-safe modifications to the collections and uses locks only when necessary, resulting in faster read operations. Additionally, the new code is more readable and accurately represents the current injector.

Related Issue

—

How Has This Been Tested?

Tested with latest spigot 1.20.6 build with latest build of orebfuscator for 1.20.5. I couldn't observe any obvious errors.

Types of Changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

[Ingrim4]

I also considered removing some more internal interfaces. Although they provide useful abstraction, they are outdated and poorly maintained. Moreover, they are only used within internal code, making them somewhat obsolete. I wanted to get your opinion on this matter, @derklaro. Here is a list of the interfaces:

• ChannelListener could get replaced by it's implementation NetworkManagerInjector
• ListenerInvoker could get replaced by it's implementation PacketFilterManager
• PlayerInjectionHandler could get moved to NetworkManagerInjector
  • if this is moved to NetworkManagerInjector then it's implementation could also be removed NetworkManagerPlayerInjector

The only interfaces I removed so far wouldn't have any functionality in the new system. I would also move some dependencies from methods calls into the constructor such as the NetworkManagerInjector because it's effectively a singleton.

[derklaro]

Gonna take a first look later, but I would like to subject to using syncronized over a concurrent map/set. It's just easier and scales better than using syncronized. Just to put it here as well: computeIfAbsent in ConcurrentHashMap locks the map even if the entry already exists in JDK8, should use something else in that case :)

[Ingrim4]

Gonna take a first look later, but I would like to subject to using syncronized over a concurrent map/set. It's just easier and scales better than using syncronized. Just to put it here as well: computeIfAbsent in ConcurrentHashMap locks the map even if the entry already exists in JDK8, should use something else in that case :)

I don't believe syncronized is an issue in this context because we only synchronize methods that are invoked when a listener is registered or unregistered, which occurs infrequently. Additionally, it is impossible to safely remove an entry from the map without some form of exclusive locking because there's no guarantee that another thread hasn't registered a new listener between the isEmpty check and the removal operation.

[Ingrim4]

This method should now handle null packets and complain about naughty plugins.

[Ingrim4]

This should now be fine without any warning etc.