From ac90e6dc4d18c4ecc3ebecba5f2f7bc370814977 Mon Sep 17 00:00:00 2001 From: Benedikt Peetz Date: Thu, 19 Feb 2026 22:40:04 +0100 Subject: pkgs/mpdpopmd: Actually consider the weight value The previous code only cared if the weight was positive, neutral, or negative; which meant that it considered a track with weight = -1 equal to one with weight = -999. This algorithm now performs a weighted random selection based on the weight. --- pkgs/by-name/mp/mpdpopm/src/dj/algorithms.rs | 137 ++++++++++++++++----------- 1 file changed, 82 insertions(+), 55 deletions(-) (limited to 'pkgs') diff --git a/pkgs/by-name/mp/mpdpopm/src/dj/algorithms.rs b/pkgs/by-name/mp/mpdpopm/src/dj/algorithms.rs index c002f055..2c3ddad6 100644 --- a/pkgs/by-name/mp/mpdpopm/src/dj/algorithms.rs +++ b/pkgs/by-name/mp/mpdpopm/src/dj/algorithms.rs @@ -26,43 +26,56 @@ pub struct Discovery { impl Algorithm for Discovery { async fn next_track(&mut self, client: &mut Client) -> Result { macro_rules! take { - ($first:expr, $second:expr, $third:expr) => {{ - $first.pop().map_or_else( - || { - $second.pop().map_or_else( - || { - $third.pop().map_or_else( - || { - unreachable!( - "This means that there are no songs in the libary" - ) - }, - |val| { - tracing::info!( - "Selecting a `{}` track for the next entry in the queue", - stringify!($third) - ); - Ok::<_, anyhow::Error>(val) - }, - ) - }, - |val| { - tracing::info!( - "Selecting a `{}` track for the next entry in the queue", - stringify!($second) - ); - Ok::<_, anyhow::Error>(val) - }, - ) - }, - |val| { - tracing::info!( - "Selecting a `{}` track for the next entry in the queue", - stringify!($first) - ); - Ok::<_, anyhow::Error>(val) - }, - ) + ($rng:expr, $from:expr) => {{ + info!(concat!( + "Trying to select a `", + stringify!($from), + "` track." + )); + + assert!(!$from.is_empty()); + + let normalized_weights = { + // We normalize the weights here, because negative values don't work for the + // distribution function we use below. + // "-5" "-3" "1" "6" "19" | +5 + // -> "0" "2" "6" "11" "24" + let mut weights = $from.iter().map(|(_, w)| *w).collect::>(); + + weights.sort_by_key(|w| *w); + + let first = *weights.first().expect( + "the value to exist, because we never run `take!` with an empty vector", + ); + + if first.is_negative() { + weights + .into_iter() + .rev() + .map(|w| w + first.abs()) + .collect::>() + } else { + weights + } + }; + + let sample = $rng.sample( + distr::weighted::WeightedIndex::new(normalized_weights.iter()) + .expect("to be okay, because the weights are normalized"), + ); + + let output = $from.remove(sample); + + info!( + concat!( + "(", + stringify!($from), + ") Selected `{}` with weight: `{}` (normalized to `{}`)" + ), + output.0, output.1, normalized_weights[sample] + ); + + Ok::<_, anyhow::Error>(output) }}; } @@ -89,24 +102,24 @@ impl Algorithm for Discovery { base }; - let mut positive = vec![]; - let mut neutral = vec![]; - let mut negative = vec![]; - + let mut sorted_tracks = Vec::with_capacity(tracks.len()); for track in tracks { let weight = Self::weight_track(client, &track).await?; - match weight { - 1..=i64::MAX => positive.push(track), - 0 => neutral.push(track), - i64::MIN..0 => negative.push(track), - } + sorted_tracks.push((track, weight)); } - // Avoid an inherit ordering, that might be returned by the `Client::get_all_songs()` function. - positive.shuffle(&mut rng); - neutral.shuffle(&mut rng); - negative.shuffle(&mut rng); + sorted_tracks.sort_by_key(|(_, weight)| *weight); + + let len = sorted_tracks.len() / 3; + + // We split the tracks into three thirds, so that we can also force a pick from e.g. + // the lower third (the negative ones). + let negative = sorted_tracks.drain(..len).collect::>(); + let neutral = sorted_tracks.drain(..len).collect::>(); + let positive = sorted_tracks; + + assert_eq!(negative.len(), neutral.len()); (positive, neutral, negative) }; @@ -124,15 +137,29 @@ impl Algorithm for Discovery { ); let next = match pick { - 0 => take!(positive, neutral, negative), - 1 => take!(neutral, positive, negative), - 2 => take!(negative, neutral, positive), + 0 if !positive.is_empty() => take!(rng, positive), + 1 if !neutral.is_empty() => take!(rng, neutral), + 2 if !negative.is_empty() => take!(rng, negative), + 0..=2 => { + // We couldn't actually satisfy the request, because we didn't have the required + // track. So we just use the first non-empty one. + if !positive.is_empty() { + take!(rng, positive) + } else if !neutral.is_empty() { + take!(rng, neutral) + } else if !negative.is_empty() { + take!(rng, negative) + } else { + assert!(positive.is_empty() && neutral.is_empty() && negative.is_empty()); + todo!("No songs available to select from, I don't know how to select one."); + } + } _ => unreachable!("These indexes are not possible"), }?; - self.already_done.insert(next.clone()); + self.already_done.insert(next.0.to_owned()); - Ok(next) + Ok(next.0) } } -- cgit 1.4.1