From 390c21f6d55c342717e907bbb86d76324379502a Mon Sep 17 00:00:00 2001 From: Dan Mulloy Date: Thu, 6 Jul 2017 13:56:30 -0400 Subject: [PATCH] Add PacketEvent#isPlayerTemporary, check for player updates This should address issues with temporary players by hopefully returning them less often --- .../protocol/events/PacketEvent.java | 37 ++++++++++++++++++- .../injector/server/SocketInjector.java | 4 +- .../injector/server/TemporaryPlayer.java} | 12 +++--- .../injector/netty/ChannelInjector.java | 2 +- .../server/AbstractInputStreamLookup.java | 2 +- .../server/TemporaryPlayerFactory.java | 14 +++---- 6 files changed, 53 insertions(+), 18 deletions(-) rename modules/{ProtocolLib => API}/src/main/java/com/comphenix/protocol/injector/server/SocketInjector.java (100%) rename modules/{ProtocolLib/src/main/java/com/comphenix/protocol/injector/server/InjectorContainer.java => API/src/main/java/com/comphenix/protocol/injector/server/TemporaryPlayer.java} (51%) diff --git a/modules/API/src/main/java/com/comphenix/protocol/events/PacketEvent.java b/modules/API/src/main/java/com/comphenix/protocol/events/PacketEvent.java index 644b119e..bb4c0caf 100644 --- a/modules/API/src/main/java/com/comphenix/protocol/events/PacketEvent.java +++ b/modules/API/src/main/java/com/comphenix/protocol/events/PacketEvent.java @@ -33,6 +33,7 @@ import com.comphenix.protocol.async.AsyncMarker; import com.comphenix.protocol.error.PluginContext; import com.comphenix.protocol.error.Report; import com.comphenix.protocol.error.ReportType; +import com.comphenix.protocol.injector.server.TemporaryPlayer; import com.google.common.base.Objects; import com.google.common.base.Preconditions; import com.google.common.collect.HashMultimap; @@ -321,7 +322,41 @@ public class PacketEvent extends EventObject implements Cancellable { * @return The player associated with this event. */ public Player getPlayer() { - return playerReference.get(); + Player player = playerReference.get(); + + // Check if the player has been updated so we can do more stuff + if (player instanceof TemporaryPlayer) { + Player updated = player.getPlayer(); + if (updated != null && !(updated instanceof TemporaryPlayer)) { + playerReference.clear(); + playerReference = new WeakReference<>(updated); + return updated; + } + } + + return player; + } + + /** + * Whether or not the player in this PacketEvent is temporary (i.e. they don't have a true player instance yet). + * Temporary players have a limited subset of methods that may be used: + * + * + * Anything else will throw an UnsupportedOperationException. Use this check before calling other methods when + * dealing with packets early in the login sequence or if you get the aforementioned exception. + * + * @return True if the player is temporary, false if not. + */ + public boolean isPlayerTemporary() { + return getPlayer() instanceof TemporaryPlayer; } /** diff --git a/modules/ProtocolLib/src/main/java/com/comphenix/protocol/injector/server/SocketInjector.java b/modules/API/src/main/java/com/comphenix/protocol/injector/server/SocketInjector.java similarity index 100% rename from modules/ProtocolLib/src/main/java/com/comphenix/protocol/injector/server/SocketInjector.java rename to modules/API/src/main/java/com/comphenix/protocol/injector/server/SocketInjector.java index 3c08a0de..9383f6d5 100644 --- a/modules/ProtocolLib/src/main/java/com/comphenix/protocol/injector/server/SocketInjector.java +++ b/modules/API/src/main/java/com/comphenix/protocol/injector/server/SocketInjector.java @@ -4,10 +4,10 @@ import java.lang.reflect.InvocationTargetException; import java.net.Socket; import java.net.SocketAddress; -import org.bukkit.entity.Player; - import com.comphenix.protocol.events.NetworkMarker; +import org.bukkit.entity.Player; + /** * Represents an injector that only gives access to a player's socket. * diff --git a/modules/ProtocolLib/src/main/java/com/comphenix/protocol/injector/server/InjectorContainer.java b/modules/API/src/main/java/com/comphenix/protocol/injector/server/TemporaryPlayer.java similarity index 51% rename from modules/ProtocolLib/src/main/java/com/comphenix/protocol/injector/server/InjectorContainer.java rename to modules/API/src/main/java/com/comphenix/protocol/injector/server/TemporaryPlayer.java index 560b0176..08a419b7 100644 --- a/modules/ProtocolLib/src/main/java/com/comphenix/protocol/injector/server/InjectorContainer.java +++ b/modules/API/src/main/java/com/comphenix/protocol/injector/server/TemporaryPlayer.java @@ -1,19 +1,19 @@ package com.comphenix.protocol.injector.server; /** - * Able to store a socket injector. + * A temporary player created by ProtocolLib when a true player instance does not exist. *

- * A necessary hack. - * @author Kristian + * Also able to store a socket injector + *

*/ -class InjectorContainer { +public class TemporaryPlayer { private volatile SocketInjector injector; - public SocketInjector getInjector() { + SocketInjector getInjector() { return injector; } - public void setInjector(SocketInjector injector) { + void setInjector(SocketInjector injector) { if (injector == null) throw new IllegalArgumentException("Injector cannot be NULL."); this.injector = injector; diff --git a/modules/ProtocolLib/src/main/java/com/comphenix/protocol/injector/netty/ChannelInjector.java b/modules/ProtocolLib/src/main/java/com/comphenix/protocol/injector/netty/ChannelInjector.java index 7d01c21a..d5551c7b 100644 --- a/modules/ProtocolLib/src/main/java/com/comphenix/protocol/injector/netty/ChannelInjector.java +++ b/modules/ProtocolLib/src/main/java/com/comphenix/protocol/injector/netty/ChannelInjector.java @@ -910,7 +910,7 @@ public class ChannelInjector extends ByteToMessageDecoder implements Injector { @Override public Player getUpdatedPlayer() { - return injector.updated; + return injector.updated != null ? injector.updated : getPlayer(); } @Override diff --git a/modules/ProtocolLib/src/main/java/com/comphenix/protocol/injector/server/AbstractInputStreamLookup.java b/modules/ProtocolLib/src/main/java/com/comphenix/protocol/injector/server/AbstractInputStreamLookup.java index e5518418..496d82b2 100644 --- a/modules/ProtocolLib/src/main/java/com/comphenix/protocol/injector/server/AbstractInputStreamLookup.java +++ b/modules/ProtocolLib/src/main/java/com/comphenix/protocol/injector/server/AbstractInputStreamLookup.java @@ -71,7 +71,7 @@ public abstract class AbstractInputStreamLookup { Player player = previous.getPlayer(); // Default implementation - if (player instanceof InjectorContainer) { + if (player instanceof TemporaryPlayer) { TemporaryPlayerFactory.setInjectorInPlayer(player, current); } } diff --git a/modules/ProtocolLib/src/main/java/com/comphenix/protocol/injector/server/TemporaryPlayerFactory.java b/modules/ProtocolLib/src/main/java/com/comphenix/protocol/injector/server/TemporaryPlayerFactory.java index 6e6f0929..052797f8 100644 --- a/modules/ProtocolLib/src/main/java/com/comphenix/protocol/injector/server/TemporaryPlayerFactory.java +++ b/modules/ProtocolLib/src/main/java/com/comphenix/protocol/injector/server/TemporaryPlayerFactory.java @@ -47,8 +47,8 @@ public class TemporaryPlayerFactory { * @return The referenced player injector, or NULL if none can be found. */ public static SocketInjector getInjectorFromPlayer(Player player) { - if (player instanceof InjectorContainer) { - return ((InjectorContainer) player).getInjector(); + if (player instanceof TemporaryPlayer) { + return ((TemporaryPlayer) player).getInjector(); } return null; } @@ -59,7 +59,7 @@ public class TemporaryPlayerFactory { * @param injector - the injector to store. */ public static void setInjectorInPlayer(Player player, SocketInjector injector) { - ((InjectorContainer) player).setInjector(injector); + ((TemporaryPlayer) player).setInjector(injector); } /** @@ -88,7 +88,7 @@ public class TemporaryPlayerFactory { @Override public Object intercept(Object obj, Method method, Object[] args, MethodProxy proxy) throws Throwable { String methodName = method.getName(); - SocketInjector injector = ((InjectorContainer) obj).getInjector(); + SocketInjector injector = ((TemporaryPlayer) obj).getInjector(); if (injector == null) throw new IllegalStateException("Unable to find injector."); @@ -152,7 +152,7 @@ public class TemporaryPlayerFactory { public int accept(Method method) { // Do not override the object method or the superclass methods if (method.getDeclaringClass().equals(Object.class) || - method.getDeclaringClass().equals(InjectorContainer.class)) + method.getDeclaringClass().equals(TemporaryPlayer.class)) return 0; else return 1; @@ -162,7 +162,7 @@ public class TemporaryPlayerFactory { // CGLib is amazing Enhancer ex = new Enhancer(); - ex.setSuperclass(InjectorContainer.class); + ex.setSuperclass(TemporaryPlayer.class); ex.setInterfaces(new Class[] { Player.class }); ex.setCallbacks(new Callback[] { NoOp.INSTANCE, implementation }); ex.setCallbackFilter(callbackFilter); @@ -179,7 +179,7 @@ public class TemporaryPlayerFactory { public Player createTemporaryPlayer(Server server, SocketInjector injector) { Player temporary = createTemporaryPlayer(server); - ((InjectorContainer) temporary).setInjector(injector); + ((TemporaryPlayer) temporary).setInjector(injector); return temporary; }