From 9874b195266d581cdce9e36b1c6184227bc2e315 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Tue, 12 May 2020 13:20:29 +1000 Subject: [PATCH] handle decks set to random new order It probably makes more sense to randomize on queue build in the future, but for now this imitates the previous Anki behaviour. --- rslib/src/deckconf.rs | 3 +- rslib/src/decks/mod.rs | 10 +++ rslib/src/notetype/cardgen.rs | 117 ++++++++++++++++++++++++++-------- 3 files changed, 103 insertions(+), 27 deletions(-) diff --git a/rslib/src/deckconf.rs b/rslib/src/deckconf.rs index e92a33297..a7a6bd754 100644 --- a/rslib/src/deckconf.rs +++ b/rslib/src/deckconf.rs @@ -51,7 +51,7 @@ pub struct NewConf { #[serde(deserialize_with = "default_on_invalid")] ints: NewCardIntervals, #[serde(deserialize_with = "default_on_invalid")] - order: NewCardOrder, + pub(crate) order: NewCardOrder, #[serde(deserialize_with = "default_on_invalid")] pub(crate) per_day: u32, @@ -199,6 +199,7 @@ impl Default for DeckConf { } impl Collection { + /// If fallback is true, guaranteed to return a deck config. pub fn get_deck_config(&self, dcid: DeckConfID, fallback: bool) -> Result> { if let Some(conf) = self.storage.get_deck_config(dcid)? { return Ok(Some(conf)); diff --git a/rslib/src/decks/mod.rs b/rslib/src/decks/mod.rs index b8d6f85ce..7053481de 100644 --- a/rslib/src/decks/mod.rs +++ b/rslib/src/decks/mod.rs @@ -9,6 +9,7 @@ pub use crate::backend_proto::{ use crate::{ card::CardID, collection::Collection, + deckconf::DeckConfID, define_newtype, err::{AnkiError, Result}, text::normalize_to_nfc, @@ -74,6 +75,15 @@ impl Deck { matches!(self.kind, DeckKind::Filtered(_)) } + /// Returns deck config ID if deck is a normal deck. + pub(crate) fn config_id(&self) -> Option { + if let DeckKind::Normal(ref norm) = self.kind { + Some(DeckConfID(norm.config_id)) + } else { + None + } + } + pub(crate) fn prepare_for_update(&mut self) { // fixme - we currently only do this when converting from human; should be done in pub methods instead diff --git a/rslib/src/notetype/cardgen.rs b/rslib/src/notetype/cardgen.rs index 41fcd2f08..697419c09 100644 --- a/rslib/src/notetype/cardgen.rs +++ b/rslib/src/notetype/cardgen.rs @@ -6,7 +6,8 @@ use crate::{ card::{Card, CardID}, cloze::add_cloze_numbers_in_string, collection::Collection, - decks::{Deck, DeckID}, + deckconf::{DeckConf, DeckConfID}, + decks::DeckID, err::{AnkiError, Result}, notes::{Note, NoteID}, notetype::NoteTypeKind, @@ -14,7 +15,8 @@ use crate::{ types::Usn, }; use itertools::Itertools; -use std::{collections::HashSet, sync::Arc}; +use rand::{rngs::StdRng, Rng, SeedableRng}; +use std::collections::{HashMap, HashSet}; /// Info about an existing card required when generating new cards #[derive(Debug, PartialEq)] @@ -48,6 +50,13 @@ pub(crate) struct CardGenContext<'a> { cards: Vec, } +// store for data that needs to be looked up multiple times +#[derive(Default)] +pub(crate) struct CardGenCache { + next_position: Option, + deck_configs: HashMap, +} + impl CardGenContext<'_> { pub(crate) fn new(nt: &NoteType, usn: Usn) -> CardGenContext<'_> { CardGenContext { @@ -207,7 +216,13 @@ impl Collection { note: &Note, target_deck_id: DeckID, ) -> Result<()> { - self.generate_cards_for_note(ctx, note, &[], Some(target_deck_id)) + self.generate_cards_for_note( + ctx, + note, + &[], + Some(target_deck_id), + &mut Default::default(), + ) } pub(crate) fn generate_cards_for_existing_note( @@ -216,7 +231,13 @@ impl Collection { note: &Note, ) -> Result<()> { let existing = self.storage.existing_cards_for_note(note.id)?; - self.generate_cards_for_note(ctx, note, &existing, Some(ctx.notetype.target_deck_id())) + self.generate_cards_for_note( + ctx, + note, + &existing, + Some(ctx.notetype.target_deck_id()), + &mut Default::default(), + ) } fn generate_cards_for_note( @@ -225,17 +246,19 @@ impl Collection { note: &Note, existing: &[AlreadyGeneratedCardInfo], target_deck_id: Option, + cache: &mut CardGenCache, ) -> Result<()> { let cards = ctx.new_cards_required(note, &existing, true); if cards.is_empty() { return Ok(()); } - self.add_generated_cards(note.id, &cards, target_deck_id) + self.add_generated_cards(note.id, &cards, target_deck_id, cache) } pub(crate) fn generate_cards_for_notetype(&mut self, ctx: &CardGenContext) -> Result<()> { let existing_cards = self.storage.existing_cards_for_notetype(ctx.notetype.id)?; let by_note = group_generated_cards_by_note(existing_cards); + let mut cache = CardGenCache::default(); for (nid, existing_cards) in by_note { if ctx.notetype.config.kind() == NoteTypeKind::Normal && existing_cards.len() == ctx.notetype.templates.len() @@ -244,8 +267,9 @@ impl Collection { // to load the note contents to know if all cards have been generated continue; } + cache.next_position = None; let note = self.storage.get_note(nid)?.unwrap(); - self.generate_cards_for_note(ctx, ¬e, &existing_cards, None)?; + self.generate_cards_for_note(ctx, ¬e, &existing_cards, None, &mut cache)?; } Ok(()) @@ -256,45 +280,86 @@ impl Collection { nid: NoteID, cards: &[CardToGenerate], target_deck_id: Option, + cache: &mut CardGenCache, ) -> Result<()> { - let mut next_pos = None; for c in cards { - let target_deck = self.deck_for_adding(c.did.or(target_deck_id))?; - let due = c.due.unwrap_or_else(|| { - if next_pos.is_none() { - next_pos = Some(self.get_and_update_next_card_position().unwrap_or(0)); - } - next_pos.unwrap() - }); - let mut card = Card::new(nid, c.ord as u16, target_deck.id, due as i32); + let (did, dcid) = self.deck_for_adding(c.did.or(target_deck_id))?; + let due = if let Some(due) = c.due { + // use existing due number if provided + due + } else { + self.due_for_deck(did, dcid, cache)? + }; + let mut card = Card::new(nid, c.ord as u16, did, due as i32); self.add_card(&mut card)?; } Ok(()) } + // not sure if entry() can be used due to get_deck_config() returning a result + #[allow(clippy::map_entry)] + fn due_for_deck(&self, did: DeckID, dcid: DeckConfID, cache: &mut CardGenCache) -> Result { + if !cache.deck_configs.contains_key(&did) { + let conf = self.get_deck_config(dcid, true)?.unwrap(); + cache.deck_configs.insert(did, conf); + } + // set if not yet set + if cache.next_position.is_none() { + cache.next_position = Some(self.get_and_update_next_card_position().unwrap_or(0)); + } + let next_pos = cache.next_position.unwrap(); + + match cache.deck_configs.get(&did).unwrap().new.order { + crate::deckconf::NewCardOrder::Random => Ok(random_position(next_pos)), + crate::deckconf::NewCardOrder::Due => Ok(next_pos), + } + } + /// If deck ID does not exist or points to a filtered deck, fall back on default. - fn deck_for_adding(&mut self, did: Option) -> Result> { + fn deck_for_adding(&mut self, did: Option) -> Result<(DeckID, DeckConfID)> { if let Some(did) = did { - if let Some(deck) = self.deck_if_normal(did)? { + if let Some(deck) = self.deck_conf_if_normal(did)? { return Ok(deck); } } - self.default_deck() + self.default_deck_conf() } - fn default_deck(&mut self) -> Result> { + fn default_deck_conf(&mut self) -> Result<(DeckID, DeckConfID)> { // currently hard-coded to 1, we could create this as needed in the future Ok(self - .get_deck(DeckID(1))? - .ok_or_else(|| AnkiError::invalid_input("missing default deck"))?) + .deck_conf_if_normal(DeckID(1))? + .ok_or_else(|| AnkiError::invalid_input("invalid default deck"))?) } - /// If deck exists and and is a normal deck, return it. - fn deck_if_normal(&mut self, did: DeckID) -> Result>> { - Ok(self - .get_deck(did)? - .and_then(|d| if !d.is_filtered() { Some(d) } else { None })) + /// If deck exists and and is a normal deck, return its ID and config + fn deck_conf_if_normal(&mut self, did: DeckID) -> Result> { + Ok(self.get_deck(did)?.and_then(|d| { + if let Some(conf_id) = d.config_id() { + Some((did, conf_id)) + } else { + None + } + })) + } +} + +fn random_position(highest_position: u32) -> u32 { + let mut rng = StdRng::seed_from_u64(highest_position as u64); + rng.gen_range(0, highest_position.max(1000)) +} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn random() { + // predictable output and a minimum range of 1000 + assert_eq!(random_position(5), 626); + assert_eq!(random_position(500), 898); + assert_eq!(random_position(5001), 2282); } }