From 87cfba1bc8b456858eb8a2366b4f1ba7ae745dd8 Mon Sep 17 00:00:00 2001 From: trivernis Date: Mon, 12 Apr 2021 12:47:29 +0200 Subject: [PATCH] Change menu pages to be wrapped in page type The pages of a menu are now wrapped in an enum that contains either a builder function or a static CreateMessage. The builder function is used for the now playing message to always return the currently playing song instead of mapping a static song. This solves the problem that the old page is being rendered when the page is recreated because of being sticky. Signed-off-by: trivernis --- bot-serenityutils/src/menu/controls.rs | 4 +- bot-serenityutils/src/menu/menu.rs | 37 +++++++++------ bot-serenityutils/src/menu/mod.rs | 2 + bot-serenityutils/src/menu/page.rs | 40 ++++++++++++++++ src/commands/music/current.rs | 12 +++-- src/messages/music.rs | 63 +++++++++++++++++++------- src/messages/sauce.rs | 8 ++-- 7 files changed, 126 insertions(+), 40 deletions(-) create mode 100644 bot-serenityutils/src/menu/page.rs diff --git a/bot-serenityutils/src/menu/controls.rs b/bot-serenityutils/src/menu/controls.rs index 83e686f..ef959b5 100644 --- a/bot-serenityutils/src/menu/controls.rs +++ b/bot-serenityutils/src/menu/controls.rs @@ -49,7 +49,9 @@ async fn display_page(ctx: &Context, menu: &mut Menu<'_>) -> SerenityUtilsResult let page = menu .pages .get(menu.current_page) - .ok_or(SerenityUtilsError::PageNotFound(menu.current_page))?; + .ok_or(SerenityUtilsError::PageNotFound(menu.current_page))? + .get() + .await?; let mut msg = menu.get_message(ctx.http()).await?; msg.edit(ctx, |e| { diff --git a/bot-serenityutils/src/menu/menu.rs b/bot-serenityutils/src/menu/menu.rs index 7f9fc23..6a5271d 100644 --- a/bot-serenityutils/src/menu/menu.rs +++ b/bot-serenityutils/src/menu/menu.rs @@ -3,10 +3,9 @@ use crate::error::{SerenityUtilsError, SerenityUtilsResult}; use crate::menu::container::get_listeners_from_context; use crate::menu::controls::{close_menu, next_page, previous_page}; use crate::menu::traits::EventDrivenMessage; -use crate::menu::EventDrivenMessagesRef; +use crate::menu::{EventDrivenMessagesRef, Page}; use futures::FutureExt; use serenity::async_trait; -use serenity::builder::CreateMessage; use serenity::client::Context; use serenity::http::Http; use serenity::model::channel::{Message, Reaction, ReactionType}; @@ -67,7 +66,7 @@ impl ActionContainer { #[derive(Clone)] pub struct Menu<'a> { pub message: Arc>, - pub pages: Vec>, + pub pages: Vec>, pub current_page: usize, pub controls: HashMap, pub timeout: Instant, @@ -100,19 +99,24 @@ impl Menu<'_> { /// Recreates the message completely pub async fn recreate(&self, http: &Http) -> SerenityUtilsResult<()> { log::debug!("Recreating message"); - let mut handle = self.message.write().await; - let old_handle = (*handle).clone(); + + let old_handle = { + let handle = self.message.read().await; + (*handle).clone() + }; log::debug!("Getting current page"); let current_page = self .pages .get(self.current_page) .cloned() - .ok_or(SerenityUtilsError::PageNotFound(self.current_page))?; + .ok_or(SerenityUtilsError::PageNotFound(self.current_page))? + .get() + .await?; log::debug!("Creating new message"); let message = http .send_message( - handle.channel_id, + old_handle.channel_id, &serde_json::to_value(current_page.0).unwrap(), ) .await?; @@ -133,13 +137,16 @@ impl Menu<'_> { } log::trace!("New message is {:?}", message); - handle.message_id = message.id.0; - let handle = (*handle).clone(); + let new_handle = { + let mut handle = self.message.write().await; + handle.message_id = message.id.0; + (*handle).clone() + }; { log::debug!("Changing key of message"); let mut listeners_lock = self.listeners.lock().await; let menu = listeners_lock.remove(&old_handle).unwrap(); - listeners_lock.insert(handle, menu); + listeners_lock.insert(new_handle, menu); } log::debug!("Deleting original message"); http.delete_message(old_handle.channel_id, old_handle.message_id) @@ -217,7 +224,7 @@ impl<'a> EventDrivenMessage for Menu<'a> { /// A builder for messages pub struct MenuBuilder { - pages: Vec>, + pages: Vec>, current_page: usize, controls: HashMap, timeout: Duration, @@ -261,7 +268,7 @@ impl MenuBuilder { } /// Adds a page to the message builder - pub fn add_page(mut self, page: CreateMessage<'static>) -> Self { + pub fn add_page(mut self, page: Page<'static>) -> Self { self.pages.push(page); self @@ -270,7 +277,7 @@ impl MenuBuilder { /// Adds multiple pages to the message pub fn add_pages(mut self, pages: I) -> Self where - I: IntoIterator>, + I: IntoIterator>, { let mut pages = pages.into_iter().collect(); self.pages.append(&mut pages); @@ -344,7 +351,9 @@ impl MenuBuilder { .pages .get(self.current_page) .ok_or(SerenityUtilsError::PageNotFound(self.current_page))? - .clone(); + .clone() + .get() + .await?; let message = channel_id.send_message(ctx, |_| &mut current_page).await?; log::trace!("Message is {:?}", message); diff --git a/bot-serenityutils/src/menu/mod.rs b/bot-serenityutils/src/menu/mod.rs index d801bcf..8eaffaf 100644 --- a/bot-serenityutils/src/menu/mod.rs +++ b/bot-serenityutils/src/menu/mod.rs @@ -1,6 +1,7 @@ pub(crate) mod container; pub(crate) mod controls; pub(crate) mod menu; +pub(crate) mod page; pub(crate) mod traits; pub use container::*; @@ -9,5 +10,6 @@ pub use menu::{ ActionContainer, ControlActionArc, Menu, MenuBuilder, CLOSE_MENU_EMOJI, NEXT_PAGE_EMOJI, PREVIOUS_PAGE_EMOJI, }; +pub use page::*; pub use traits::EventDrivenMessage; diff --git a/bot-serenityutils/src/menu/page.rs b/bot-serenityutils/src/menu/page.rs new file mode 100644 index 0000000..91b3426 --- /dev/null +++ b/bot-serenityutils/src/menu/page.rs @@ -0,0 +1,40 @@ +use crate::error::SerenityUtilsResult; +use serenity::builder::CreateMessage; +use std::future::Future; +use std::pin::Pin; +use std::sync::Arc; + +pub type MessageBuildOutput<'b> = + Pin>> + Send + 'b>>; +pub type MessageBuilderFn<'b> = Arc MessageBuildOutput<'b> + Send + Sync>; + +#[derive(Clone)] +/// A page that stores a builder function for message pages +/// or static pages +pub enum Page<'b> { + Builder(MessageBuilderFn<'b>), + Static(CreateMessage<'b>), +} + +impl<'b> Page<'b> { + /// Creates a new page with the given builder function + pub fn new_builder(builder_fn: F) -> Self + where + F: Fn() -> MessageBuildOutput<'b> + Send + Sync, + { + Self::Builder(Arc::new(builder_fn)) + } + + /// Creates a new page with a static message + pub fn new_static(page: CreateMessage<'b>) -> Self { + Self::Static(page) + } + + /// Returns the CreateMessage of the page + pub async fn get(&self) -> SerenityUtilsResult> { + match self { + Page::Builder(b) => b().await, + Page::Static(inner) => Ok(inner.clone()), + } + } +} diff --git a/src/commands/music/current.rs b/src/commands/music/current.rs index 6ce5191..924ac0f 100644 --- a/src/commands/music/current.rs +++ b/src/commands/music/current.rs @@ -20,14 +20,18 @@ async fn current(ctx: &Context, msg: &Message) -> CommandResult { log::debug!("Displaying current song for queue in {}", guild.id); let queue = get_queue_for_guild(ctx, &guild.id).await?; - let mut queue_lock = queue.lock().await; - if let Some(current) = queue_lock.current() { + let current = { + let queue_lock = queue.lock().await; + queue_lock.current().clone() + }; + + if let Some(current) = current { let metadata = current.metadata().clone(); log::trace!("Metadata is {:?}", metadata); - let np_msg = - create_now_playing_msg(ctx, msg.channel_id, &metadata, queue_lock.paused()).await?; + let np_msg = create_now_playing_msg(ctx, queue.clone(), msg.channel_id).await?; + let mut queue_lock = queue.lock().await; if let Some(old_np) = mem::replace(&mut queue_lock.now_playing_msg, Some(np_msg)) { let old_np = old_np.read().await; if let Ok(message) = old_np.get_message(&ctx.http).await { diff --git a/src/messages/music.rs b/src/messages/music.rs index 448517e..62a0e2a 100644 --- a/src/messages/music.rs +++ b/src/messages/music.rs @@ -6,15 +6,16 @@ use serenity::model::prelude::ChannelId; use songbird::input::Metadata; use crate::commands::music::{get_queue_for_guild, get_voice_manager, is_dj}; +use crate::providers::music::queue::MusicQueue; use crate::utils::error::*; use bot_serenityutils::core::MessageHandle; use bot_serenityutils::error::SerenityUtilsResult; -use bot_serenityutils::menu::{Menu, MenuBuilder}; +use bot_serenityutils::menu::{Menu, MenuBuilder, Page}; use serenity::builder::CreateMessage; use serenity::client::Context; use serenity::model::channel::Reaction; use std::time::Duration; -use tokio::sync::RwLock; +use tokio::sync::{Mutex, RwLock}; static PAUSE_BUTTON: &str = "⏯️"; static SKIP_BUTTON: &str = "⏭️"; @@ -23,14 +24,10 @@ static STOP_BUTTON: &str = "⏹️"; /// Creates a new now playing message and returns the embed for that message pub async fn create_now_playing_msg( ctx: &Context, + queue: Arc>, channel_id: ChannelId, - meta: &Metadata, - paused: bool, ) -> BotResult>> { - log::debug!("Creating now playing message"); - let mut page = CreateMessage::default(); - page.embed(|e| create_now_playing_embed(meta, e, paused)); - + log::debug!("Creating now playing menu"); let handle = MenuBuilder::default() .add_control(0, STOP_BUTTON, |c, m, r| { Box::pin(stop_button_action(c, m, r)) @@ -41,7 +38,24 @@ pub async fn create_now_playing_msg( .add_control(2, SKIP_BUTTON, |c, m, r| { Box::pin(skip_button_action(c, m, r)) }) - .add_page(page) + .add_page(Page::new_builder(move || { + let queue = Arc::clone(&queue); + Box::pin(async move { + log::debug!("Creating now playing embed for page"); + let queue = queue.lock().await; + log::debug!("Queue locked"); + let mut page = CreateMessage::default(); + + if let Some(current) = queue.current() { + page.embed(|e| create_now_playing_embed(current.metadata(), e, queue.paused())); + } else { + page.embed(|e| e.description("Queue is empty")); + } + log::debug!("Embed created"); + + Ok(page) + }) + })) .sticky(true) .timeout(Duration::from_secs(60 * 60 * 24)) .build(ctx, channel_id) @@ -60,11 +74,13 @@ pub async fn update_now_playing_msg( log::debug!("Updating now playing message"); let handle = handle.read().await; let mut message = handle.get_message(http).await?; + message .edit(http, |m| { m.embed(|e| create_now_playing_embed(meta, e, paused)) }) .await?; + log::debug!("Message updated."); Ok(()) } @@ -97,6 +113,7 @@ async fn play_pause_button_action( _: &mut Menu<'_>, reaction: Reaction, ) -> SerenityUtilsResult<()> { + log::debug!("Play/Pause button pressed"); let guild_id = reaction.guild_id.unwrap(); let user = reaction.user(&ctx).await?; @@ -105,12 +122,21 @@ async fn play_pause_button_action( } { let queue = get_queue_for_guild(ctx, &guild_id).await?; - let mut queue = queue.lock().await; - queue.pause(); - let message = queue.now_playing_msg.clone().unwrap(); - if let Some(current) = queue.current() { - update_now_playing_msg(&ctx.http, &message, current.metadata(), queue.paused()).await?; + let (current, message, paused) = { + log::debug!("Queue is locked"); + let mut queue = queue.lock().await; + queue.pause(); + ( + queue.current().clone(), + queue.now_playing_msg.clone().unwrap(), + queue.paused(), + ) + }; + log::debug!("Queue is unlocked"); + + if let Some(current) = current { + update_now_playing_msg(&ctx.http, &message, current.metadata(), paused).await?; } } @@ -130,10 +156,13 @@ async fn skip_button_action( return Ok(()); } { - let queue = get_queue_for_guild(ctx, &guild_id).await?; - let queue = queue.lock().await; + let current = { + let queue = get_queue_for_guild(ctx, &guild_id).await?; + let queue = queue.lock().await; + queue.current().clone() + }; - if let Some(current) = queue.current() { + if let Some(current) = current { let _ = current.stop(); } } diff --git a/src/messages/sauce.rs b/src/messages/sauce.rs index 30c4c3b..b5448e5 100644 --- a/src/messages/sauce.rs +++ b/src/messages/sauce.rs @@ -7,7 +7,7 @@ use serenity::{model::channel::Message, prelude::*}; use bot_coreutils::url::get_domain_for_url; use crate::utils::error::BotResult; -use bot_serenityutils::menu::MenuBuilder; +use bot_serenityutils::menu::{MenuBuilder, Page}; use rand::prelude::SliceRandom; use std::time::Duration; @@ -26,7 +26,7 @@ pub async fn show_sauce_menu( msg: &Message, sources: Vec, ) -> BotResult<()> { - let pages: Vec = sources.into_iter().map(create_sauce_page).collect(); + let pages: Vec = sources.into_iter().map(create_sauce_page).collect(); if pages.len() == 1 { MenuBuilder::default() @@ -46,7 +46,7 @@ pub async fn show_sauce_menu( } /// Creates a single sauce page -fn create_sauce_page<'a>(mut result: SauceResult) -> CreateMessage<'a> { +fn create_sauce_page<'a>(mut result: SauceResult) -> Page<'a> { let mut message = CreateMessage::default(); let mut description_lines = Vec::new(); let original = result.original_url; @@ -95,5 +95,5 @@ fn create_sauce_page<'a>(mut result: SauceResult) -> CreateMessage<'a> { .footer(|f| f.text("Powered by SauceNAO")) }); - message + Page::new_static(message) }