Implement improved scoring (#13)

Closes #10

Reviewed-on: #13
Co-authored-by: Rhys Lloyd <krakow20@gmail.com>
Co-committed-by: Rhys Lloyd <krakow20@gmail.com>

card_game/src/lib.rs

@@ -7,9 +7,11 @@ use core::ops::RangeBounds;
 
 // TODO: pub struct ValidInstruction<I>(I);
 pub trait Game {
+	type Score;
 	type Stats;
 	type Config;
 	type Instruction;
+	fn score(&self, stats: &Self::Stats, config: &Self::Config) -> Self::Score;
 	fn possible_instructions(
 		&self,
 		config: &Self::Config,
@@ -247,10 +249,13 @@ impl<const DN: usize, const UP: usize> Pile<DN, UP> {
 			face_up: Stack::new(),
 		}
 	}
-	pub fn flip_up(&mut self) {
+	/// Returns whether a card was flipped up.
+	pub fn flip_up(&mut self) -> bool {
 		if let Some(card) = self.face_down.pop() {
 			self.face_up.push(card);
+			return true;
 		}
+		false
 	}
 	pub fn is_empty(&self) -> bool {
 		self.face_down.is_empty() && self.face_up.is_empty()
@@ -258,25 +263,31 @@ impl<const DN: usize, const UP: usize> Pile<DN, UP> {
 	pub fn pop(&mut self) -> Option<Card> {
 		self.face_up.pop()
 	}
-	pub fn pop_flip_up(&mut self) -> Option<Card> {
+	/// Returns the popped card and whether a card was flipped up.
-		let card = self.face_up.pop()?;
+	pub fn pop_flip_up(&mut self) -> (Option<Card>, bool) {
-		if self.face_up.is_empty() {
+		let card = match self.face_up.pop() {
-			self.flip_up();
+			Some(card) => card,
-		}
+			None => return (None, false),
-		Some(card)
+		};
+		let did_flip_up = if self.face_up.is_empty() {
+			self.flip_up()
+		} else {
+			false
+		};
+		(Some(card), did_flip_up)
 	}
 	pub fn take_range<R: RangeBounds<usize>>(&mut self, range: R) -> Stack<UP> {
-		// if self.face_up.get(range).is_none() {
-		// 	return None;
-		// }
 		self.face_up.take_range(range)
 	}
-	pub fn take_range_flip_up<R: RangeBounds<usize>>(&mut self, range: R) -> Stack<UP> {
+	/// Returns the card range and whether a card was flipped up.
+	pub fn take_range_flip_up<R: RangeBounds<usize>>(&mut self, range: R) -> (Stack<UP>, bool) {
 		let cards = self.take_range(range);
-		if self.face_up.is_empty() {
+		let did_flip_up = if self.face_up.is_empty() {
-			self.flip_up();
+			self.flip_up()
-		}
+		} else {
-		cards
+			false
+		};
+		(cards, did_flip_up)
 	}
 	pub fn push(&mut self, card: Card) {
 		self.face_up.push(card);
@@ -309,24 +320,42 @@ pub enum SessionInstruction<I> {
 
 #[derive(Clone, Debug, Default)]
 pub struct SessionStats<S> {
-	inner_stats: S,
+	inner: S,
-	undos: usize,
+	undos: u32,
 }
 impl<S> SessionStats<S> {
 	pub const fn stats(&self) -> &S {
-		&self.inner_stats
+		&self.inner
 	}
 	const fn increment_undos(&mut self) {
 		self.undos += 1;
 	}
-	pub const fn undos(&self) -> usize {
+	pub const fn undos(&self) -> u32 {
 		self.undos
 	}
 }
+#[derive(Clone, Debug)]
+pub struct SessionConfig<C> {
+	pub inner: C,
+	pub undo_penalty: i32,
+}
+impl<C> SessionConfig<C> {
+	fn new_default(inner: C) -> Self {
+		Self {
+			inner,
+			undo_penalty: -15,
+		}
+	}
+}
+impl<C: Default> Default for SessionConfig<C> {
+	fn default() -> Self {
+		Self::new_default(C::default())
+	}
+}
 
 pub struct Session<G: Game> {
 	stats: SessionStats<G::Stats>,
-	config: G::Config,
+	config: SessionConfig<G::Config>,
 	state: SessionState<G>,
 }
 #[derive(Clone, Eq, Hash, PartialEq)]
@@ -344,13 +373,18 @@ impl<G: Game + Clone> SessionState<G> {
 		}
 	}
 }
-impl<G: Game> Session<G>
+impl<G: Game> SessionState<G> {
+	pub const fn state(&self) -> &G {
+		&self.state
+	}
+}
+impl<G: Game<Score = i32>> Session<G>
 where
 	G: Clone + Eq + core::hash::Hash,
 	G::Stats: Clone + Default,
 	G::Instruction: Clone + Eq + core::hash::Hash,
 {
-	pub fn new(state: G, config: G::Config) -> Self {
+	pub fn new(state: G, config: SessionConfig<G::Config>) -> Self {
 		Self {
 			stats: SessionStats::default(),
 			config,
@@ -366,10 +400,10 @@ where
 	pub const fn stats(&self) -> &SessionStats<G::Stats> {
 		&self.stats
 	}
-	pub const fn state(&self) -> &G {
+	pub const fn state(&self) -> &SessionState<G> {
-		&self.state.state
+		&self.state
 	}
-	pub const fn config(&self) -> &G::Config {
+	pub const fn config(&self) -> &SessionConfig<G::Config> {
 		&self.config
 	}
 	pub fn history(&self) -> &[G::Instruction] {
@@ -380,7 +414,7 @@ where
 			.process_instruction(&mut self.stats, &self.config, SessionInstruction::Undo)
 	}
 	pub fn possible_instructions(&self) -> impl Iterator<Item = G::Instruction> + use<G> {
-		self.state.state.possible_instructions(&self.config)
+		self.state.state.possible_instructions(&self.config.inner)
 	}
 	pub fn process_instruction(&mut self, instruction: G::Instruction) {
 		self.state.process_instruction(
@@ -393,28 +427,32 @@ where
 		self.state.is_win()
 	}
 }
-impl<G: Game> Game for SessionState<G>
+impl<G: Game<Score = i32>> Game for SessionState<G>
 where
 	G: Clone,
 	G::Stats: Default,
 	G::Instruction: Clone,
 {
+	type Score = i32;
 	type Stats = SessionStats<G::Stats>;
-	type Config = G::Config;
+	type Config = SessionConfig<G::Config>;
 	type Instruction = SessionInstruction<G::Instruction>;
+	fn score(&self, stats: &Self::Stats, config: &Self::Config) -> Self::Score {
+		self.state.score(&stats.inner, &config.inner) + stats.undos as i32 * config.undo_penalty
+	}
 	fn possible_instructions(
 		&self,
 		config: &Self::Config,
 	) -> impl Iterator<Item = Self::Instruction> + use<G> {
 		self.state
-			.possible_instructions(config)
+			.possible_instructions(&config.inner)
 			.map(SessionInstruction::InnerInstruction)
 	}
 	fn is_instruction_valid(&self, config: &Self::Config, instruction: Self::Instruction) -> bool {
 		match instruction {
 			SessionInstruction::Undo => !self.history.is_empty(),
 			SessionInstruction::InnerInstruction(instruction) => {
-				self.state.is_instruction_valid(config, instruction)
+				self.state.is_instruction_valid(&config.inner, instruction)
 			}
 		}
 	}
@@ -431,16 +469,16 @@ where
 				let mut inner_stats = G::Stats::default();
 				let mut state = self.seed.clone();
 				for instruction in &self.history {
-					state.process_instruction(&mut inner_stats, config, instruction.clone());
+					state.process_instruction(&mut inner_stats, &config.inner, instruction.clone());
 				}
 				self.state = state;
-				stats.inner_stats = inner_stats;
+				stats.inner = inner_stats;
 				stats.increment_undos();
 			}
 			SessionInstruction::InnerInstruction(instruction) => {
 				self.history.push(instruction.clone());
 				self.state
-					.process_instruction(&mut stats.inner_stats, config, instruction);
+					.process_instruction(&mut stats.inner, &config.inner, instruction);
 			}
 		}
 	}

klondike-bench/src/main.rs

@@ -1,5 +1,5 @@
 use card_game::Game;
-use klondike::{Klondike, KlondikeConfig, KlondikeStats, Rng};
+use klondike::{Klondike, KlondikeConfig, KlondikeStats, Rng, ScoringConfig};
 
 const MAX_MOVES: usize = 250;
 
@@ -10,13 +10,14 @@ fn play_to_win(rng: &mut Rng) -> Option<KlondikeStats> {
 	const CONFIG: KlondikeConfig = KlondikeConfig {
 		draw_stock: klondike::DrawStockConfig::DrawOne,
 		move_from_foundation: klondike::MoveFromFoundationConfig::Allowed,
+		scoring: ScoringConfig::DEFAULT,
 	};
 	// play game a bit
 	while let Some(instruction) = game.get_auto_move(&CONFIG)
 		&& !game.is_win()
 	{
 		// quit before 250 moves
-		if MAX_MOVES < stats.moves() + 1 {
+		if (MAX_MOVES as u32) < stats.moves() + 1 {
 			return None;
 		}
 
@@ -35,9 +36,9 @@ fn main() {
 	for _ in 0..GAMES {
 		if let Some(stats) = play_to_win(&mut rng) {
 			wins += 1;
-			score_tally[stats.score() / 5] += 1;
+			score_tally[(stats.score(&ScoringConfig::DEFAULT) / 5) as usize] += 1;
-			recycle_tally[stats.recycle_count()] += 1;
+			recycle_tally[stats.recycle_count() as usize] += 1;
-			moves_tally[stats.moves()] += 1;
+			moves_tally[stats.moves() as usize] += 1;
 		}
 	}
 	println!("score_tally={score_tally:?}");

klondike-cli/src/main.rs

@@ -1,7 +1,7 @@
-use card_game::{Card, Game, Pile, Rank, Session, SessionStats, Suit};
+use card_game::{Card, Game, Pile, Rank, Session, Suit};
 use klondike::{
 	DstFoundation, DstTableau, Foundation, Klondike, KlondikeConfig, KlondikeInstruction,
-	KlondikePile, KlondikePileStack, KlondikeStats, SkipCards, Tableau, TableauStack,
+	KlondikePile, KlondikePileStack, SkipCards, Tableau, TableauStack,
 };
 
 // #[cfg(test)]
@@ -108,15 +108,16 @@ impl Display for Displayed<&Klondike> {
 	}
 }
 
-impl Display for Displayed<&SessionStats<KlondikeStats>> {
+struct DisplayStats<'a>(&'a Session<Klondike>);
+impl Display for DisplayStats<'_> {
 	fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
 		write!(
 			f,
 			"recycles: {} moves: {} undos: {} score:{}",
-			self.0.stats().recycle_count(),
+			self.0.stats().stats().recycle_count(),
-			self.0.stats().moves(),
+			self.0.stats().stats().moves(),
-			self.0.undos(),
+			self.0.stats().undos(),
-			self.0.stats().score() as isize - self.0.undos() as isize * 15,
+			self.0.state().score(self.0.stats(), &self.0.config()),
 		)
 	}
 }
@@ -248,9 +249,9 @@ fn main() -> Result<(), std::io::Error> {
 	loop {
 		// display stats
 		println!("seed: {seed} ");
-		println!("{}", Displayed(session.stats()));
+		println!("{}", DisplayStats(&session));
 		// display game
-		println!("{}", Displayed(session.state()));
+		println!("{}", Displayed(session.state().state()));
 
 		// parse input
 		input.clear();
@@ -274,7 +275,11 @@ fn main() -> Result<(), std::io::Error> {
 				}
 			}
 			SessionInstruction::Auto => {
-				if let Some(instruction) = session.state().get_auto_move(session.config()) {
+				if let Some(instruction) = session
+					.state()
+					.state()
+					.get_auto_move(&session.config().inner)
+				{
 					session.process_instruction(instruction);
 				} else {
 					println!("No valid moves!");
@@ -284,9 +289,11 @@ fn main() -> Result<(), std::io::Error> {
 				session.process_instruction(KlondikeInstruction::RotateStock)
 			}
 			SessionInstruction::Klondike(naive_instruction) => {
-				if let Some(instruction) =
+				if let Some(instruction) = find_valid_instruction(
-					find_valid_instruction(session.config(), session.state(), naive_instruction)
+					&session.config().inner,
-				{
+					session.state().state(),
+					naive_instruction,
+				) {
 					session.process_instruction(instruction);
 				} else {
 					println!("Invalid move!");

klondike/src/lib.rs

@@ -7,7 +7,7 @@ use card_game::{Card, Game, Pile, Rank, Stack};
 #[cfg(doctest)]
 struct ReadmeDoctests;
 
-#[derive(Clone, Copy, Debug, Default)]
+#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)]
 pub enum DrawStockConfig {
 	#[default]
 	DrawOne = 1,
@@ -21,42 +21,96 @@ pub enum MoveFromFoundationConfig {
 	Disallowed,
 }
 
+#[derive(Clone, Copy, Debug)]
+pub struct ScoringConfig {
+	pub move_to_foundation: i32,
+	pub flip_up_bonus: i32,
+	pub move_to_tableau: i32,
+	pub move_from_foundation: i32,
+	pub recycle: i32,
+}
+impl ScoringConfig {
+	pub const DEFAULT: Self = Self {
+		move_to_foundation: 10,
+		flip_up_bonus: 5,
+		move_to_tableau: 5,
+		move_from_foundation: -15,
+		recycle: 0,
+	};
+}
+impl Default for ScoringConfig {
+	fn default() -> Self {
+		Self::DEFAULT
+	}
+}
+
 #[derive(Clone, Debug, Default)]
 pub struct KlondikeConfig {
 	pub draw_stock: DrawStockConfig,
 	pub move_from_foundation: MoveFromFoundationConfig,
+	pub scoring: ScoringConfig,
 }
 
 #[derive(Clone, Debug, Default)]
 pub struct KlondikeStats {
-	score: usize,
+	moves: u32,
-	recycle_count: usize,
+	move_to_foundation_count: u32,
-	moves: usize,
+	flip_up_bonus_count: u32,
+	move_to_tableau_count: u32,
+	move_from_foundation_count: u32,
+	recycle_count: u32,
 }
 impl KlondikeStats {
 	pub const fn new() -> Self {
 		KlondikeStats {
-			score: 0,
-			recycle_count: 0,
 			moves: 0,
+			move_to_foundation_count: 0,
+			flip_up_bonus_count: 0,
+			move_to_tableau_count: 0,
+			move_from_foundation_count: 0,
+			recycle_count: 0,
 		}
 	}
-	pub const fn score(&self) -> usize {
+	pub const fn score(&self, config: &ScoringConfig) -> i32 {
-		self.score
+		self.move_to_foundation_count as i32 * config.move_to_foundation
+			+ self.flip_up_bonus_count as i32 * config.flip_up_bonus
+			+ self.move_to_tableau_count as i32 * config.move_to_tableau
+			+ self.move_from_foundation_count as i32 * config.move_from_foundation
+			+ self.recycle_count as i32 * config.recycle
 	}
-	pub const fn recycle_count(&self) -> usize {
+	pub const fn moves(&self) -> u32 {
-		self.recycle_count
-	}
-	pub const fn moves(&self) -> usize {
 		self.moves
 	}
+	pub const fn move_to_foundation_count(&self) -> u32 {
+		self.move_to_foundation_count
+	}
+	pub const fn flip_up_bonus_count(&self) -> u32 {
+		self.flip_up_bonus_count
+	}
+	pub const fn move_to_tableau_count(&self) -> u32 {
+		self.move_to_tableau_count
+	}
+	pub const fn move_from_foundation_count(&self) -> u32 {
+		self.move_from_foundation_count
+	}
+	pub const fn recycle_count(&self) -> u32 {
+		self.recycle_count
+	}
 	/// A card was moved to a foundation.
-	const fn increment_score_foundation(&mut self) {
+	const fn increment_move_to_foundation(&mut self) {
-		self.score += 10;
+		self.move_to_foundation_count += 1;
+	}
+	/// A card on the tableau was flipped up.
+	const fn increment_flip_up_bonus(&mut self) {
+		self.flip_up_bonus_count += 1;
 	}
 	/// A card was moved from stock to tableau.
-	const fn increment_score_tableau(&mut self) {
+	const fn increment_move_to_tableau(&mut self) {
-		self.score += 5;
+		self.move_to_tableau_count += 1;
+	}
+	/// A card was moved from foundation to tableau.
+	const fn increment_move_from_foundation(&mut self) {
+		self.move_from_foundation_count += 1;
 	}
 	const fn increment_recycle_count(&mut self) {
 		self.recycle_count += 1;
@@ -414,7 +468,7 @@ impl KlondikeState {
 			KlondikePile::Stock => self.stock.face_up().last(),
 		}
 	}
-	fn take_stack(&mut self, src: KlondikePileStack) -> Stack<13> {
+	fn take_stack(&mut self, src: KlondikePileStack) -> (Stack<13>, bool) {
 		match src {
 			KlondikePileStack::Tableau(TableauStack {
 				tableau,
@@ -428,13 +482,14 @@ impl KlondikeState {
 				Tableau::Tableau6 => self.tableau6.take_range_flip_up(skip_cards as usize..),
 				Tableau::Tableau7 => self.tableau7.take_range_flip_up(skip_cards as usize..),
 			},
-			KlondikePileStack::Foundation(foundation) => {
+			KlondikePileStack::Foundation(foundation) => (
-				Stack::from_iter(self.foundations[foundation as usize].pop())
+				Stack::from_iter(self.foundations[foundation as usize].pop()),
-			}
+				false,
-			KlondikePileStack::Stock => Stack::from_iter(self.stock.pop()),
+			),
+			KlondikePileStack::Stock => (Stack::from_iter(self.stock.pop()), false),
 		}
 	}
-	fn take_top_card<S: Into<KlondikePile>>(&mut self, src: S) -> Option<Card> {
+	fn take_top_card<S: Into<KlondikePile>>(&mut self, src: S) -> (Option<Card>, bool) {
 		match src.into() {
 			KlondikePile::Tableau(tableau) => match tableau {
 				Tableau::Tableau1 => self.tableau1.pop_flip_up(),
@@ -445,8 +500,10 @@ impl KlondikeState {
 				Tableau::Tableau6 => self.tableau6.pop_flip_up(),
 				Tableau::Tableau7 => self.tableau7.pop_flip_up(),
 			},
-			KlondikePile::Foundation(foundation) => self.foundations[foundation as usize].pop(),
+			KlondikePile::Foundation(foundation) => {
-			KlondikePile::Stock => self.stock.pop(),
+				(self.foundations[foundation as usize].pop(), false)
+			}
+			KlondikePile::Stock => (self.stock.pop(), false),
 		}
 	}
 	fn extend_foundation<I: IntoIterator<Item = Card>>(
@@ -654,9 +711,13 @@ impl Klondike {
 }
 
 impl Game for Klondike {
+	type Score = i32;
 	type Stats = KlondikeStats;
 	type Config = KlondikeConfig;
 	type Instruction = KlondikeInstruction;
+	fn score(&self, stats: &Self::Stats, config: &Self::Config) -> Self::Score {
+		stats.score(&config.scoring)
+	}
 	fn possible_instructions(
 		&self,
 		config: &Self::Config,
@@ -690,16 +751,24 @@ impl Game for Klondike {
 			}
 			// Move a card from anywhere to a foundation
 			KlondikeInstruction::DstFoundation(DstFoundation { src, foundation }) => {
-				stats.increment_score_foundation();
+				stats.increment_move_to_foundation();
-				let card = self.state.take_top_card(src);
+				let (card, did_flip_up) = self.state.take_top_card(src);
+				if did_flip_up {
+					stats.increment_flip_up_bonus();
+				}
 				self.state.extend_foundation(foundation, card);
 			}
 			// Move a stack of cards from anywhere to a tableau
 			KlondikeInstruction::DstTableau(DstTableau { src, tableau }) => {
-				if src == KlondikePileStack::Stock {
+				match src {
-					stats.increment_score_tableau();
+					KlondikePileStack::Stock => stats.increment_move_to_tableau(),
+					KlondikePileStack::Foundation(_) => stats.increment_move_from_foundation(),
+					_ => {}
+				}
+				let (cards, did_flip_up) = self.state.take_stack(src);
+				if did_flip_up {
+					stats.increment_flip_up_bonus();
 				}
-				let cards = self.state.take_stack(src);
 				self.state.extend_tableau(tableau, cards);
 			}
 		}