From a41178fb9ca99474bd577428c685820e7e2fbc89 Mon Sep 17 00:00:00 2001 From: theEvilReaper Date: Fri, 9 Oct 2026 08:47:43 +0200 Subject: [PATCH 1/3] feat(phase): allow multiple finish callbacks --- .../xerus/api/phase/LinearPhaseSeries.java | 2 +- .../theevilreaper/xerus/api/phase/Phase.java | 85 +++++++++++++++++-- 2 files changed, 78 insertions(+), 9 deletions(-) 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; } } From d6a51e6d32dd5b414dabdd9c456ad855ba70cbb5 Mon Sep 17 00:00:00 2001 From: theEvilReaper Date: Fri, 9 Oct 2026 08:47:58 +0200 Subject: [PATCH 2/3] test(phase): update callback cases --- .../theevilreaper/xerus/api/phase/CyclicPhaseSeriesTest.java | 4 ++-- .../theevilreaper/xerus/api/phase/PhaseStructureTest.java | 5 +++-- .../java/net/theevilreaper/xerus/api/phase/PhaseTest.java | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) 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/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(); From 9a9b33bdf55416dc198756889eaabf006ec0b47b Mon Sep 17 00:00:00 2001 From: theEvilReaper Date: Fri, 9 Oct 2026 08:48:14 +0200 Subject: [PATCH 3/3] test(phase): add more cases --- .../xerus/api/phase/GamePhaseTest.java | 37 +++++ .../xerus/api/phase/PhaseCallbackTest.java | 139 ++++++++++++++++++ 2 files changed, 176 insertions(+) create mode 100644 src/test/java/net/theevilreaper/xerus/api/phase/GamePhaseTest.java create mode 100644 src/test/java/net/theevilreaper/xerus/api/phase/PhaseCallbackTest.java 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 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); + } +}