From ee3ec18e0620320b2fd4f73574ea04556c29e241 Mon Sep 17 00:00:00 2001 From: projectmoon Date: Thu, 30 Sep 2021 21:16:00 +0000 Subject: [PATCH] Refactor keep-drop parsing into function, better error handling. (#93) This commit refactors the keep-drop parsing into two separate functions: one for extracting keep-drop text, and one for actually doing something with the extracted values. An intermediate enum is introduced to contain extracted text, instead of relying on Ok/Err values directly for figuring out what to do with the values. This allows us to express "this behavior is correct, and all others are not" instead of using a "fall back to secondary functionality" approach. Reviewed-on: https://git.agnos.is/projectmoon/tenebrous-dicebot/pulls/93 Co-Authored-By: projectmoon Co-Committed-By: projectmoon --- dicebot/src/basic/dice.rs | 39 ++++++++++---- dicebot/src/basic/parser.rs | 104 +++++++++++++++++++++++------------- 2 files changed, 94 insertions(+), 49 deletions(-) diff --git a/dicebot/src/basic/dice.rs b/dicebot/src/basic/dice.rs index e35a744..f8c4050 100644 --- a/dicebot/src/basic/dice.rs +++ b/dicebot/src/basic/dice.rs @@ -6,8 +6,9 @@ use std::fmt; use std::ops::{Deref, DerefMut}; -//Old stuff, for regular dice rolling. To be moved elsewhere. - +/// A basic dice roll, in XdY notation, like "1d4" or "3d6". +/// Optionally supports D&D advantage/disadvantge keep-or-drop +/// functionality. #[derive(Debug, PartialEq, Eq, Clone, Copy)] pub struct Dice { pub(crate) count: u32, @@ -15,26 +16,42 @@ pub struct Dice { pub(crate) keep_drop: KeepOrDrop, } +/// Enum indicating how to handle bonuses or penalties using extra +/// dice. If set to Keep, the roll will keep the highest X number of +/// dice in the roll, and add those together. If set to Drop, the +/// opposite is performed, and the lowest X number of dice are added +/// instead. If set to None, then all dice in the roll are added up as +/// normal. +#[derive(Debug, PartialEq, Eq, Clone, Copy)] +pub enum KeepOrDrop { + /// Keep only the X highest dice for adding up to the total. + Keep(u32), + + /// Keep only the X lowest dice (i.e. drop the highest) for adding + /// up to the total. + Drop(u32), + + /// Add up all dice in the roll for the total. + None, +} + impl fmt::Display for Dice { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self.keep_drop { KeepOrDrop::Keep(keep) => write!(f, "{}d{}k{}", self.count, self.sides, keep), KeepOrDrop::Drop(drop) => write!(f, "{}d{}dh{}", self.count, self.sides, drop), - KeepOrDrop::None => write!(f, "{}d{}", self.count, self.sides), + KeepOrDrop::None => write!(f, "{}d{}", self.count, self.sides), } } } -#[derive(Debug, PartialEq, Eq, Clone, Copy)] -pub enum KeepOrDrop { - Keep (u32), - Drop (u32), - None, -} - impl Dice { pub fn new(count: u32, sides: u32, keep_drop: KeepOrDrop) -> Dice { - Dice { count, sides, keep_drop } + Dice { + count, + sides, + keep_drop, + } } } diff --git a/dicebot/src/basic/parser.rs b/dicebot/src/basic/parser.rs index 8302951..80f7f04 100644 --- a/dicebot/src/basic/parser.rs +++ b/dicebot/src/basic/parser.rs @@ -33,6 +33,14 @@ enum Sign { Minus, } +/// Intermediate parsed value for a keep-drop expression to indicate +/// which one it is. +enum ParsedKeepOrDrop<'a> { + Keep(&'a str), + Drop(&'a str), + NotPresent, +} + macro_rules! too_big { ($input: expr) => { NomErr::Error(($input, NomErrorKind::TooLarge)) @@ -41,50 +49,58 @@ macro_rules! too_big { /// Parse a dice expression. Does not eat whitespace fn parse_dice(input: &str) -> IResult<&str, Dice> { - // parse main dice expression let (input, (count, _, sides)) = tuple((digit1, tag("d"), digit1))(input)?; - - // check for keep expression to keep highest dice (2d20k1) - let (keep, input) = match tuple::<&str, _, (_, _), _>((tag("k"), digit1))(input) { - // if ok, keep expression is present - Ok((rest, (_, keep_amount))) => (keep_amount, rest), - // otherwise absent and keep all dice - Err(_) => ("", input), - }; - - // check for drop expression to drop highest dice (2d20dh1) - let (drop, input) = match tuple::<&str, _, (_, _), _>((tag("dh"), digit1))(input) { - // if ok, keep expression is present - Ok((rest, (_, drop_amount))) => (drop_amount, rest), - // otherwise absent and keep all dice - Err(_) => ("", input), - }; - let count: u32 = count.parse().map_err(|_| too_big!(count))?; + let sides = sides.parse().map_err(|_| too_big!(sides))?; + let (input, keep_drop) = parse_keep_or_drop(input, count)?; + Ok((input, Dice::new(count, sides, keep_drop))) +} - // don't allow keep greater than number of dice, and don't allow keep zero - let keep_drop = match keep.parse::() { - // Ok, there's a keep value, check and create Keep - Ok(i) => match i { - _i if _i > count || _i == 0 => KeepOrDrop::None, - i => KeepOrDrop::Keep(i), +/// Extract keep/drop number as a string. Fails if the value is not a +/// string. +fn parse_keep_or_drop_text<'a>( + symbol: &'a str, + input: &'a str, +) -> IResult<&'a str, ParsedKeepOrDrop<'a>> { + let (parsed_kd, input) = match tuple::<&str, _, (_, _), _>((tag(symbol), digit1))(input) { + // if ok, one of the expressions is present + Ok((rest, (_, kd_expr))) => match symbol { + "k" => (ParsedKeepOrDrop::Keep(kd_expr), rest), + "dh" => (ParsedKeepOrDrop::Drop(kd_expr), rest), + _ => panic!("Unrecogized keep-drop symbol: {}", symbol), }, - // Err, check if drop works - Err(_) => { - match drop.parse::() { - // Ok, there's a drop value, check and create Drop - Ok(i) => match i { - _i if i >= count => KeepOrDrop::None, - i => KeepOrDrop::Drop(i), - }, - // Err, there's neither keep nor drop - Err(_) => KeepOrDrop::None, - } - } + // otherwise absent (attempt to keep all dice) + Err(_) => (ParsedKeepOrDrop::NotPresent, input), }; - let sides = sides.parse().map_err(|_| too_big!(sides))?; - Ok((input, Dice::new(count, sides, keep_drop))) + Ok((input, parsed_kd)) +} + +/// Parse keep/drop expression, which consits of "k" or "dh" following +/// a dice expression. For example, "1d4h3" or "1d4dh2". +fn parse_keep_or_drop<'a>(input: &'a str, count: u32) -> IResult<&'a str, KeepOrDrop> { + let (input, keep) = parse_keep_or_drop_text("k", input)?; + let (input, drop) = parse_keep_or_drop_text("dh", input)?; + + use ParsedKeepOrDrop::*; + let keep_drop: KeepOrDrop = match (keep, drop) { + //Potential valid Keep expression. + (Keep(keep), NotPresent) => match keep.parse().map_err(|_| too_big!(input))? { + _i if _i > count || _i == 0 => Ok(KeepOrDrop::None), + i => Ok(KeepOrDrop::Keep(i)), + }, + //Potential valid Drop expression. + (NotPresent, Drop(drop)) => match drop.parse().map_err(|_| too_big!(input))? { + _i if _i >= count => Ok(KeepOrDrop::None), + i => Ok(KeepOrDrop::Drop(i)), + }, + //No Keep or Drop specified; regular behavior. + (NotPresent, NotPresent) => Ok(KeepOrDrop::None), + //Anything else is an error. + _ => Err(NomErr::Error((input, NomErrorKind::Many1))), + }?; + + Ok((input, keep_drop)) } // Parse a single digit expression. Does not eat whitespace @@ -205,6 +221,18 @@ mod tests { ); } + #[test] + fn cant_have_both_keep_and_drop_test() { + let res = parse_dice("1d4k3dh2"); + assert!(res.is_err()); + match res { + Err(NomErr::Error((_, kind))) => { + assert_eq!(kind, NomErrorKind::Many1); + } + _ => panic!("Got success, expected error"), + } + } + #[test] fn big_number_of_dice_doesnt_crash_test() { let res = parse_dice("64378631476346123874527551481376547657868536d4");