fix(phase): fix GamePhase, TimedPhase and LinearPhaseSeries bugs

src/main/java/net/theevilreaper/xerus/api/phase/GamePhase.java

@@ -46,7 +46,7 @@ protected GamePhase(@NotNull String name) {
     public <T extends Event> EventListener<T> addListener(@NotNull Class<T> eventClass, @NotNull Consumer<T> 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,21 +67,24 @@ public void initNode(@NotNull EventNode<Event> node) {
      *
      * @param eventClass the class, which should be removed
      * @param <T>        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 <T extends Event> EventListener<? extends Event> removeListener(@NotNull Class<T> 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;
     }
 
     /**
      * 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) {

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.
      * <p>
-     * 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();
+        }
     }
 }

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()}.
      * <p>
+     * If the phase is already finished, this method has no effect.
+     * </p>
+     * <p>
      * Subclasses overriding this method must call {@code super.finish()}.
      * </p>
      */
     @Override
     @MustBeInvokedByOverriders
     public void finish() {
+        if (isFinished()) return;
         onFinish();
         super.finish();
     }

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));
+    }
 }

src/test/java/net/theevilreaper/xerus/api/phase/GamePhaseTest.java

@@ -2,12 +2,17 @@
 
 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.assertNotNull;
+import static org.junit.jupiter.api.Assertions.*;
 
+@ExtendWith(MicrotusExtension.class)
 class GamePhaseTest {
 
     private static class TestEvent implements Event {
@@ -34,4 +39,36 @@ void testAddAndRemoveListener() {
         EventListener<? extends Event> removed = phase.removeListener(TestEvent.class);
         assertNotNull(removed);
     }
+
+    @Test
+    void testRemovedListenerIsNotCalled() {
+        DummyGamePhase phase = new DummyGamePhase();
+        EventNode<Event> node = EventNode.all("test");
+        phase.initNode(node);
+        AtomicBoolean called = new AtomicBoolean(false);
+
+        EventListener<TestEvent> 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));
+    }
+
+    @Test
+    void testStartTwiceWithNode(Env env) {
+        DummyGamePhase phase = new DummyGamePhase();
+        phase.addListener(TestEvent.class, event -> {});
+
+        phase.start();
+        assertDoesNotThrow(phase::start);
+        phase.finish();
+    }
 }

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);
+    }
+}