From 02d6703da779a5876ceaf5d2fd2dd9833a7a6621 Mon Sep 17 00:00:00 2001 From: Wesley Wolfe Date: Fri, 3 Aug 2012 06:24:55 -0500 Subject: [PATCH] Implement new AsyncPlayerChatEvent. Addresses BUKKIT-2064 Added two utility collections for use with PlayerChatEvents allowing lazier initialization of events and less need to synchronize against the player list. Provided a hidden queue system for similar logic to pre-1.3 chat. When a plugin is listening for the deprecated PlayerChatEvent, all chat will be delayed to be mirror executed from the main thread. All developers are encouraged to immediately update to the developmental Bukkit chat API as a minimum transition for server stability. Additionally, changes were required to bring thread-safety to the flow logic. CopyOnWriteArrayList is the only viable means to produce thread safety with minimal diff; using a sane pre-implemented collection would require reworking of sections of NMS logic. As a minor change, implemented expected functionality for PlayerCommandPreProcessEvent. Setting the player should now change the player executing the command. --- .../net/minecraft/server/MinecraftServer.java | 24 +++++ .../minecraft/server/NetServerHandler.java | 37 ++++--- .../ServerConfigurationManagerAbstract.java | 2 +- .../craftbukkit/entity/CraftPlayer.java | 2 +- .../bukkit/craftbukkit/util/LazyHashSet.java | 98 +++++++++++++++++++ .../craftbukkit/util/LazyPlayerSet.java | 25 +++++ 6 files changed, 173 insertions(+), 15 deletions(-) create mode 100644 src/main/java/org/bukkit/craftbukkit/util/LazyHashSet.java create mode 100644 src/main/java/org/bukkit/craftbukkit/util/LazyPlayerSet.java diff --git a/src/main/java/net/minecraft/server/MinecraftServer.java b/src/main/java/net/minecraft/server/MinecraftServer.java index bba9515690..3e779357a4 100644 --- a/src/main/java/net/minecraft/server/MinecraftServer.java +++ b/src/main/java/net/minecraft/server/MinecraftServer.java @@ -19,6 +19,7 @@ import joptsimple.OptionSet; import org.bukkit.World.Environment; import org.bukkit.event.server.RemoteServerCommandEvent; import org.bukkit.event.world.WorldSaveEvent; +import org.bukkit.event.player.PlayerChatEvent; // CraftBukkit end public abstract class MinecraftServer implements Runnable, IMojangStatistics, ICommandListener { @@ -78,6 +79,7 @@ public abstract class MinecraftServer implements Runnable, IMojangStatistics, IC public ConsoleReader reader; public static int currentTick; public final Thread primaryThread; + public java.util.Queue chatQueue = new java.util.concurrent.ConcurrentLinkedQueue(); // CraftBukkit end public MinecraftServer(OptionSet options) { // CraftBukkit - signature file -> OptionSet @@ -509,6 +511,28 @@ public abstract class MinecraftServer implements Runnable, IMojangStatistics, IC // CraftBukkit start - only send timeupdates to the people in that world this.server.getScheduler().mainThreadHeartbeat(this.ticks); + // Fix for old plugins still using deprecated event + while (!chatQueue.isEmpty()) { + PlayerChatEvent event = chatQueue.remove(); + org.bukkit.Bukkit.getPluginManager().callEvent(event); + + if (event.isCancelled()) { + continue; + } + + String message = String.format(event.getFormat(), event.getPlayer().getDisplayName(), event.getMessage()); + console.sendMessage(message); + if (((org.bukkit.craftbukkit.util.LazyPlayerSet) event.getRecipients()).isLazy()) { + for (Object player : getServerConfigurationManager().players) { + ((EntityPlayer) player).sendMessage(message); + } + } else { + for (org.bukkit.entity.Player player : event.getRecipients()) { + player.sendMessage(message); + } + } + } + // Send timeupdates to everyone, it will get the right time from the world the player is in. if (this.ticks % 20 == 0) { for (int i = 0; i < this.getServerConfigurationManager().players.size(); ++i) { diff --git a/src/main/java/net/minecraft/server/NetServerHandler.java b/src/main/java/net/minecraft/server/NetServerHandler.java index aa43435e02..e512b2b75c 100644 --- a/src/main/java/net/minecraft/server/NetServerHandler.java +++ b/src/main/java/net/minecraft/server/NetServerHandler.java @@ -16,12 +16,14 @@ import java.util.HashSet; import org.bukkit.Location; import org.bukkit.craftbukkit.inventory.CraftInventoryView; import org.bukkit.craftbukkit.inventory.CraftItemStack; +import org.bukkit.craftbukkit.util.LazyPlayerSet; import org.bukkit.craftbukkit.entity.CraftPlayer; import org.bukkit.craftbukkit.event.CraftEventFactory; import org.bukkit.entity.Player; import org.bukkit.event.Event; import org.bukkit.event.block.Action; import org.bukkit.event.block.SignChangeEvent; +import org.bukkit.event.player.AsyncPlayerChatEvent; import org.bukkit.event.player.PlayerAnimationEvent; import org.bukkit.event.player.PlayerChatEvent; import org.bukkit.event.player.PlayerCommandPreprocessEvent; @@ -801,12 +803,12 @@ public class NetServerHandler extends NetHandler { return; } - this.chat(s); + this.chat(s, packet3chat.a_()); } } } - public boolean chat(String s) { + public boolean chat(String s, boolean async) { if (!this.player.dead) { if (s.length() == 0) { logger.warning(this.player.name + " tried to send an empty message"); @@ -823,17 +825,26 @@ public class NetServerHandler extends NetHandler { return true; } else { Player player = this.getPlayer(); - PlayerChatEvent event = new PlayerChatEvent(player, s); + AsyncPlayerChatEvent event = new AsyncPlayerChatEvent(async, player, s, new LazyPlayerSet()); this.server.getPluginManager().callEvent(event); - if (event.isCancelled()) { - return true; - } - - s = String.format(event.getFormat(), event.getPlayer().getDisplayName(), event.getMessage()); - minecraftServer.console.sendMessage(s); - for (Player recipient : event.getRecipients()) { - recipient.sendMessage(s); + if (PlayerChatEvent.getHandlerList().getRegisteredListeners().length != 0) { + // Evil plugins still listening to deprecated event + PlayerChatEvent queueEvent = new PlayerChatEvent(player, event.getMessage(), event.getFormat(), event.getRecipients()); + queueEvent.setCancelled(event.isCancelled()); + minecraftServer.chatQueue.add(queueEvent); + } else { + s = String.format(event.getFormat(), event.getPlayer().getDisplayName(), event.getMessage()); + minecraftServer.console.sendMessage(s); + if (((LazyPlayerSet) event.getRecipients()).isLazy()) { + for (Object recipient : minecraftServer.getServerConfigurationManager().players) { + ((EntityPlayer) recipient).sendMessage(s); + } + } else { + for (org.bukkit.entity.Player recipient : event.getRecipients()) { + recipient.sendMessage(s); + } + } } } @@ -851,7 +862,7 @@ public class NetServerHandler extends NetHandler { // CraftBukkit start CraftPlayer player = this.getPlayer(); - PlayerCommandPreprocessEvent event = new PlayerCommandPreprocessEvent(player, s); + PlayerCommandPreprocessEvent event = new PlayerCommandPreprocessEvent(player, s, new LazyPlayerSet()); this.server.getPluginManager().callEvent(event); if (event.isCancelled()) { @@ -859,7 +870,7 @@ public class NetServerHandler extends NetHandler { } try { - if (this.server.dispatchCommand(player, event.getMessage().substring(1))) { + if (this.server.dispatchCommand(event.getPlayer(), event.getMessage().substring(1))) { return; } } catch (org.bukkit.command.CommandException ex) { diff --git a/src/main/java/net/minecraft/server/ServerConfigurationManagerAbstract.java b/src/main/java/net/minecraft/server/ServerConfigurationManagerAbstract.java index ba2480080e..35fa2cddb2 100644 --- a/src/main/java/net/minecraft/server/ServerConfigurationManagerAbstract.java +++ b/src/main/java/net/minecraft/server/ServerConfigurationManagerAbstract.java @@ -30,7 +30,7 @@ public abstract class ServerConfigurationManagerAbstract { private static final SimpleDateFormat e = new SimpleDateFormat("yyyy-MM-dd \'at\' HH:mm:ss z"); public static final Logger a = Logger.getLogger("Minecraft"); private final MinecraftServer server; - public final List players = new ArrayList(); + public final List players = new java.util.concurrent.CopyOnWriteArrayList(); // CraftBukkit - ArrayList -> CopyOnWriteArrayList: Iterator safety private final BanList banByName = new BanList(new File("banned-players.txt")); private final BanList banByIP = new BanList(new File("banned-ips.txt")); private Set operators = new HashSet(); diff --git a/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java b/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java index dd82cb32ba..b1906046a4 100644 --- a/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java +++ b/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java @@ -229,7 +229,7 @@ public class CraftPlayer extends CraftHumanEntity implements Player { public void chat(String msg) { if (getHandle().netServerHandler == null) return; - getHandle().netServerHandler.chat(msg); + getHandle().netServerHandler.chat(msg, false); } public boolean performCommand(String command) { diff --git a/src/main/java/org/bukkit/craftbukkit/util/LazyHashSet.java b/src/main/java/org/bukkit/craftbukkit/util/LazyHashSet.java new file mode 100644 index 0000000000..ad83fd87dd --- /dev/null +++ b/src/main/java/org/bukkit/craftbukkit/util/LazyHashSet.java @@ -0,0 +1,98 @@ +package org.bukkit.craftbukkit.util; + +import java.util.Collection; +import java.util.Iterator; +import java.util.Set; + + +public abstract class LazyHashSet implements Set { + Set reference = null; + + public int size() { + return getReference().size(); + } + + public boolean isEmpty() { + return getReference().isEmpty(); + } + + public boolean contains(Object o) { + return getReference().contains(o); + } + + public Iterator iterator() { + return getReference().iterator(); + } + + public Object[] toArray() { + return getReference().toArray(); + } + + public T[] toArray(T[] a) { + return getReference().toArray(a); + } + + public boolean add(E o) { + return getReference().add(o); + } + + public boolean remove(Object o) { + return getReference().remove(o); + } + + public boolean containsAll(Collection c) { + return getReference().containsAll(c); + } + + public boolean addAll(Collection c) { + return getReference().addAll(c); + } + + public boolean retainAll(Collection c) { + return getReference().retainAll(c); + } + + public boolean removeAll(Collection c) { + return getReference().removeAll(c); + } + + public void clear() { + getReference().clear(); + } + + public Set getReference() { + Set reference = this.reference ; + if (reference != null) { + return reference; + } + return this.reference = makeReference(); + } + + abstract Set makeReference(); + + public boolean isLazy() { + return reference == null; + } + + @Override + public int hashCode() { + return 157 * getReference().hashCode(); + } + + @Override + public boolean equals(Object obj) { + if (obj == this) { + return true; + } + if (obj == null || this.getClass() != obj.getClass()) { + return false; + } + LazyHashSet that = (LazyHashSet) obj; + return (this.isLazy() && that.isLazy()) || this.getReference().equals(that.getReference()); + } + + @Override + public String toString() { + return getReference().toString(); + } +} diff --git a/src/main/java/org/bukkit/craftbukkit/util/LazyPlayerSet.java b/src/main/java/org/bukkit/craftbukkit/util/LazyPlayerSet.java new file mode 100644 index 0000000000..df09019056 --- /dev/null +++ b/src/main/java/org/bukkit/craftbukkit/util/LazyPlayerSet.java @@ -0,0 +1,25 @@ +package org.bukkit.craftbukkit.util; + +import java.util.HashSet; +import java.util.List; +import net.minecraft.server.EntityPlayer; +import net.minecraft.server.MinecraftServer; + +import org.bukkit.entity.Player; + +public class LazyPlayerSet extends LazyHashSet { + + @Override + HashSet makeReference() { + if (reference != null) { + throw new IllegalStateException("Reference already created!"); + } + List players = MinecraftServer.getServer().getServerConfigurationManager().players; + HashSet reference = new HashSet(players.size()); + for (EntityPlayer player : players) { + reference.add(player.getBukkitEntity()); + } + return reference; + } + +}