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 <trivernis@protonmail.com>

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| {

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<RwLock<MessageHandle>>,
-    pub pages: Vec<CreateMessage<'a>>,
+    pub pages: Vec<Page<'a>>,
     pub current_page: usize,
     pub controls: HashMap<String, ActionContainer>,
     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<CreateMessage<'static>>,
+    pages: Vec<Page<'static>>,
     current_page: usize,
     controls: HashMap<String, ActionContainer>,
     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<I>(mut self, pages: I) -> Self
     where
-        I: IntoIterator<Item = CreateMessage<'static>>,
+        I: IntoIterator<Item = Page<'static>>,
     {
         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);

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;

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<Box<dyn Future<Output = SerenityUtilsResult<CreateMessage<'b>>> + Send + 'b>>;
+pub type MessageBuilderFn<'b> = Arc<dyn Fn() -> 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<F: 'static>(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<CreateMessage<'b>> {
+        match self {
+            Page::Builder(b) => b().await,
+            Page::Static(inner) => Ok(inner.clone()),
+        }
+    }
+}

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 {

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<Mutex<MusicQueue>>,
     channel_id: ChannelId,
-    meta: &Metadata,
-    paused: bool,
 ) -> BotResult<Arc<RwLock<MessageHandle>>> {
-    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();
         }
     }

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<SauceResult>,
 ) -> BotResult<()> {
-    let pages: Vec<CreateMessage> = sources.into_iter().map(create_sauce_page).collect();
+    let pages: Vec<Page> = 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)
 }