From 00ddff59444afa6c859c28a2cecad71156441a87 Mon Sep 17 00:00:00 2001 From: theEvilReaper Date: Fri, 9 Oct 2026 09:05:59 +0200 Subject: [PATCH 1/4] fix(phase): remove the registered listener in GamePhase --- .../xerus/api/phase/GamePhase.java | 5 ++-- .../xerus/api/phase/GamePhaseTest.java | 25 ++++++++++++++++++- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/main/java/net/theevilreaper/xerus/api/phase/GamePhase.java b/src/main/java/net/theevilreaper/xerus/api/phase/GamePhase.java index 000d7e5..4bd6874 100644 --- a/src/main/java/net/theevilreaper/xerus/api/phase/GamePhase.java +++ b/src/main/java/net/theevilreaper/xerus/api/phase/GamePhase.java @@ -46,7 +46,7 @@ protected GamePhase(@NotNull String name) { public EventListener addListener(@NotNull Class eventClass, @NotNull Consumer listener) { this.verifyValueIntegrity(); var eventListener = EventListener.of(eventClass, listener); - this.listenerHashMap.put(eventClass, EventListener.of(eventClass, listener)); + this.listenerHashMap.put(eventClass, eventListener); this.phaseNode.addListener(eventListener); return eventListener; } @@ -67,11 +67,12 @@ public void initNode(@NotNull EventNode node) { * * @param eventClass the class, which should be removed * @param the event class must inherit from the {@link Event} - * @return the removed listener + * @return the removed listener or {@code null} if no listener is registered for the given class */ public EventListener removeListener(@NotNull Class eventClass) { if (phaseNode == null || listenerHashMap == null) return null; var eventListener = this.listenerHashMap.remove(eventClass); + if (eventListener == null) return null; this.phaseNode.removeListener(eventListener); return eventListener; } diff --git a/src/test/java/net/theevilreaper/xerus/api/phase/GamePhaseTest.java b/src/test/java/net/theevilreaper/xerus/api/phase/GamePhaseTest.java index 975e659..e21d36b 100644 --- a/src/test/java/net/theevilreaper/xerus/api/phase/GamePhaseTest.java +++ b/src/test/java/net/theevilreaper/xerus/api/phase/GamePhaseTest.java @@ -2,11 +2,12 @@ import net.minestom.server.event.Event; import net.minestom.server.event.EventListener; +import net.minestom.server.event.EventNode; import org.junit.jupiter.api.Test; import java.util.concurrent.atomic.AtomicBoolean; -import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.*; class GamePhaseTest { @@ -34,4 +35,26 @@ void testAddAndRemoveListener() { EventListener removed = phase.removeListener(TestEvent.class); assertNotNull(removed); } + + @Test + void testRemovedListenerIsNotCalled() { + DummyGamePhase phase = new DummyGamePhase(); + EventNode node = EventNode.all("test"); + phase.initNode(node); + AtomicBoolean called = new AtomicBoolean(false); + + EventListener listener = phase.addListener(TestEvent.class, event -> called.set(true)); + assertSame(listener, phase.removeListener(TestEvent.class)); + + node.call(new TestEvent()); + assertFalse(called.get()); + } + + @Test + void testRemoveUnknownListener() { + DummyGamePhase phase = new DummyGamePhase(); + phase.addListener(TestEvent.class, event -> {}); + + assertNull(phase.removeListener(Event.class)); + } } From 2eb5b1abeb4473b2125b9bd684c325b75ab52b12 Mon Sep 17 00:00:00 2001 From: theEvilReaper Date: Fri, 9 Oct 2026 09:05:59 +0200 Subject: [PATCH 2/4] fix(phase): ignore repeated start calls on a running GamePhase --- .../theevilreaper/xerus/api/phase/GamePhase.java | 2 ++ .../xerus/api/phase/GamePhaseTest.java | 14 ++++++++++++++ 2 files changed, 16 insertions(+) diff --git a/src/main/java/net/theevilreaper/xerus/api/phase/GamePhase.java b/src/main/java/net/theevilreaper/xerus/api/phase/GamePhase.java index 4bd6874..4780f4a 100644 --- a/src/main/java/net/theevilreaper/xerus/api/phase/GamePhase.java +++ b/src/main/java/net/theevilreaper/xerus/api/phase/GamePhase.java @@ -80,9 +80,11 @@ public EventListener removeListener(@NotNull /** * Starts the phase by calling the overridden start method from the superclass. * Adds the phaseNode to the global event handler if phaseNode is not null. + * Calling this method on an already running phase has no effect. */ @Override public void start() { + if (isRunning()) return; super.start(); if (phaseNode != null) { diff --git a/src/test/java/net/theevilreaper/xerus/api/phase/GamePhaseTest.java b/src/test/java/net/theevilreaper/xerus/api/phase/GamePhaseTest.java index e21d36b..5c6df3b 100644 --- a/src/test/java/net/theevilreaper/xerus/api/phase/GamePhaseTest.java +++ b/src/test/java/net/theevilreaper/xerus/api/phase/GamePhaseTest.java @@ -3,12 +3,16 @@ import net.minestom.server.event.Event; import net.minestom.server.event.EventListener; import net.minestom.server.event.EventNode; +import net.minestom.testing.Env; +import net.minestom.testing.extension.MicrotusExtension; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import java.util.concurrent.atomic.AtomicBoolean; import static org.junit.jupiter.api.Assertions.*; +@ExtendWith(MicrotusExtension.class) class GamePhaseTest { private static class TestEvent implements Event { @@ -57,4 +61,14 @@ void testRemoveUnknownListener() { assertNull(phase.removeListener(Event.class)); } + + @Test + void testStartTwiceWithNode(Env env) { + DummyGamePhase phase = new DummyGamePhase(); + phase.addListener(TestEvent.class, event -> {}); + + phase.start(); + assertDoesNotThrow(phase::start); + phase.finish(); + } } From 542c41f007aee423dcc18b0481fdb41643b4880b Mon Sep 17 00:00:00 2001 From: theEvilReaper Date: Fri, 9 Oct 2026 09:05:59 +0200 Subject: [PATCH 3/4] fix(phase): invoke TimedPhase#onFinish only once --- .../xerus/api/phase/TimedPhase.java | 4 ++ .../xerus/api/phase/TimedPhaseTest.java | 38 +++++++++++++++++++ 2 files changed, 42 insertions(+) create mode 100644 src/test/java/net/theevilreaper/xerus/api/phase/TimedPhaseTest.java diff --git a/src/main/java/net/theevilreaper/xerus/api/phase/TimedPhase.java b/src/main/java/net/theevilreaper/xerus/api/phase/TimedPhase.java index e89861c..701b154 100644 --- a/src/main/java/net/theevilreaper/xerus/api/phase/TimedPhase.java +++ b/src/main/java/net/theevilreaper/xerus/api/phase/TimedPhase.java @@ -111,12 +111,16 @@ public void onUpdate0() { /** * Marks the phase as finished and invokes {@link #onFinish()}. *

+ * If the phase is already finished, this method has no effect. + *

+ *

* Subclasses overriding this method must call {@code super.finish()}. *

*/ @Override @MustBeInvokedByOverriders public void finish() { + if (isFinished()) return; onFinish(); super.finish(); } diff --git a/src/test/java/net/theevilreaper/xerus/api/phase/TimedPhaseTest.java b/src/test/java/net/theevilreaper/xerus/api/phase/TimedPhaseTest.java new file mode 100644 index 0000000..d68ccef --- /dev/null +++ b/src/test/java/net/theevilreaper/xerus/api/phase/TimedPhaseTest.java @@ -0,0 +1,38 @@ +package net.theevilreaper.xerus.api.phase; + +import org.junit.jupiter.api.Test; + +import java.time.temporal.ChronoUnit; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +class TimedPhaseTest { + + private static class CountingTimedPhase extends TimedPhase { + + private int finishCalls; + + CountingTimedPhase() { + super("Timed", ChronoUnit.SECONDS); + } + + @Override + protected void onFinish() { + finishCalls++; + } + + @Override + public void onUpdate() { + } + } + + @Test + void testOnFinishCalledOnce() { + CountingTimedPhase phase = new CountingTimedPhase(); + + phase.finish(); + phase.finish(); + + assertEquals(1, phase.finishCalls); + } +} From c71e73e9e40159b9b7af320719cec8a1dd11cb2a Mon Sep 17 00:00:00 2001 From: theEvilReaper Date: Fri, 9 Oct 2026 09:05:59 +0200 Subject: [PATCH 4/4] fix(phase): do not skip the running phase when resuming a LinearPhaseSeries --- .../xerus/api/phase/LinearPhaseSeries.java | 7 +++- .../phase/AdvancedLinearPhaseSeriesTest.java | 41 ++++++++++++++++++- 2 files changed, 45 insertions(+), 3 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 db57502..73f317f 100644 --- a/src/main/java/net/theevilreaper/xerus/api/phase/LinearPhaseSeries.java +++ b/src/main/java/net/theevilreaper/xerus/api/phase/LinearPhaseSeries.java @@ -148,12 +148,15 @@ public boolean isPaused() { /** * Sets the paused state of the phase series. *

- * If set to {@code false}, the series will attempt to advance. + * If set to {@code false} and the current phase has finished while the series was paused, + * the series advances to the next phase. A still running phase is not skipped. * * @param paused {@code true} to pause the series, {@code false} to resume */ public void setPaused(boolean paused) { this.paused = paused; - advance(); + if (!paused && isRunning() && currentPhase != null && currentPhase.isFinished()) { + advance(); + } } } diff --git a/src/test/java/net/theevilreaper/xerus/api/phase/AdvancedLinearPhaseSeriesTest.java b/src/test/java/net/theevilreaper/xerus/api/phase/AdvancedLinearPhaseSeriesTest.java index 01a4354..e0f5d4a 100644 --- a/src/test/java/net/theevilreaper/xerus/api/phase/AdvancedLinearPhaseSeriesTest.java +++ b/src/test/java/net/theevilreaper/xerus/api/phase/AdvancedLinearPhaseSeriesTest.java @@ -7,8 +7,9 @@ import org.mockito.junit.jupiter.MockitoExtension; import java.util.ArrayList; +import java.util.List; -import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.*; /** * @author Patrick Zdarsky / Rxcki @@ -39,4 +40,42 @@ void testPhasesConstructor() { phaseSeries.start(); assertEquals(phase1, phaseSeries.getCurrentPhase()); } + + @Test + void testResumeDoesNotSkipRunningPhase() { + var phase1 = new SimplePhase("Phase #1"); + var phase2 = new SimplePhase("Phase #2"); + var phaseSeries = new LinearPhaseSeries<>("Test Phase Series", new ArrayList<>(List.of(phase1, phase2))); + + phaseSeries.start(); + phaseSeries.setPaused(true); + phaseSeries.setPaused(false); + + assertEquals(phase1, phaseSeries.getCurrentPhase()); + assertFalse(phase2.isRunning()); + } + + @Test + void testResumeAdvancesWhenPhaseFinishedWhilePaused() { + var phase1 = new SimplePhase("Phase #1"); + var phase2 = new SimplePhase("Phase #2"); + var phaseSeries = new LinearPhaseSeries<>("Test Phase Series", new ArrayList<>(List.of(phase1, phase2))); + + phaseSeries.start(); + phaseSeries.setPaused(true); + phase1.finish(); + assertEquals(phase1, phaseSeries.getCurrentPhase()); + + phaseSeries.setPaused(false); + assertEquals(phase2, phaseSeries.getCurrentPhase()); + assertTrue(phase2.isRunning()); + } + + @Test + void testPauseBeforeStart() { + var phaseSeries = new LinearPhaseSeries<>("Test Phase Series", new ArrayList<>(List.of(new SimplePhase()))); + + assertDoesNotThrow(() -> phaseSeries.setPaused(true)); + assertDoesNotThrow(() -> phaseSeries.setPaused(false)); + } }