Added more flexible dependency injection to MusicManager for unittesting. Moved to event driven design for less complex code and lower runtime complexity.

2026-02-04 10:54:51 +00:00 · 2025-10-11 20:45:57 +02:00
parent 1ecdb9a555
commit 9749d3eee8
8 changed files with 109 additions and 60 deletions
--- a/app/src/main/java/org/toop/Main.java
+++ b/app/src/main/java/org/toop/Main.java
@@ -1,17 +1,12 @@
 package org.toop;

-import javafx.scene.media.MediaPlayer;
 import org.toop.app.App;
-import org.toop.framework.audio.AudioEventListener;
-import org.toop.framework.audio.AudioVolumeManager;
-import org.toop.framework.audio.MusicManager;
-import org.toop.framework.audio.SoundEffectManager;
+import org.toop.framework.audio.*;
 import org.toop.framework.networking.NetworkingClientManager;
 import org.toop.framework.networking.NetworkingInitializationException;
 import org.toop.framework.resource.ResourceLoader;
 import org.toop.framework.resource.ResourceManager;
 import org.toop.framework.resource.resources.MusicAsset;
-import org.toop.framework.resource.resources.SoundEffectAsset;

 public final class Main {
    static void main(String[] args) {
--- a/framework/src/main/java/org/toop/framework/audio/JavaFXDispatcher.java
+++ b/framework/src/main/java/org/toop/framework/audio/JavaFXDispatcher.java
@@ -0,0 +1,12 @@
+package org.toop.framework.audio;
+
+import javafx.application.Platform;
+import org.toop.framework.audio.interfaces.Dispatcher;
+
+// TODO isn't specific to audio
+public class JavaFXDispatcher implements Dispatcher {
+    @Override
+    public void run(Runnable task) {
+        Platform.runLater(task);
+    }
+}
--- a/framework/src/main/java/org/toop/framework/audio/MusicManager.java
+++ b/framework/src/main/java/org/toop/framework/audio/MusicManager.java
@@ -1,31 +1,40 @@
 package org.toop.framework.audio;

-import javafx.application.Platform;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.toop.framework.audio.interfaces.Dispatcher;
 import org.toop.framework.resource.ResourceManager;
 import org.toop.framework.resource.resources.BaseResource;
 import org.toop.framework.resource.types.AudioResource;

 import java.util.*;
-import java.util.concurrent.*;
-import java.util.concurrent.atomic.AtomicReference;

 public class MusicManager<T extends AudioResource> implements org.toop.framework.audio.interfaces.MusicManager<T> {
    private static final Logger logger = LogManager.getLogger(MusicManager.class);
-    private final Class<T> type;
+
    private final List<T> backgroundMusic = new LinkedList<>();
+    private final Dispatcher dispatcher;
+    private final List<T> resources;
    private int playingIndex = 0;
-    private ScheduledExecutorService scheduler;
+    private boolean playing = false;

    public MusicManager(Class<T> type) {
-        this.type = type;
-
-        Runtime.getRuntime().addShutdownHook(new Thread(this::shutdownScheduler));
+        this.dispatcher = new JavaFXDispatcher();
+        this.resources = new ArrayList<>(ResourceManager.getAllOfType((Class<? extends BaseResource>) type)
+                .stream()
+                .map(e -> (T) e.getResource())
+                .toList());
+        createShuffled();
    }

-    private void increasePlayingIndex() {
-        playingIndex = (playingIndex + 1) % backgroundMusic.size();
+    // Used in unit testing
+    MusicManager(Class<T> type, Dispatcher dispatcher, ResourceManager resourceManager) {
+        this.dispatcher = dispatcher;
+        this.resources = new ArrayList<>(resourceManager.getAllOfType((Class<? extends BaseResource>) type)
+            .stream()
+            .map(e -> (T) e.getResource())
+            .toList());
+        createShuffled();
    }

    @Override
@@ -37,52 +46,61 @@ public class MusicManager<T extends AudioResource> implements org.toop.framework
        backgroundMusic.add(musicAsset);
    }

-    private void shutdownScheduler() {
-        if (scheduler != null && !scheduler.isShutdown()) {
-            scheduler.shutdownNow();
-            scheduler = null;
-            logger.debug("MusicManager scheduler shut down.");
-        }
-    }
-
-    @Override
-    public void stop() {
-        shutdownScheduler();
-        Platform.runLater(() -> backgroundMusic.forEach(T::stop));
+    private void createShuffled() {
+        backgroundMusic.clear();
+        Collections.shuffle(resources);
+        backgroundMusic.addAll(resources);
    }

    public void play() {
-        backgroundMusic.clear();
-        @SuppressWarnings("unchecked")
-        List<T> resources = new ArrayList<>(ResourceManager.getAllOfType((Class<? extends BaseResource>) type)
-                .stream()
-                .map(e -> (T) e.getResource())
-                .toList());
-        Collections.shuffle(resources);
-        backgroundMusic.addAll(resources);
+        if (playing) {
+            logger.warn("MusicManager is already playing.");
+            return;
+        }

        if (backgroundMusic.isEmpty()) return;

-        shutdownScheduler();
-        scheduler = Executors.newSingleThreadScheduledExecutor();
+        playingIndex = 0;
+        playing = true;
+        playCurrentTrack();
+    }

-        AtomicReference<T> current = new AtomicReference<>(backgroundMusic.get(playingIndex));
+    private void playCurrentTrack() {
+        if (playingIndex >= backgroundMusic.size()) {
+            playingIndex = 0;
+        }

-        Platform.runLater(() -> {
-            T first = current.get();
-            if (!first.isPlaying()) first.play();
+        T current = backgroundMusic.get(playingIndex);
+
+        if (current == null) {
+            logger.error("Current track is null!");
+            return;
+        }
+
+        dispatcher.run(() -> {
+            current.play();
+
+            current.setOnEnd(() -> {
+                playingIndex++;
+                playCurrentTrack();
+            });
+
+            current.setOnError(() -> {
+                logger.error("Error playing track: {}", current);
+                backgroundMusic.remove(current);
+                if (!backgroundMusic.isEmpty()) {
+                    playCurrentTrack();
+                } else {
+                    playing = false;
+                }
+            });
        });
+    }

-        scheduler.scheduleAtFixedRate(() -> {
-            T track = current.get();
-            if (!track.isPlaying()) {
-                increasePlayingIndex();
-                T next = backgroundMusic.get(playingIndex);
-                current.set(next);
-                Platform.runLater(() -> {
-                    if (!next.isPlaying()) next.play();
-                });
-            }
-        }, 500, 500, TimeUnit.MILLISECONDS);
+    public void stop() {
+        if (!playing) return;
+
+        playing = false;
+        dispatcher.run(() -> backgroundMusic.forEach(T::stop));
    }
 }
--- a/framework/src/main/java/org/toop/framework/audio/interfaces/Dispatcher.java
+++ b/framework/src/main/java/org/toop/framework/audio/interfaces/Dispatcher.java
@@ -0,0 +1,6 @@
+package org.toop.framework.audio.interfaces;
+
+// TODO isn't specific to audio
+public interface Dispatcher {
+    void run(Runnable task);
+}
--- a/framework/src/main/java/org/toop/framework/resource/ResourceManager.java
+++ b/framework/src/main/java/org/toop/framework/resource/ResourceManager.java
@@ -7,7 +7,6 @@ import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.toop.framework.resource.exceptions.ResourceNotFoundException;
 import org.toop.framework.resource.resources.*;
-import org.toop.framework.resource.types.AudioResource;

 /**
 * Centralized manager for all loaded assets in the application.
@@ -52,12 +51,20 @@ import org.toop.framework.resource.types.AudioResource;
 * </ul>
 */
 public class ResourceManager {
-        private static final Logger logger = LogManager.getLogger(ResourceManager.class);
+    private static final Logger logger = LogManager.getLogger(ResourceManager.class);
    private static final Map<String, ResourceMeta<? extends BaseResource>> assets =
            new ConcurrentHashMap<>();
+    private static ResourceManager instance;

    private ResourceManager() {}

+    public static ResourceManager getInstance() {
+        if (instance == null) {
+            instance = new ResourceManager();
+        }
+        return instance;
+    }
+
    /**
     * Loads all assets from a given {@link ResourceLoader} into the manager.
     *
--- a/framework/src/main/java/org/toop/framework/resource/resources/MusicAsset.java
+++ b/framework/src/main/java/org/toop/framework/resource/resources/MusicAsset.java
@@ -62,8 +62,13 @@ public class MusicAsset extends BaseResource implements LoadableResource, AudioR
    }

    @Override
-    public boolean isPlaying() {
-        return isPlaying;
+    public void setOnEnd(Runnable run) {
+        mediaPlayer.setOnEndOfMedia(run);
+    }
+
+    @Override
+    public void setOnError(Runnable run) {
+        mediaPlayer.setOnError(run);
    }

    @Override
--- a/framework/src/main/java/org/toop/framework/resource/resources/SoundEffectAsset.java
+++ b/framework/src/main/java/org/toop/framework/resource/resources/SoundEffectAsset.java
@@ -101,9 +101,13 @@ public class SoundEffectAsset extends BaseResource implements LoadableResource,
    }

    @Override
-    public boolean isPlaying() {
+    public void setOnEnd(Runnable run) {
+        // TODO
+    }
+
+    @Override
+    public void setOnError(Runnable run) {
        // TODO
-        return false;
    }

    @Override
--- a/framework/src/main/java/org/toop/framework/resource/types/AudioResource.java
+++ b/framework/src/main/java/org/toop/framework/resource/types/AudioResource.java
@@ -2,7 +2,9 @@ package org.toop.framework.resource.types;

 public interface AudioResource {
    void updateVolume(double volume);
-    boolean isPlaying();
+//    boolean isPlaying();
    void play();
    void stop();
+    void setOnEnd(Runnable run);
+    void setOnError(Runnable run);
 }