Don't print pointless warnings on Spigot.

ProtocolLib/src/main/java/com/comphenix/protocol/ProtocolLibrary.java

@@ -33,6 +33,7 @@ import org.bukkit.plugin.PluginManager;
 import org.bukkit.plugin.java.JavaPlugin;
 
 import com.comphenix.protocol.async.AsyncFilterManager;
+import com.comphenix.protocol.error.BasicErrorReporter;
 import com.comphenix.protocol.error.DetailedErrorReporter;
 import com.comphenix.protocol.error.ErrorReporter;
 import com.comphenix.protocol.error.Report;
@@ -92,7 +93,7 @@ public class ProtocolLibrary extends JavaPlugin {
 	private static PacketFilterManager protocolManager;
 	
 	// Error reporter
-	private static ErrorReporter reporter;
+	private static ErrorReporter reporter = new BasicErrorReporter();
 	
 	// Metrics and statistisc
 	private Statistics statistisc;
@@ -490,7 +491,9 @@ public class ProtocolLibrary extends JavaPlugin {
 			unhookTask.close();
 		protocolManager = null;
 		statistisc = null;
-		reporter = null;
+
+		// To clean up global parameters
+		reporter = new BasicErrorReporter();
 		
 		// Leaky ClassLoader begone!
 		if (updater == null || updater.getResult() != UpdateResult.SUCCESS) {
@@ -515,6 +518,8 @@ public class ProtocolLibrary extends JavaPlugin {
 	
 	/**
 	 * Retrieve the current error reporter.
+	 * <p>
+	 * This is guaranteed to not be NULL in 2.5.0 and later.
 	 * @return Current error reporter.
 	 */
 	public static ErrorReporter getErrorReporter() {

ProtocolLib/src/main/java/com/comphenix/protocol/error/BasicErrorReporter.java

@@ -0,0 +1,96 @@
+package com.comphenix.protocol.error;
+
+import java.io.PrintStream;
+import org.bukkit.plugin.Plugin;
+
+import com.comphenix.protocol.error.Report.ReportBuilder;
+import com.comphenix.protocol.reflect.PrettyPrinter;
+
+/**
+ * Represents a basic error reporter that prints error reports to the standard error stream.
+ * <p>
+ * Note that this implementation doesn't distinguish between {@link #reportWarning(Object, Report)} 
+ * and {@link #reportDetailed(Object, Report)} - they both have the exact same behavior.
+ * @author Kristian
+ */
+public class BasicErrorReporter implements ErrorReporter {
+	private final PrintStream output;
+	
+	/**
+	 * Construct a new basic error reporter that prints directly the standard error stream.
+	 */
+	public BasicErrorReporter() {
+		 this(System.err);
+	}
+	
+	/**
+	 * Construct a error reporter that prints to the given output stream.
+	 * @param output - the output stream.
+	 */
+	public BasicErrorReporter(PrintStream output) {
+		this.output = output;
+	}
+
+	@Override
+	public void reportMinimal(Plugin sender, String methodName, Throwable error) {
+		output.println("Unhandled exception occured in " +  methodName + " for " + sender.getName());
+		error.printStackTrace(output);
+	}
+
+	@Override
+	public void reportMinimal(Plugin sender, String methodName, Throwable error, Object... parameters) {
+		reportMinimal(sender, methodName, error);
+		
+		// Also print parameters
+		printParameters(parameters);
+	}
+
+	@Override
+	public void reportWarning(Object sender, Report report) {
+		// Basic warning
+		output.println("[" + sender.getClass().getSimpleName() + "] " + report.getReportMessage());
+		
+		if (report.getException() != null) {
+			report.getException().printStackTrace(output);
+		}
+		printParameters(report.getCallerParameters());
+	}
+
+	@Override
+	public void reportWarning(Object sender, ReportBuilder reportBuilder) {
+		reportWarning(sender, reportBuilder.build());
+	}
+
+	@Override
+	public void reportDetailed(Object sender, Report report) {
+		// No difference from warning
+		reportWarning(sender, report);
+	}
+
+	@Override
+	public void reportDetailed(Object sender, ReportBuilder reportBuilder) {
+		reportWarning(sender, reportBuilder);
+	}
+	
+	/**
+	 * Print the given parameters to the standard error stream.
+	 * @param parameters - the output parameters.
+	 */
+	private void printParameters(Object[] parameters) {
+		if (parameters != null && parameters.length > 0) {
+			output.println("Parameters: ");
+			
+			try {
+				for (Object parameter : parameters) {
+					if (parameter == null)
+						output.println("[NULL]");
+					else
+						output.println(PrettyPrinter.printObject(parameter));
+				}
+			} catch (IllegalAccessException e) {
+				// Damn it
+				e.printStackTrace();
+			}
+		}
+	}
+}

ProtocolLib/src/main/java/com/comphenix/protocol/error/DelegatedErrorReporter.java

@@ -0,0 +1,80 @@
+package com.comphenix.protocol.error;
+
+import org.bukkit.plugin.Plugin;
+
+import com.comphenix.protocol.error.Report.ReportBuilder;
+
+/**
+ * Construct an error reporter that delegates to another error reporter.
+ * @author Kristian
+ */
+public class DelegatedErrorReporter implements ErrorReporter {
+	private final ErrorReporter delegated;
+
+	/**
+	 * Construct a new error reporter that forwards all reports to a given reporter.
+	 * @param delegated - the delegated reporter.
+	 */
+	public DelegatedErrorReporter(ErrorReporter delegated) {
+		this.delegated = delegated;
+	}
+	
+	/**
+	 * Retrieve the underlying error reporter.
+	 * @return Underlying error reporter.
+	 */
+	public ErrorReporter getDelegated() {
+		return delegated;
+	}
+
+	@Override
+	public void reportMinimal(Plugin sender, String methodName, Throwable error) {
+		delegated.reportMinimal(sender, methodName, error);
+	}
+
+	@Override
+	public void reportMinimal(Plugin sender, String methodName, Throwable error, Object... parameters) {
+		delegated.reportMinimal(sender, methodName, error, parameters);
+	}
+
+	@Override
+	public void reportWarning(Object sender, Report report) {
+		Report transformed = filterReport(sender, report, false);
+		
+		if (transformed != null) {
+			delegated.reportWarning(sender, transformed);
+		}
+	}
+
+	@Override
+	public void reportDetailed(Object sender, Report report) {
+		Report transformed = filterReport(sender, report, true);
+		
+		if (transformed != null) {
+			delegated.reportDetailed(sender, transformed);
+		}
+	}
+
+	/**
+	 * Invoked before an error report is passed on to the underlying error reporter.
+	 * <p>
+	 * To cancel a report, return NULL.
+	 * @param sender - the sender component.
+	 * @param report - the error report.
+	 * @param detailed - whether or not the report will be displayed in detail.
+	 * @return The report to pass on, or NULL to cancel it.
+	 */
+	protected Report filterReport(Object sender, Report report, boolean detailed) {
+		return report;
+	}
+	
+	@Override
+	public void reportWarning(Object sender, ReportBuilder reportBuilder) {
+		reportWarning(sender, reportBuilder.build());
+	}
+	
+	@Override
+	public void reportDetailed(Object sender, ReportBuilder reportBuilder) {
+		reportDetailed(sender, reportBuilder.build());
+	}
+}

ProtocolLib/src/main/java/com/comphenix/protocol/error/DetailedErrorReporter.java

@@ -342,7 +342,6 @@ public class DetailedErrorReporter implements ErrorReporter {
 	 * @return String representation.
 	 */
 	protected String getStringDescription(Object value) {
-		
 		// We can't only rely on toString.
 		if (value == null) {
 			return "[NULL]";

ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/InjectedArrayList.java

@@ -23,7 +23,6 @@ import java.util.ArrayList;
 import java.util.Set;
 
 import com.comphenix.protocol.ProtocolLibrary;
-import com.comphenix.protocol.error.ErrorReporter;
 import com.comphenix.protocol.error.Report;
 import com.comphenix.protocol.error.ReportType;
 import com.comphenix.protocol.injector.ListenerInvoker;
@@ -88,15 +87,10 @@ class InjectedArrayList extends ArrayList<Object> {
 			return true;
 			
 		} catch (InvocationTargetException e) {
-			ErrorReporter reporter = ProtocolLibrary.getErrorReporter();
-			
 			// Prefer to report this to the user, instead of risking sending it to Minecraft
-			if (reporter != null) {
-				reporter.reportDetailed(this, Report.newBuilder(REPORT_CANNOT_REVERT_CANCELLED_PACKET).error(e).callerParam(packet));
-			} else {
-				System.out.println("[ProtocolLib] Reverting cancelled packet failed.");
-				e.printStackTrace();
-			}
+			ProtocolLibrary.getErrorReporter().reportDetailed(this, 
+					Report.newBuilder(REPORT_CANNOT_REVERT_CANCELLED_PACKET).error(e).callerParam(packet)
+			);
 			
 			// Failure
 			return false;

ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjector.java

@@ -368,7 +368,7 @@ public abstract class PlayerInjector implements SocketInjector {
 					return null;
 				
 				hasProxyType = true;
-				reporter.reportWarning(this, Report.newBuilder(REPORT_DETECTED_CUSTOM_SERVER_HANDLER).callerParam(notchEntity, serverField));
+				reporter.reportWarning(this, Report.newBuilder(REPORT_DETECTED_CUSTOM_SERVER_HANDLER).callerParam(serverField));
 				
 				// No? Is it a Proxy type?
 				try {

ProtocolLib/src/main/java/com/comphenix/protocol/injector/spigot/SpigotPacketInjector.java

@@ -22,7 +22,9 @@ import net.sf.cglib.proxy.NoOp;
 
 import com.comphenix.protocol.Packets;
 import com.comphenix.protocol.concurrency.IntegerSet;
+import com.comphenix.protocol.error.DelegatedErrorReporter;
 import com.comphenix.protocol.error.ErrorReporter;
+import com.comphenix.protocol.error.Report;
 import com.comphenix.protocol.events.PacketContainer;
 import com.comphenix.protocol.events.PacketEvent;
 import com.comphenix.protocol.injector.ListenerInvoker;
@@ -275,7 +277,8 @@ public class SpigotPacketInjector implements SpigotPacketListener {
 		if (dummyInjector == null) {
 			// Inject the network manager
 			try {
-				NetworkObjectInjector created = new NetworkObjectInjector(classLoader, reporter, null, invoker, null);
+				NetworkObjectInjector created = new NetworkObjectInjector(
+						classLoader, filterImpossibleWarnings(reporter), null, invoker, null);
 				
 				if (MinecraftReflection.isLoginHandler(connection)) {
 					created.initialize(connection);
@@ -303,6 +306,23 @@ public class SpigotPacketInjector implements SpigotPacketListener {
 		return dummyInjector;
 	}
 
+	/**
+	 * Return a delegated error reporter that ignores certain warnings that are irrelevant on Spigot.
+	 * @param reporter - error reporter to delegate.
+	 * @return The filtered error reporter.
+	 */
+	private ErrorReporter filterImpossibleWarnings(ErrorReporter reporter) {
+		return new DelegatedErrorReporter(reporter) {
+			@Override
+			protected Report filterReport(Object sender, Report report, boolean detailed) {
+				// This doesn't matter - ignore it
+				if (report.getType() == NetworkObjectInjector.REPORT_DETECTED_CUSTOM_SERVER_HANDLER)
+					return null;
+				return report;
+			}
+		};
+	}
+	
 	/**
 	 * Save a given player injector for later.
 	 * @param networkManager - the associated network manager.
@@ -400,7 +420,8 @@ public class SpigotPacketInjector implements SpigotPacketListener {
 	 */
 	void injectPlayer(Player player) {
 		try {
-			NetworkObjectInjector dummy = new NetworkObjectInjector(classLoader, reporter, player, invoker, null);
+			NetworkObjectInjector dummy = new NetworkObjectInjector(
+					classLoader, filterImpossibleWarnings(reporter), player, invoker, null);
 			dummy.initializePlayer(player);
 			
 			// Save this player for the network manager

ProtocolLib/src/main/java/com/comphenix/protocol/reflect/compiler/BackgroundCompiler.java

@@ -29,8 +29,6 @@ import java.util.concurrent.RejectedExecutionException;
 import java.util.concurrent.ThreadFactory;
 import java.util.concurrent.TimeUnit;
 
-import javax.annotation.Nullable;
-
 import com.comphenix.protocol.error.ErrorReporter;
 import com.comphenix.protocol.error.Report;
 import com.comphenix.protocol.error.ReportType;
@@ -122,11 +120,13 @@ public class BackgroundCompiler {
 	}
 
 	// Avoid "Constructor call must be the first statement".
-	private void initializeCompiler(ClassLoader loader, @Nullable ErrorReporter reporter, ExecutorService executor) {
+	private void initializeCompiler(ClassLoader loader, ErrorReporter reporter, ExecutorService executor) {
 		if (loader == null)
 			throw new IllegalArgumentException("loader cannot be NULL");
 		if (executor == null)
 			throw new IllegalArgumentException("executor cannot be NULL");
+		if (reporter == null)
+			throw new IllegalArgumentException("reporter cannot be NULL.");
 		
 		this.compiler = new StructureCompiler(loader);
 		this.reporter = reporter;
@@ -226,14 +226,9 @@ public class BackgroundCompiler {
 						setEnabled(false);
 						
 						// Inform about this error as best as we can
-						if (reporter != null) {
-							reporter.reportDetailed(BackgroundCompiler.this,
-									Report.newBuilder(REPORT_CANNOT_COMPILE_STRUCTURE_MODIFIER).callerParam(uncompiled).error(e)
-							);
-						} else {
-							System.err.println("Exception occured in structure compiler: ");
-							e.printStackTrace();
-						}
+						reporter.reportDetailed(BackgroundCompiler.this,
+								Report.newBuilder(REPORT_CANNOT_COMPILE_STRUCTURE_MODIFIER).callerParam(uncompiled).error(e)
+						);
 					}
 					
 					// We'll also return the new structure modifier