Fixed a thread stuck scenario while trying to load unknown sources

Fixed /skip command not executing because of a negation in the if statement
Improved skip command by using the duplicate track detection

src/main/java/netneutrality/command/music/SkipCommand.java

@@ -74,7 +74,7 @@ public class SkipCommand extends AbstractMusicCommand {
         int skips = context.getEvent().getOption("skip", 0, OptionMapping::getAsInt);
         context.ackAndWait();
 
-        if (!MusicUtils.checkIfConnected(manager, context)) {
+        if (MusicUtils.checkIfConnected(manager, context)) {
             if (skips > 0) {
                 context.reply("Skipping %s track%s", skips, (skips > 1 ? "s" : ""));
                 manager.getScheduler().getQueueManager().skip(skips);

src/main/java/netneutrality/music/MusicManager.java

@@ -42,7 +42,7 @@ public class MusicManager {
 
     private final static Logger logger = LoggerFactory.getLogger(MusicManager.class);
     private final static Map<String, MusicManager> managers = new ConcurrentHashMap<>();
-    private final static ScheduledExecutorService executor = (ScheduledExecutorService) ThreadManager.getExecutor("music", ThreadManager.ExecutorType.SCHEDULED);
+    private final static ScheduledExecutorService executor = ThreadManager.getMusicExecutor();
 
     private final static AudioPlayerManager playerManager = new DefaultAudioPlayerManager();
     private final Guild guild;

src/main/java/netneutrality/music/QueueManager.java

@@ -46,16 +46,6 @@ public class QueueManager {
             }
         }
 
-        public void toggleSkip(Member member) {
-            if (hasSkip(member)) {
-                logger.debug("VoteSkip: user={}, skip already exist, before={}, after={}", member.getId(), skips.size(), skips.size() - 1);
-                skips.remove(member.getId());
-            } else {
-                logger.debug("VoteSkip: user={}, skip doesn't exist, before={}, after={}", member.getId(), skips.size(), skips.size() + 1);
-                skips.add(member.getId());
-            }
-        }
-
         public boolean hasSkip(Member member) {
             return skips.contains(member.getId());
         }
@@ -134,9 +124,9 @@ public class QueueManager {
         }
     }
 
-    public TrackInfo pollHeadOfQueue() {
+    public void removeHeadOfQueue() {
         synchronized (queue) {
-            return queue.poll();
+            queue.poll();
         }
     }
 
@@ -207,7 +197,7 @@ public class QueueManager {
         }
     }
 
-    public void playNextTrack(AudioPlayer player) {
+    public void startNextTrack() {
         synchronized (queue) {
             TrackInfo last = queue.poll();
             TrackInfo next = queue.peek();
@@ -215,7 +205,7 @@ public class QueueManager {
             if (next != null) {
                 if (last != null && last.getTrack().equals(next.getTrack())) {
                     logger.debug("Guild {}: duplicate track in queue found, skipping to next. last={}, next={}", guild.getId(), last.getTrack().getIdentifier(), next.getTrack().getIdentifier());
-                    playNextTrack(player);
+                    startNextTrack();
                 } else {
                     player.startTrack(next.getTrack(), false);
                 }
@@ -223,7 +213,10 @@ public class QueueManager {
                 player.stopTrack();
             }
         }
+    }
 
+    public void skip() {
+        skip(1);
     }
 
     public void skip(int nbTrack) {
@@ -241,18 +234,12 @@ public class QueueManager {
                 nbTrack = 1;
             }
 
-            logger.debug("Guild {}: skipping tracks, toSkip={}", guild.getId(), nbTrack);
-            for (int i = 0; i < nbTrack; i++) {
-                TrackInfo last = queue.poll();
-            }
-
-            TrackInfo current = queue.peek();
-            if (current != null) {
-                player.startTrack(current.getTrack(), false);
-            } else {
-                player.stopTrack();
+            logger.debug("Guild {}: skipping {} tracks", guild.getId(), nbTrack);
+            for (int i = 1; i < nbTrack; i++) {
+                queue.poll();
             }
 
+            startNextTrack();
         }
     }
 

src/main/java/netneutrality/music/TrackLoader.java

@@ -13,11 +13,14 @@ import netneutrality.music.loadable.CachedSource;
 import netneutrality.music.loadable.LoadableMusicSource;
 import netneutrality.music.loadable.LoadableSong;
 import netneutrality.thread.ThreadManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.util.List;
 
 public class TrackLoader implements Runnable {
 
+    private final Logger logger = LoggerFactory.getLogger(TrackLoader.class);
     private final CommandContext context;
     private final MusicManager musicManager;
     private final AudioPlayerManager audioPlayerManager;
@@ -40,10 +43,10 @@ public class TrackLoader implements Runnable {
     public void loadAllSongs(List<LoadableSong> songs) {
         this.songs = songs;
         if (songs.size() > 1) {
-            silent = true;
+            this.silent = true;
         }
 
-        ThreadManager.getExecutor("Track loader").submit(this);
+        ThreadManager.getMusicExecutor().submit(this);
     }
 
     public void loadAllSongs(CachedSource cachedSource) {
@@ -81,7 +84,7 @@ public class TrackLoader implements Runnable {
 
                         totalNumberOfTracks++;
 
-                        System.out.println("notify, loaded: " + numberOfTracksLoaded + ", total: " + totalNumberOfTracks);
+                        logger.debug("callback notify; total={}, loaded={}", totalNumberOfTracks, numberOfTracksLoaded);
                         sync.notify();
                     }
 
@@ -90,6 +93,8 @@ public class TrackLoader implements Runnable {
 
                 audioPlayerManager.loadItemOrdered(this, identifier, loadingHandler);
             } else {
+                totalNumberOfTracks++;
+
                 if (songs.size() == 1) {
                     context.replyToMessage("No source found to play the given url");
                 }
@@ -99,7 +104,7 @@ public class TrackLoader implements Runnable {
         try {
             synchronized (sync) {
                 while (!(songs.size() == totalNumberOfTracks)) {
-                    System.out.println("wait");
+                    logger.debug("waiting for load; size={}, total={}, loaded={}", songs.size(), totalNumberOfTracks, numberOfTracksLoaded);
                     sync.wait();
                 }
 
@@ -109,6 +114,7 @@ public class TrackLoader implements Runnable {
                 }
             }
         } catch (InterruptedException exception) {
+            stopLoading();
             LogHandler.log(exception);
         }
 

src/main/java/netneutrality/music/TrackLoadingHandler.java

@@ -71,7 +71,7 @@ public class TrackLoadingHandler implements AudioLoadResultHandler {
     }
 
     @Override
-    public void playlistLoaded(AudioPlaylist audioPlaylist) {
+    public void playlistLoaded(AudioPlaylist playlist) {
         if (loadConsumer != null) {
             loadConsumer.accept(true);
         }
@@ -81,29 +81,29 @@ public class TrackLoadingHandler implements AudioLoadResultHandler {
         }
         logger.debug("Guild {}: playlist loaded: {}", guild.getId(), identifier);
 
-        if (!audioPlaylist.getTracks().isEmpty()) {
-            if (audioPlaylist.isSearchResult()) {
-                AudioTrack track = audioPlaylist.getTracks().get(0);
+        if (!playlist.getTracks().isEmpty()) {
+            if (playlist.isSearchResult()) {
+
+                AudioTrack track = playlist.getTracks().get(0);
                 scheduler.getQueueManager().queue(track, context.getGuildUser());
 
                 if (!silent) {
                     context.reply(MessageFunction.buildAddingTrackMessage(track, type));
                 }
 
-                return;
-            }
-
-            context.deleteMessage();
-            if (!silent) {
-                context.reply(MessageFunction.buildAddingPlaylistMessage(audioPlaylist, identifier, type));
-            }
-
-            for (AudioTrack track : audioPlaylist.getTracks()) {
-                if (loadingDisabled) {
-                    break;
+            } else {
+                context.deleteMessage();
+                if (!silent) {
+                    context.reply(MessageFunction.buildAddingPlaylistMessage(playlist, identifier, type));
                 }
 
-                scheduler.getQueueManager().queue(track, context.getGuildUser());
+                for (AudioTrack track : playlist.getTracks()) {
+                    if (loadingDisabled) {
+                        break;
+                    }
+
+                    scheduler.getQueueManager().queue(track, context.getGuildUser());
+                }
             }
         }
     }

src/main/java/netneutrality/music/TrackScheduler.java

@@ -99,11 +99,11 @@ public class TrackScheduler extends AudioEventAdapter {
 
         // check if mayStartNext
         if (endReason == AudioTrackEndReason.FINISHED || endReason == AudioTrackEndReason.LOAD_FAILED) {
-            queueManager.playNextTrack(player);
+            queueManager.startNextTrack();
         }
 
         if (endReason == AudioTrackEndReason.STOPPED) {
-            queueManager.pollHeadOfQueue();
+            queueManager.removeHeadOfQueue();
         }
 
         // endReason == FINISHED: A track finished or died by an exception (mayStartNext = true).
@@ -125,6 +125,6 @@ public class TrackScheduler extends AudioEventAdapter {
     public void onTrackStuck(AudioPlayer player, AudioTrack track, long thresholdMs) {
         logger.debug("Guild {}: currently playing track is stuck: {}", guild.getId(), track.getIdentifier());
         sendMessageToChannel("Current track is stuck, skipping to next");
-        queueManager.playNextTrack(player);
+        queueManager.startNextTrack();
     }
 }

src/main/java/netneutrality/thread/ThreadManager.java

@@ -7,10 +7,7 @@ package netneutrality.thread;
  */
 
 import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
-import java.util.concurrent.TimeUnit;
+import java.util.concurrent.*;
 
 public class ThreadManager {
 
@@ -20,6 +17,20 @@ public class ThreadManager {
 
     private static final Map<String, ExecutorService> executors = new ConcurrentHashMap<>();
 
+    public static ScheduledExecutorService getMusicExecutor() {
+        return (ScheduledExecutorService) executors.computeIfAbsent(
+                "music-executor",
+                s -> Executors.newScheduledThreadPool(
+                        Runtime.getRuntime().availableProcessors(),
+                        new BaseThreadFactory("music-executor")
+                )
+        );
+    }
+
+    public static ExecutorService getExecutor() {
+        return executors.computeIfAbsent("general-executor", s -> Executors.newCachedThreadPool(new BaseThreadFactory("general-executor")));
+    }
+
     public static ExecutorService getExecutor(String name) {
         return executors.computeIfAbsent(name, s -> Executors.newSingleThreadExecutor(new BaseThreadFactory(name)));
     }