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
This commit is contained in:
Thomas Graves 2025-03-14 05:32:28 -04:00 committed by GitHub
parent 9b5da546be
commit f5f22edb6a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 44 additions and 13 deletions

View file

@ -213,6 +213,7 @@ GithubAnon0000 <GithubAnon0000@users.noreply.github.com>
Mike Hardy <github@mikehardy.net> Mike Hardy <github@mikehardy.net>
Danika_Dakika <https://github.com/Danika-Dakika> Danika_Dakika <https://github.com/Danika-Dakika>
Mumtaz Hajjo Alrifai <mumtazrifai@protonmail.com> Mumtaz Hajjo Alrifai <mumtazrifai@protonmail.com>
Thomas Graves <fate@hey.com>
Jakub Fidler <jakub.fidler@protonmail.com> Jakub Fidler <jakub.fidler@protonmail.com>
******************** ********************

View file

@ -237,19 +237,25 @@ impl Card {
/// greater delay, but output will be at least 1. /// greater delay, but output will be at least 1.
fn new_remaining_steps(&self, new_steps: &[f32], old_steps: &[f32]) -> Option<u32> { fn new_remaining_steps(&self, new_steps: &[f32], old_steps: &[f32]) -> Option<u32> {
let remaining = self.remaining_steps(); let remaining = self.remaining_steps();
let new_remaining = old_steps
.len() let new_remaining = if old_steps.is_empty() {
.checked_sub(remaining as usize + 1) remaining
.and_then(|last_index| { } else {
new_steps old_steps
.iter() .len()
.rev() .checked_sub(remaining as usize + 1)
.position(|&step| step <= old_steps[last_index]) .and_then(|last_index| {
}) new_steps
// no last delay or last delay is less than all new steps → all steps remain .iter()
.unwrap_or(new_steps.len()) .rev()
// (re)learning card must have at least 1 step remaining .position(|&step| step <= old_steps[last_index])
.max(1) as u32; })
// 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) (remaining != new_remaining).then_some(new_remaining)
} }
@ -490,6 +496,7 @@ impl From<MemoryState> for FsrsMemoryState {
#[cfg(test)] #[cfg(test)]
mod test { mod test {
use crate::prelude::*;
use crate::tests::open_test_collection_with_learning_card; use crate::tests::open_test_collection_with_learning_card;
use crate::tests::open_test_collection_with_relearning_card; use crate::tests::open_test_collection_with_relearning_card;
use crate::tests::DeckAdder; use crate::tests::DeckAdder;
@ -515,4 +522,27 @@ mod test {
col.set_deck(&[card_id], deck.id).unwrap(); col.set_deck(&[card_id], deck.id).unwrap();
assert_eq!(col.get_first_card().remaining_steps, 2); 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(())
}
} }