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

@@ -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<Option<DeckConf>> {
         if let Some(conf) = self.storage.get_deck_config(dcid)? {
             return Ok(Some(conf));

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<DeckConfID> {
+        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
 

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<SingleCardGenContext>,
 }
 
+// store for data that needs to be looked up multiple times
+#[derive(Default)]
+pub(crate) struct CardGenCache {
+    next_position: Option<u32>,
+    deck_configs: HashMap<DeckID, DeckConf>,
+}
+
 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<DeckID>,
+        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, &note, &existing_cards, None)?;
+            self.generate_cards_for_note(ctx, &note, &existing_cards, None, &mut cache)?;
         }
 
         Ok(())
@@ -256,45 +280,86 @@ impl Collection {
         nid: NoteID,
         cards: &[CardToGenerate],
         target_deck_id: Option<DeckID>,
+        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 (did, dcid) = self.deck_for_adding(c.did.or(target_deck_id))?;
-            let due = c.due.unwrap_or_else(|| {
+            let due = if let Some(due) = c.due {
-                if next_pos.is_none() {
+                // use existing due number if provided
-                    next_pos = Some(self.get_and_update_next_card_position().unwrap_or(0));
+                due
-                }
+            } else {
-                next_pos.unwrap()
+                self.due_for_deck(did, dcid, cache)?
-            });
+            };
-            let mut card = Card::new(nid, c.ord as u16, target_deck.id, due as i32);
+            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<u32> {
+        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<DeckID>) -> Result<Arc<Deck>> {
+    fn deck_for_adding(&mut self, did: Option<DeckID>) -> 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<Arc<Deck>> {
+    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))?
+            .deck_conf_if_normal(DeckID(1))?
-            .ok_or_else(|| AnkiError::invalid_input("missing default deck"))?)
+            .ok_or_else(|| AnkiError::invalid_input("invalid default deck"))?)
     }
 
-    /// If deck exists and and is a normal deck, return it.
+    /// If deck exists and and is a normal deck, return its ID and config
-    fn deck_if_normal(&mut self, did: DeckID) -> Result<Option<Arc<Deck>>> {
+    fn deck_conf_if_normal(&mut self, did: DeckID) -> Result<Option<(DeckID, DeckConfID)>> {
-        Ok(self
+        Ok(self.get_deck(did)?.and_then(|d| {
-            .get_deck(did)?
+            if let Some(conf_id) = d.config_id() {
-            .and_then(|d| if !d.is_filtered() { Some(d) } else { None }))
+                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);
     }
 }