From 29392dc72d94fefecfb5cbe12198b13bd3eda60c Mon Sep 17 00:00:00 2001 From: Miguel da Mota Date: Sun, 10 Mar 2024 17:46:35 +0100 Subject: [PATCH 1/2] chore: improve checks --- src/commands/info.rs | 4 +--- src/commands/mod.rs | 2 +- src/commands/play.rs | 2 +- src/commands/stop.rs | 2 +- src/main.rs | 26 ++++++++++++-------------- src/music/music_manager.rs | 4 +--- src/util/user_util.rs | 9 +++------ 7 files changed, 20 insertions(+), 29 deletions(-) diff --git a/src/commands/info.rs b/src/commands/info.rs index 2c93d36..a708372 100644 --- a/src/commands/info.rs +++ b/src/commands/info.rs @@ -3,9 +3,7 @@ use serenity::builder::{CreateCommand, CreateEmbed}; use crate::util::embed::Embed; -pub async fn run(_ctx: &Context, command: &CommandInteraction) -> CreateEmbed { - let username = command.user.name.as_str(); - +pub async fn run(_ctx: &Context, _command: &CommandInteraction) -> CreateEmbed { Embed::create( "", "Botendo v7", diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 6eaf7b9..755a0ee 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -1,3 +1,3 @@ pub mod info; pub mod play; -pub mod stop; \ No newline at end of file +pub mod stop; diff --git a/src/commands/play.rs b/src/commands/play.rs index 8613fd2..1e05661 100644 --- a/src/commands/play.rs +++ b/src/commands/play.rs @@ -6,7 +6,7 @@ use serenity::model::application::CommandOptionType; use crate::util::embed::Embed; -pub async unsafe fn run(ctx: &Context, command: &CommandInteraction) -> CreateEmbed { +pub async fn run(ctx: &Context, command: &CommandInteraction) -> CreateEmbed { let username = command.user.name.as_str(); let options = &command.data.options; diff --git a/src/commands/stop.rs b/src/commands/stop.rs index 8a65e35..665a24d 100644 --- a/src/commands/stop.rs +++ b/src/commands/stop.rs @@ -3,7 +3,7 @@ use crate::util::embed::Embed; use serenity::all::{CommandInteraction, Context}; use serenity::builder::{CreateCommand, CreateEmbed}; -pub async unsafe fn run(ctx: &Context, command: &CommandInteraction) -> CreateEmbed { +pub async fn run(ctx: &Context, command: &CommandInteraction) -> CreateEmbed { let username = command.user.name.as_str(); let guild_id = match &command.guild_id { diff --git a/src/main.rs b/src/main.rs index 55eeaf4..20611a3 100644 --- a/src/main.rs +++ b/src/main.rs @@ -63,22 +63,20 @@ struct Handler; #[async_trait] impl EventHandler for Handler { async fn interaction_create(&self, ctx: Context, interaction: Interaction) { - unsafe { - if let Interaction::Command(command) = interaction { - let _ = &command.defer(&ctx.http()).await.expect("Cannot defer"); + if let Interaction::Command(command) = interaction { + let _ = &command.defer(&ctx.http()).await.expect("Cannot defer"); - let content = Some(match command.data.name.as_str() { - "info" => commands::info::run(&ctx, &command).await, - "play" => commands::play::run(&ctx, &command).await, - "stop" => commands::stop::run(&ctx, &command).await, - _ => respond_with_error(&ctx, &command).await, - }); + let content = Some(match command.data.name.as_str() { + "info" => commands::info::run(&ctx, &command).await, + "play" => commands::play::run(&ctx, &command).await, + "stop" => commands::stop::run(&ctx, &command).await, + _ => respond_with_error(&ctx, &command).await, + }); - if let Some(embed) = content { - let followup = CreateInteractionResponseFollowup::new().embed(embed); - if let Err(why) = command.create_followup(&ctx.http, followup).await { - println!("Cannot followup to slash command: {why}") - } + if let Some(embed) = content { + let followup = CreateInteractionResponseFollowup::new().embed(embed); + if let Err(why) = command.create_followup(&ctx.http, followup).await { + println!("Cannot followup to slash command: {why}") } } } diff --git a/src/music/music_manager.rs b/src/music/music_manager.rs index 2f42673..e1e9f13 100644 --- a/src/music/music_manager.rs +++ b/src/music/music_manager.rs @@ -224,9 +224,7 @@ pub async fn stop(ctx: &Context, guild_id: &GuildId) -> Result .expect("Cannot get Songbird") .clone(); - let has_handler = manager.get(*guild_id).is_some(); - - if has_handler { + if manager.get(*guild_id).is_some() { manager.remove(*guild_id).await?; return Ok(true); // Handler removed } diff --git a/src/util/user_util.rs b/src/util/user_util.rs index 32c5423..c3ef10a 100644 --- a/src/util/user_util.rs +++ b/src/util/user_util.rs @@ -12,6 +12,7 @@ pub fn request_guild(ctx: &Context, guild_id: &GuildId) -> Guild { } /// Request a guild by id, get it from Discord, not from cache, this is a partial guild +#[allow(dead_code)] pub async fn request_partial_guild(ctx: &Context, guild_id: &GuildId) -> PartialGuild { match ctx.http.get_guild(*guild_id).await { Ok(guild) => guild, @@ -32,16 +33,12 @@ pub async fn get_vc_id(ctx: &Context, guild_id: &GuildId, user_id: &UserId) -> O /// Check if the bot is connected to a voice channel pub async fn is_self_connected_to_vc(ctx: &Context, guild_id: &GuildId) -> bool { - let channel_id = get_self_vc_id(ctx, guild_id); - - channel_id.await.is_some() + get_self_vc_id(ctx, guild_id).await.is_some() } /// Check if a user is connected to a voice channel pub async fn is_user_connected_to_vc(ctx: &Context, guild_id: &GuildId, user_id: &UserId) -> bool { - let channel_id = get_vc_id(ctx, guild_id, user_id).await; - - channel_id.is_some() + get_vc_id(ctx, guild_id, user_id).await.is_some() } /// Get the voice channel id of the bot From 1879b4ee0f5a9f7ec67c08d9853c3e77745085dc Mon Sep 17 00:00:00 2001 From: Miguel da Mota Date: Sun, 10 Mar 2024 19:36:28 +0100 Subject: [PATCH 2/2] fix: music queue improvements --- src/main.rs | 30 +-------------- src/music/music_events.rs | 21 +++++----- src/music/music_queue.rs | 81 ++++++++++++++++++++++++++++++--------- 3 files changed, 75 insertions(+), 57 deletions(-) diff --git a/src/main.rs b/src/main.rs index 20611a3..4ed6306 100644 --- a/src/main.rs +++ b/src/main.rs @@ -3,7 +3,7 @@ mod music; mod util; use serenity::all::{ - CommandInteraction, CreateInteractionResponseFollowup, GuildId, OnlineStatus, VoiceState, + CommandInteraction, CreateInteractionResponseFollowup, OnlineStatus, VoiceState, }; use serenity::async_trait; use serenity::builder::CreateEmbed; @@ -30,34 +30,6 @@ impl TypeMapKey for HttpKey { #[macro_use] extern crate lazy_static; -use crate::music::music_queue::MusicQueue; -use std::collections::HashMap; - -lazy_static! { - static ref HASHMAP: Mutex> = Mutex::new(HashMap::new()); -} - -pub async fn get_queue(guild_id: &GuildId) -> MusicQueue { - let mut queues = HASHMAP.lock().await; - - match queues.get(&guild_id) { - Some(music_queue) => music_queue.clone(), - None => { - let q = MusicQueue { - guild_id: *guild_id, - queue: Vec::new(), - now_playing: None, - }; - queues.insert(*guild_id, q.clone()); - q - } - } -} - -pub async fn set_queue(guild_id: &GuildId, queue: MusicQueue) { - HASHMAP.lock().await.insert(*guild_id, queue); -} - struct Handler; #[async_trait] diff --git a/src/music/music_events.rs b/src/music/music_events.rs index 3b52714..905048a 100644 --- a/src/music/music_events.rs +++ b/src/music/music_events.rs @@ -1,4 +1,5 @@ use crate::music::{music_manager, music_queue}; + use serenity::all::{ChannelId, GuildId, Http}; use serenity::async_trait; use songbird::{Event, EventContext, EventHandler}; @@ -17,9 +18,8 @@ impl EventHandler for TrackEndNotifier { // TODO: Does this need to be unsafe? if let EventContext::Track(..) = ctx { println!("The track ended!"); - let music_queue = crate::get_queue(&self.guild_id).await; - let q = &music_queue.queue; - if q.is_empty() { + + if music_queue::is_empty(&self.guild_id).await { // No more songs in queue, exit the vc let stopped = match music_manager::stop(&self.cmdctx, &self.guild_id).await { Ok(stopped) => stopped, @@ -35,12 +35,15 @@ impl EventHandler for TrackEndNotifier { } return None; } - let head = music_queue::get_head(&self.guild_id).await; - if head.is_none() { - println!("Cannot get head of queue"); - return None; - } - let head = head.unwrap(); + + let head = match music_queue::get_head(&self.guild_id).await { + Some(head) => head, + None => { + println!("Cannot get head of queue"); + return None; + } + }; + music_manager::play_song(&self.cmdctx, &self.guild_id, &head).await; } diff --git a/src/music/music_queue.rs b/src/music/music_queue.rs index c7501f9..741720e 100644 --- a/src/music/music_queue.rs +++ b/src/music/music_queue.rs @@ -1,7 +1,41 @@ use serenity::all::GuildId; use songbird::input::YoutubeDl; +use tokio::sync::{Mutex, MutexGuard}; -#[derive(Debug, Clone)] +use std::collections::HashMap; +use std::sync::Arc; + +type MusicQueueItem = Arc>; + +lazy_static! { + static ref HASHMAP: Mutex> = Mutex::new(HashMap::new()); +} + +async fn get_music_queue(guild_id: &GuildId) -> MusicQueueItem { + let mut queues = HASHMAP.lock().await; + + queues + .entry(*guild_id) + .or_insert(Arc::new(Mutex::new(MusicQueue { + guild_id: *guild_id, + queue: Vec::new(), + now_playing: None, + }))) + .clone() +} + +pub async fn with_music_queue(guild_id: &GuildId, f: F) -> T +where + F: FnOnce(&mut MusicQueue) -> T, + T: Send, +{ + let queue = get_music_queue(guild_id).await; + let mut queue = queue.lock().await; + + f(&mut *queue) +} + +#[derive(Debug)] pub struct MusicQueue { // God this sucks. This needs to be reprogrammed properly. pub guild_id: GuildId, @@ -10,35 +44,44 @@ pub struct MusicQueue { } pub async fn delete_queue(guild_id: &GuildId) { - let mut queue = crate::get_queue(guild_id).await; - queue.now_playing = None; - queue.queue = Vec::new(); - crate::set_queue(guild_id, queue).await; + with_music_queue(guild_id, |queue| { + queue.now_playing = None; + queue.queue.clear(); + }) + .await; } pub async fn add_to_queue(guild_id: &GuildId, input: YoutubeDl) { - let mut queue = crate::get_queue(guild_id).await; - queue.queue.push(input); - crate::set_queue(guild_id, queue).await; + with_music_queue(guild_id, |queue| { + queue.queue.push(input); + }) + .await; } pub async fn get_head(guild_id: &GuildId) -> Option { - let mut music_queue = crate::get_queue(guild_id).await; - if music_queue.queue.is_empty() { - return None; - } - let result = music_queue.queue.remove(0); - crate::set_queue(guild_id, music_queue).await; - Some(result) + with_music_queue(guild_id, |queue| { + if queue.queue.is_empty() { + None + } else { + Some(queue.queue.remove(0)) + } + }) + .await } pub async fn set_now_playing(guild_id: &GuildId, now_playing: Option) { - let mut queue = crate::get_queue(guild_id).await; + let queue = get_music_queue(guild_id).await; + let mut queue = queue.lock().await; + queue.now_playing = now_playing; - crate::set_queue(guild_id, queue).await; } pub async fn get_now_playing(guild_id: &GuildId) -> Option { - let queue = crate::get_queue(guild_id).await; - queue.now_playing + let queue = get_queue(guild_id).await; + + queue.now_playing.to_owned() +} + +pub async fn is_empty(guild_id: &GuildId) -> bool { + with_music_queue(guild_id, |queue| queue.queue.is_empty()).await }