Add event phases (#1669)

* Proof of concept

* Simplify and document

* Allow events to be registered with default phases

* Use modified Kosaraju for the toposort, and add test for cyclic dependency graphs

* Separate phase-related functionality in an EventPhase class

* Revert "Separate phase-related functionality in an EventPhase class"

This reverts commit e433f34.

* Ensure that the phase order is deterministic

* Add pretty graphs

* Add a test, fix a bug, only do one sort for every constraint registration

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

@@ -16,12 +16,17 @@
 
 package net.fabricmc.fabric.api.event;
 
+import org.jetbrains.annotations.ApiStatus;
+
+import net.minecraft.util.Identifier;
+
 /**
- * Base class for Event implementations.
+ * Base class for Fabric's event implementations.
  *
  * @param <T> The listener type.
  * @see EventFactory
  */
+@ApiStatus.NonExtendable // Should only be extended by fabric API.
 public abstract class Event<T> {
 	/**
 	 * The invoker field. This should be updated by the implementation to
@@ -44,9 +49,43 @@ public final T invoker() {
 	}
 
 	/**
-	 * Register a listener to the event.
+	 * 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);
+	}
+
+	/**
+	 * 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.
+	 */
+	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.
+	}
 }

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

@@ -18,6 +18,8 @@
 
 import java.util.function.Function;
 
+import net.minecraft.util.Identifier;
+
 import net.fabricmc.fabric.impl.base.event.EventFactoryImpl;
 
 /**
@@ -93,6 +95,39 @@ 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.
+	 * 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 <T>            The listener type.
+	 * @return The Event instance.
+	 */
+	public static <T> Event<T> createWithPhases(Class<? super T> type, Function<T[], T> invokerFactory, Identifier... defaultPhases) {
+		EventFactoryImpl.ensureContainsDefault(defaultPhases);
+		EventFactoryImpl.ensureNoDuplicates(defaultPhases);
+
+		Event<T> event = createArrayBacked(type, invokerFactory);
+
+		for (int i = 1; i < defaultPhases.length; ++i) {
+			event.addPhaseOrdering(defaultPhases[i-1], defaultPhases[i]);
+		}
+
+		return event;
+	}
+
 	/**
 	 * Get the listener object name. This can be used in debugging/profiling
 	 * scenarios.

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

@@ -17,18 +17,34 @@
 package net.fabricmc.fabric.impl.base.event;
 
 import java.lang.reflect.Array;
-import java.util.Arrays;
+import java.util.ArrayList;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
 import java.util.Objects;
-import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReentrantLock;
 import java.util.function.Function;
 
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import net.minecraft.util.Identifier;
+
 import net.fabricmc.fabric.api.event.Event;
 
 class ArrayBackedEvent<T> extends Event<T> {
+	static final Logger LOGGER = LogManager.getLogger("fabric-api-base");
+
 	private final Function<T[], T> invokerFactory;
-	private final Lock lock = new ReentrantLock();
+	private final Object lock = new Object();
 	private T[] handlers;
+	/**
+	 * Registered event phases.
+	 */
+	private final Map<Identifier, EventPhaseData<T>> phases = new LinkedHashMap<>();
+	/**
+	 * Phases sorted in the correct dependency order.
+	 */
+	private final List<EventPhaseData<T>> sortedPhases = new ArrayList<>();
 
 	@SuppressWarnings("unchecked")
 	ArrayBackedEvent(Class<? super T> type, Function<T[], T> invokerFactory) {
@@ -43,16 +59,72 @@ 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!");
 
-		lock.lock();
+		synchronized (lock) {
+			getOrCreatePhase(phaseIdentifier, true).addListener(listener);
+			rebuildInvoker(handlers.length + 1);
+		}
+	}
+
+	private EventPhaseData<T> getOrCreatePhase(Identifier id, boolean sortIfCreate) {
+		EventPhaseData<T> phase = phases.get(id);
+
+		if (phase == null) {
+			phase = new EventPhaseData<>(id, handlers.getClass().getComponentType());
+			phases.put(id, phase);
+			sortedPhases.add(phase);
+
+			if (sortIfCreate) {
+				PhaseSorting.sortPhases(sortedPhases);
+			}
+		}
+
+		return phase;
+	}
+
+	private void rebuildInvoker(int newLength) {
+		// Rebuild handlers.
+		if (sortedPhases.size() == 1) {
+			// Special case with a single phase: use the array of the phase directly.
+			handlers = sortedPhases.get(0).listeners;
+		} else {
+			@SuppressWarnings("unchecked")
+			T[] newHandlers = (T[]) Array.newInstance(handlers.getClass().getComponentType(), newLength);
+			int newHandlersIndex = 0;
+
+			for (EventPhaseData<T> existingPhase : sortedPhases) {
+				int length = existingPhase.listeners.length;
+				System.arraycopy(existingPhase.listeners, 0, newHandlers, newHandlersIndex, length);
+				newHandlersIndex += length;
+			}
+
+			handlers = newHandlers;
+		}
+
+		// Rebuild invoker.
+		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.");
 
-		try {
-			handlers = Arrays.copyOf(handlers, handlers.length + 1);
-			handlers[handlers.length - 1] = listener;
-			update();
-		} finally {
-			lock.unlock();
+		synchronized (lock) {
+			EventPhaseData<T> first = getOrCreatePhase(firstPhase, false);
+			EventPhaseData<T> second = getOrCreatePhase(secondPhase, false);
+			first.subsequentPhases.add(second);
+			second.previousPhases.add(first);
+			PhaseSorting.sortPhases(this.sortedPhases);
+			rebuildInvoker(handlers.length);
 		}
 	}
 }

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

@@ -27,6 +27,8 @@
 import java.util.List;
 import java.util.function.Function;
 
+import net.minecraft.util.Identifier;
+
 import net.fabricmc.fabric.api.event.Event;
 
 public final class EventFactoryImpl {
@@ -44,6 +46,26 @@ public static <T> Event<T> createArrayBacked(Class<? super T> type, Function<T[]
 		return event;
 	}
 
+	public static void ensureContainsDefault(Identifier[] defaultPhases) {
+		for (Identifier id : defaultPhases) {
+			if (id.equals(Event.DEFAULT_PHASE)) {
+				return;
+			}
+		}
+
+		throw new IllegalArgumentException("The event phases must contain Event.DEFAULT_PHASE.");
+	}
+
+	public static void ensureNoDuplicates(Identifier[] defaultPhases) {
+		for (int i = 0; i < defaultPhases.length; ++i) {
+			for (int j = i+1; j < defaultPhases.length; ++j) {
+				if (defaultPhases[i].equals(defaultPhases[j])) {
+					throw new IllegalArgumentException("Duplicate event phase: " + defaultPhases[i]);
+				}
+			}
+		}
+	}
+
 	// Code originally by sfPlayer1.
 	// Unfortunately, it's slightly slower than just passing an empty array in the first place.
 	private static <T> T buildEmptyInvoker(Class<T> handlerClass, Function<T[], T> invokerSetup) {

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

@@ -0,0 +1,47 @@
+/*
+ * 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.impl.base.event;
+
+import java.lang.reflect.Array;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+import net.minecraft.util.Identifier;
+
+/**
+ * Data of an {@link ArrayBackedEvent} phase.
+ */
+class EventPhaseData<T> {
+	final Identifier id;
+	T[] listeners;
+	final List<EventPhaseData<T>> subsequentPhases = new ArrayList<>();
+	final List<EventPhaseData<T>> previousPhases = new ArrayList<>();
+	int visitStatus = 0; // 0: not visited, 1: visiting, 2: visited
+
+	@SuppressWarnings("unchecked")
+	EventPhaseData(Identifier id, Class<?> listenerClass) {
+		this.id = id;
+		this.listeners = (T[]) Array.newInstance(listenerClass, 0);
+	}
+
+	void addListener(T listener) {
+		int oldLength = listeners.length;
+		listeners = Arrays.copyOf(listeners, oldLength + 1);
+		listeners[oldLength] = listener;
+	}
+}