logaritmisk/trueskill-tt
1
0
Fork 0
Code Issues 1 Pull Requests Packages Projects Releases Activity

feat: let observers be shared, boxed, or borrowed

Browse Source 
`History` takes its observer by value and never hands it back, so a
caller who wanted to read what an observer recorded had no way to keep a
handle to it. The natural spelling did not compile:

    let recorder = Arc::new(Recorder::default());
    History::builder().observer(Arc::clone(&recorder))
    // error[E0277]: `Arc<Recorder>: Observer<i64>` is not satisfied

The workaround was for every observer to wrap each of its own fields in
an `Arc` and derive `Clone` — one allocation and one lock per field, a
pattern each implementor had to rediscover, and nothing documenting it.

Adds blanket `Observer` impls for `Arc<O>`, `Box<O>` and `&O`. All are
`?Sized`, so `Arc<dyn Observer<T>>` and `Box<dyn Observer<T>>` work too
and an observer can be chosen at runtime. Also adds
`History::observer()` and `into_observer()`, so a non-shared observer's
state can be inspected in place or reclaimed after `converge` without
needing interior mutability at all.

`tests/observer.rs` is simplified to the shared spelling, so the
recommended pattern is the one demonstrated rather than the workaround.

Closes #40

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
This commit is contained in:
logaritmisk
2026-09-07 15:13:28 +02:00
co-authored by Claude Opus 5
parent 3c2f9ac64c
commit 2fff745c3b
3 changed files with 166 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/history.rs
+20
View File
@@ -538,6 +538,26 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
.sum()
}
/// The configured observer.
///
/// `History` takes its observer by value, so this is how a caller inspects
/// one it did not keep a handle to. For an observer that accumulates
/// state, prefer passing an `Arc` and keeping a clone — see the
/// [`Observer`] docs.
#[must_use]
pub fn observer(&self) -> &O {
&self.observer
}
/// Consume the history and return its observer.
///
/// Useful for reclaiming a non-shared observer's accumulated state after
/// `converge` without needing interior mutability.
#[must_use]
pub fn into_observer(self) -> O {
self.observer
}
/// Every team's member skills, validated.
///
/// # Errors
src/observer.rs
+77
View File
@@ -26,6 +26,83 @@ pub trait Observer<T: Time>: Send + Sync {
fn on_converged(&self, _iters: usize, _final_step: (f64, f64), _converged: bool) {}
}
/// Shared and boxed observers forward to what they point at.
///
/// `History` takes its observer by value, so a caller who wants to *read* what
/// an observer recorded has to keep a handle to it. Without these impls the
/// natural spelling does not compile:
///
/// ```
/// # use std::sync::{Arc, Mutex};
/// # use trueskill_tt::{History, Observer};
/// #[derive(Default)]
/// struct Recorder {
/// iterations: Mutex<Vec<usize>>,
/// }
///
/// impl Observer<i64> for Recorder {
/// fn on_iteration_end(&self, iter: usize, _step: (f64, f64)) {
/// self.iterations.lock().unwrap().push(iter);
/// }
/// }
///
/// let recorder = Arc::new(Recorder::default());
/// let mut h = History::builder().observer(Arc::clone(&recorder)).build();
/// h.record_winner(&"a", &"b", 1).unwrap();
/// h.converge().unwrap();
///
/// // The caller's handle sees what the history's copy recorded.
/// assert!(!recorder.iterations.lock().unwrap().is_empty());
/// ```
///
/// The alternative was for every observer to wrap each of its own fields in an
/// `Arc` and derive `Clone` — one allocation and one lock per field, and a
/// pattern each implementor had to rediscover.
///
/// `?Sized` is deliberate: it makes `Arc<dyn Observer<T>>` and
/// `Box<dyn Observer<T>>` work, so observers can be chosen at runtime.
impl<T: Time, O: Observer<T> + ?Sized> Observer<T> for std::sync::Arc<O> {
fn on_iteration_end(&self, iter: usize, max_step: (f64, f64)) {
(**self).on_iteration_end(iter, max_step);
}
fn on_slice_processed(&self, time: &T, slice_idx: usize, n_events: usize) {
(**self).on_slice_processed(time, slice_idx, n_events);
}
fn on_converged(&self, iters: usize, final_step: (f64, f64), converged: bool) {
(**self).on_converged(iters, final_step, converged);
}
}
impl<T: Time, O: Observer<T> + ?Sized> Observer<T> for Box<O> {
fn on_iteration_end(&self, iter: usize, max_step: (f64, f64)) {
(**self).on_iteration_end(iter, max_step);
}
fn on_slice_processed(&self, time: &T, slice_idx: usize, n_events: usize) {
(**self).on_slice_processed(time, slice_idx, n_events);
}
fn on_converged(&self, iters: usize, final_step: (f64, f64), converged: bool) {
(**self).on_converged(iters, final_step, converged);
}
}
impl<T: Time, O: Observer<T> + ?Sized> Observer<T> for &O {
fn on_iteration_end(&self, iter: usize, max_step: (f64, f64)) {
(**self).on_iteration_end(iter, max_step);
}
fn on_slice_processed(&self, time: &T, slice_idx: usize, n_events: usize) {
(**self).on_slice_processed(time, slice_idx, n_events);
}
fn on_converged(&self, iters: usize, final_step: (f64, f64), converged: bool) {
(**self).on_converged(iters, final_step, converged);
}
}
/// ZST no-op observer; the default when none is configured.
#[derive(Copy, Clone, Debug, Default)]
pub struct NullObserver;
tests/observer.rs
+69 -13
View File
@@ -8,14 +8,13 @@ use std::sync::{Arc, Mutex};
use trueskill_tt::{History, Observer};
/// `History` takes its observer by value and never hands it back, so a test
/// that wants to read what was recorded shares the storage rather than the
/// observer: the handles are cloned, the buffers are not.
#[derive(Clone, Default)]
/// Plain fields. `Arc<O>` implements `Observer`, so the caller shares the
/// observer itself rather than wrapping each field in its own `Arc`.
#[derive(Default)]
struct Recorder {
iterations: Arc<Mutex<Vec<usize>>>,
slices: Arc<Mutex<Vec<(i64, usize, usize)>>>,
converged: Arc<Mutex<Vec<(usize, bool)>>>,
iterations: Mutex<Vec<usize>>,
slices: Mutex<Vec<(i64, usize, usize)>>,
converged: Mutex<Vec<(usize, bool)>>,
}
impl Observer<i64> for Recorder {
@@ -37,8 +36,8 @@ impl Observer<i64> for Recorder {
#[test]
fn every_observer_callback_fires() {
let recorder = Recorder::default();
let mut h = History::builder().observer(recorder.clone()).build();
let recorder = Arc::new(Recorder::default());
let mut h = History::builder().observer(Arc::clone(&recorder)).build();
h.record_winner(&"a", &"b", 1).unwrap();
h.record_winner(&"b", &"c", 2).unwrap();
@@ -61,8 +60,8 @@ fn every_observer_callback_fires() {
#[test]
fn slice_callbacks_report_the_slice_they_swept() {
let recorder = Recorder::default();
let mut h = History::builder().observer(recorder.clone()).build();
let recorder = Arc::new(Recorder::default());
let mut h = History::builder().observer(Arc::clone(&recorder)).build();
h.record_winner(&"a", &"b", 10).unwrap();
h.record_winner(&"a", &"b", 20).unwrap();
@@ -90,8 +89,8 @@ fn slice_callbacks_report_the_slice_they_swept() {
#[test]
fn a_single_slice_history_still_reports_its_sweep() {
let recorder = Recorder::default();
let mut h = History::builder().observer(recorder.clone()).build();
let recorder = Arc::new(Recorder::default());
let mut h = History::builder().observer(Arc::clone(&recorder)).build();
h.record_winner(&"a", &"b", 1).unwrap();
h.converge().unwrap();
@@ -103,3 +102,60 @@ fn a_single_slice_history_still_reports_its_sweep() {
);
assert!(slices.iter().all(|&(t, idx, _)| t == 1 && idx == 0));
}
/// The gap #40 closed: without `impl Observer for Arc<O>`, an observer that
/// accumulates anything had to wrap every field in its own `Arc` and derive
/// `Clone`, because `History` consumes the observer and never hands it back.
#[test]
fn a_shared_observer_reaches_the_callers_handle() {
let recorder = Arc::new(Recorder::default());
let mut h = History::builder().observer(Arc::clone(&recorder)).build();
h.record_winner(&"a", &"b", 1).unwrap();
h.converge().unwrap();
assert!(!recorder.iterations.lock().unwrap().is_empty());
assert!(!recorder.slices.lock().unwrap().is_empty());
assert!(!recorder.converged.lock().unwrap().is_empty());
}
/// `?Sized` on the blanket impls means the observer can be chosen at runtime.
#[test]
fn a_trait_object_observer_works() {
let boxed: Box<dyn Observer<i64>> = Box::new(Recorder::default());
let mut h = History::builder().observer(boxed).build();
h.record_winner(&"a", &"b", 1).unwrap();
h.converge().unwrap();
let shared: Arc<dyn Observer<i64>> = Arc::new(Recorder::default());
let mut h = History::builder().observer(Arc::clone(&shared)).build();
h.record_winner(&"a", &"b", 1).unwrap();
h.converge().unwrap();
}
/// A non-shared observer can be reclaimed after convergence instead.
#[test]
fn into_observer_returns_the_accumulated_state() {
let mut h = History::builder().observer(Recorder::default()).build();
h.record_winner(&"a", &"b", 1).unwrap();
h.converge().unwrap();
// Readable in place...
assert!(!h.observer().iterations.lock().unwrap().is_empty());
// ...and reclaimable by value.
let recorder = h.into_observer();
assert!(!recorder.slices.lock().unwrap().is_empty());
}
/// Borrowing works too, for an observer that outlives the history.
#[test]
fn a_borrowed_observer_works() {
let recorder = Recorder::default();
{
let mut h = History::builder().observer(&recorder).build();
h.record_winner(&"a", &"b", 1).unwrap();
h.converge().unwrap();
}
assert!(!recorder.iterations.lock().unwrap().is_empty());
}