diff --git a/src/main/java/net/theevilreaper/xerus/api/phase/LinearPhaseSeries.java b/src/main/java/net/theevilreaper/xerus/api/phase/LinearPhaseSeries.java
index c899d66..db57502 100644
--- a/src/main/java/net/theevilreaper/xerus/api/phase/LinearPhaseSeries.java
+++ b/src/main/java/net/theevilreaper/xerus/api/phase/LinearPhaseSeries.java
@@ -113,7 +113,7 @@ public boolean isLastPhase() {
* Starts the currently selected phase and registers a callback to advance when it finishes.
*/
public void startCurrentPhase() {
- currentPhase.setFinishedCallback(this::advance);
+ currentPhase.setSeriesCallback(this::advance);
currentPhase.start();
}
diff --git a/src/main/java/net/theevilreaper/xerus/api/phase/Phase.java b/src/main/java/net/theevilreaper/xerus/api/phase/Phase.java
index 360cded..86f21cb 100644
--- a/src/main/java/net/theevilreaper/xerus/api/phase/Phase.java
+++ b/src/main/java/net/theevilreaper/xerus/api/phase/Phase.java
@@ -1,12 +1,17 @@
package net.theevilreaper.xerus.api.phase;
import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.List;
/**
* Represents a unit of execution within a phase-based system.
*
* A {@code Phase} is started externally and can complete at any time. Once finished,
- * it can notify an optional listener via a {@code finishedCallback}.
+ * it notifies all registered finished callbacks.
*
*
* Each phase has a unique name and maintains internal state indicating whether it is running,
@@ -19,11 +24,14 @@
*/
public abstract class Phase {
+ private static final Logger PHASE_LOGGER = LoggerFactory.getLogger(Phase.class);
+
private final String name;
+ private final List finishedCallbacks;
private boolean running;
private boolean finished;
private boolean skipping;
- private Runnable finishedCallback;
+ private Runnable seriesCallback;
/**
* Constructs a new {@code Phase} with the specified name.
@@ -32,6 +40,7 @@ public abstract class Phase {
*/
protected Phase(@NotNull String name) {
this.name = name;
+ this.finishedCallbacks = new ArrayList<>();
}
/**
@@ -64,9 +73,11 @@ public void onSkip() {
}
/**
- * Marks the phase as finished and invokes the {@code finishedCallback}, if set.
+ * Marks the phase as finished and invokes all registered finished callbacks in registration order.
*
* A phase can only be finished once. Subsequent calls will be ignored.
+ * An exception thrown by a callback is logged and does not prevent the remaining callbacks
+ * or the owning series from being notified.
*
*/
public void finish() {
@@ -78,8 +89,29 @@ public void finish() {
running = false;
skipping = false;
- if (finishedCallback != null) {
- finishedCallback.run();
+ notifyFinishedCallbacks();
+
+ if (seriesCallback != null) {
+ seriesCallback.run();
+ }
+ }
+
+ /**
+ * Invokes all registered finished callbacks in registration order.
+ *
+ * Iterates over a copy so callbacks may remove themselves while being invoked.
+ * Exceptions are logged and do not prevent the remaining callbacks from running.
+ *
+ */
+ private void notifyFinishedCallbacks() {
+ if (finishedCallbacks.isEmpty()) return;
+
+ for (Runnable callback : List.copyOf(finishedCallbacks)) {
+ try {
+ callback.run();
+ } catch (Exception exception) {
+ PHASE_LOGGER.error("A finished callback of phase '{}' threw an exception", name, exception);
+ }
}
}
@@ -150,11 +182,48 @@ public void setSkipping(boolean skipping) {
}
/**
- * Sets the callback to be invoked when this phase finishes.
+ * Registers a callback to be invoked when this phase finishes.
+ *
+ * @param callback the {@link Runnable} to be executed on phase completion
+ */
+ public void addFinishedCallback(@NotNull Runnable callback) {
+ this.finishedCallbacks.add(callback);
+ }
+
+ /**
+ * Removes a previously registered finished callback.
+ *
+ * @param callback the {@link Runnable} to remove
+ * @return {@code true} if the callback was registered, otherwise {@code false}
+ */
+ public boolean removeFinishedCallback(@NotNull Runnable callback) {
+ return this.finishedCallbacks.remove(callback);
+ }
+
+ /**
+ * Replaces all registered finished callbacks with the given one.
*
- * @param finishedCallback the {@link Runnable} to be executed on phase completion
+ * @param finishedCallback the {@link Runnable} to be executed on phase completion, or {@code null} to clear all callbacks
+ * @deprecated use {@link #addFinishedCallback(Runnable)} and {@link #removeFinishedCallback(Runnable)} instead
*/
+ @Deprecated(forRemoval = true)
public void setFinishedCallback(Runnable finishedCallback) {
- this.finishedCallback = finishedCallback;
+ this.finishedCallbacks.clear();
+ if (finishedCallback != null) {
+ this.finishedCallbacks.add(finishedCallback);
+ }
+ }
+
+ /**
+ * Sets the internal callback used by an owning {@link LinearPhaseSeries} to advance once this phase finishes.
+ *
+ * It is kept separate from the user-defined finished callbacks so it can not be removed by them.
+ * The series callback is always invoked after all finished callbacks.
+ *
+ *
+ * @param seriesCallback the {@link Runnable} to be executed by the owning series
+ */
+ void setSeriesCallback(Runnable seriesCallback) {
+ this.seriesCallback = seriesCallback;
}
}
diff --git a/src/test/java/net/theevilreaper/xerus/api/phase/CyclicPhaseSeriesTest.java b/src/test/java/net/theevilreaper/xerus/api/phase/CyclicPhaseSeriesTest.java
index 7b9cfc0..8d00ace 100644
--- a/src/test/java/net/theevilreaper/xerus/api/phase/CyclicPhaseSeriesTest.java
+++ b/src/test/java/net/theevilreaper/xerus/api/phase/CyclicPhaseSeriesTest.java
@@ -39,7 +39,7 @@ void testSetMaxIterations() {
/* @Test
void testSingleLoop() {
var runnable = Mockito.mock(Runnable.class);
- phaseSeries.setFinishedCallback(runnable);
+ phaseSeries.addFinishedCallback(runnable);
phaseSeries.setMaxIterations(1);
assertEquals(2, phaseSeries.getIterations());
@@ -57,7 +57,7 @@ void testSingleLoop() {
@Test
void testDoubleLoop() {
var runnable = Mockito.mock(Runnable.class);
- phaseSeries.setFinishedCallback(runnable);
+ phaseSeries.addFinishedCallback(runnable);
phaseSeries.setMaxIterations(2);
phaseSeries.start();
diff --git a/src/test/java/net/theevilreaper/xerus/api/phase/GamePhaseTest.java b/src/test/java/net/theevilreaper/xerus/api/phase/GamePhaseTest.java
new file mode 100644
index 0000000..975e659
--- /dev/null
+++ b/src/test/java/net/theevilreaper/xerus/api/phase/GamePhaseTest.java
@@ -0,0 +1,37 @@
+package net.theevilreaper.xerus.api.phase;
+
+import net.minestom.server.event.Event;
+import net.minestom.server.event.EventListener;
+import org.junit.jupiter.api.Test;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+class GamePhaseTest {
+
+ private static class TestEvent implements Event {
+ }
+
+ private static class DummyGamePhase extends GamePhase {
+ protected DummyGamePhase() {
+ super("Dummy");
+ }
+
+ @Override
+ protected void onStart() {
+ }
+ }
+
+ @Test
+ void testAddAndRemoveListener() {
+ DummyGamePhase phase = new DummyGamePhase();
+ AtomicBoolean called = new AtomicBoolean(false);
+
+ EventListener listener = phase.addListener(TestEvent.class, event -> called.set(true));
+ assertNotNull(listener);
+
+ EventListener extends Event> removed = phase.removeListener(TestEvent.class);
+ assertNotNull(removed);
+ }
+}
diff --git a/src/test/java/net/theevilreaper/xerus/api/phase/PhaseCallbackTest.java b/src/test/java/net/theevilreaper/xerus/api/phase/PhaseCallbackTest.java
new file mode 100644
index 0000000..4a8b19d
--- /dev/null
+++ b/src/test/java/net/theevilreaper/xerus/api/phase/PhaseCallbackTest.java
@@ -0,0 +1,139 @@
+package net.theevilreaper.xerus.api.phase;
+
+import org.junit.jupiter.api.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+class PhaseCallbackTest {
+
+ @Test
+ void testLinearSeriesKeepsUserCallback() {
+ SimplePhase phase1 = new SimplePhase("Phase #1");
+ SimplePhase phase2 = new SimplePhase("Phase #2");
+ LinearPhaseSeries series = new LinearPhaseSeries<>("Series", new ArrayList<>(List.of(phase1, phase2)));
+
+ AtomicBoolean called = new AtomicBoolean(false);
+ phase1.addFinishedCallback(() -> called.set(true));
+
+ series.start();
+ phase1.finish();
+
+ assertTrue(called.get());
+ assertEquals(phase2, series.getCurrentPhase());
+ }
+
+ @Test
+ void testUserCallbackRunsBeforeAdvance() {
+ SimplePhase phase1 = new SimplePhase("Phase #1");
+ SimplePhase phase2 = new SimplePhase("Phase #2");
+ LinearPhaseSeries series = new LinearPhaseSeries<>("Series", new ArrayList<>(List.of(phase1, phase2)));
+
+ List order = new ArrayList<>();
+ phase1.addFinishedCallback(() -> order.add("phase1"));
+ phase2.addFinishedCallback(() -> order.add("phase2"));
+ series.addFinishedCallback(() -> order.add("series"));
+
+ series.start();
+ phase1.finish();
+ assertFalse(phase2.isFinished());
+ phase2.finish();
+
+ assertEquals(List.of("phase1", "phase2", "series"), order);
+ assertTrue(series.isFinished());
+ }
+
+ @Test
+ void testCyclicSeriesKeepsUserCallback() {
+ SimplePhase phase1 = new SimplePhase("Phase #1");
+ SimplePhase phase2 = new SimplePhase("Phase #2");
+ CyclicPhaseSeries series = new CyclicPhaseSeries<>("Series", new ArrayList<>(List.of(phase1, phase2)));
+ series.setMaxIterations(2);
+
+ List calls = new ArrayList<>();
+ phase1.addFinishedCallback(() -> calls.add("phase1"));
+
+ series.start();
+ phase1.finish();
+
+ assertEquals(List.of("phase1"), calls);
+ assertEquals(phase2, series.getCurrentPhase());
+ }
+
+ @Test
+ void testMultipleCallbacksRunInOrder() {
+ SimplePhase phase = new SimplePhase("Phase");
+ List order = new ArrayList<>();
+ phase.addFinishedCallback(() -> order.add("first"));
+ phase.addFinishedCallback(() -> order.add("second"));
+
+ phase.finish();
+
+ assertEquals(List.of("first", "second"), order);
+ }
+
+ @Test
+ void testRemoveFinishedCallback() {
+ SimplePhase phase = new SimplePhase("Phase");
+ AtomicBoolean called = new AtomicBoolean(false);
+ Runnable callback = () -> called.set(true);
+ phase.addFinishedCallback(callback);
+
+ assertTrue(phase.removeFinishedCallback(callback));
+ assertFalse(phase.removeFinishedCallback(callback));
+
+ phase.finish();
+ assertFalse(called.get());
+ }
+
+ @Test
+ void testCallbackCanRemoveItselfDuringFinish() {
+ SimplePhase phase = new SimplePhase("Phase");
+ AtomicBoolean secondCalled = new AtomicBoolean(false);
+ phase.addFinishedCallback(new Runnable() {
+ @Override
+ public void run() {
+ phase.removeFinishedCallback(this);
+ }
+ });
+ phase.addFinishedCallback(() -> secondCalled.set(true));
+
+ assertDoesNotThrow(phase::finish);
+ assertTrue(secondCalled.get());
+ }
+
+ @Test
+ void testFailingCallbackDoesNotBlockOthersOrSeries() {
+ SimplePhase phase1 = new SimplePhase("Phase #1");
+ SimplePhase phase2 = new SimplePhase("Phase #2");
+ LinearPhaseSeries series = new LinearPhaseSeries<>("Series", new ArrayList<>(List.of(phase1, phase2)));
+
+ AtomicBoolean secondCalled = new AtomicBoolean(false);
+ phase1.addFinishedCallback(() -> {
+ throw new IllegalStateException("Broken callback");
+ });
+ phase1.addFinishedCallback(() -> secondCalled.set(true));
+
+ series.start();
+ assertDoesNotThrow(phase1::finish);
+
+ assertTrue(secondCalled.get());
+ assertEquals(phase2, series.getCurrentPhase());
+ }
+
+ @Test
+ @SuppressWarnings("removal")
+ void testDeprecatedSetterReplacesCallbacks() {
+ SimplePhase phase = new SimplePhase("Phase");
+ List calls = new ArrayList<>();
+ phase.addFinishedCallback(() -> calls.add("old"));
+ phase.setFinishedCallback(() -> calls.add("new"));
+
+ phase.finish();
+
+ assertEquals(List.of("new"), calls);
+ }
+}
diff --git a/src/test/java/net/theevilreaper/xerus/api/phase/PhaseStructureTest.java b/src/test/java/net/theevilreaper/xerus/api/phase/PhaseStructureTest.java
index b1d5747..45586e7 100644
--- a/src/test/java/net/theevilreaper/xerus/api/phase/PhaseStructureTest.java
+++ b/src/test/java/net/theevilreaper/xerus/api/phase/PhaseStructureTest.java
@@ -23,7 +23,7 @@ protected void onStart() {
throw new RuntimeException("Start called");
}
};
- this.phase.setFinishedCallback(() -> {
+ this.phase.addFinishedCallback(() -> {
throw new RuntimeException("Finish callback");
});
}
@@ -49,7 +49,7 @@ void testStartMethod() {
@Order(3)
@Test
void testFinishMethod() {
- assertThrowsExactly(RuntimeException.class, phase::finish, "Finish callback");
+ assertDoesNotThrow(phase::finish);
}
@Order(4)
@@ -92,6 +92,7 @@ void testSetSkipping() {
@Order(10)
@Test
+ @SuppressWarnings("removal")
void testSetFinishCallback() {
this.phase.setFinishedCallback(null);
this.phase.finish();
diff --git a/src/test/java/net/theevilreaper/xerus/api/phase/PhaseTest.java b/src/test/java/net/theevilreaper/xerus/api/phase/PhaseTest.java
index 98a6692..5dbda68 100644
--- a/src/test/java/net/theevilreaper/xerus/api/phase/PhaseTest.java
+++ b/src/test/java/net/theevilreaper/xerus/api/phase/PhaseTest.java
@@ -45,7 +45,7 @@ void testStartDouble() {
void startFinishCallback() {
var phase = new SimplePhase("Test=");
var callback = Mockito.mock(Runnable.class);
- phase.setFinishedCallback(callback);
+ phase.addFinishedCallback(callback);
phase.start();
phase.finish();
@@ -94,7 +94,7 @@ void isFinishedFull() {
void testDoubleFinished() {
var phase = new SimplePhase("Test=");
var callback = Mockito.mock(Runnable.class);
- phase.setFinishedCallback(callback);
+ phase.addFinishedCallback(callback);
phase.finish();
phase.finish();