From fe4da25e15d3a07e7c1dc266c044cf291dc95fc8 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Fri, 8 Jan 2021 20:32:36 +1000 Subject: [PATCH] fix reposition not honoring provided order, and add unit test --- rslib/src/backend/mod.rs | 8 ++- rslib/src/sched/new.rs | 109 ++++++++++++++++++++++++++++++++++----- 2 files changed, 103 insertions(+), 14 deletions(-) diff --git a/rslib/src/backend/mod.rs b/rslib/src/backend/mod.rs index 13487163f..22868773f 100644 --- a/rslib/src/backend/mod.rs +++ b/rslib/src/backend/mod.rs @@ -33,6 +33,7 @@ use crate::{ RenderCardOutput, }, sched::cutoff::local_minutes_west_for_stamp, + sched::new::NewCardSortOrder, sched::timespan::{answer_button_time, time_span}, search::{ concatenate_searches, negate_search, normalize_search, replace_search_term, BoolSeparator, @@ -597,8 +598,13 @@ impl BackendService for Backend { input.randomize, input.shift_existing, ); + let order = if random { + NewCardSortOrder::Random + } else { + NewCardSortOrder::Preserve + }; self.with_col(|col| { - col.sort_cards(&cids, start, step, random, shift) + col.sort_cards(&cids, start, step, order, shift) .map(Into::into) }) } diff --git a/rslib/src/sched/new.rs b/rslib/src/sched/new.rs index ea8737050..4043fb1b1 100644 --- a/rslib/src/sched/new.rs +++ b/rslib/src/sched/new.rs @@ -40,15 +40,21 @@ pub(crate) struct NewCardSorter { position: HashMap, } +#[derive(PartialEq)] +pub enum NewCardSortOrder { + NoteId, + Random, + Preserve, +} + impl NewCardSorter { - pub(crate) fn new(cards: &[Card], starting_from: u32, step: u32, random: bool) -> Self { - let nids: HashSet<_> = cards.iter().map(|c| c.note_id).collect(); - let mut nids: Vec<_> = nids.into_iter().collect(); - if random { - nids.shuffle(&mut rand::thread_rng()); - } else { - nids.sort_unstable(); - } + pub(crate) fn new( + cards: &[Card], + starting_from: u32, + step: u32, + order: NewCardSortOrder, + ) -> Self { + let nids = nids_in_desired_order(cards, order); NewCardSorter { position: nids @@ -67,6 +73,39 @@ impl NewCardSorter { } } +fn nids_in_desired_order(cards: &[Card], order: NewCardSortOrder) -> Vec { + if order == NewCardSortOrder::Preserve { + nids_in_preserved_order(cards) + } else { + let nids: HashSet<_> = cards.iter().map(|c| c.note_id).collect(); + let mut nids: Vec<_> = nids.into_iter().collect(); + match order { + NewCardSortOrder::NoteId => { + nids.sort_unstable(); + } + NewCardSortOrder::Random => { + nids.shuffle(&mut rand::thread_rng()); + } + NewCardSortOrder::Preserve => unreachable!(), + } + nids + } +} + +fn nids_in_preserved_order(cards: &[Card]) -> Vec { + let mut seen = HashSet::new(); + cards + .iter() + .filter_map(|card| { + if seen.insert(card.note_id) { + Some(card.note_id) + } else { + None + } + }) + .collect() +} + impl Collection { pub fn reschedule_cards_as_new(&mut self, cids: &[CardID], log: bool) -> Result<()> { let usn = self.usn()?; @@ -94,12 +133,12 @@ impl Collection { cids: &[CardID], starting_from: u32, step: u32, - random: bool, + order: NewCardSortOrder, shift: bool, ) -> Result<()> { let usn = self.usn()?; self.transact(None, |col| { - col.sort_cards_inner(cids, starting_from, step, random, shift, usn) + col.sort_cards_inner(cids, starting_from, step, order, shift, usn) }) } @@ -108,7 +147,7 @@ impl Collection { cids: &[CardID], starting_from: u32, step: u32, - random: bool, + order: NewCardSortOrder, shift: bool, usn: Usn, ) -> Result<()> { @@ -117,7 +156,7 @@ impl Collection { } self.storage.set_search_table_to_card_ids(cids, true)?; let cards = self.storage.all_searched_cards_in_search_order()?; - let sorter = NewCardSorter::new(&cards, starting_from, step, random); + let sorter = NewCardSorter::new(&cards, starting_from, step, order); for mut card in cards { let original = card.clone(); card.set_new_position(sorter.position(&card)); @@ -130,7 +169,12 @@ impl Collection { /// in the future if calling it as part of a deck options update. pub fn sort_deck(&mut self, deck: DeckID, random: bool) -> Result<()> { let cids = self.search_cards(&format!("did:{}", deck), SortMode::NoOrder)?; - self.sort_cards(&cids, 1, 1, random, false) + let order = if random { + NewCardSortOrder::Random + } else { + NewCardSortOrder::NoteId + }; + self.sort_cards(&cids, 1, 1, order, false) } fn shift_existing_cards(&mut self, start: u32, by: u32, usn: Usn) -> Result<()> { @@ -144,3 +188,42 @@ impl Collection { Ok(()) } } + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn new_order() { + let mut c1 = Card::new(NoteID(6), 0, DeckID(0), 0); + c1.id.0 = 2; + let mut c2 = Card::new(NoteID(5), 0, DeckID(0), 0); + c2.id.0 = 3; + let mut c3 = Card::new(NoteID(4), 0, DeckID(0), 0); + c3.id.0 = 1; + let cards = vec![c1.clone(), c2.clone(), c3.clone()]; + + // Preserve + let sorter = NewCardSorter::new(&cards, 0, 1, NewCardSortOrder::Preserve); + assert_eq!(sorter.position(&c1), 0); + assert_eq!(sorter.position(&c2), 1); + assert_eq!(sorter.position(&c3), 2); + + // NoteID/step/starting + let sorter = NewCardSorter::new(&cards, 3, 2, NewCardSortOrder::NoteId); + assert_eq!(sorter.position(&c3), 3); + assert_eq!(sorter.position(&c2), 5); + assert_eq!(sorter.position(&c1), 7); + + // Random + let mut c1_positions = HashSet::new(); + for _ in 1..100 { + let sorter = NewCardSorter::new(&cards, 0, 1, NewCardSortOrder::Random); + c1_positions.insert(sorter.position(&c1)); + if c1_positions.len() == cards.len() { + return; + } + } + unreachable!("not random"); + } +}