From 96ad954cf725be77c3a565c147399079b18c8088 Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Tue, 23 Oct 2012 00:37:35 +0200 Subject: [PATCH] Prevent the PlayerQuitEvent from being fired twice. FIXES Ticket-2. --- ProtocolLib/dependency-reduced-pom.xml | 2 +- .../injector/PacketFilterManager.java | 13 +++++-- .../player/InjectedServerConnection.java | 2 +- .../injector/player/NetworkFieldInjector.java | 5 +++ .../player/NetworkObjectInjector.java | 5 +++ .../player/NetworkServerInjector.java | 37 +++++++++++++++++++ .../player/PlayerInjectionHandler.java | 12 ++++++ .../injector/player/PlayerInjector.java | 5 +++ .../injector/player/ReplacedArrayList.java | 32 +++++++++++++++- 9 files changed, 105 insertions(+), 8 deletions(-) diff --git a/ProtocolLib/dependency-reduced-pom.xml b/ProtocolLib/dependency-reduced-pom.xml index 9ab24145..657d089f 100644 --- a/ProtocolLib/dependency-reduced-pom.xml +++ b/ProtocolLib/dependency-reduced-pom.xml @@ -4,7 +4,7 @@ com.comphenix.protocol ProtocolLib ProtocolLib - 1.4.2 + 1.4.3-SNAPSHOT Provides read/write access to the Minecraft protocol. http://dev.bukkit.org/server-mods/protocollib/ diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketFilterManager.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketFilterManager.java index b4af084f..afbb5fcd 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketFilterManager.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketFilterManager.java @@ -606,6 +606,7 @@ public final class PacketFilterManager implements ProtocolManager, ListenerInvok @EventHandler(priority = EventPriority.MONITOR, ignoreCancelled = true) public void onPlayerQuit(PlayerQuitEvent event) { + playerInjection.handleDisconnect(event.getPlayer()); playerInjection.uninjectPlayer(event.getPlayer()); } @@ -689,10 +690,14 @@ public final class PacketFilterManager implements ProtocolManager, ListenerInvok Object event = args[0]; // Check for the correct event - if (event instanceof PlayerJoinEvent) - playerInjection.injectPlayer(((PlayerJoinEvent) event).getPlayer()); - else if (event instanceof PlayerQuitEvent) - playerInjection.uninjectPlayer(((PlayerQuitEvent) event).getPlayer()); + if (event instanceof PlayerJoinEvent) { + Player player = ((PlayerJoinEvent) event).getPlayer(); + playerInjection.injectPlayer(player); + } else if (event instanceof PlayerQuitEvent) { + Player player = ((PlayerQuitEvent) event).getPlayer(); + playerInjection.handleDisconnect(player); + playerInjection.uninjectPlayer(player); + } } return null; } diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/InjectedServerConnection.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/InjectedServerConnection.java index d65fd405..83d61646 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/InjectedServerConnection.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/InjectedServerConnection.java @@ -241,7 +241,7 @@ class InjectedServerConnection { // Clean up? if (removing instanceof NetLoginHandler) { netLoginInjector.cleanup(removing); - } + } } }; } diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetworkFieldInjector.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetworkFieldInjector.java index 10bf4f1d..19104e74 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetworkFieldInjector.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetworkFieldInjector.java @@ -191,6 +191,11 @@ class NetworkFieldInjector extends PlayerInjector { overridenLists.clear(); } + @Override + public void handleDisconnect() { + // No need to do anything + } + @Override public boolean canInject(GamePhase phase) { // All phases should work diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetworkObjectInjector.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetworkObjectInjector.java index 9cf12a21..e0b77d46 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetworkObjectInjector.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetworkObjectInjector.java @@ -159,6 +159,11 @@ class NetworkObjectInjector extends PlayerInjector { } } + @Override + public void handleDisconnect() { + // No need to do anything + } + @Override public boolean canInject(GamePhase phase) { // Works for all phases diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetworkServerInjector.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetworkServerInjector.java index 9872947a..1fcb28ed 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetworkServerInjector.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/NetworkServerInjector.java @@ -17,8 +17,10 @@ package com.comphenix.protocol.injector.player; +import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.util.logging.Level; import java.util.logging.Logger; import net.minecraft.server.Packet; @@ -50,6 +52,7 @@ import com.comphenix.protocol.reflect.instances.ExistingGenerator; */ public class NetworkServerInjector extends PlayerInjector { + private static Field disconnectField; private static Method sendPacketMethod; private InjectedServerConnection serverInjection; @@ -59,6 +62,9 @@ public class NetworkServerInjector extends PlayerInjector { // Used to create proxy objects private ClassLoader classLoader; + // Whether or not the player has disconnected + private boolean hasDisconnected; + public NetworkServerInjector( ClassLoader classLoader, Logger logger, Player player, ListenerInvoker invoker, IntegerSet sendingFilters, @@ -250,11 +256,42 @@ public class NetworkServerInjector extends PlayerInjector { } catch (IllegalAccessException e) { e.printStackTrace(); } + + // Prevent the PlayerQuitEvent from being sent twice + if (hasDisconnected) { + setDisconnect(serverHandlerRef.getValue(), true); + } } serverInjection.revertServerHandler(serverHandler); } + @Override + public void handleDisconnect() { + hasDisconnected = true; + } + + /** + * Set the disconnected field in a NetServerHandler. + * @param handler - the NetServerHandler. + * @param value - the new value. + */ + private void setDisconnect(Object handler, boolean value) { + // Set it + try { + // Load the field + if (disconnectField == null) { + disconnectField = FuzzyReflection.fromObject(handler).getFieldByName("disconnected.*"); + } + FieldUtils.writeField(disconnectField, handler, value); + + } catch (IllegalArgumentException e) { + logger.log(Level.WARNING, "Unable to find disconnect field. Is ProtocolLib up to date?"); + } catch (IllegalAccessException e) { + logger.log(Level.WARNING, "Unable to update disconnected field. Player quit event may be sent twice."); + } + } + @Override public void checkListener(PacketListener listener) { // We support everything diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjectionHandler.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjectionHandler.java index 14b3279a..e601dc72 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjectionHandler.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjectionHandler.java @@ -358,6 +358,18 @@ public class PlayerInjectionHandler { } } + /** + * Invoke special routines for handling disconnect before a player is uninjected. + * @param player - player to process. + */ + public void handleDisconnect(Player player) { + PlayerInjector injector = getInjector(player); + + if (injector != null) { + injector.handleDisconnect(); + } + } + /** * Unregisters the given player. * @param player - player to unregister. diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjector.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjector.java index 739df11c..a4c2d605 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjector.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjector.java @@ -451,6 +451,11 @@ abstract class PlayerInjector { cleanHook(); clean = true; } + + /** + * Clean up after the player has disconnected. + */ + public abstract void handleDisconnect(); /** * Override to add custom cleanup behavior. diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/ReplacedArrayList.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/ReplacedArrayList.java index 6ebb9cb7..8fe8b3e3 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/ReplacedArrayList.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/ReplacedArrayList.java @@ -68,7 +68,7 @@ class ReplacedArrayList extends ArrayList { } /** - * Invoksed when an element is being removed. + * Invoked when an element is being removed. * @param removing - the element being removed. */ protected void onRemoved(TKey removing) { @@ -264,6 +264,15 @@ class ReplacedArrayList extends ArrayList { addMapping(target, replacement, false); } + /** + * Retrieve the old value, if it exists. + * @param target - the key. + * @return The value that was replaced, or NULL. + */ + public TKey getMapping(TKey target) { + return replaceMap.get(target); + } + /** * Add a replace rule. *

@@ -284,8 +293,9 @@ class ReplacedArrayList extends ArrayList { /** * Revert the given mapping. * @param target - the instance we replaced. + * @return The old mapped value, or NULL if nothing was replaced. */ - public synchronized void removeMapping(TKey target) { + public synchronized TKey removeMapping(TKey target) { // Make sure the mapping exist if (replaceMap.containsKey(target)) { TKey replacement = replaceMap.get(target); @@ -293,7 +303,25 @@ class ReplacedArrayList extends ArrayList { // Revert existing elements replaceAll(replacement, target); + return replacement; } + return null; + } + + /** + * Swap the new replaced value with its old value. + * @param target - the instance we replaced. + * @param The old mapped value, or NULL if nothing was swapped. + */ + public synchronized TKey swapMapping(TKey target) { + // Make sure the mapping exist + TKey replacement = removeMapping(target); + + // Add the reverse + if (replacement != null) { + replaceMap.put(replacement, target); + } + return replacement; } /**