remove separate trait

card_game/src/lib.rs

@@ -5,7 +5,6 @@ struct ReadmeDoctests;
 
 use core::ops::RangeBounds;
 
-// TODO: pub struct ValidInstruction<I>(I);
 pub trait Game: Clone {
 	type Score: Clone + core::fmt::Debug;
 	type Stats: Clone + core::fmt::Debug;
@@ -16,14 +15,51 @@ pub trait Game: Clone {
 		&self,
 		config: &Self::Config,
 	) -> impl Iterator<Item = Self::Instruction> + use<Self>;
-	fn is_instruction_valid(&self, config: &Self::Config, instruction: Self::Instruction) -> bool;
+	fn is_instruction_valid(&self, config: &Self::Config, instruction: &Self::Instruction) -> bool;
-	fn process_instruction(
+	fn process_instruction(valid_instruction: StagedInstruction<'_, Self>);
-		&mut self,
-		stats: &mut Self::Stats,
-		config: &Self::Config,
-		instruction: Self::Instruction,
-	);
 	fn is_win(&self) -> bool;
+	/// Validate an instruction to prepare to run it.
+	fn stage_instruction<'a>(
+		&'a mut self,
+		stats: &'a mut Self::Stats,
+		config: &'a Self::Config,
+		instruction: Self::Instruction,
+	) -> Option<StagedInstruction<'a, Self>> {
+		self.is_instruction_valid(config, &instruction)
+			.then_some(StagedInstruction {
+				game: self,
+				stats,
+				config,
+				instruction,
+			})
+	}
+}
+
+/// Call .commit() to run the staged instruction.
+/// Cannot be constructed other than by Game::stage_instruction.
+pub struct StagedInstruction<'a, G: Game> {
+	game: &'a mut G,
+	stats: &'a mut G::Stats,
+	config: &'a G::Config,
+	instruction: G::Instruction,
+}
+impl<'a, G: Game> StagedInstruction<'a, G> {
+	/// Consume the staged instruction.  Use this in your implementation of InstructionConsumer::process_instruction for your game.
+	pub fn consume(self) -> (&'a mut G, &'a mut G::Stats, &'a G::Config, G::Instruction) {
+		let StagedInstruction {
+			game,
+			stats,
+			config,
+			instruction,
+		} = self;
+		(game, stats, config, instruction)
+	}
+}
+impl<G: Game> StagedInstruction<'_, G> {
+	/// Process the staged instruction.
+	pub fn commit(self) {
+		G::process_instruction(self);
+	}
 }
 
 /// card_game supports up to 4 identifiably separate decks.
@@ -542,14 +578,21 @@ where
 		&self.state.history
 	}
 	pub fn undo(&mut self) {
+		if let Some(staged) =
 			self.state
-			.process_instruction(&mut self.stats, &self.config, SessionInstruction::Undo)
+				.stage_instruction(&mut self.stats, &self.config, SessionInstruction::Undo)
+		{
+			staged.commit();
+		}
 	}
 	pub fn possible_instructions(&self) -> impl Iterator<Item = G::Instruction> + use<G> {
 		self.state.state.possible_instructions(&self.config.inner)
 	}
-	pub fn process_instruction(&mut self, instruction: G::Instruction) {
+	pub fn stage_instruction(
-		self.state.process_instruction(
+		&mut self,
+		instruction: G::Instruction,
+	) -> Option<StagedInstruction<'_, SessionState<G>>> {
+		self.state.stage_instruction(
 			&mut self.stats,
 			&self.config,
 			SessionInstruction::InnerInstruction(instruction),
@@ -583,7 +626,8 @@ where
 
 			// Run one possible move
 			if let Some(instruction) = it.next() {
-				session.process_instruction(instruction);
+				// TODO: no unwrap
+				session.stage_instruction(instruction).unwrap().commit();
 				continue;
 			}
 
@@ -618,7 +662,7 @@ where
 			.possible_instructions(&config.inner)
 			.map(SessionInstruction::InnerInstruction)
 	}
-	fn is_instruction_valid(&self, config: &Self::Config, instruction: Self::Instruction) -> bool {
+	fn is_instruction_valid(&self, config: &Self::Config, instruction: &Self::Instruction) -> bool {
 		match instruction {
 			SessionInstruction::Undo => !self.history.is_empty(),
 			SessionInstruction::InnerInstruction(instruction) => {
@@ -626,26 +670,27 @@ where
 			}
 		}
 	}
-	fn process_instruction(
+	fn process_instruction(valid_instruction: StagedInstruction<'_, Self>) {
-		&mut self,
+		let (game, stats, config, instruction) = valid_instruction.consume();
-		stats: &mut Self::Stats,
-		config: &Self::Config,
-		instruction: Self::Instruction,
-	) {
 		match instruction {
 			SessionInstruction::Undo => {
-				if let Some(snapshot) = self.history.pop() {
+				if let Some(snapshot) = game.history.pop() {
-					self.state = snapshot.state;
+					game.state = snapshot.state;
 					stats.increment_undos();
 				}
 			}
 			SessionInstruction::InnerInstruction(instruction) => {
-				self.history.push(StateSnapshot {
+				game.history.push(StateSnapshot {
-					state: self.state.clone(),
+					state: game.state.clone(),
 					instruction: instruction.clone(),
 				});
-				self.state
+				// This might be considered cheaing a bit, we're pre-approving the instruction here by accessing the inner fields.
-					.process_instruction(&mut stats.inner, &config.inner, instruction);
+				G::process_instruction(StagedInstruction {
+					game: &mut game.state,
+					stats: &mut stats.inner,
+					config: &config.inner,
+					instruction,
+				});
 			}
 		}
 	}
@@ -681,7 +726,8 @@ where
 		let serialized = SerializedSession::deserialize(deserializer)?;
 		let mut session = Session::new(serialized.initial_state, serialized.config);
 		for instruction in serialized.instructions {
-			session.process_instruction(instruction);
+			// TODO: no unwrap
+			session.stage_instruction(instruction).unwrap().commit();
 		}
 		Ok(session)
 	}

klondike-bench/src/main.rs

@@ -21,7 +21,9 @@ fn play_to_win(rng: &mut Rng) -> Option<KlondikeStats> {
 			return None;
 		}
 
-		game.process_instruction(&mut stats, &CONFIG, instruction);
+		game.stage_instruction(&mut stats, &CONFIG, instruction)
+			.unwrap()
+			.commit();
 	}
 	game.is_win().then_some(stats)
 }

klondike-cli/src/main.rs

@@ -210,7 +210,7 @@ fn find_valid_instruction(
 						});
 						let instruction =
 							KlondikeInstruction::DstTableau(DstTableau { tableau, src });
-						if state.is_instruction_valid(config, instruction) {
+						if state.is_instruction_valid(config, &instruction) {
 							return Some(instruction);
 						}
 					}
@@ -228,7 +228,7 @@ fn find_valid_instruction(
 		_ => return None,
 	};
 	state
-		.is_instruction_valid(config, instruction)
+		.is_instruction_valid(config, &instruction)
 		.then_some(instruction)
 }
 
@@ -277,13 +277,16 @@ fn main() -> Result<(), std::io::Error> {
 					.state()
 					.get_auto_move(&session.config().inner)
 				{
-					session.process_instruction(instruction);
+					session.stage_instruction(instruction).unwrap().commit();
 				} else {
 					println!("No valid moves!");
 				}
 			}
 			SessionInstruction::Stock => {
-				session.process_instruction(KlondikeInstruction::RotateStock)
+				session
+					.stage_instruction(KlondikeInstruction::RotateStock)
+					.unwrap()
+					.commit();
 			}
 			SessionInstruction::Klondike(naive_instruction) => {
 				if let Some(instruction) = find_valid_instruction(
@@ -291,7 +294,7 @@ fn main() -> Result<(), std::io::Error> {
 					session.state().state(),
 					naive_instruction,
 				) {
-					session.process_instruction(instruction);
+					session.stage_instruction(instruction).unwrap().commit();
 				} else {
 					println!("Invalid move!");
 				}

klondike/README.md

@@ -16,7 +16,7 @@ let mut session = Session::new_default(game);
 
 // play game a bit
 while let Some(instruction) = session.state().state().get_auto_move(&config) {
-	session.process_instruction(instruction);
+	session.stage_instruction(instruction).unwrap().commit();
 
 	// quit after 200 moves or win
 	if session.is_win() || 200 < session.stats().stats().moves() {

klondike/src/lib.rs

@@ -1,6 +1,6 @@
 pub type Rng = rand::rngs::StdRng;
 
-use card_game::{Card, Game, Pile, Rank, Stack};
+use card_game::{Card, Game, Pile, Rank, Stack, StagedInstruction};
 
 #[cfg(test)]
 mod test;
@@ -621,7 +621,7 @@ impl KlondikeState {
 	pub fn is_instruction_valid(
 		&self,
 		config: &KlondikeConfig,
-		instruction: KlondikeInstruction,
+		instruction: &KlondikeInstruction,
 	) -> bool {
 		match instruction {
 			// Stock -> Stock draws a card or resets the stock
@@ -830,38 +830,34 @@ impl Game for Klondike {
 		let state = self.state.clone();
 		let config = config.clone();
 		KlondikeIter::new()
-			.filter(move |&instruction| state.is_instruction_valid(&config, instruction))
+			.filter(move |instruction| state.is_instruction_valid(&config, instruction))
 	}
-	fn is_instruction_valid(&self, config: &Self::Config, instruction: Self::Instruction) -> bool {
+	fn is_instruction_valid(&self, config: &Self::Config, instruction: &Self::Instruction) -> bool {
 		self.state.is_instruction_valid(config, instruction)
 	}
-	fn process_instruction(
+	fn process_instruction(valid_instruction: StagedInstruction<'_, Self>) {
-		&mut self,
+		let (game, stats, config, instruction) = valid_instruction.consume();
-		stats: &mut Self::Stats,
-		config: &Self::Config,
-		instruction: Self::Instruction,
-	) {
 		stats.increment_moves();
 		match instruction {
 			// Reset the stock if it's empty
 			KlondikeInstruction::RotateStock => {
-				if self.state.stock.face_down().is_empty() {
+				if game.state.stock.face_down().is_empty() {
-					self.state.stock.flip_it_and_reverse_it();
+					game.state.stock.flip_it_and_reverse_it();
 					stats.increment_recycle_count();
 				} else {
 					for _ in 0..config.draw_stock as usize {
-						self.state.stock.flip_up();
+						game.state.stock.flip_up();
 					}
 				}
 			}
 			// Move a card from anywhere to a foundation
 			KlondikeInstruction::DstFoundation(DstFoundation { src, foundation }) => {
 				stats.increment_move_to_foundation();
-				let (card, did_flip_up) = self.state.take_top_card_flip_up(src);
+				let (card, did_flip_up) = game.state.take_top_card_flip_up(src);
 				if did_flip_up {
 					stats.increment_flip_up_bonus();
 				}
-				self.state.extend_foundation(foundation, card);
+				game.state.extend_foundation(foundation, card);
 			}
 			// Move a stack of cards from anywhere to a tableau
 			KlondikeInstruction::DstTableau(DstTableau { src, tableau }) => {
@@ -870,11 +866,11 @@ impl Game for Klondike {
 					KlondikePileStack::Foundation(_) => stats.increment_move_from_foundation(),
 					KlondikePileStack::Tableau(_) => {}
 				}
-				let (cards, did_flip_up) = self.state.take_stack_flip_up(src);
+				let (cards, did_flip_up) = game.state.take_stack_flip_up(src);
 				if did_flip_up {
 					stats.increment_flip_up_bonus();
 				}
-				self.state.extend_tableau(tableau, cards);
+				game.state.extend_tableau(tableau, cards);
 			}
 		}
 	}

klondike/src/test.rs

@@ -23,7 +23,10 @@ fn test_json() {
 	let solution_result = session.solve();
 	if let Ok(Some(solution)) = solution_result {
 		for snapshot in solution.clean_solution() {
-			session.process_instruction(snapshot.instruction().clone());
+			session
+				.stage_instruction(snapshot.instruction().clone())
+				.unwrap()
+				.commit();
 		}
 	}
 	let serialized = serde_json::to_string(&session).unwrap();
@@ -42,7 +45,10 @@ fn test_rmp() {
 	let solution_result = session.solve();
 	if let Ok(Some(solution)) = solution_result {
 		for snapshot in solution.clean_solution() {
-			session.process_instruction(snapshot.instruction().clone());
+			session
+				.stage_instruction(snapshot.instruction().clone())
+				.unwrap()
+				.commit();
 		}
 	}
 	let serialized = rmp_serde::to_vec(&session).unwrap();