From 125bfbad302a7a93fef62e306f32a8e9c5f588a8 Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Sat, 23 Nov 2013 05:00:34 +0100 Subject: [PATCH] TemporaryPlayer doesn't support getName(), so use getAddress() for maps In addition, adding a useful ConcurrentPlayerMap that doesn't store player instances directly, and can even handle TemporaryPlayers. --- .../protocol/async/PlayerSendingHandler.java | 17 +- .../concurrency/ConcurrentPlayerMap.java | 325 ++++++++++++++++++ .../player/ProxyPlayerInjectionHandler.java | 4 +- 3 files changed, 334 insertions(+), 12 deletions(-) create mode 100644 ProtocolLib/src/main/java/com/comphenix/protocol/concurrency/ConcurrentPlayerMap.java diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/async/PlayerSendingHandler.java b/ProtocolLib/src/main/java/com/comphenix/protocol/async/PlayerSendingHandler.java index e2aa0f09..6954504e 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/async/PlayerSendingHandler.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/async/PlayerSendingHandler.java @@ -19,13 +19,14 @@ package com.comphenix.protocol.async; import java.util.ArrayList; import java.util.List; -import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; import java.util.concurrent.Executor; import java.util.concurrent.Executors; import java.util.concurrent.ThreadFactory; import org.bukkit.entity.Player; +import com.comphenix.protocol.concurrency.ConcurrentPlayerMap; import com.comphenix.protocol.error.ErrorReporter; import com.comphenix.protocol.events.PacketEvent; import com.comphenix.protocol.injector.SortedPacketListenerList; @@ -37,9 +38,8 @@ import com.google.common.util.concurrent.ThreadFactoryBuilder; * @author Kristian */ class PlayerSendingHandler { - private ErrorReporter reporter; - private ConcurrentHashMap playerSendingQueues; + private ConcurrentMap playerSendingQueues; // Timeout listeners private SortedPacketListenerList serverTimeoutListeners; @@ -105,7 +105,7 @@ class PlayerSendingHandler { this.clientTimeoutListeners = clientTimeoutListeners; // Initialize storage of queues - this.playerSendingQueues = new ConcurrentHashMap(); + this.playerSendingQueues = ConcurrentPlayerMap.usingAddress(); } /** @@ -137,15 +137,14 @@ class PlayerSendingHandler { * @return The server or client sending queue the packet belongs to. */ public PacketSendingQueue getSendingQueue(PacketEvent packet, boolean createNew) { - String name = packet.getPlayer().getName(); - QueueContainer queues = playerSendingQueues.get(name); + QueueContainer queues = playerSendingQueues.get(packet.getPlayer()); // Safe concurrent initialization if (queues == null && createNew) { final QueueContainer newContainer = new QueueContainer(); // Attempt to map the queue - queues = playerSendingQueues.putIfAbsent(name, newContainer); + queues = playerSendingQueues.putIfAbsent(packet.getPlayer(), newContainer); if (queues == null) { queues = newContainer; @@ -258,9 +257,7 @@ class PlayerSendingHandler { * @param player - the player that just logged out. */ public void removePlayer(Player player) { - String name = player.getName(); - // Every packet will be dropped - there's nothing we can do - playerSendingQueues.remove(name); + playerSendingQueues.remove(player); } } diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/concurrency/ConcurrentPlayerMap.java b/ProtocolLib/src/main/java/com/comphenix/protocol/concurrency/ConcurrentPlayerMap.java new file mode 100644 index 00000000..66694b04 --- /dev/null +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/concurrency/ConcurrentPlayerMap.java @@ -0,0 +1,325 @@ +package com.comphenix.protocol.concurrency; + +import java.util.AbstractMap; +import java.util.AbstractSet; +import java.util.Iterator; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.ExecutionException; + +import org.bukkit.Bukkit; +import org.bukkit.entity.Player; + +import com.google.common.base.Function; +import com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; +import com.google.common.cache.CacheLoader; +import com.google.common.cache.RemovalListener; +import com.google.common.cache.RemovalNotification; +import com.google.common.collect.AbstractIterator; +import com.google.common.collect.Maps; +import com.google.common.util.concurrent.UncheckedExecutionException; + +/** + * Represents a concurrent player map. + *

+ * This map may use player addresses as keys. + * @author Kristian + */ +public class ConcurrentPlayerMap extends AbstractMap implements ConcurrentMap { + /** + * Represents the different standard player keys, + * @author Kristian + */ + public enum PlayerKey implements Function { + /** + * Use a player's {@link Player#getAddress()} as key in the map. + */ + ADDRESS { + @Override + public Object apply(Player player) { + return player.getAddress(); + } + }, + + /** + * Use a player's {@link Player#getName()} as key in the map. + */ + NAME { + @Override + public Object apply(Player player) { + return player.getName(); + } + }, + } + + /** + * An internal map of player keys to values. + */ + protected ConcurrentMap valueLookup = createValueMap(); + + /** + * A cache of the associated keys for each player. + */ + protected Cache keyLookup = createKeyCache(); + + /** + * The method used to retrieve a unique key for a player. + */ + protected final Function keyMethod; + + /** + * Construct a new concurrent player map that uses each player's address as key. + * @return Concurrent player map. + */ + public static ConcurrentPlayerMap usingAddress() { + return new ConcurrentPlayerMap(PlayerKey.ADDRESS); + } + + /** + * Construct a new concurrent player map that uses each player's name as key. + * @return Concurrent player map. + */ + public static ConcurrentPlayerMap usingName() { + return new ConcurrentPlayerMap(PlayerKey.NAME); + } + + /** + * Construct a new concurrent player map using the given standard key method. + * @param standardMethod - the standard key method. + */ + public ConcurrentPlayerMap(PlayerKey standardMethod) { + this.keyMethod = standardMethod; + } + + /** + * Construct a new concurrent player map using the given custom key method. + * @param method - custom key method. + */ + public ConcurrentPlayerMap(Function method) { + this.keyMethod = method; + } + + /** + * Construct the map that will store the associated values. + *

+ * The default implementation uses a {@link ConcurrentHashMap}. + * @return The value map. + */ + protected ConcurrentMap createValueMap() { + return Maps.newConcurrentMap(); + } + + /** + * Construct a cache of keys and the associated player. + * @return The key map. + */ + protected Cache createKeyCache() { + return CacheBuilder.newBuilder(). + weakValues(). + removalListener( + new RemovalListener() { + @Override + public void onRemoval(RemovalNotification removed) { + // We ignore explicit removal + if (removed.wasEvicted()) { + onCacheEvicted(removed.getKey()); + } + } + }). + build( + new CacheLoader() { + @Override + public Player load(Object key) throws Exception { + Player player = findOnlinePlayer(key); + + if (player != null) + return player; + + // Per the contract, this method should not return NULL + throw new IllegalArgumentException( + "Unable to find a player associated with: " + key); + } + }); + } + + /** + * Invoked when an entry in the cache has been evicted, typically by the garbage collector. + * @param key - the key. + * @param player - the value that was evicted or collected. + */ + private void onCacheEvicted(Object key) { + Player newPlayer = findOnlinePlayer(key); + + if (newPlayer != null) { + // Update the reference + keyLookup.asMap().put(key, newPlayer); + } else { + valueLookup.remove(key); + } + } + + /** + * Find an online player from the given key. + * @param key - a non-null key. + * @return The player with the given key, or NULL if not found. + */ + protected Player findOnlinePlayer(Object key) { + for (Player player : Bukkit.getOnlinePlayers()) { + if (key.equals(keyMethod.apply(player))) { + return player; + } + } + return null; + } + + /** + * Lookup a player by key in the cache, optionally searching every online player. + * @param key - the key of the player we are locating. + * @return The player, or NULL if not found. + * @throws ExecutionException + */ + protected Player lookupPlayer(Object key) { + try { + return keyLookup.get(key); + } catch (ExecutionException e) { + return null; + } catch (UncheckedExecutionException e) { + return null; + } + } + + /** + * Retrieve the key of a particular player, ensuring it is cached. + * @param player - the player whose key we want to retrieve. + * @return The key. + */ + protected Object cachePlayerKey(Player player) { + Object key = keyMethod.apply(player); + + keyLookup.asMap().put(key, player); + return key; + } + + @Override + public TValue put(Player key, TValue value) { + return valueLookup.put(cachePlayerKey(key), value); + } + + @Override + public TValue putIfAbsent(Player key, TValue value) { + return valueLookup.putIfAbsent(cachePlayerKey(key), value); + } + + @Override + public TValue replace(Player key, TValue value) { + return valueLookup.replace(cachePlayerKey(key), value); + } + + @Override + public boolean replace(Player key, TValue oldValue, TValue newValue) { + return valueLookup.replace(cachePlayerKey(key), oldValue, newValue); + } + + @Override + public TValue remove(Object key) { + if (key instanceof Player) { + Object playerKey = keyMethod.apply((Player) key); + TValue value = valueLookup.remove(playerKey); + + keyLookup.asMap().remove(playerKey); + return value; + } + return null; + } + + @Override + public boolean remove(Object key, Object value) { + if (key instanceof Player) { + Object playerKey = keyMethod.apply((Player) key); + + if (valueLookup.remove(playerKey, value)) { + keyLookup.asMap().remove(playerKey); + return true; + } + } + return false; + } + + @Override + public TValue get(Object key) { + if (key instanceof Player) + return valueLookup.get(keyMethod.apply((Player) key)); + return null; + } + + @Override + public boolean containsKey(Object key) { + if (key instanceof Player) + return valueLookup.containsKey(keyMethod.apply((Player) key)); + return false; + } + + @Override + public Set> entrySet() { + return new AbstractSet>() { + @Override + public Iterator> iterator() { + return entryIterator(); + } + + @Override + public int size() { + return valueLookup.size(); + } + + @Override + public void clear() { + valueLookup.clear(); + keyLookup.invalidateAll(); + } + }; + } + + /** + * Retrieve an iterator of entries that supports removal of elements. + * @return Entry iterator. + */ + private Iterator> entryIterator() { + // Skip entries with stale data + final Iterator> source = valueLookup.entrySet().iterator(); + final AbstractIterator> filtered = + new AbstractIterator>() { + @Override + protected Entry computeNext() { + while (source.hasNext()) { + Entry entry = source.next(); + Player player = lookupPlayer(entry.getKey()); + + if (player == null) { + // Remove entries that cannot be found + source.remove(); + keyLookup.asMap().remove(entry.getKey()); + } else { + return new SimpleEntry(player, entry.getValue()); + } + } + return endOfData(); + } + }; + + // We can't return AbstractIterator directly, as it doesn't permitt the remove() method + return new Iterator>() { + public boolean hasNext() { + return filtered.hasNext(); + } + public Entry next() { + return filtered.next(); + } + public void remove() { + source.remove(); + } + }; + } +} diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/ProxyPlayerInjectionHandler.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/ProxyPlayerInjectionHandler.java index 9d1c7d90..07c9b35e 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/ProxyPlayerInjectionHandler.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/ProxyPlayerInjectionHandler.java @@ -545,7 +545,7 @@ class ProxyPlayerInjectionHandler implements PlayerInjectionHandler { } else { throw new PlayerLoggedOutException(String.format( "Unable to send packet %s (%s): Player %s has logged out.", - packet.getID(), packet, reciever.getName() + packet.getID(), packet, reciever )); } } @@ -567,7 +567,7 @@ class ProxyPlayerInjectionHandler implements PlayerInjectionHandler { else throw new PlayerLoggedOutException(String.format( "Unable to receieve packet %s. Player %s has logged out.", - mcPacket, player.getName() + mcPacket, player )); }