Encapsulate Player instance lookup in a PlayerMap object.

Mostly for legacy Player instance getting, future aims are:
* Efficient lookup by name.
* Efficient lookup by prefixes of names (e.g. for command use).
* Efficient lookup name->uuid and uuid->name.
* Keep name->uuid mappings for history lookup and similar.

NCPCore/src/main/java/fr/neatmonster/nocheatplus/players/DataManager.java

@@ -13,6 +13,7 @@ import java.util.Map.Entry;
 import java.util.Set;
 import java.util.UUID;
 
+import org.bukkit.Bukkit;
 import org.bukkit.entity.Player;
 import org.bukkit.event.EventHandler;
 import org.bukkit.event.EventPriority;
@@ -29,6 +30,7 @@ import fr.neatmonster.nocheatplus.checks.access.ICheckConfig;
 import fr.neatmonster.nocheatplus.checks.access.ICheckData;
 import fr.neatmonster.nocheatplus.checks.combined.CombinedData;
 import fr.neatmonster.nocheatplus.compat.BridgeMisc;
+import fr.neatmonster.nocheatplus.compat.versions.ServerVersion;
 import fr.neatmonster.nocheatplus.components.ComponentRegistry;
 import fr.neatmonster.nocheatplus.components.ComponentWithName;
 import fr.neatmonster.nocheatplus.components.ConsistencyChecker;
@@ -65,6 +67,7 @@ public class DataManager implements Listener, INotifyReload, INeedConfig, Compon
     // Not static
     private int foundInconsistencies = 0;
 
+    // TODO: Switch to UUIDs as keys, get data by uuid when possible, use PlayerMap for getting
     /** PlayerData storage. */
     protected final Map<String, PlayerData> playerData = new LinkedHashMap<String, PlayerData>(100);
 
@@ -79,12 +82,11 @@ public class DataManager implements Listener, INotifyReload, INeedConfig, Compon
     private final Map<String, Long> lastLogout = new LinkedHashMap<String, Long>(50, 0.75f, true);
 
     /**
-     * Keeping track of online players.<br>
-     * Mappings:
-     * <li>exact player name -> Player instance</li>
-     * <li>lower case player name -> Player instance</li>
+     * Keeping track of online players. Currently id/name mappings are not kept
+     * on logout, but might be later.
      */
-    protected final Map<String, Player> onlinePlayers = new LinkedHashMap<String, Player>(100);
+    // TODO: Switch to UUIDs as keys, get data by uuid when possible, use PlayerMap for getting the UUID.
+    protected final PlayerMap playerMap;
 
     /**
      * IRemoveData instances.
@@ -116,6 +118,14 @@ public class DataManager implements Listener, INotifyReload, INeedConfig, Compon
      */
     public DataManager() {
         instance = this;
+        final String version = ServerVersion.getMinecraftVersion();
+        if (ServerVersion.compareVersions(version, "1.8") >= 0 || version.equals("1.7.10") && Bukkit.getServer().getVersion().toLowerCase().indexOf("spigot") != -1) {
+            // Safe to assume Spigot, don't store Player instances.
+            playerMap = new PlayerMap(false);
+        } else {
+            // Likely an older version without efficient mapping.
+            playerMap = new PlayerMap(true);
+        }
     }
 
     /**
@@ -419,7 +429,7 @@ public class DataManager implements Listener, INotifyReload, INeedConfig, Compon
      * @return
      */
     public static Player getPlayerExact(final String playerName) {
-        return instance.onlinePlayers.get(playerName);
+        return instance.playerMap.getPlayerExact(playerName);
     }
 
     /**
@@ -445,7 +455,7 @@ public class DataManager implements Listener, INotifyReload, INeedConfig, Compon
      * @return
      */
     public static Player getPlayer(final String playerName) {
-        return instance.onlinePlayers.get(playerName.toLowerCase());
+        return instance.playerMap.getPlayer(playerName);
     }
 
     @Override
@@ -478,9 +488,7 @@ public class DataManager implements Listener, INotifyReload, INeedConfig, Compon
      * @param player
      */
     private void addOnlinePlayer(final Player player) {
-        final String name = player.getName();
-        onlinePlayers.put(name, player);
-        onlinePlayers.put(name.toLowerCase(), player);
+        playerMap.updatePlayer(player);
     }
 
     /**
@@ -488,9 +496,8 @@ public class DataManager implements Listener, INotifyReload, INeedConfig, Compon
      * @param player
      */
     private void removeOnlinePlayer(final Player player) {
-        final String name = player.getName();
-        onlinePlayers.remove(name);
-        onlinePlayers.remove(name.toLowerCase()); 
+        // TODO: Consider to only remove the Player instance?
+        playerMap.remove(player);
     }
 
     /**
@@ -503,7 +510,7 @@ public class DataManager implements Listener, INotifyReload, INeedConfig, Compon
         clearConfigs();
         lastLogout.clear();
         executionHistories.clear();
-        onlinePlayers.clear();
+        playerMap.clear();
         // Finally alert (summary) if inconsistencies found.
         if (foundInconsistencies > 0) {
             StaticLog.logWarning("[NoCheatPlus] DataMan found " + foundInconsistencies + " inconsistencies (warnings suppressed).");
@@ -521,17 +528,16 @@ public class DataManager implements Listener, INotifyReload, INeedConfig, Compon
         // Check online player tracking consistency.
         int missing = 0;
         int changed = 0;
-        int expectedSize = 0;
         for (int i = 0; i < onlinePlayers.length; i++) {
             final Player player = onlinePlayers[i];
-            final String name = player.getName();
+            final UUID id = player.getUniqueId();
             //			if (player.isOnline()) {
-            expectedSize += 1 + (name.equals(name.toLowerCase()) ? 0 : 1);
-            if (!this.onlinePlayers.containsKey(name)) {
+            // TODO: Add a consistency check method !?
+            if (!playerMap.hasPlayerInfo(id)) {
                 missing ++;
                 // TODO: Add the player [problem: messy NPC plugins?]?
             }
-            if (player != this.onlinePlayers.get(name)) {
+            if (playerMap.storesPlayerInstances() && player != playerMap.getPlayer(id)) {
                 changed ++;
                 // Update the reference.
                 addOnlinePlayer(player);
@@ -541,17 +547,18 @@ public class DataManager implements Listener, INotifyReload, INeedConfig, Compon
 
         // TODO: Consider checking lastLogout for too long gone players.
 
-        final int storedSize = this.onlinePlayers.size();
-        if (missing != 0 || changed != 0 || expectedSize != storedSize) {
+        // TODO: Later the map size will not work, if we keep name/id mappings after logout. Other checking methods are possible.
+        final int storedSize = this.playerMap.size();
+        if (missing != 0 || changed != 0 || onlinePlayers.length != storedSize) {
             foundInconsistencies ++;
             if (!ConfigManager.getConfigFile().getBoolean(ConfPaths.DATA_CONSISTENCYCHECKS_SUPPRESSWARNINGS)) {
                 final List<String> details = new LinkedList<String>();
                 if (missing != 0) {
                     details.add("missing online players (" + missing + ")");
                 }
-                if (expectedSize != storedSize) {
+                if (onlinePlayers.length != storedSize) {
                     // TODO: Consider checking for not online players and remove them.
-                    details.add("wrong number of online players (" + storedSize + " instead of " + expectedSize + ")");
+                    details.add("wrong number of online players (" + storedSize + " instead of " + onlinePlayers.length + ")");
                 }
                 if (changed != 0) {
                     details.add("changed player instances (" + changed + ")");

NCPCore/src/main/java/fr/neatmonster/nocheatplus/players/PlayerMap.java

@@ -0,0 +1,289 @@
+package fr.neatmonster.nocheatplus.players;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.UUID;
+
+import org.bukkit.Bukkit;
+import org.bukkit.entity.Player;
+
+import fr.neatmonster.nocheatplus.logging.StaticLog;
+import fr.neatmonster.nocheatplus.utilities.ReflectionUtil;
+
+/**
+ * Map (online) players in various ways. Fall back to Bukkit methods if not
+ * mapped.
+ * 
+ * @author asofold
+ *
+ */
+public final class PlayerMap {
+
+    /**
+     * Carry basic information about players.
+     * 
+     * @author asofold
+     *
+     */
+    public static final class PlayerInfo {
+
+        public final UUID id;
+        public final String exactName;
+        public final String lowerCaseName;
+        public Player player = null;
+
+        public PlayerInfo (UUID id, String exactName) {
+            this.id = id;
+            this.exactName = exactName;
+            this.lowerCaseName = exactName.toLowerCase();
+        }
+
+        public boolean matchesExact(UUID id, String exactName) {
+            return this.id.equals(id) && this.exactName.equals(exactName);
+        }
+
+    }
+
+
+    private final boolean storePlayerInstances;
+    private final boolean hasGetPlayer_UUID = ReflectionUtil.getMethod(Bukkit.class, "getPlayer", UUID.class) != null;
+
+    // TODO: Map types (copy on write, lazy erase, or just keep ordinary maps?)
+    private Map<UUID, PlayerInfo> idInfoMap = new HashMap<UUID, PlayerMap.PlayerInfo>();
+    private Map<String, PlayerInfo> exactNameInfoMap = new HashMap<String, PlayerMap.PlayerInfo>();
+    private Map<String, PlayerInfo> lowerCaseNameInfoMap = new HashMap<String, PlayerMap.PlayerInfo>();
+    // TODO: Consider: Get players by prefix (primary thread only, e.g. for use with commands).
+    // TODO: get uuid/name methods?
+    // TODO: unlink Player references on remove for better gc?
+
+
+    public PlayerMap(boolean storePlayerInstances) {
+        this.storePlayerInstances = storePlayerInstances;
+        if (storePlayerInstances) {
+            StaticLog.logInfo("Player instances are stored for efficiency.");
+        }
+    }
+
+    // Public methods.
+
+    public boolean storesPlayerInstances() {
+        return storePlayerInstances;
+    }
+
+    public boolean hasPlayerInfo(final UUID id) {
+        return idInfoMap.containsKey(id);
+    }
+
+    public boolean hasPlayerInfoExact(final String exactName) {
+        return exactNameInfoMap.containsKey(exactName);
+    }
+
+    public boolean hasPlayerInfo(final String probableName) {
+        return hasPlayerInfoLowerCase(probableName.toLowerCase());
+    }
+
+    public boolean hasPlayerInfoLowerCase(final String lowerCaseName) {
+        return lowerCaseNameInfoMap.containsKey(lowerCaseName);
+    }
+
+    public PlayerInfo getPlayerInfo(final UUID id) {
+        return idInfoMap.get(id);
+    }
+
+    public PlayerInfo getPlayerInfoExact(final String exactName) {
+        return exactNameInfoMap.get(exactName);
+    }
+
+    public PlayerInfo getPlayerInfo(final String probableName) {
+        return getPlayerInfoLowerCase(probableName.toLowerCase());
+    }
+
+    public PlayerInfo getPlayerInfoLowerCase(final String lowerCaseName) {
+        return lowerCaseNameInfoMap.get(lowerCaseName);
+    }
+
+    public Player getPlayer(final UUID id) {
+        final PlayerInfo info = idInfoMap.get(id);
+        if (info != null) {
+            if (info.player != null) {
+                return info.player;
+            }
+            if (storePlayerInstances) {
+                info.player = getPlayerBukkit(info);
+                return info.player;
+            } else  {
+                return getPlayerBukkit(info);
+            }
+        } else {
+            return getPlayerBukkit(id);
+        }
+    }
+
+    @SuppressWarnings("deprecation")
+    public Player getPlayerExact(final String exactName) {
+        final PlayerInfo info = exactNameInfoMap.get(exactName);
+        if (info != null) {
+            if (info.player != null) {
+                return info.player;
+            }
+            if (storePlayerInstances) {
+                info.player = getPlayerBukkit(info);
+                return info.player;
+            } else  {
+                return getPlayerBukkit(info);
+            }
+        } else {
+            return Bukkit.getPlayerExact(exactName);
+        }
+    }
+
+    public Player getPlayer(final String probableName) {
+        return getPlayerLowerCase(probableName.toLowerCase());
+    }
+
+    @SuppressWarnings("deprecation")
+    public Player getPlayerLowerCase(final String lowerCaseName) {
+        final PlayerInfo info = lowerCaseNameInfoMap.get(lowerCaseName);
+        if (info != null) {
+            if (info.player != null) {
+                return info.player;
+            }
+            if (storePlayerInstances) {
+                info.player = getPlayerBukkit(info);
+                return info.player;
+            } else  {
+                return getPlayerBukkit(info);
+            }
+        } else {
+            return Bukkit.getPlayer(lowerCaseName);
+        }
+    }
+
+    public PlayerInfo updatePlayer(final Player player) {
+        final UUID id = player.getUniqueId();
+        final String exactName = player.getName();
+        PlayerInfo info = idInfoMap.get(id);
+        if (info != null) {
+            if (info.matchesExact(id, exactName)) {
+                // Nothing to do, except updating the player instance.
+                if (storePlayerInstances) {
+                    info.player = player;
+                }
+                return info;
+            } else {
+                // Remove the info.
+                remove(info);
+            }
+        }
+        // Create and link a new info.
+        info = new PlayerInfo(id, exactName);
+        if (storePlayerInstances) {
+            info.player = player;
+        }
+        ensureRemoved(info);
+        idInfoMap.put(id, info);
+        exactNameInfoMap.put(exactName, info);
+        lowerCaseNameInfoMap.put(info.lowerCaseName, info);
+        return info;
+    }
+
+    /**
+     * Remove the instance reference, if present at all.
+     * 
+     * @param id
+     */
+    public void removePlayerInstance(final UUID id) {
+        final PlayerInfo info = idInfoMap.get(id);
+        if (info != null) {
+            info.player = null;
+        }
+    }
+
+    public boolean remove(final Player player) {
+        return ensureRemoved(new PlayerInfo(player.getUniqueId(), player.getName()));
+    }
+
+    public void clear() {
+        idInfoMap.clear();
+        exactNameInfoMap.clear();
+        lowerCaseNameInfoMap.clear();
+    }
+
+    public int size() {
+        // TODO: Consistency?
+        return idInfoMap.size();
+    }
+
+    // Private methods.
+
+    private Player getPlayerBukkit(final UUID id) {
+        if (hasGetPlayer_UUID) {
+            return Bukkit.getPlayer(id);
+        } else {
+            // Backwards compatibility.
+            return scanForPlayer(id);
+        }
+    }
+
+    private Player scanForPlayer(final UUID id) {
+        // TODO: Add a mapping for id->name for this case?
+        for (final Player player : Bukkit.getOnlinePlayers()) {
+            if (id.equals(player.getUniqueId())) {
+                return player;
+            }
+        }
+        return null;
+    }
+
+    @SuppressWarnings("deprecation")
+    private Player getPlayerBukkit(final PlayerInfo info) {
+        if (hasGetPlayer_UUID) {
+            return Bukkit.getPlayer(info.id);
+        } else {
+            return Bukkit.getPlayerExact(info.exactName);
+        }
+    }
+
+    /**
+     * Ensure there are no entries for the given info (for the case of
+     * inconsistent combinations).
+     * 
+     * @param info
+     * @return
+     */
+    private boolean ensureRemoved(final PlayerInfo info) {
+        PlayerInfo ref = idInfoMap.get(info.id);
+        boolean changed = false;
+        if (ref != null) {
+            remove(ref);
+            changed = true;
+        }
+        ref = exactNameInfoMap.get(info.exactName);
+        if (ref != null) {
+            remove(ref);
+            changed = true;
+        }
+        ref = lowerCaseNameInfoMap.get(info.lowerCaseName);
+        if (ref != null) {
+            remove(ref);
+            changed = true;
+        }
+        return changed;
+    }
+
+    /**
+     * Remove an existing info from all mappings, only call for consistent
+     * states.
+     * 
+     * @param info
+     * @return
+     */
+    private boolean remove(final PlayerInfo info) {
+        boolean altered = false;
+        altered |= idInfoMap.remove(info.id) != null;
+        altered |= exactNameInfoMap.remove(info.exactName) != null;
+        altered |= lowerCaseNameInfoMap.remove(info.lowerCaseName) != null;
+        return altered;
+    }
+
+}