From 7ca0daa48e9c1dfd940b63890d60f2947df68dc1 Mon Sep 17 00:00:00 2001 From: Anders Olsson Date: Wed, 9 Sep 2026 21:29:16 +0200 Subject: [PATCH] feat: PartialEq on the config types, and pin the public trait impls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Rating` already derived `PartialEq`, but that derive is only reachable through `D: PartialEq` — and `ConstantDrift`, the crate's own only `Drift` impl, did not satisfy it. So the derive was there and unusable. Found by writing the comparison from a consumer's position rather than reading the derive list. `ConstantDrift`, `ConvergenceOptions` and `GameOptions` now derive `PartialEq`. All three are pure configuration; comparing two is the natural thing to want and nothing about them makes equality ambiguous. `tests/trait_impls.rs` pins the surface, written the way the failure was reported: a consumer struct that *holds* a `History` and derives `Debug`. It also asserts `History`'s `Debug` summarises rather than dumping its skill stores, so a future derive cannot quietly replace the hand-written impl. `Clone` on `History` stays off. It is a decision, not an omission: a history owns every slice's skill store and arena, so cloning one is proportional to the whole fit, and no consumer has wanted it. Closes #76. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ --- src/convergence.rs | 2 +- src/drift.rs | 2 +- src/game.rs | 2 +- tests/trait_impls.rs | 96 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 99 insertions(+), 3 deletions(-) create mode 100644 tests/trait_impls.rs diff --git a/src/convergence.rs b/src/convergence.rs index ee7e87a..04d8b55 100644 --- a/src/convergence.rs +++ b/src/convergence.rs @@ -4,7 +4,7 @@ use std::time::Duration; use smallvec::SmallVec; -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug, PartialEq)] pub struct ConvergenceOptions { pub max_iter: usize, pub epsilon: f64, diff --git a/src/drift.rs b/src/drift.rs index ba36486..d4bf247 100644 --- a/src/drift.rs +++ b/src/drift.rs @@ -43,7 +43,7 @@ pub trait Drift: Copy + Debug + Send + Sync { /// A non-finite gamma is caught a second time regardless: /// `History::converge` validates the drift variance each competitor actually /// accumulates, which also covers a custom [`Drift`] implementation. -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug, PartialEq)] pub struct ConstantDrift(f64); impl ConstantDrift { diff --git a/src/game.rs b/src/game.rs index b0d7e67..9f9838f 100644 --- a/src/game.rs +++ b/src/game.rs @@ -68,7 +68,7 @@ impl DiffFactor { /// `p_draw` and `convergence` apply to ranked outcomes (`Game::ranked`). /// `score_sigma` applies only to scored outcomes (`Game::scored`); it controls /// how much the engine trusts the observed score margin (smaller σ = more trust). -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug, PartialEq)] pub struct GameOptions { pub p_draw: f64, pub score_sigma: f64, diff --git a/tests/trait_impls.rs b/tests/trait_impls.rs new file mode 100644 index 0000000..438ba75 --- /dev/null +++ b/tests/trait_impls.rs @@ -0,0 +1,96 @@ +//! The traits a consumer needs on the public types, pinned so they cannot be +//! removed by accident. +//! +//! This is written from a consumer's position — deriving `Debug` on a struct +//! that *holds* a `History` — because that is the thing that failed. Asserting +//! `History: Debug` in isolation would not have caught the generic-bound half: +//! `Rating` derives `PartialEq`, but that is only usable if `D: PartialEq`, and +//! the crate's own only `Drift` impl did not satisfy it. + +use trueskill_tt::{ + ConstantDrift, ConvergenceOptions, ConvergenceReport, Event, GameOptions, Gaussian, History, + HistoryBuilder, InferenceError, Member, Outcome, Rating, Team, +}; + +/// The reported failure, verbatim: a consumer holding a history in app state. +#[derive(Debug)] +#[allow( + dead_code, + reason = "held only so `derive(Debug)` has something to render" +)] +struct App { + history: History, +} + +#[test] +fn a_struct_holding_a_history_can_derive_debug() { + let app = App { + history: History::default(), + }; + + let rendered = format!("{app:?}"); + + // Summarising, not a dump of every skill store — the same choice `Joint`'s + // manual `Debug` makes about its n² factorisation. + assert!(rendered.contains("competitors"), "{rendered}"); + assert!(rendered.contains("time_slices"), "{rendered}"); + assert!( + !rendered.contains("SkillStore"), + "History's Debug should summarise, not dump: {rendered}" + ); +} + +#[test] +fn history_builder_is_debug_and_clone() { + let b: HistoryBuilder = History::builder(); + let cloned = b.clone(); + assert!(!format!("{cloned:?}").is_empty()); +} + +#[test] +fn config_and_input_value_types_are_comparable() { + assert_eq!(ConstantDrift::new(0.1), ConstantDrift::new(0.1)); + assert_ne!(ConstantDrift::new(0.1), ConstantDrift::new(0.2)); + + assert_eq!(ConvergenceOptions::default(), ConvergenceOptions::default()); + assert_eq!(GameOptions::default(), GameOptions::default()); + + // `Rating: PartialEq` is only reachable through `D: PartialEq`. + assert_eq!(Rating::::default(), Rating::default()); + assert_ne!( + Rating::default(), + Rating::::default().with_drift_scale(2.0) + ); + + assert_eq!(Member::new("a"), Member::new("a")); + assert_ne!(Member::new("a"), Member::new("b")); + assert_eq!( + Team::with_members([Member::new("a")]), + Team::with_members([Member::new("a")]) + ); + + let event = || Event { + time: 1, + teams: [ + Team::with_members([Member::new("a")]), + Team::with_members([Member::new("b")]), + ] + .into_iter() + .collect(), + outcome: Outcome::winner(0, 2), + }; + assert_eq!(event(), event()); + + assert_eq!(Gaussian::default(), Gaussian::default()); +} + +#[test] +fn a_history_is_send_and_sync_and_default() { + fn assert_send_sync() {} + assert_send_sync::(); + assert_send_sync::(); + + let mut h = History::default(); + let report: ConvergenceReport = h.converge().expect("an empty history converges"); + assert_eq!(report, report.clone()); +}