Code review

src/model/constants.rs

@@ -9,17 +9,16 @@ pub const BETA: f64 = DEFAULT_VOLATILITY / 2.0;
 pub const DECAY_DAYS: u64 = 121; // Approximately 4 months
 
 /// Minimum rating that any player can decay to, based on their peak rating
-pub const DECAY_MINIMUM: f64 = 15.0 * MULTIPLIER; // 900.0
+pub const DECAY_MINIMUM: f64 = 15.0 * MULTIPLIER;
 
-/// Amount of rating lost per decay cycle (weekly after DECAY_DAYS)
-pub const DECAY_RATE: f64 = 0.06 * MULTIPLIER; // 3.6 per week
+/// Amount of rating lost per decay cycle
+pub const DECAY_RATE: f64 = 0.06 * MULTIPLIER;
 
-/// Base volatility for new players
-/// Higher values indicate more uncertainty in the rating
-pub const DEFAULT_VOLATILITY: f64 = 5.0 * MULTIPLIER; // 300.0
+/// Initial volatility, higher values indicate more uncertainty in the rating
+pub const DEFAULT_VOLATILITY: f64 = 5.0 * MULTIPLIER;
 
 /// Fallback default rating used when rating cannot be identified from osu! rank information
-pub const FALLBACK_DEFAULT_RATING: f64 = 15.0 * MULTIPLIER; // 900.0
+pub const FALLBACK_RATING: f64 = 15.0 * MULTIPLIER;
 
 /// Arbitrary regularization parameter
 pub const KAPPA: f64 = 0.0001;

src/model/decay.rs

@@ -8,7 +8,10 @@
 /// - Decay Floor: A minimum rating threshold based on a player's peak rating
 /// - Weekly Decay: Rating reductions occur in weekly intervals after the decay period
 /// - Volatility Growth: Player volatility increases with each decay cycle
-use super::constants::{DECAY_DAYS, DECAY_MINIMUM, DECAY_RATE, DECAY_VOLATILITY_GROWTH_RATE, DEFAULT_VOLATILITY};
+use super::{
+    constants::{DECAY_DAYS, DECAY_MINIMUM, DECAY_RATE, DECAY_VOLATILITY_GROWTH_RATE, DEFAULT_VOLATILITY},
+    structures::rating_adjustment_type::RatingAdjustmentType
+};
 use crate::{
     database::db_structs::{PlayerRating, RatingAdjustment},
     model::structures::rating_adjustment_type::RatingAdjustmentType::{Decay, Initial}
@@ -22,6 +25,9 @@ pub enum DecayError {
     /// Player has no rating adjustments in their history
     #[error("Player rating has no adjustments")]
     NoAdjustments,
+    /// Player has no rating adjustments in their history of type Match
+    #[error("Player rating has no match adjustments")]
+    NoMatchAdjustments,
     /// Player has played a match within the decay period
     #[error("Player is still active")]
     PlayerActive,
@@ -118,7 +124,7 @@ impl DecaySystem {
 
         let last_play_time = self.get_last_play_time(player_rating)?;
 
-        if self.is_player_active(last_play_time) {
+        if self.is_active(last_play_time) {
             return Err(DecayError::PlayerActive);
         }
 
@@ -140,16 +146,18 @@ impl DecaySystem {
     fn get_last_play_time(&self, player_rating: &PlayerRating) -> Result<DateTime<FixedOffset>, DecayError> {
         player_rating
             .adjustments
-            .last()
+            .iter()
+            .rev()
+            .find(|adj| adj.adjustment_type == RatingAdjustmentType::Match)
             .map(|adj| adj.timestamp)
-            .ok_or(DecayError::NoAdjustments)
+            .ok_or(DecayError::NoMatchAdjustments)
     }
 
-    /// Determines if a player is still within their active period
+    /// Determines if a player's `last_play_time` is within the active period
     ///
     /// A player is considered active if their last play time was within
     /// DECAY_DAYS of the current reference time.
-    fn is_player_active(&self, last_play_time: DateTime<FixedOffset>) -> bool {
+    fn is_active(&self, last_play_time: DateTime<FixedOffset>) -> bool {
         self.current_time - last_play_time < Duration::days(DECAY_DAYS as i64)
     }
 
@@ -170,19 +178,9 @@ impl DecaySystem {
         let mut timestamps = Vec::new();
         let floor = self.calculate_decay_floor(player_rating);
 
-        let mut current_rating = player_rating.rating;
         let mut current_time = decay_start;
-
         while current_time <= self.current_time {
-            let new_rating = self.calculate_decay_rating(current_rating, floor);
-
-            // Stop if we've hit the floor (no more decay possible)
-            if current_rating == new_rating {
-                break;
-            }
-
             timestamps.push(current_time);
-            current_rating = new_rating;
             current_time += Duration::weeks(1);
         }
 
@@ -206,6 +204,11 @@ impl DecaySystem {
             let new_rating = self.calculate_decay_rating(current_rating, floor);
             let new_volatility = self.calculate_decay_volatility(current_volatility);
 
+            // Stop if we've hit the floor (no more decay possible)
+            if new_rating == current_rating {
+                break;
+            }
+
             adjustments.push(RatingAdjustment {
                 player_id: player_rating.player_id,
                 ruleset: player_rating.ruleset,
@@ -271,18 +274,18 @@ mod tests {
     #[test]
     fn test_decay_error_initial_rating() {
         let last_played = Utc::now().fixed_offset();
-        let current_time = last_played + Duration::days(DECAY_DAYS as i64 + 1);
+        let current_time = last_played + Duration::days(DECAY_DAYS as i64);
         let system = DecaySystem::new(current_time);
         let mut rating =
             generate_player_rating(1, Ruleset::Osu, 2000.0, 200.0, 1, Some(last_played), Some(last_played));
 
-        assert_eq!(system.decay(&mut rating), Err(DecayError::InitialRating));
+        assert_eq!(system.decay(&mut rating), Err(DecayError::NoMatchAdjustments));
     }
 
     #[test]
     fn test_decay_error_below_floor() {
         let last_played = Utc::now().fixed_offset();
-        let current_time = last_played + Duration::days(DECAY_DAYS as i64 + 1);
+        let current_time = last_played + Duration::days(DECAY_DAYS as i64);
         let system = DecaySystem::new(current_time);
         let mut rating = generate_player_rating(
             1,

src/model/otr_model.rs

@@ -115,11 +115,11 @@ impl OtrModel {
     fn process_match(&mut self, match_: &Match) {
         self.apply_decay(match_);
 
-        let ratings_standard = self.generate_ratings(match_);
-        let ratings_with_penalties = self.generate_penalized_ratings(match_);
+        let ratings_a = self.generate_ratings_a(match_);
+        let ratings_b = self.generate_ratings_b(match_);
 
-        let calc_standard = self.calc_a(ratings_standard, match_);
-        let calc_penalized = self.calc_b(ratings_with_penalties, match_);
+        let calc_standard = self.calc_a(ratings_a, match_);
+        let calc_penalized = self.calc_b(ratings_b, match_);
         let final_results = self.calc_weighted_rating(&calc_standard, &calc_penalized);
 
         self.apply_results(match_, &final_results)
@@ -129,7 +129,7 @@ impl OtrModel {
     ///
     /// This method only considers games that players actually participated in,
     /// providing a "pure" performance rating for each game played.
-    fn generate_ratings(&self, match_: &Match) -> HashMap<i32, Vec<Rating>> {
+    fn generate_ratings_a(&self, match_: &Match) -> HashMap<i32, Vec<Rating>> {
         let mut map: HashMap<i32, Vec<Rating>> = HashMap::new();
         for game in &match_.games {
             let game_rating_result = self.rate(game);
@@ -145,11 +145,11 @@ impl OtrModel {
     /// This method assumes players who missed games would have placed last,
     /// providing a "worst-case" rating scenario for players who don't participate
     /// in all games of a match.
-    fn generate_penalized_ratings(&self, match_: &Match) -> HashMap<i32, Vec<Rating>> {
+    fn generate_ratings_b(&self, match_: &Match) -> HashMap<i32, Vec<Rating>> {
         let mut cloned_match = match_.clone();
         let participants = self.get_match_participants(&cloned_match);
         self.apply_tie_for_last_scores(&mut cloned_match, &participants);
-        self.generate_ratings(&cloned_match)
+        self.generate_ratings_a(&cloned_match)
     }
 
     /// Gets a unique list of all players who participated in any game of the match.

src/model/rating_tracker.rs

@@ -280,7 +280,7 @@ mod tests {
     use crate::{
         database::db_structs::PlayerRating,
         model::{
-            constants::{DEFAULT_VOLATILITY, FALLBACK_DEFAULT_RATING},
+            constants::{DEFAULT_VOLATILITY, FALLBACK_RATING},
             rating_tracker::RatingTracker,
             structures::{
                 rating_adjustment_type::RatingAdjustmentType,
@@ -408,7 +408,7 @@ mod tests {
         let player_ratings = vec![generate_player_rating(
             1,
             Osu,
-            FALLBACK_DEFAULT_RATING,
+            FALLBACK_RATING,
             DEFAULT_VOLATILITY,
             1,
             None,

src/model/rating_utils.rs

@@ -1,4 +1,4 @@
-use super::constants::FALLBACK_DEFAULT_RATING;
+use super::constants::FALLBACK_RATING;
 use crate::{
     database::db_structs::{Match, Player, PlayerRating, RatingAdjustment},
     model::{
@@ -100,10 +100,10 @@ fn initial_rating(player: &Player, ruleset: &Ruleset) -> f64 {
 
             match rank {
                 Some(r) => mu_from_rank(r, *ruleset),
-                None => FALLBACK_DEFAULT_RATING
+                None => FALLBACK_RATING
             }
         }
-        None => FALLBACK_DEFAULT_RATING
+        None => FALLBACK_RATING
     }
 }