Separate phase-related functionality in an EventPhase class

fabric-api-base/src/main/java/net/fabricmc/fabric/api/event/Event.java

@@ -20,6 +20,8 @@
 
 import net.minecraft.util.Identifier;
 
+import net.fabricmc.fabric.impl.base.event.UnsupportedEventPhase;
+
 /**
  * Base class for Fabric's event implementations.
  *
@@ -50,42 +52,19 @@ public final T invoker() {
 
 	/**
 	 * Register a listener to the event, in the default phase.
-	 * Have a look at {@link #addPhaseOrdering} for an explanation of event phases.
 	 *
 	 * @param listener The desired listener.
 	 */
-	public abstract void register(T listener);
-
-	/**
-	 * The identifier of the default phase.
-	 * Have a look at {@link EventFactory#createWithPhases} for an explanation of event phases.
-	 */
-	public static final Identifier DEFAULT_PHASE = new Identifier("fabric", "default");
-
-	/**
-	 * Register a listener to the event for the specified phase.
-	 * Have a look at {@link EventFactory#createWithPhases} for an explanation of event phases.
-	 *
-	 * @param phase Identifier of the phase this listener should be registered for. It will be created if it didn't exist yet.
-	 * @param listener The desired listener.
-	 */
-	public void register(Identifier phase, T listener) {
-		// This is done to keep compatibility with existing Event subclasses, but they should really not be subclassing Event.
-		register(listener);
+	public void register(T listener) {
+		phase(EventPhase.DEFAULT).register(listener);
 	}
 
 	/**
-	 * Request that listeners registered for one phase be executed before listeners registered for another phase.
-	 * Relying on the default phases supplied to {@link EventFactory#createWithPhases} should be preferred over manually
-	 * registering phase ordering dependencies.
-	 *
-	 * <p>Incompatible ordering constraints such as cycles will lead to inconsistent behavior:
-	 * some constraints will be respected and some will be ignored. If this happens, a warning will be logged.
-	 *
-	 * @param firstPhase The identifier of the phase that should run before the other. It will be created if it didn't exist yet.
-	 * @param secondPhase The identifier of the phase that should run after the other. It will be created if it didn't exist yet.
+	 * Retrieve a phase for this event, or create it if it didn't exist yet.
+	 * @see EventPhase
 	 */
-	public void addPhaseOrdering(Identifier firstPhase, Identifier secondPhase) {
-		// This is not abstract to avoid breaking existing Event subclasses, but they should really not be subclassing Event.
+	public EventPhase<T> phase(Identifier phaseIdentifier) {
+		// This is done to keep compatibility with existing Event subclasses, but they should really not be subclassing Event.
+		return new UnsupportedEventPhase<>(this);
 	}
 }

fabric-api-base/src/main/java/net/fabricmc/fabric/api/event/EventFactory.java

@@ -96,33 +96,24 @@ public static <T> Event<T> createArrayBacked(Class<T> type, T emptyInvoker, Func
 	}
 
 	/**
-	 * Create an array-backed event with a list of default phases that get invoked in order.
+	 * Create an array-backed event with a list of default {@linkplain EventPhase phases} that get invoked in order.
 	 * Exposing the identifiers of the default phases as {@code public static final} constants is encouraged.
 	 *
-	 * <p>An event phase is a named group of listeners, which may be ordered before or after other groups of listeners.
-	 * This allows some listeners to take priority over other listeners.
-	 * Adding separate events should be considered before making use of multiple event phases.
-	 *
-	 * <p>Phases may be freely added to events created with any of the factory functions,
-	 * however using this function is preferred for widely used event phases.
-	 * If more phases are necessary, discussion with the author of the Event is encouraged.
-	 *
-	 * <p>Refer to {@link Event#addPhaseOrdering} for an explanation of event phases.
-	 *
 	 * @param type           The listener class type.
 	 * @param invokerFactory The invoker factory, combining multiple listeners into one instance.
-	 * @param defaultPhases  The default phases of this event, in the correct order. Must contain {@link Event#DEFAULT_PHASE}.
+	 * @param defaultPhases  The default phases of this event, in the correct order. Must contain {@link EventPhase#DEFAULT}.
 	 * @param <T>            The listener type.
 	 * @return The Event instance.
+	 * @see EventPhase
 	 */
 	public static <T> Event<T> createWithPhases(Class<? super T> type, Function<T[], T> invokerFactory, Identifier... defaultPhases) {
-		EventFactoryImpl.ensureContainsDefault(defaultPhases);
-		EventFactoryImpl.ensureNoDuplicates(defaultPhases);
+		if (!EventFactoryImpl.contains(defaultPhases, EventPhase.DEFAULT)) throw new IllegalArgumentException("The default phases must contain EventPhase.DEFAULT");
+		EventFactoryImpl.ensureNoDuplicatesNoNull(defaultPhases);
 
 		Event<T> event = createArrayBacked(type, invokerFactory);
 
 		for (int i = 1; i < defaultPhases.length; ++i) {
-			event.addPhaseOrdering(defaultPhases[i-1], defaultPhases[i]);
+			event.phase(defaultPhases[i-1]).runBefore(defaultPhases[i]);
 		}
 
 		return event;

fabric-api-base/src/main/java/net/fabricmc/fabric/api/event/EventPhase.java

@@ -0,0 +1,61 @@
+/*
+ * Copyright (c) 2016, 2017, 2018, 2019 FabricMC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package net.fabricmc.fabric.api.event;
+
+import net.minecraft.util.Identifier;
+
+/**
+ * An event phase is a named group of event listeners, which may be ordered before or after other groups of listeners.
+ * This allows some listeners to take priority over other listeners.
+ * Adding separate events should be considered before making use of multiple event phases.
+ *
+ * <p>Phases may be freely added to any event, however using the default phases passed to {@link EventFactory#createWithPhases}
+ * is preferred to manually adding them. If more phases are necessary, discussion with the author of the Event is encouraged.
+ */
+public interface EventPhase<T> {
+	/**
+	 * The identifier of the default phase.
+	 */
+	Identifier DEFAULT = new Identifier("fabric", "default");
+
+	/**
+	 * Register a listener to this event phase.
+	 *
+	 * @param listener The desired listener.
+	 */
+	void register(T listener);
+
+	/**
+	 * Request that listeners registered to this phase be ran before listeners registered to some other phases.
+	 * Relying on the default phases supplied to {@link EventFactory#createWithPhases} should be preferred over manually
+	 * registering phase ordering dependencies.
+	 *
+	 * <p>Incompatible ordering constraints such as cycles will lead to inconsistent ordering of listeners inside the cycle.
+	 * If this happens, a warning will be logged.
+	 *
+	 * @param subsequentPhases Identifiers of the phases that should run AFTER this one.
+	 */
+	void runBefore(Identifier... subsequentPhases);
+
+	/**
+	 * Request that listeners registered to this phase be ran after listeners registered to some other phases.
+	 *
+	 * @param previousPhases Identifiers of the phases that should run BEFORE this one.
+	 * @see #runBefore
+	 */
+	void runAfter(Identifier... previousPhases);
+}

fabric-api-base/src/main/java/net/fabricmc/fabric/impl/base/event/ArrayBackedEvent.java

@@ -32,6 +32,7 @@
 import net.minecraft.util.Identifier;
 
 import net.fabricmc.fabric.api.event.Event;
+import net.fabricmc.fabric.api.event.EventPhase;
 
 class ArrayBackedEvent<T> extends Event<T> {
 	private static final Logger LOGGER = LogManager.getLogger("fabric-api-base");
@@ -42,11 +43,11 @@ class ArrayBackedEvent<T> extends Event<T> {
 	/**
 	 * Registered event phases.
 	 */
-	private final Map<Identifier, EventPhaseData<T>> phases = new LinkedHashMap<>();
+	private final Map<Identifier, EventPhaseData> phases = new LinkedHashMap<>();
 	/**
 	 * Phases sorted in the correct dependency order.
 	 */
-	private final List<EventPhaseData<T>> sortedPhases = new ArrayList<>();
+	private final List<EventPhaseData> sortedPhases = new ArrayList<>();
 
 	@SuppressWarnings("unchecked")
 	ArrayBackedEvent(Class<? super T> type, Function<T[], T> invokerFactory) {
@@ -60,26 +61,19 @@ void update() {
 	}
 
 	@Override
-	public void register(T listener) {
-		register(DEFAULT_PHASE, listener);
-	}
-
-	@Override
-	public void register(Identifier phaseIdentifier, T listener) {
-		Objects.requireNonNull(phaseIdentifier, "Tried to register a listener for a null phase!");
-		Objects.requireNonNull(listener, "Tried to register a null listener!");
+	public EventPhase<T> phase(Identifier phaseIdentifier) {
+		Objects.requireNonNull(phaseIdentifier, "Tried to retrieve a null phase.");
 
 		synchronized (lock) {
-			getOrCreatePhase(phaseIdentifier).addListener(listener);
-			rebuildInvoker(handlers.length + 1);
+			return getOrCreatePhase(phaseIdentifier);
 		}
 	}
 
-	private EventPhaseData<T> getOrCreatePhase(Identifier id) {
-		EventPhaseData<T> phase = phases.get(id);
+	private EventPhaseData getOrCreatePhase(Identifier id) {
+		EventPhaseData phase = phases.get(id);
 
 		if (phase == null) {
-			phase = new EventPhaseData<>(id, handlers.getClass().getComponentType());
+			phase = new EventPhaseData(id, handlers.getClass().getComponentType());
 			phases.put(id, phase);
 			sortedPhases.add(phase);
 		}
@@ -97,7 +91,7 @@ private void rebuildInvoker(int newLength) {
 			T[] newHandlers = (T[]) Array.newInstance(handlers.getClass().getComponentType(), newLength);
 			int newHandlersIndex = 0;
 
-			for (EventPhaseData<T> existingPhase : sortedPhases) {
+			for (EventPhaseData existingPhase : sortedPhases) {
 				for (T handler : existingPhase.listeners) {
 					newHandlers[newHandlersIndex++] = handler;
 				}
@@ -110,52 +104,36 @@ private void rebuildInvoker(int newLength) {
 		update();
 	}
 
-	@Override
-	public void addPhaseOrdering(Identifier firstPhase, Identifier secondPhase) {
-		Objects.requireNonNull(firstPhase, "Tried to add an ordering for a null phase.");
-		Objects.requireNonNull(secondPhase, "Tried to add an ordering for a null phase.");
-		if (firstPhase.equals(secondPhase)) throw new IllegalArgumentException("Tried to add a phase that depends on itself.");
-
-		synchronized (lock) {
-			EventPhaseData<T> first = getOrCreatePhase(firstPhase);
-			EventPhaseData<T> second = getOrCreatePhase(secondPhase);
-			first.subsequentPhases.add(second);
-			second.previousPhases.add(first);
-			sortPhases();
-			rebuildInvoker(handlers.length);
-		}
-	}
-
 	/**
 	 * Uses a modified Kosaraju SCC to sort the phases.
 	 */
 	private void sortPhases() {
 		sortedPhases.clear();
 
 		// FIRST VISIT
-		List<EventPhaseData<T>> toposort = new ArrayList<>(phases.size());
+		List<EventPhaseData> toposort = new ArrayList<>(phases.size());
 
-		for (EventPhaseData<T> phase : phases.values()) {
+		for (EventPhaseData phase : phases.values()) {
 			forwardVisit(phase, null, toposort);
 		}
 
 		clearStatus(toposort);
 		Collections.reverse(toposort);
 
 		// SECOND VISIT
-		for (EventPhaseData<T> phase : toposort) {
+		for (EventPhaseData phase : toposort) {
 			backwardVisit(phase);
 		}
 
 		clearStatus(toposort);
 	}
 
-	private void forwardVisit(EventPhaseData<T> phase, EventPhaseData<T> parent, List<EventPhaseData<T>> toposort) {
+	private void forwardVisit(EventPhaseData phase, EventPhaseData parent, List<EventPhaseData> toposort) {
 		if (phase.visitStatus == 0) {
 			// Not yet visited.
 			phase.visitStatus = 1;
 
-			for (EventPhaseData<T> data : phase.subsequentPhases) {
+			for (EventPhaseData data : phase.subsequentPhases) {
 				forwardVisit(data, phase, toposort);
 			}
 
@@ -171,28 +149,28 @@ private void forwardVisit(EventPhaseData<T> phase, EventPhaseData<T> parent, Lis
 		}
 	}
 
-	private void clearStatus(List<EventPhaseData<T>> phases) {
-		for (EventPhaseData<T> phase : phases) {
+	private void clearStatus(List<EventPhaseData> phases) {
+		for (EventPhaseData phase : phases) {
 			phase.visitStatus = 0;
 		}
 	}
 
-	private void backwardVisit(EventPhaseData<T> phase) {
+	private void backwardVisit(EventPhaseData phase) {
 		if (phase.visitStatus == 0) {
 			phase.visitStatus = 1;
 			sortedPhases.add(phase);
 
-			for (EventPhaseData<T> data : phase.previousPhases) {
+			for (EventPhaseData data : phase.previousPhases) {
 				backwardVisit(data);
 			}
 		}
 	}
 
-	private static class EventPhaseData<T> {
+	private class EventPhaseData implements EventPhase<T> {
 		final Identifier id;
 		T[] listeners;
-		final List<EventPhaseData<T>> subsequentPhases = new ArrayList<>();
-		final List<EventPhaseData<T>> previousPhases = new ArrayList<>();
+		final List<EventPhaseData> subsequentPhases = new ArrayList<>();
+		final List<EventPhaseData> previousPhases = new ArrayList<>();
 		int visitStatus = 0; // 0: not visited, 1: visiting, 2: visited
 
 		@SuppressWarnings("unchecked")
@@ -206,5 +184,51 @@ private void addListener(T listener) {
 			listeners = Arrays.copyOf(listeners, oldLength + 1);
 			listeners[oldLength] = listener;
 		}
+
+		@Override
+		public void register(T listener) {
+			Objects.requireNonNull(listener, "Tried to register a null listener!");
+
+			synchronized (lock) {
+				addListener(listener);
+				rebuildInvoker(handlers.length + 1);
+			}
+		}
+
+		@Override
+		public void runBefore(Identifier... subsequentPhases) {
+			EventFactoryImpl.ensureNoDuplicatesNoNull(subsequentPhases);
+			if (subsequentPhases.length == 0) throw new IllegalArgumentException("Must register at least one subsequent phase.");
+			if (EventFactoryImpl.contains(subsequentPhases, id)) throw new IllegalArgumentException("Event phase may not depend on itself.");
+
+			synchronized (lock) {
+				for (Identifier other : subsequentPhases) {
+					EventPhaseData second = getOrCreatePhase(other);
+					this.subsequentPhases.add(second);
+					second.previousPhases.add(this);
+				}
+
+				sortPhases();
+				rebuildInvoker(handlers.length);
+			}
+		}
+
+		@Override
+		public void runAfter(Identifier... previousPhases) {
+			EventFactoryImpl.ensureNoDuplicatesNoNull(previousPhases);
+			if (previousPhases.length == 0) throw new IllegalArgumentException("Must register at least one previous phase.");
+			if (EventFactoryImpl.contains(previousPhases, id)) throw new IllegalArgumentException("Event phase may not depend on itself.");
+
+			synchronized (lock) {
+				for (Identifier other : previousPhases) {
+					EventPhaseData second = getOrCreatePhase(other);
+					this.previousPhases.add(second);
+					second.subsequentPhases.add(this);
+				}
+
+				sortPhases();
+				rebuildInvoker(handlers.length);
+			}
+		}
 	}
 }