From df133ce00d07e0d784fb42eb59434e28058a7d76 Mon Sep 17 00:00:00 2001
From: "google-labs-jules[bot]"
 <161369871+google-labs-jules[bot]@users.noreply.github.com>
Date: Thu, 14 Aug 2025 14:17:58 +0000
Subject: [PATCH] Refactor and Stabilize Music Module

This commit includes a full suite of fixes and improvements to the music module, addressing critical bugs, improving code quality, and enhancing stability.

Key Bug Fixes:
- Fixes a race condition in the `trackEnd` event that caused random song skipping by removing a redundant `player.play()` call.
- Adds `trackStuck` and `trackError` event handlers to gracefully skip songs that fail to play, fixing issues where playback would stop mid-song.
- Fixes the `/queue skipto` command to correctly modify the queue.
- Fixes a validation conflict in the `/volume` command.
- Fixes the `/loop` command to use the correct API method.
- Fixes a race condition in the `/play` command by re-validating player state after an async search.

Refactoring and Improvements:
- Refactors duplicated `formatDuration` and `generateProgressBar` functions into a shared utility file.
- Corrects the `/stop` command's behavior to no longer disconnect the bot.
- Adds a guard against seeking in live streams.
- Improves performance of the `/queue list` command for very large queues.
- Removes a conflicting `queueEnd` event handler in favor of the configured `onEmptyQueue` option.
---
 modules/music/handlers/loop.js         | 17 ++++++--------
 modules/music/handlers/nowplaying.js   | 17 +++-----------
 modules/music/handlers/play.js         |  7 ++++++
 modules/music/handlers/queue/list.js   | 23 ++++++++++--------
 modules/music/handlers/queue/skipto.js | 10 +++++---
 modules/music/handlers/seek.js         | 11 ++++-----
 modules/music/handlers/stop.js         |  5 ++--
 modules/music/handlers/volume.js       |  2 +-
 modules/music/index.js                 | 32 ++++++++++++++++++--------
 modules/music/services/settings.js     |  2 +-
 modules/music/utils/formatters.js      | 14 +++++++++++
 11 files changed, 84 insertions(+), 56 deletions(-)
 create mode 100644 modules/music/utils/formatters.js

diff --git a/modules/music/handlers/loop.js b/modules/music/handlers/loop.js
index 875a2d3..20c9afa 100644
--- a/modules/music/handlers/loop.js
+++ b/modules/music/handlers/loop.js
@@ -30,16 +30,13 @@ export function createLoopCommand(ctx) {
     const mode = interaction.options.getString("mode");
 
     try {
-      if (mode === "off") {
-        player.repeatMode = "none";
-        await interaction.editReply({ embeds: [embed.success("Loop mode set to off.")] });
-      } else if (mode === "song") {
-        player.repeatMode = "track";
-        await interaction.editReply({ embeds: [embed.success("Loop mode set to song.")] });
-      } else if (mode === "queue") {
-        player.repeatMode = "queue";
-        await interaction.editReply({ embeds: [embed.success("Loop mode set to queue.")] });
-      }
+      // The setRepeatMode method ensures the state is set correctly.
+      // The valid modes are "off", "track", and "queue".
+      player.setRepeatMode(mode);
+
+      // Capitalize first letter for the reply message
+      const friendlyMode = mode.charAt(0).toUpperCase() + mode.slice(1);
+      await interaction.editReply({ embeds: [embed.success(`Loop mode set to ${friendlyMode}.`)] });
     } catch (error) {
       logger.error(`[Music] Error setting loop mode: ${error.message}`);
       await interaction.editReply({ embeds: [embed.error(`An error occurred while trying to set the loop mode: ${error.message}`)] });
diff --git a/modules/music/handlers/nowplaying.js b/modules/music/handlers/nowplaying.js
index b4755bd..c65759d 100644
--- a/modules/music/handlers/nowplaying.js
+++ b/modules/music/handlers/nowplaying.js
@@ -1,3 +1,5 @@
+import { formatDuration, generateProgressBar } from "../utils/formatters.js";
+
 export function createNowPlayingCommand(ctx) {
   const { v2, logger, music, embed } = ctx;
   const { manager } = music;
@@ -19,7 +21,7 @@ export function createNowPlayingCommand(ctx) {
     const isStream = song.info.isStream;
     const totalDuration = isStream ? "LIVE" : formatDuration(player.queue.current.info.duration);
     const currentPosition = isStream ? "0:00" : formatDuration(player.position);
-    const progressBar = isStream ? "[▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬▬]" : generateProgressBar(player.position, player.queue.current.info.duration);
+    const progressBar = isStream ? "" : generateProgressBar(player.position, player.queue.current.info.duration);
 
     const nowPlayingEmbed = embed.info("Now Playing");
     nowPlayingEmbed.setTitle(song.info.title);
@@ -35,16 +37,3 @@ export function createNowPlayingCommand(ctx) {
 
   return cmdNowPlaying;
 }
-
-function formatDuration(ms) {
-  const minutes = Math.floor(ms / 60000);
-  const seconds = ((ms % 60000) / 1000).toFixed(0);
-  return minutes + ":" + (seconds < 10 ? '0' : '') + seconds;
-}
-
-function generateProgressBar(current, total, size = 20) {
-  const percentage = current / total;
-  const progress = Math.round(size * percentage);
-  const empty = size - progress;
-  return "[" + "=".repeat(progress) + "-".repeat(empty) + "]";
-}
diff --git a/modules/music/handlers/play.js b/modules/music/handlers/play.js
index 6badeb5..fafc8b1 100644
--- a/modules/music/handlers/play.js
+++ b/modules/music/handlers/play.js
@@ -67,6 +67,13 @@ export function createPlayCommand(ctx) {
 
       const res = await player.search({ query, source: "ytsearch" }, interaction.user);
 
+      // Re-validate player state after await, in case it was destroyed during search
+      const currentPlayer = manager.players.get(interaction.guild.id);
+      if (!currentPlayer || currentPlayer.state === "DESTROYED") {
+        logger.info(`[Music] Player for guild ${interaction.guild.id} was destroyed during song search. Aborting play command.`);
+        return;
+      }
+
       if (!res || !res.tracks.length) {
         return interaction.editReply({ embeds: [embed.error(`No results found for 
 ${query}
diff --git a/modules/music/handlers/queue/list.js b/modules/music/handlers/queue/list.js
index 75056db..4e1d21b 100644
--- a/modules/music/handlers/queue/list.js
+++ b/modules/music/handlers/queue/list.js
@@ -1,5 +1,6 @@
 import { ApplicationCommandOptionType } from "discord.js";
 import { createPaginatedEmbed } from "../../../../core/ui.js";
+import { formatDuration } from "../../utils/formatters.js";
 
 export function createListCommand(ctx, cmdQueue) {
   const { logger, music, embed, lifecycle } = ctx;
@@ -15,7 +16,9 @@ export function createListCommand(ctx, cmdQueue) {
     }
 
     const itemsPerPage = 10;
-    const totalPages = Math.ceil(player.queue.tracks.length / itemsPerPage);
+    const maxPages = 25; // Cap to prevent performance issues with massive queues
+    const totalTracks = player.queue.tracks.length;
+    const totalPages = Math.min(Math.ceil(totalTracks / itemsPerPage), maxPages);
     const page = interaction.options.getInteger("page") || 1;
 
     if (page < 1 || page > totalPages) {
@@ -36,14 +39,20 @@ export function createListCommand(ctx, cmdQueue) {
       let pageDescription = "";
       if (player.queue.current && i === 0) { // Only show "Now Playing" on the first page
         const currentDuration = player.queue.current.info.isStream ? "LIVE" : formatDuration(player.queue.current.info.duration);
-        pageDescription += `**Now Playing:** [${player.queue.current.info.title}](${player.queue.current.info.uri}) - ${player.queue.current.info.author} (${currentDuration})`;
+        pageDescription += `**Now Playing:** [${player.queue.current.info.title}](${player.queue.current.info.uri}) - ${player.queue.current.info.author} (${currentDuration})\n`;
       }
       pageDescription += formattedQueue.join("\n");
 
-      pages.push({
+      const pageEmbed = {
         title: `Music Queue (Page ${i + 1}/${totalPages})`,
         description: pageDescription,
-      });
+      };
+
+      if (i === maxPages - 1 && totalTracks > maxPages * itemsPerPage) {
+        pageEmbed.footer = { text: `Displaying first ${maxPages * itemsPerPage} of ${totalTracks} songs.` };
+      }
+
+      pages.push(pageEmbed);
     }
 
     const { message, dispose } = createPaginatedEmbed(ctx, cmdQueue, "music", pages, {
@@ -54,10 +63,4 @@ export function createListCommand(ctx, cmdQueue) {
     await interaction.editReply(message);
     lifecycle.addDisposable(dispose);
   };
-}
-
-function formatDuration(ms) {
-  const minutes = Math.floor(ms / 60000);
-  const seconds = ((ms % 60000) / 1000).toFixed(0);
-  return minutes + ":" + (seconds < 10 ? '0' : '') + seconds;
 }
\ No newline at end of file
diff --git a/modules/music/handlers/queue/skipto.js b/modules/music/handlers/queue/skipto.js
index 3e7e52a..4eb36c7 100644
--- a/modules/music/handlers/queue/skipto.js
+++ b/modules/music/handlers/queue/skipto.js
@@ -18,9 +18,13 @@ export function createSkipToCommand(ctx) {
     }
 
     try {
-      // Lavalink.js queue is 0-indexed, so position - 1
-      // The play method can take an index to skip to
-      await player.play(player.queue.tracks[position - 1]);
+      // Remove all tracks before the target position.
+      // The `remove` method modifies the queue in place.
+      player.queue.remove(0, position - 1);
+
+      // Skip the current song to start playing the new first song in the queue.
+      await player.skip();
+
       await interaction.editReply({ embeds: [embed.success(`Skipped to song at position ${position}.`)] });
     } catch (error) {
       logger.error(`[Music] Error skipping to song: ${error.message}`);
diff --git a/modules/music/handlers/seek.js b/modules/music/handlers/seek.js
index 03d1416..4829eb8 100644
--- a/modules/music/handlers/seek.js
+++ b/modules/music/handlers/seek.js
@@ -1,4 +1,5 @@
 import { ApplicationCommandOptionType } from "discord.js";
+import { formatDuration } from "../utils/formatters.js";
 
 export function createSeekCommand(ctx) {
   const { v2, logger, music, embed } = ctx;
@@ -22,6 +23,10 @@ export function createSeekCommand(ctx) {
       return interaction.editReply({ embeds: [embed.error("No song is currently playing.")] });
     }
 
+    if (player.queue.current.info.isStream) {
+      return interaction.editReply({ embeds: [embed.error("You cannot seek in a live stream.")] });
+    }
+
     const timeString = interaction.options.getString("time");
     let seekToMs = 0;
 
@@ -54,9 +59,3 @@ export function createSeekCommand(ctx) {
 
   return cmdSeek;
 }
-
-function formatDuration(ms) {
-  const minutes = Math.floor(ms / 60000);
-  const seconds = ((ms % 60000) / 1000).toFixed(0);
-  return minutes + ":" + (seconds < 10 ? '0' : '') + seconds;
-}
diff --git a/modules/music/handlers/stop.js b/modules/music/handlers/stop.js
index 222110d..e358746 100644
--- a/modules/music/handlers/stop.js
+++ b/modules/music/handlers/stop.js
@@ -16,8 +16,9 @@ export function createStopCommand(ctx) {
     }
 
     try {
-      await player.destroy("User requested stop");
-      await interaction.editReply({ embeds: [embed.success("Music stopped and queue cleared.")] });
+      player.queue.clear();
+      await player.stop();
+      await interaction.editReply({ embeds: [embed.success("Music stopped and queue cleared. The bot remains in the voice channel.")] });
     } catch (error) {
       logger.error(`[Music] Error stopping music: ${error.message}`);
       await interaction.editReply({ embeds: [embed.error(`An error occurred while trying to stop the music: ${error.message}`)] });
diff --git a/modules/music/handlers/volume.js b/modules/music/handlers/volume.js
index 7c1ccff..e9ea239 100644
--- a/modules/music/handlers/volume.js
+++ b/modules/music/handlers/volume.js
@@ -10,7 +10,7 @@ export function createVolumeCommand(ctx) {
     .setDescription("Sets the player volume.")
     .addIntegerOption(opt =>
       opt.setName("level")
-        .setDescription("The volume level (0-100).")
+        .setDescription("The volume level (0-1000). Values above 100 may cause distortion.")
         .setRequired(false)
         .setMinValue(0)
         .setMaxValue(1000)
diff --git a/modules/music/index.js b/modules/music/index.js
index cb4854f..047c3bb 100644
--- a/modules/music/index.js
+++ b/modules/music/index.js
@@ -87,22 +87,36 @@ export default async function init(ctx) {
     logger.info(`[Music] Started playing ${track.info.title} by ${track.info.author} on guild ${player.guildId} in channel ${player.voiceChannelId}`);
     logger.debug(`[Music] Track details: ${JSON.stringify(track.info)}`);
   });
-  manager.on("trackEnd", (player, track) => {
-    logger.info(`[Music] Finished playing ${track.info.title} on guild ${player.guildId}.`);
+  manager.on("trackEnd", (player, track, payload) => {
+    logger.info(`[Music] Finished playing ${track.info.title} on guild ${player.guildId}. Reason: ${payload.reason}.`);
+    // autoSkip is enabled, so we don't need to manually call player.play() here.
+    // lavalink-client will handle playing the next track.
     if (player.queue.size > 0) {
-      player.play();
+      logger.debug(`[Music] Queue has ${player.queue.size} more tracks. autoSkip will handle playback.`);
     } else {
-      logger.info(`[Music] Queue ended on guild ${player.guildId}.`);
-      // Optionally destroy player after a delay if no more tracks and nobody is in voice channel
-      // For now, let's rely on onEmptyQueue destroyAfterMs
+      logger.info(`[Music] Queue is empty on guild ${player.guildId}.`);
     }
   });
 
-  manager.on("queueEnd", player => {
-    logger.info(`[Music] Queue ended on guild ${player.guildId} in channel ${player.voiceChannelId}. Destroying player.`);
-    player.destroy();
+  manager.on("trackStuck", (player, track, payload) => {
+    logger.warn(`[Music] Track stuck: ${track.info.title} on guild ${player.guildId}. Reason: ${payload.type}. Threshold: ${payload.thresholdMs}ms.`);
+    if (player.queue.size > 0) {
+      logger.info(`[Music] Skipping to next track on guild ${player.guildId}.`);
+      player.skip();
+    }
   });
 
+  manager.on("trackError", (player, track, payload) => {
+    logger.error(`[Music] Track error: ${track.info.title} on guild ${player.guildId}. Error: ${payload.error}.`);
+    if (player.queue.size > 0) {
+      logger.info(`[Music] Skipping to next track on guild ${player.guildId}.`);
+      player.skip();
+    }
+  });
+
+  // The onEmptyQueue option is configured to handle this automatically after a 30s delay.
+  // No explicit queueEnd handler is needed.
+
   // CRITICAL: Handle Discord raw events for voice connections
   ctx.client.on("raw", d => manager.sendRawData(d));
   lifecycle.addDisposable(() => ctx.client.off("raw", d => manager.sendRawData(d)));
diff --git a/modules/music/services/settings.js b/modules/music/services/settings.js
index c9b48ff..c3014be 100644
--- a/modules/music/services/settings.js
+++ b/modules/music/services/settings.js
@@ -7,7 +7,7 @@ const DEFAULT_VOLUME = 50; // Default volume if not set
 const GuildMusicSettingsSchema = z.object({
   _id: z.any().optional(), // MongoDB ObjectId
   guildId: z.string(),
-  volume: z.number().min(0).max(100).default(DEFAULT_VOLUME),
+  volume: z.number().min(0).max(1000).default(DEFAULT_VOLUME),
 });
 
 const CACHE_TTL_MS = 60_000; // Cache for 1 minute
diff --git a/modules/music/utils/formatters.js b/modules/music/utils/formatters.js
new file mode 100644
index 0000000..140a09c
--- /dev/null
+++ b/modules/music/utils/formatters.js
@@ -0,0 +1,14 @@
+export function formatDuration(ms) {
+  if (isNaN(ms) || ms < 0) return "0:00";
+  const minutes = Math.floor(ms / 60000);
+  const seconds = ((ms % 60000) / 1000).toFixed(0);
+  return minutes + ":" + (seconds < 10 ? '0' : '') + seconds;
+}
+
+export function generateProgressBar(current, total, size = 20) {
+  if (isNaN(current) || isNaN(total) || total === 0) return `[${'-'.repeat(size)}]`;
+  const percentage = Math.min(current / total, 1); // Ensure percentage doesn't exceed 100%
+  const progress = Math.round(size * percentage);
+  const empty = size - progress;
+  return "[" + "=".repeat(progress) + "-".repeat(empty) + "]";
+}