From 45ca3f116f967010e8fb3e6e481017f28e066e9b Mon Sep 17 00:00:00 2001 From: Ecconia Date: Thu, 14 Jun 2018 05:04:37 +0200 Subject: [PATCH 1/6] Missing BindableEvent These three events did not have an interface extending from BindableEvent causing a NPE on classloading of EventBuilder. Which blocked all usage of that class. Fix: adding MCPalyerEvent like in all other player events. --- .../abstraction/events/MCPlayerToggleFlightEvent.java | 6 +----- .../abstraction/events/MCPlayerToggleSneakEvent.java | 6 +----- .../abstraction/events/MCPlayerToggleSprintEvent.java | 6 +----- 3 files changed, 3 insertions(+), 15 deletions(-) diff --git a/src/main/java/com/laytonsmith/abstraction/events/MCPlayerToggleFlightEvent.java b/src/main/java/com/laytonsmith/abstraction/events/MCPlayerToggleFlightEvent.java index 0d163725f4..9f8312ea79 100644 --- a/src/main/java/com/laytonsmith/abstraction/events/MCPlayerToggleFlightEvent.java +++ b/src/main/java/com/laytonsmith/abstraction/events/MCPlayerToggleFlightEvent.java @@ -1,13 +1,9 @@ package com.laytonsmith.abstraction.events; -import com.laytonsmith.abstraction.MCPlayer; - -public interface MCPlayerToggleFlightEvent { +public interface MCPlayerToggleFlightEvent extends MCPlayerEvent { boolean isFlying(); - MCPlayer getPlayer(); - void setCancelled(boolean state); boolean isCancelled(); diff --git a/src/main/java/com/laytonsmith/abstraction/events/MCPlayerToggleSneakEvent.java b/src/main/java/com/laytonsmith/abstraction/events/MCPlayerToggleSneakEvent.java index 0bd9747827..a83a698970 100644 --- a/src/main/java/com/laytonsmith/abstraction/events/MCPlayerToggleSneakEvent.java +++ b/src/main/java/com/laytonsmith/abstraction/events/MCPlayerToggleSneakEvent.java @@ -1,13 +1,9 @@ package com.laytonsmith.abstraction.events; -import com.laytonsmith.abstraction.MCPlayer; - -public interface MCPlayerToggleSneakEvent { +public interface MCPlayerToggleSneakEvent extends MCPlayerEvent { boolean isSneaking(); - MCPlayer getPlayer(); - void setCancelled(boolean state); boolean isCancelled(); diff --git a/src/main/java/com/laytonsmith/abstraction/events/MCPlayerToggleSprintEvent.java b/src/main/java/com/laytonsmith/abstraction/events/MCPlayerToggleSprintEvent.java index ce7cd1b5c4..d633ed47bc 100644 --- a/src/main/java/com/laytonsmith/abstraction/events/MCPlayerToggleSprintEvent.java +++ b/src/main/java/com/laytonsmith/abstraction/events/MCPlayerToggleSprintEvent.java @@ -1,13 +1,9 @@ package com.laytonsmith.abstraction.events; -import com.laytonsmith.abstraction.MCPlayer; - -public interface MCPlayerToggleSprintEvent { +public interface MCPlayerToggleSprintEvent extends MCPlayerEvent { boolean isSprinting(); - MCPlayer getPlayer(); - void setCancelled(boolean state); boolean isCancelled(); From 1031c8b52f57d733a9663411f1a4c7bdd62a6dd7 Mon Sep 17 00:00:00 2001 From: Ecconia Date: Thu, 14 Jun 2018 05:07:39 +0200 Subject: [PATCH 2/6] Rewriting ManualTrigger to send serverwide ManualTrigger was only able to trigger an event if there was a bind() which used that event in MS. If one intends to send an only serverwide message. it was not possible. General rewrite as cleanup. Features stay the same. --- .../laytonsmith/core/events/EventUtils.java | 56 ++++++++++--------- 1 file changed, 30 insertions(+), 26 deletions(-) diff --git a/src/main/java/com/laytonsmith/core/events/EventUtils.java b/src/main/java/com/laytonsmith/core/events/EventUtils.java index 6fb252f214..43b8ff9579 100644 --- a/src/main/java/com/laytonsmith/core/events/EventUtils.java +++ b/src/main/java/com/laytonsmith/core/events/EventUtils.java @@ -10,7 +10,9 @@ import com.laytonsmith.core.constructs.Target; import com.laytonsmith.core.events.BoundEvent.Priority; import com.laytonsmith.core.exceptions.CRE.CREBindException; +import com.laytonsmith.core.exceptions.CRE.CRECastException; import com.laytonsmith.core.exceptions.CRE.CREEventException; +import com.laytonsmith.core.exceptions.CRE.CREIllegalArgumentException; import com.laytonsmith.core.extensions.Extension; import com.laytonsmith.core.extensions.ExtensionManager; import com.laytonsmith.core.extensions.ExtensionTracker; @@ -140,24 +142,34 @@ public static SortedSet GetEvents(Driver type) { } public static void ManualTrigger(String eventName, CArray object, Target t, boolean serverWide) { - for(Driver type : event_handles.keySet()) { + // Get the event + Event event = EventList.getEvent(eventName); + if(event == null) { + // Abort if event is not existing + throw new CREIllegalArgumentException("Non existant event is being triggered: " + eventName, t); + } + + BindableEvent convertedEvent = null; + try { + // Either an exception will be thrown, or null returned + convertedEvent = event.convert(object, t); + } catch(UnsupportedOperationException ex) { + } + if(convertedEvent == null) { + throw new CREBindException(eventName + " doesn't support the use of trigger() yet.", t); + } + + // By Javadoc, if supportsExternal() false there is no use to call it serverwide + if(serverWide && event.supportsExternal()) { + event.manualTrigger(convertedEvent); + } else { SortedSet toRun = new TreeSet<>(); - SortedSet bounded = GetEvents(type); - Event driver = EventList.getEvent(type, eventName); - if(bounded != null) { - for(BoundEvent b : bounded) { - if(b.getEventName().equalsIgnoreCase(eventName)) { + for(Driver type : event_handles.keySet()) { + for(BoundEvent boundEvent: GetEvents(type)) { + if(boundEvent.getEventName().equalsIgnoreCase(eventName)) { try { - BindableEvent convertedEvent = null; - try { - convertedEvent = driver.convert(object, t); - } catch(UnsupportedOperationException ex) { - // The event will stay null, and be caught below - } - if(convertedEvent == null) { - throw new CREBindException(eventName + " doesn't support the use of trigger() yet.", t); - } else if(driver.matches(b.getPrefilter(), convertedEvent)) { - toRun.add(b); + if(event.matches(boundEvent.getPrefilter(), convertedEvent)) { + toRun.add(boundEvent); } } catch(PrefilterNonMatchException ex) { //Not running this one @@ -165,17 +177,9 @@ public static void ManualTrigger(String eventName, CArray object, Target t, bool } } } - //If it's not a serverwide event, or this event doesn't support external events. + if(!toRun.isEmpty()) { - if(!serverWide || !driver.supportsExternal()) { - FireListeners(toRun, driver, driver.convert(object, t)); - } else { - //It's serverwide, so we can just trigger it normally with the driver, and it should trickle back down to us - driver.manualTrigger(driver.convert(object, t)); - } - } else { - //They have fired a non existant event - ConfigRuntimeException.DoWarning(ConfigRuntimeException.CreateUncatchableException("Non existant event is being triggered: " + eventName, object.getTarget())); + FireListeners(toRun, event, convertedEvent); } } } From c48a71667b091d9774bb6f011c0f02d1235e4cc5 Mon Sep 17 00:00:00 2001 From: Ecconia Date: Thu, 14 Jun 2018 05:08:50 +0200 Subject: [PATCH 3/6] Updating docs of trigger with exceptions New exceptions are now thrown. Old ones had been ignored. Formatting the docs to the 120 character limit. --- .../core/functions/EventBinding.java | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/laytonsmith/core/functions/EventBinding.java b/src/main/java/com/laytonsmith/core/functions/EventBinding.java index c3cc3b7f25..f7d182d2b2 100644 --- a/src/main/java/com/laytonsmith/core/functions/EventBinding.java +++ b/src/main/java/com/laytonsmith/core/functions/EventBinding.java @@ -30,11 +30,13 @@ import com.laytonsmith.core.events.EventUtils; import com.laytonsmith.core.exceptions.CRE.CREBindException; import com.laytonsmith.core.exceptions.CRE.CRECastException; +import com.laytonsmith.core.exceptions.CRE.CREIllegalArgumentException; import com.laytonsmith.core.exceptions.CRE.CREInsufficientArgumentsException; import com.laytonsmith.core.exceptions.CRE.CREThrowable; import com.laytonsmith.core.exceptions.ConfigCompileException; import com.laytonsmith.core.exceptions.ConfigRuntimeException; import com.laytonsmith.core.exceptions.EventException; + import java.util.ArrayList; import java.util.EnumSet; import java.util.List; @@ -456,17 +458,22 @@ public Integer[] numArgs() { @Override public String docs() { - return "void {eventName, eventObject, [serverWide]} Manually triggers bound events. The event object passed to this function is " - + " sent directly as-is to the bound events. Check the documentation for each event to see what is required." - + " No checks will be done on the data here, but it is not recommended to fail to send all parameters required." - + " If serverWide is true, the event is triggered directly in the server, unless it is a CommandHelper specific" - + " event, in which case, serverWide is irrelevant. Defaults to false, which means that only CommandHelper code" - + " will receive the event."; + return "void {eventName, eventObject, [serverWide]} Manually triggers bound events." + + " The event object passed to this function is sent directly as-is to the bound events." + + " Check the documentation for each event to see what is required." + + " No checks will be done on the data here," + + " but it is not recommended to fail to send all parameters required." + + " If serverWide is true, the event is triggered directly in the server," + + " unless it is a CommandHelper specific event, in which case, serverWide is irrelevant." + + " Defaults to false, which means that only CommandHelper code will receive the event." + + " Throws a CastException when eventObject is not an array and not null." + + " Throws a BindException when " + getName() + "() is not yet supported by the given event." + + " Throws a IllegalArgumentException exception, if the event does not exist."; } @Override public Class[] thrown() { - return new Class[]{CRECastException.class}; + return new Class[]{CRECastException.class, CREBindException.class, CREIllegalArgumentException.class}; } @Override From 62827276cec5202e4f77772e383294f471e1b2f9 Mon Sep 17 00:00:00 2001 From: Ecconia Date: Thu, 14 Jun 2018 05:14:36 +0200 Subject: [PATCH 4/6] Fixing EventBuilder -> Using EventBuilder resulted in spam in console on proper usage. EventBuilder is checking in static code if each Event has a _instantiate method to create it. Some events don't need to be created since convert() returns null or throws an exception. By removing the "warmup" for all events on classloading, the loads is distributed and the actual checks if _instantiate exists will only be done, when a convert() method actually needs to create an event. -> instantiate() expects _instantiate() to return a Bukkit event, but all except one event return a BukkitMC*Event from CH. There is no need for EventBuilder to insert the Bukkit event into the BukkitMC*Event with reflection and overhead. --- .../laytonsmith/core/events/EventBuilder.java | 39 +------------------ 1 file changed, 2 insertions(+), 37 deletions(-) diff --git a/src/main/java/com/laytonsmith/core/events/EventBuilder.java b/src/main/java/com/laytonsmith/core/events/EventBuilder.java index daf6ee62f0..8614aa1047 100644 --- a/src/main/java/com/laytonsmith/core/events/EventBuilder.java +++ b/src/main/java/com/laytonsmith/core/events/EventBuilder.java @@ -4,9 +4,6 @@ import com.laytonsmith.PureUtilities.Common.StreamUtils; import com.laytonsmith.abstraction.Implementation; import com.laytonsmith.annotations.abstraction; -import com.laytonsmith.core.constructs.Target; -import com.laytonsmith.core.exceptions.CRE.CREPluginInternalException; -import java.lang.reflect.Constructor; import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.util.HashMap; @@ -22,7 +19,6 @@ private EventBuilder() { } private static final Map, Method> methods = new HashMap, Method>(); - private static final Map, Constructor> constructors = new HashMap, Constructor>(); private static final Map, Class> eventImplementations = new HashMap, Class>(); static { @@ -39,8 +35,6 @@ private EventBuilder() { } } eventImplementations.put(cinterface, c); - //Also, warm it up - warmup(cinterface); } } } @@ -64,7 +58,7 @@ private static void warmup(Class clazz) { if(method == null) { StreamUtils.GetSystemErr().println("UNABLE TO CACHE A CONSTRUCTOR FOR " + clazz.getSimpleName() + ". Manual triggering will be impossible, and errors will occur" - + " if an attempt is made. Did you forget to add" + + " if an attempt is made. Did someone forget to add" + " public static _instantiate(...) to " + clazz.getSimpleName() + "?"); } methods.put((Class) clazz, method); @@ -76,33 +70,7 @@ public static T instantiate(Class) clazz)) { warmup(clazz); } - Object o = methods.get((Class) clazz).invoke(null, params); - //Now, we have an instance of the underlying object, which the instance - //of the event BindableEvent should know how to handle in a constructor. - if(!constructors.containsKey((Class) clazz)) { - Class bindableEvent = eventImplementations.get((Class) clazz); - Constructor constructor = null; - for(Constructor c : bindableEvent.getConstructors()) { - if(c.getParameterTypes().length == 1) { - //looks promising - if(c.getParameterTypes()[0].equals(o.getClass())) { - //This is it - constructor = c; - break; - } - } - } - if(constructor == null) { - throw new CREPluginInternalException("Cannot find an acceptable constructor that follows the format:" - + " public " + bindableEvent.getClass().getSimpleName() + "(" + o.getClass().getSimpleName() + " event)." - + " Please notify the plugin author of this error.", Target.UNKNOWN); - } - constructors.put((Class) clazz, constructor); - } - //Construct a new instance, then return it. - Constructor constructor = constructors.get((Class) clazz); - BindableEvent be = (BindableEvent) constructor.newInstance(o); - return (T) be; + return (T) methods.get((Class) clazz).invoke(null, params); } catch(Exception e) { e.printStackTrace(); @@ -110,7 +78,4 @@ public static T instantiate(Class Date: Thu, 14 Jun 2018 05:16:45 +0200 Subject: [PATCH 5/6] Changing inconsistent code This is the only event which does not return a BukkitMC*Event instead it returned a Bukkit event. Which is no longer accepted. --- .../abstraction/bukkit/events/BukkitPlayerEvents.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/laytonsmith/abstraction/bukkit/events/BukkitPlayerEvents.java b/src/main/java/com/laytonsmith/abstraction/bukkit/events/BukkitPlayerEvents.java index e6caf082c8..d3570eba02 100644 --- a/src/main/java/com/laytonsmith/abstraction/bukkit/events/BukkitPlayerEvents.java +++ b/src/main/java/com/laytonsmith/abstraction/bukkit/events/BukkitPlayerEvents.java @@ -552,8 +552,8 @@ public void setJoinMessage(String message) { pje.setJoinMessage(message); } - public static PlayerJoinEvent _instantiate(MCPlayer player, String message) { - return new PlayerJoinEvent(((BukkitMCPlayer) player)._Player(), message); + public static BukkitMCPlayerJoinEvent _instantiate(MCPlayer player, String message) { + return new BukkitMCPlayerJoinEvent(new PlayerJoinEvent(((BukkitMCPlayer) player)._Player(), message)); } } From 08e86d248cc78b6f03cd0932080d077fbf1d62f7 Mon Sep 17 00:00:00 2001 From: Ecconia Date: Sat, 21 Jul 2018 03:55:02 +0200 Subject: [PATCH 6/6] Style-Update Updating Style to get rid of merge conflicts. --- .../laytonsmith/core/events/EventBuilder.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/laytonsmith/core/events/EventBuilder.java b/src/main/java/com/laytonsmith/core/events/EventBuilder.java index 8614aa1047..b0444d7fdf 100644 --- a/src/main/java/com/laytonsmith/core/events/EventBuilder.java +++ b/src/main/java/com/laytonsmith/core/events/EventBuilder.java @@ -18,8 +18,9 @@ public final class EventBuilder { private EventBuilder() { } - private static final Map, Method> methods = new HashMap, Method>(); - private static final Map, Class> eventImplementations = new HashMap, Class>(); + private static final Map, Method> METHODS = new HashMap, Method>(); + private static final Map, Class> EVENT_IMPLEMENTATIONS = + new HashMap, Class>(); static { //First, we need to pull all the event implementors @@ -34,7 +35,7 @@ private EventBuilder() { break; } } - eventImplementations.put(cinterface, c); + EVENT_IMPLEMENTATIONS.put(cinterface, c); } } } @@ -46,8 +47,8 @@ private EventBuilder() { * @param clazz */ private static void warmup(Class clazz) { - if(!methods.containsKey((Class) clazz)) { - Class implementor = eventImplementations.get((Class) clazz); + if(!METHODS.containsKey((Class) clazz)) { + Class implementor = EVENT_IMPLEMENTATIONS.get((Class) clazz); Method method = null; for(Method m : implementor.getMethods()) { if(m.getName().equals("_instantiate") && (m.getModifiers() & Modifier.STATIC) != 0) { @@ -61,16 +62,16 @@ private static void warmup(Class clazz) { + " if an attempt is made. Did someone forget to add" + " public static _instantiate(...) to " + clazz.getSimpleName() + "?"); } - methods.put((Class) clazz, method); + METHODS.put((Class) clazz, method); } } public static T instantiate(Class clazz, Object... params) { try { - if(!methods.containsKey((Class) clazz)) { + if(!METHODS.containsKey((Class) clazz)) { warmup(clazz); } - return (T) methods.get((Class) clazz).invoke(null, params); + return (T) METHODS.get((Class) clazz).invoke(null, params); } catch(Exception e) { e.printStackTrace();