Add PacketEvent#isPlayerTemporary, check for player updates

This should address issues with temporary players by hopefully returning them less often
This commit is contained in:
Dan Mulloy 2017-07-06 13:56:30 -04:00
parent 20d78832b0
commit 390c21f6d5
6 changed files with 53 additions and 18 deletions

View File

@ -33,6 +33,7 @@ import com.comphenix.protocol.async.AsyncMarker;
import com.comphenix.protocol.error.PluginContext; import com.comphenix.protocol.error.PluginContext;
import com.comphenix.protocol.error.Report; import com.comphenix.protocol.error.Report;
import com.comphenix.protocol.error.ReportType; import com.comphenix.protocol.error.ReportType;
import com.comphenix.protocol.injector.server.TemporaryPlayer;
import com.google.common.base.Objects; import com.google.common.base.Objects;
import com.google.common.base.Preconditions; import com.google.common.base.Preconditions;
import com.google.common.collect.HashMultimap; import com.google.common.collect.HashMultimap;
@ -321,7 +322,41 @@ public class PacketEvent extends EventObject implements Cancellable {
* @return The player associated with this event. * @return The player associated with this event.
*/ */
public Player getPlayer() { 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:
* <ul>
* <li>getPlayer</li>
* <li>getAddress</li>
* <li>getServer</li>
* <li>chat</li>
* <li>sendMessage</li>
* <li>kickPlayer</li>
* <li>isOnline</li>
* </ul>
*
* 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;
} }
/** /**

View File

@ -4,10 +4,10 @@ import java.lang.reflect.InvocationTargetException;
import java.net.Socket; import java.net.Socket;
import java.net.SocketAddress; import java.net.SocketAddress;
import org.bukkit.entity.Player;
import com.comphenix.protocol.events.NetworkMarker; import com.comphenix.protocol.events.NetworkMarker;
import org.bukkit.entity.Player;
/** /**
* Represents an injector that only gives access to a player's socket. * Represents an injector that only gives access to a player's socket.
* *

View File

@ -1,19 +1,19 @@
package com.comphenix.protocol.injector.server; 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.
* <p> * <p>
* A necessary hack. * Also able to store a socket injector
* @author Kristian * </p>
*/ */
class InjectorContainer { public class TemporaryPlayer {
private volatile SocketInjector injector; private volatile SocketInjector injector;
public SocketInjector getInjector() { SocketInjector getInjector() {
return injector; return injector;
} }
public void setInjector(SocketInjector injector) { void setInjector(SocketInjector injector) {
if (injector == null) if (injector == null)
throw new IllegalArgumentException("Injector cannot be NULL."); throw new IllegalArgumentException("Injector cannot be NULL.");
this.injector = injector; this.injector = injector;

View File

@ -910,7 +910,7 @@ public class ChannelInjector extends ByteToMessageDecoder implements Injector {
@Override @Override
public Player getUpdatedPlayer() { public Player getUpdatedPlayer() {
return injector.updated; return injector.updated != null ? injector.updated : getPlayer();
} }
@Override @Override

View File

@ -71,7 +71,7 @@ public abstract class AbstractInputStreamLookup {
Player player = previous.getPlayer(); Player player = previous.getPlayer();
// Default implementation // Default implementation
if (player instanceof InjectorContainer) { if (player instanceof TemporaryPlayer) {
TemporaryPlayerFactory.setInjectorInPlayer(player, current); TemporaryPlayerFactory.setInjectorInPlayer(player, current);
} }
} }

View File

@ -47,8 +47,8 @@ public class TemporaryPlayerFactory {
* @return The referenced player injector, or NULL if none can be found. * @return The referenced player injector, or NULL if none can be found.
*/ */
public static SocketInjector getInjectorFromPlayer(Player player) { public static SocketInjector getInjectorFromPlayer(Player player) {
if (player instanceof InjectorContainer) { if (player instanceof TemporaryPlayer) {
return ((InjectorContainer) player).getInjector(); return ((TemporaryPlayer) player).getInjector();
} }
return null; return null;
} }
@ -59,7 +59,7 @@ public class TemporaryPlayerFactory {
* @param injector - the injector to store. * @param injector - the injector to store.
*/ */
public static void setInjectorInPlayer(Player player, SocketInjector injector) { public static void setInjectorInPlayer(Player player, SocketInjector injector) {
((InjectorContainer) player).setInjector(injector); ((TemporaryPlayer) player).setInjector(injector);
} }
/** /**
@ -88,7 +88,7 @@ public class TemporaryPlayerFactory {
@Override @Override
public Object intercept(Object obj, Method method, Object[] args, MethodProxy proxy) throws Throwable { public Object intercept(Object obj, Method method, Object[] args, MethodProxy proxy) throws Throwable {
String methodName = method.getName(); String methodName = method.getName();
SocketInjector injector = ((InjectorContainer) obj).getInjector(); SocketInjector injector = ((TemporaryPlayer) obj).getInjector();
if (injector == null) if (injector == null)
throw new IllegalStateException("Unable to find injector."); throw new IllegalStateException("Unable to find injector.");
@ -152,7 +152,7 @@ public class TemporaryPlayerFactory {
public int accept(Method method) { public int accept(Method method) {
// Do not override the object method or the superclass methods // Do not override the object method or the superclass methods
if (method.getDeclaringClass().equals(Object.class) || if (method.getDeclaringClass().equals(Object.class) ||
method.getDeclaringClass().equals(InjectorContainer.class)) method.getDeclaringClass().equals(TemporaryPlayer.class))
return 0; return 0;
else else
return 1; return 1;
@ -162,7 +162,7 @@ public class TemporaryPlayerFactory {
// CGLib is amazing // CGLib is amazing
Enhancer ex = new Enhancer(); Enhancer ex = new Enhancer();
ex.setSuperclass(InjectorContainer.class); ex.setSuperclass(TemporaryPlayer.class);
ex.setInterfaces(new Class[] { Player.class }); ex.setInterfaces(new Class[] { Player.class });
ex.setCallbacks(new Callback[] { NoOp.INSTANCE, implementation }); ex.setCallbacks(new Callback[] { NoOp.INSTANCE, implementation });
ex.setCallbackFilter(callbackFilter); ex.setCallbackFilter(callbackFilter);
@ -179,7 +179,7 @@ public class TemporaryPlayerFactory {
public Player createTemporaryPlayer(Server server, SocketInjector injector) { public Player createTemporaryPlayer(Server server, SocketInjector injector) {
Player temporary = createTemporaryPlayer(server); Player temporary = createTemporaryPlayer(server);
((InjectorContainer) temporary).setInjector(injector); ((TemporaryPlayer) temporary).setInjector(injector);
return temporary; return temporary;
} }