Replace getOnlinePlayers to provide a view. Adds BUKKIT-5668

5 files changed, 62 insertions, 9 deletions

diff --git a/src/main/java/org/bukkit/Bukkit.java b/src/main/java/org/bukkit/Bukkit.java
index dafea2e6..1eaf92d7 100644
--- a/src/main/java/org/bukkit/Bukkit.java
+++ b/src/main/java/org/bukkit/Bukkit.java
@@ -2,6 +2,7 @@ package org.bukkit;
 
 import java.awt.image.BufferedImage;
 import java.io.File;
+import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -90,9 +91,22 @@ public final class Bukkit {
     }
 
     /**
+     * This method exists for legacy reasons to provide backwards
+     * compatibility. It will not exist at runtime and should not be used
+     * under any circumstances.
+     *
+     * @Deprecated
+     * @see Server#_INVALID_getOnlinePlayers()
+     */
+    @Deprecated
+    public static Player[] _INVALID_getOnlinePlayers() {
+        return server._INVALID_getOnlinePlayers();
+    }
+
+    /**
      * @see Server#getOnlinePlayers()
      */
-    public static Player[] getOnlinePlayers() {
+    public static Collection<? extends Player> getOnlinePlayers() {
         return server.getOnlinePlayers();
     }
 
diff --git a/src/main/java/org/bukkit/Server.java b/src/main/java/org/bukkit/Server.java
index 3c4d5418..e14e9f1d 100644
--- a/src/main/java/org/bukkit/Server.java
+++ b/src/main/java/org/bukkit/Server.java
@@ -2,6 +2,9 @@ package org.bukkit;
 
 import java.awt.image.BufferedImage;
 import java.io.File;
+import java.io.Serializable;
+import java.util.Collection;
+import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -14,6 +17,7 @@ import org.bukkit.command.CommandException;
 import org.bukkit.command.CommandSender;
 import org.bukkit.command.ConsoleCommandSender;
 import org.bukkit.command.PluginCommand;
+import org.bukkit.entity.Entity;
 import org.bukkit.entity.Player;
 import org.bukkit.event.inventory.InventoryType;
 import org.bukkit.event.server.ServerListPingEvent;
@@ -33,6 +37,7 @@ import org.bukkit.scoreboard.ScoreboardManager;
 import org.bukkit.util.CachedServerIcon;
 
 import com.avaje.ebean.config.ServerConfig;
+import com.google.common.collect.ImmutableList;
 
 import org.bukkit.inventory.ItemFactory;
 import org.bukkit.inventory.meta.ItemMeta;
@@ -80,11 +85,46 @@ public interface Server extends PluginMessageRecipient {
     public String getBukkitVersion();
 
     /**
-     * Gets a list of all currently logged in players.
+     * Gets an array copy of all currently logged in players.
+     * <p>
+     * This method exists for legacy reasons to provide backwards
+     * compatibility. It will not exist at runtime and should not be used
+     * under any circumstances.
      *
+     * @Deprecated superseded by {@link #getOnlinePlayers()}
      * @return an array of Players that are currently online
      */
-    public Player[] getOnlinePlayers();
+    @Deprecated
+    public Player[] _INVALID_getOnlinePlayers();
+
+    /**
+     * Gets a view of all currently logged in players. This {@linkplain
+     * Collections#unmodifiableCollection(Collection) view} is a reused
+     * object, making some operations like {@link Collection#size()}
+     * zero-allocation.
+     * <p>
+     * The collection is a view backed by the internal representation, such
+     * that, changes to the internal state of the server will be reflected
+     * immediately. However, the reuse of the returned collection (identity)
+     * is not strictly guaranteed for future or all implementations. Casting
+     * the collection, or relying on interface implementations (like {@link
+     * Serializable} or {@link List}), is deprecated.
+     * <p>
+     * Iteration behavior is undefined outside of self-contained main-thread
+     * uses. Normal and immediate iterator use without consequences that
+     * affect the collection are fully supported. The effects following
+     * (non-exhaustive) {@link Entity#teleport(Location) teleportation},
+     * {@link Player#setHealth(double) death}, and {@link Player#kickPlayer(
+     * String) kicking} are undefined. Any use of this collection from
+     * asynchronous threads is unsafe.
+     * <p>
+     * For safe consequential iteration or mimicking the old array behavior,
+     * using {@link Collection#toArray(Object[])} is recommended. For making
+     * snapshots, {@link ImmutableList#copyOf(Collection)} is recommended.
+     *
+     * @return a view of currently online players.
+     */
+    public Collection<? extends Player> getOnlinePlayers();
 
     /**
      * Get the maximum amount of players which can login to this server.
diff --git a/src/main/java/org/bukkit/command/defaults/ListCommand.java b/src/main/java/org/bukkit/command/defaults/ListCommand.java
index 80c6135a..eb8a6a91 100644
--- a/src/main/java/org/bukkit/command/defaults/ListCommand.java
+++ b/src/main/java/org/bukkit/command/defaults/ListCommand.java
@@ -1,5 +1,6 @@
 package org.bukkit.command.defaults;
 
+import java.util.Collection;
 import java.util.List;
 
 import org.apache.commons.lang.Validate;
@@ -23,7 +24,7 @@ public class ListCommand extends VanillaCommand {
 
         StringBuilder online = new StringBuilder();
 
-        Player[] players = Bukkit.getOnlinePlayers();
+        final Collection<? extends Player> players = Bukkit.getOnlinePlayers();
 
         for (Player player : players) {
             // If a player is hidden from the sender don't show them in the list
@@ -37,7 +38,7 @@ public class ListCommand extends VanillaCommand {
             online.append(player.getDisplayName());
         }
 
-        sender.sendMessage("There are " + players.length + "/" + Bukkit.getMaxPlayers() + " players online:\n" + online.toString());
+        sender.sendMessage("There are " + players.size() + "/" + Bukkit.getMaxPlayers() + " players online:\n" + online.toString());
 
         return true;
     }
diff --git a/src/main/java/org/bukkit/event/player/PlayerChatEvent.java b/src/main/java/org/bukkit/event/player/PlayerChatEvent.java
index b48ea360..1fb5cd75 100644
--- a/src/main/java/org/bukkit/event/player/PlayerChatEvent.java
+++ b/src/main/java/org/bukkit/event/player/PlayerChatEvent.java
@@ -1,6 +1,5 @@
 package org.bukkit.event.player;
 
-import java.util.Arrays;
 import java.util.HashSet;
 import java.util.Set;
 
@@ -33,7 +32,7 @@ public class PlayerChatEvent extends PlayerEvent implements Cancellable {
         super(player);
         this.message = message;
         this.format = "<%1$s> %2$s";
-        this.recipients = new HashSet<Player>(Arrays.asList(player.getServer().getOnlinePlayers()));
+        this.recipients = new HashSet<Player>(player.getServer().getOnlinePlayers());
     }
 
     public PlayerChatEvent(final Player player, final String message, final String format, final Set<Player> recipients) {
diff --git a/src/main/java/org/bukkit/event/player/PlayerCommandPreprocessEvent.java b/src/main/java/org/bukkit/event/player/PlayerCommandPreprocessEvent.java
index 7f3cae67..1ec81732 100644
--- a/src/main/java/org/bukkit/event/player/PlayerCommandPreprocessEvent.java
+++ b/src/main/java/org/bukkit/event/player/PlayerCommandPreprocessEvent.java
@@ -1,6 +1,5 @@
 package org.bukkit.event.player;
 
-import java.util.Arrays;
 import java.util.HashSet;
 import java.util.Set;
 
@@ -55,7 +54,7 @@ public class PlayerCommandPreprocessEvent extends PlayerEvent implements Cancell
 
     public PlayerCommandPreprocessEvent(final Player player, final String message) {
         super(player);
-        this.recipients = new HashSet<Player>(Arrays.asList(player.getServer().getOnlinePlayers()));
+        this.recipients = new HashSet<Player>(player.getServer().getOnlinePlayers());
         this.message = message;
     }