This is because multiple plugins depend on us, and are not properly
notified after ProtocolLib has been reloaded.
The only possible solution is to reload every dependent plugin after
ProtocolLib has been reloaded, but unfortunately, I ran into
LinkageErrors when I tried it. So it's probably not possible with the
current architecture to support reloaders.
Instead, we'll simply print a BIG BOLD warning telling any users of
these plugins that ProtocolLib cannot be reloaded except through the
built in "/reload" command.
Previously, we have used a BlockingHashMap to simply lock the packet
read thread until we have had a chance to intercept the
NetLoginHandler/PendingConnection and store InputStream ->
PlayerInjector -> TemporaryPlayer.
Problem is, this could potentially cause problems if, for some reason, a
packet is intercepted after the player has logged out and the player
injector has been removed from the lookup map. In that case, the read
thread would wait until it reaches the default timeout of 2 seconds.
Locking threads is fairly inefficient in general, and waiting for the
server connection thread to update the NetLoginHandler list could take a
while.
Instead, ProtocolLib will now intercept any Socket accepted in the
server's main ServerSocket, and record any calls to getInputStream().
That way, we can get a InputStream -> Socket mapping before the server
thread ever creates the read and write threads in NetLoginHandler ->
NetworkManager.
Unfortunately, it's not trivial to swap out the ServerSocket in the
DedicatedServerConnectionThread - we actually have to trigger the
accept() thread and move through a cycle of the loop before our custom
ServerSocket is used. To do this, we will actually connect to the server
and read its MOTD manually, hopefully getting to it before any other
players.
This creates a slight overhead of a couple of threads per server start,
but it's probably much better than locking the read thread. More testing
is needed though before this can be confirmed.
Perhaps NbtBase shouldn't have implemented getValue() after all - it
would have been better to have a shared base interface with getName()
and getType(), and only let the primitive elements implement getValue().
Too late to change it now though.
Added a missing remove method in NbtCompound. In addition, the
getValue() method in NbtCompount has been depreciated. It is far better
to use the put and get methods in NbtCompound instead.
This required extensive reworking of the inner packet injection system
in ProtocolLib.
I've also fixed a minior bug in the fuzzy member contract class.
Our custom IOC system (DefaultInstances) would call new Block(0) on a
MCPC based CraftBukkit system, causing air blocks to be treated as
solid blocks. We prevented this by explicitly forbidding it from
creating instances of ItemStack or Block in our structure modifier's
default field generator.
A second bug was caused by mismatched methods in WrappedDataWatcher,
which we solved by simply trying to use them, and then swapping them
if we encounter any errors or weird state.
It is now possible to specify exactly what method or field you're
looking for, based on a number of different critera such as return
value, parameter count or parameter type, exceptions, modifiers, name -
all using a very fluent builder syntax.
The entity modifier would try to cache each equivalent converter by its
specific type - i.e. whether or not it converts entities or watchable
collections - but this isn't enough to distinguish entity modifiers
as they store a hidden reference to the world the entity is supposed
to be occupying.
Thus, after the first entity modifier is used for "world", it would
be subsequently cached for every world ("world_nether", "world_end",
etc.). The end result was that entity modifiers would only work for
one world.
This patch fixes this problem by adding a proper equals() method to
the entity equivalent converter class, along with all the other
equvialent converter classes, in case it should prove to become useful
in the future.
We'll move the "load class" call to the main thread, hopefully fixing
ticket 27. The secondary thread will still call the "define class"
protected method, which may not be thread safe.