From f5f22edb6aff57745a1c53b691b8a5c58d232236 Mon Sep 17 00:00:00 2001 From: Thomas Graves Date: Fri, 14 Mar 2025 05:32:28 -0400 Subject: [PATCH] Don't recalculate remaining steps, conditionally (#3849) * Don't recalculate remaining steps, conditionally Bug report reproduction steps: Create a new profile so that everything is set to default. Create a new card. Click Good. Open deck options and empty learning steps. Save. No go back and put 1m 10m as LS. Go back to the card and it should show 10m on the Good button. Check if old_steps is empty and if it is just use remaining steps for the new_remaining steps. Add test. * Update contributers * Format code * Fix clippy error * Use more idiomatic imports --- CONTRIBUTORS | 1 + rslib/src/card/mod.rs | 56 +++++++++++++++++++++++++++++++++---------- 2 files changed, 44 insertions(+), 13 deletions(-) diff --git a/CONTRIBUTORS b/CONTRIBUTORS index 07b3e8489..9524eb675 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -213,6 +213,7 @@ GithubAnon0000 Mike Hardy Danika_Dakika Mumtaz Hajjo Alrifai +Thomas Graves Jakub Fidler ******************** diff --git a/rslib/src/card/mod.rs b/rslib/src/card/mod.rs index f943ee3b5..98b7ef90c 100644 --- a/rslib/src/card/mod.rs +++ b/rslib/src/card/mod.rs @@ -237,19 +237,25 @@ impl Card { /// greater delay, but output will be at least 1. fn new_remaining_steps(&self, new_steps: &[f32], old_steps: &[f32]) -> Option { let remaining = self.remaining_steps(); - let new_remaining = old_steps - .len() - .checked_sub(remaining as usize + 1) - .and_then(|last_index| { - new_steps - .iter() - .rev() - .position(|&step| step <= old_steps[last_index]) - }) - // no last delay or last delay is less than all new steps → all steps remain - .unwrap_or(new_steps.len()) - // (re)learning card must have at least 1 step remaining - .max(1) as u32; + + let new_remaining = if old_steps.is_empty() { + remaining + } else { + old_steps + .len() + .checked_sub(remaining as usize + 1) + .and_then(|last_index| { + new_steps + .iter() + .rev() + .position(|&step| step <= old_steps[last_index]) + }) + // no last delay or last delay is less than all new steps → all steps remain + .unwrap_or(new_steps.len()) + // (re)learning card must have at least 1 step remaining + .max(1) as u32 + }; + (remaining != new_remaining).then_some(new_remaining) } @@ -490,6 +496,7 @@ impl From for FsrsMemoryState { #[cfg(test)] mod test { + use crate::prelude::*; use crate::tests::open_test_collection_with_learning_card; use crate::tests::open_test_collection_with_relearning_card; use crate::tests::DeckAdder; @@ -515,4 +522,27 @@ mod test { col.set_deck(&[card_id], deck.id).unwrap(); assert_eq!(col.get_first_card().remaining_steps, 2); } + + #[test] + fn should_not_recalculate_remaining_steps_if_there_are_no_old_steps() -> Result<(), AnkiError> { + let mut col = Collection::new(); + + let nt = col.get_notetype_by_name("Basic")?.unwrap(); + let mut note = nt.new_note(); + col.add_note(&mut note, DeckId(1))?; + + let card_id = col.get_first_card().id; + col.set_deck(&[card_id], DeckId(1))?; + + col.set_default_learn_steps(vec![1., 10.]); + + let _post_answer = col.answer_good(); + + col.set_default_learn_steps(vec![]); + col.set_default_learn_steps(vec![1., 10.]); + + assert_eq!(col.get_first_card().remaining_steps, 1); + + Ok(()) + } }