From 4f871c64d76b965e9558b5fe21a00e9cd197ebed Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Sun, 30 Mar 2014 00:50:45 +0100 Subject: [PATCH] Warn instead of throwing an exception on packet type change. We don't want to crash plugins over this, since it doesn't automatically cause problems. But it may trip up plugins that assume the packet types they set when registering a listener is the only ones they'll ever recieve in the method body, which is not true if a preceeding packet listener can change a packet to an arbitrary type. I'm open for better suggestions here. But for now, I'll just print a warning and hope people use sendServerPacket() instead. --- .../protocol/events/PacketEvent.java | 32 +++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/events/PacketEvent.java b/ProtocolLib/src/main/java/com/comphenix/protocol/events/PacketEvent.java index 97bc9fb1..6a0e8530 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/events/PacketEvent.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/events/PacketEvent.java @@ -28,11 +28,28 @@ import org.bukkit.event.Cancellable; import com.comphenix.protocol.Application; import com.comphenix.protocol.PacketType; +import com.comphenix.protocol.ProtocolLibrary; import com.comphenix.protocol.async.AsyncMarker; +import com.comphenix.protocol.error.Report; +import com.comphenix.protocol.error.ReportType; import com.google.common.base.Objects; import com.google.common.base.Preconditions; +import com.google.common.collect.HashMultimap; +import com.google.common.collect.Multimaps; +import com.google.common.collect.SetMultimap; +/** + * Represents a packet sending or receiving event. Changes to the packet will be + * reflected in the final version to be sent or received. It is also possible to cancel an event. + * @author Kristian + */ public class PacketEvent extends EventObject implements Cancellable { + public static final ReportType REPORT_CHANGING_PACKET_TYPE_IS_CONFUSING = new ReportType( + "Plugin %s changed packet type from %s to %s in packet listener. This is confusing for other plugins! (Not an error, though!)"); + + private static final SetMultimap CHANGE_WARNINGS = + Multimaps.synchronizedSetMultimap(HashMultimap.create()); + /** * Automatically generated by Eclipse. */ @@ -170,8 +187,19 @@ public class PacketEvent extends EventObject implements Cancellable { throw new IllegalStateException("The packet event is read-only."); if (packet == null) throw new IllegalArgumentException("Cannot set packet to NULL. Use setCancelled() instead."); - if (this.packet != null && !Objects.equal(this.packet.getType(), packet.getType())) - throw new IllegalArgumentException("Cannot change packet type from " + this.packet.getType() + " to " + packet.getType()); + + // Change warnings + final PacketType oldType = this.packet.getType(); + final PacketType newType = packet.getType(); + if (this.packet != null && !Objects.equal(oldType, newType)) { + // Only report this once + if (CHANGE_WARNINGS.put(oldType, newType)) { + ProtocolLibrary.getErrorReporter().reportWarning(this, + Report.newBuilder(REPORT_CHANGING_PACKET_TYPE_IS_CONFUSING). + messageParam(oldType, newType). + build()); + } + } this.packet = packet; }