From 0e00c4a4618e9c483e30b1a5a9a9dbd202c9e6ef Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Fri, 29 Oct 2021 11:11:48 +1000 Subject: [PATCH] fix new cards not being correctly limited https://forums.ankiweb.net/t/ios-beta-20080-2-more-new-cards-after-review-limit-is-met/13728/10 --- rslib/src/decks/limits.rs | 16 +++++-- rslib/src/decks/tree.rs | 2 +- rslib/src/scheduler/answering/mod.rs | 55 +++++++++++++++++++++++- rslib/src/scheduler/queue/builder/mod.rs | 2 +- 4 files changed, 69 insertions(+), 6 deletions(-) diff --git a/rslib/src/decks/limits.rs b/rslib/src/decks/limits.rs index 10286ff77..34bbe44a1 100644 --- a/rslib/src/decks/limits.rs +++ b/rslib/src/decks/limits.rs @@ -16,10 +16,14 @@ pub(crate) struct RemainingLimits { } impl RemainingLimits { - pub(crate) fn new(deck: &Deck, config: Option<&DeckConfig>, today: u32) -> Self { + pub(crate) fn new(deck: &Deck, config: Option<&DeckConfig>, today: u32, v3: bool) -> Self { config .map(|config| { - let (new_today, rev_today) = deck.new_rev_counts(today); + let (new_today, mut rev_today) = deck.new_rev_counts(today); + if v3 { + // any reviewed new cards contribute to the review limit + rev_today += new_today; + } RemainingLimits { review: ((config.inner.reviews_per_day as i32) - rev_today).max(0) as u32, new: ((config.inner.new_per_day as i32) - new_today).max(0) as u32, @@ -47,12 +51,18 @@ pub(crate) fn remaining_limits_map<'a>( decks: impl Iterator, config: &'a HashMap, today: u32, + v3: bool, ) -> HashMap { decks .map(|deck| { ( deck.id, - RemainingLimits::new(deck, deck.config_id().and_then(|id| config.get(&id)), today), + RemainingLimits::new( + deck, + deck.config_id().and_then(|id| config.get(&id)), + today, + v3, + ), ) }) .collect() diff --git a/rslib/src/decks/tree.rs b/rslib/src/decks/tree.rs index 481dc4578..903338dfd 100644 --- a/rslib/src/decks/tree.rs +++ b/rslib/src/decks/tree.rs @@ -316,7 +316,7 @@ impl Collection { let counts = self.due_counts(days_elapsed, learn_cutoff, limit)?; let dconf = self.storage.get_deck_config_map()?; add_counts(&mut tree, &counts); - let limits = remaining_limits_map(decks_map.values(), &dconf, days_elapsed); + let limits = remaining_limits_map(decks_map.values(), &dconf, days_elapsed, v3); if sched_ver == SchedulerVersion::V2 { if v3 { sum_counts_and_apply_limits_v3(&mut tree, &limits); diff --git a/rslib/src/scheduler/answering/mod.rs b/rslib/src/scheduler/answering/mod.rs index 3b255af15..caba4af55 100644 --- a/rslib/src/scheduler/answering/mod.rs +++ b/rslib/src/scheduler/answering/mod.rs @@ -431,7 +431,9 @@ fn get_fuzz_seed(card: &Card) -> Option { #[cfg(test)] mod test { use super::*; - use crate::{card::CardType, collection::open_test_collection}; + use crate::{ + card::CardType, collection::open_test_collection, deckconfig::ReviewMix, search::SortMode, + }; fn current_state(col: &mut Collection, card_id: CardId) -> CardState { col.get_next_card_states(card_id).unwrap().current @@ -554,4 +556,55 @@ mod test { Ok(()) } + + fn v3_test_collection(cards: usize) -> Result<(Collection, Vec)> { + let mut col = open_test_collection(); + let nt = col.get_notetype_by_name("Basic")?.unwrap(); + for _ in 0..cards { + let mut note = Note::new(&nt); + col.add_note(&mut note, DeckId(1))?; + } + col.set_config_bool(BoolKey::Sched2021, true, false)?; + let cids = col.search_cards("", SortMode::NoOrder)?; + Ok((col, cids)) + } + + macro_rules! assert_counts { + ($col:ident, $new:expr, $learn:expr, $review:expr) => {{ + let tree = $col.deck_tree(Some(TimestampSecs::now()), None).unwrap(); + assert_eq!(tree.new_count, $new); + assert_eq!(tree.learn_count, $learn); + assert_eq!(tree.review_count, $review); + let queued = $col.get_queued_cards(1, false).unwrap(); + assert_eq!(queued.new_count, $new); + assert_eq!(queued.learning_count, $learn); + assert_eq!(queued.review_count, $review); + }}; + } + + #[test] + fn new_limited_by_reviews() -> Result<()> { + let (mut col, cids) = v3_test_collection(4)?; + col.set_due_date(&cids[0..2], "0", None)?; + // set a limit of 3 reviews, which should give us 2 reviews and 1 new card + let mut conf = col.get_deck_config(DeckConfigId(1), false)?.unwrap(); + conf.inner.reviews_per_day = 3; + conf.inner.set_new_mix(ReviewMix::BeforeReviews); + col.storage.update_deck_conf(&conf)?; + + assert_counts!(col, 1, 0, 2); + // first card is the new card + col.answer_good(); + assert_counts!(col, 0, 1, 2); + // then the two reviews + col.answer_good(); + assert_counts!(col, 0, 1, 1); + col.answer_good(); + assert_counts!(col, 0, 1, 0); + // after the final 10 minute step, the queues should be empty + col.answer_good(); + assert_counts!(col, 0, 0, 0); + + Ok(()) + } } diff --git a/rslib/src/scheduler/queue/builder/mod.rs b/rslib/src/scheduler/queue/builder/mod.rs index 07786f2d8..e4b642b3a 100644 --- a/rslib/src/scheduler/queue/builder/mod.rs +++ b/rslib/src/scheduler/queue/builder/mod.rs @@ -235,7 +235,7 @@ impl Collection { // fetch remaining limits, and cap to selected deck limits so that we don't // do more work than necessary - let mut remaining = remaining_limits_map(decks.iter(), &config, timing.days_elapsed); + let mut remaining = remaining_limits_map(decks.iter(), &config, timing.days_elapsed, true); let selected_deck_limits_at_start = *remaining.get(&deck_id).unwrap(); let mut selected_deck_limits = selected_deck_limits_at_start; for limit in remaining.values_mut() {