diff --git a/soulseek-rs-lib/src/client/children.rs b/soulseek-rs-lib/src/client/children.rs index 0a054d0..45fad3f 100644 --- a/soulseek-rs-lib/src/client/children.rs +++ b/soulseek-rs-lib/src/client/children.rs @@ -32,17 +32,24 @@ use crate::{debug, trace}; pub const MAX_CHILDREN: usize = 10; /// The child limit the server's figures imply, following Nicotine+: -/// `speed / ratio / 100`, capped, and none at all below the minimum speed the -/// server set. `None` when the server has not reported our speed yet, which -/// leaves the caller's own cap standing. +/// `speed / ratio / 100`, capped, and none at all when our recorded speed is +/// below the minimum the server set. +/// +/// `None` means the server's figures do not decide it — either it has not +/// reported our speed yet, or it never announced a ratio at all (soulfind +/// does not) — and the caller's own cap stands, which is how slskd runs the +/// whole time. #[must_use] pub fn limit_from_speed( own_speed: Option, min_speed: u32, ratio: u32, ) -> Option { + if ratio == 0 { + return None; + } let speed = own_speed?; - if ratio == 0 || speed < min_speed { + if speed < min_speed { return Some(0); } Some((speed / ratio / 100) as usize) @@ -336,8 +343,8 @@ mod tests { ); assert_eq!( limit_from_speed(Some(10_000), 1_024, 0), - Some(0), - "a zero ratio means the server is not allowing children" + None, + "a server that never announced a ratio does not decide the limit" ); assert_eq!( limit_from_speed(None, 1_024, 50), @@ -364,6 +371,12 @@ mod tests { children.set_limits(Some(100), 1_024, 50); assert!(!children.has_room(), "a slow client carries none"); + + children.set_limits(Some(100), 1_024, 0); + assert!( + children.has_room(), + "a server with no figures leaves our own cap standing" + ); assert_eq!(ends.len(), 2); } diff --git a/soulseek-rs-lib/src/client/distributed.rs b/soulseek-rs-lib/src/client/distributed.rs index df84a2b..98344cf 100644 --- a/soulseek-rs-lib/src/client/distributed.rs +++ b/soulseek-rs-lib/src/client/distributed.rs @@ -321,8 +321,11 @@ pub fn announce_move( ) { let server = ctx.server_sender.clone(); // What the server needs to know is whether it may offer us to another - // peer right now, not whether we serve children in principle. + // peer right now, not whether we serve children in principle. The stance + // carries it, so record it as told: a separate AcceptChildren right + // behind this one would say nothing new. let has_room = ctx.children.has_room(); + let _ = ctx.accept_children_change(); announce(server.as_ref(), branch, has_parent, has_room); ctx.children.broadcast_stance(&branch.root, branch.level); } diff --git a/soulseek-rs-lib/src/client/mod.rs b/soulseek-rs-lib/src/client/mod.rs index f17ec45..c6ab69c 100644 --- a/soulseek-rs-lib/src/client/mod.rs +++ b/soulseek-rs-lib/src/client/mod.rs @@ -466,6 +466,9 @@ pub struct ClientContext { leaf: distributed::Leaf, /// The children hanging from us, when the client serves any. pub(crate) children: children::Children, + /// The last `AcceptChildren` answer we gave the server, so the same + /// answer is not repeated: it is a standing state, not a heartbeat. + announced_accept_children: Option, /// The upload speed the server records for us, once it has told us. The /// child limit is derived from it. own_average_speed: Option, @@ -642,6 +645,7 @@ impl ClientContext { operations: None, leaf: distributed::Leaf::new(""), children: children::Children::default(), + announced_accept_children: None, own_average_speed: None, parent_min_speed: 0, parent_speed_ratio: 0, @@ -829,6 +833,16 @@ impl ClientContext { self.own_interests.clone() } + /// Take the `AcceptChildren` answer to send, or `None` when the server + /// already has this one. + pub fn accept_children_change(&mut self) -> Option { + let has_room = self.children.has_room(); + (self.announced_accept_children != Some(has_room)).then(|| { + self.announced_accept_children = Some(has_room); + has_room + }) + } + /// The upload speed the server records for us, if it has said. #[must_use] pub const fn own_average_speed(&self) -> Option { diff --git a/soulseek-rs-lib/src/client/operations.rs b/soulseek-rs-lib/src/client/operations.rs index 578aee6..57b6072 100644 --- a/soulseek-rs-lib/src/client/operations.rs +++ b/soulseek-rs-lib/src/client/operations.rs @@ -1253,8 +1253,16 @@ impl Client { /// (`AcceptChildren`, code 100). Sent whenever that answer changes: the /// server only offers us to peers while it is true. fn announce_child_capacity(client_context: &Arc>) { - let (sender, has_room) = match client_context.read_safe() { - Ok(ctx) => (ctx.server_sender.clone(), ctx.children.has_room()), + let (sender, has_room) = match client_context.write_safe() { + Ok(mut ctx) => { + // Only on a change: the server holds this as a standing + // state, and repeating it on every message we happen to + // receive would be noise. + let Some(has_room) = ctx.accept_children_change() else { + return; + }; + (ctx.server_sender.clone(), has_room) + } Err(_) => return, }; let Some(sender) = sender else { return }; diff --git a/soulseek-rs-lib/src/client/tests.rs b/soulseek-rs-lib/src/client/tests.rs index d9d75fa..7506e66 100644 --- a/soulseek-rs-lib/src/client/tests.rs +++ b/soulseek-rs-lib/src/client/tests.rs @@ -1126,3 +1126,15 @@ fn leaving_a_room_drops_its_ticker_board() { "a board for a room we left is stale" ); } + +#[test] +fn the_server_is_told_our_child_capacity_only_when_it_changes() { + // AcceptChildren is a standing state on the server, not a heartbeat. + let mut ctx = ClientContext::new(); + assert_eq!( + ctx.accept_children_change(), + Some(false), + "the first answer is always worth sending" + ); + assert_eq!(ctx.accept_children_change(), None, "unchanged, so silent"); +}