Change modifier order in Cthulhu

dicebot/src/cthulhu/parser.rs

@@ -4,16 +4,13 @@ use crate::parser::dice::DiceParsingError;
 //TOOD convert these to use parse_amounts from the common dice code.
 
 fn parse_modifier(input: &str) -> Result<DiceRollModifier, DiceParsingError> {
-    if input.ends_with("bb") {
+    match input.trim() {
-        Ok(DiceRollModifier::TwoBonus)
+        "bb" => Ok(DiceRollModifier::TwoBonus),
-    } else if input.ends_with("b") {
+        "b" => Ok(DiceRollModifier::OneBonus),
-        Ok(DiceRollModifier::OneBonus)
+        "pp" => Ok(DiceRollModifier::TwoPenalty),
-    } else if input.ends_with("pp") {
+        "p" => Ok(DiceRollModifier::OnePenalty),
-        Ok(DiceRollModifier::TwoPenalty)
+        "" => Ok(DiceRollModifier::Normal),
-    } else if input.ends_with("p") {
+        _ => Err(DiceParsingError::InvalidModifiers),
-        Ok(DiceRollModifier::OnePenalty)
-    } else {
-        Ok(DiceRollModifier::Normal)
     }
 }
 
@@ -21,32 +18,70 @@ fn parse_modifier(input: &str) -> Result<DiceRollModifier, DiceParsingError> {
 //Split based on :, send first part to parse_modifier.
 //Send second part to parse_amounts
 pub fn parse_regular_roll(input: &str) -> Result<DiceRoll, DiceParsingError> {
-    let input: Vec<&str> = input.trim().split(":").collect();
+    let (amount, modifiers_str) = crate::parser::dice::parse_single_amount(input)?;
-
-    let (modifiers_str, amounts_str) = match input[..] {
-        [amounts] => Ok(("", amounts)),
-        [modifiers, amounts] => Ok((modifiers, amounts)),
-        _ => Err(DiceParsingError::UnconsumedInput),
-    }?;
-
     let modifier = parse_modifier(modifiers_str)?;
-    let amount = crate::parser::dice::parse_single_amount(amounts_str)?;
     Ok(DiceRoll { modifier, amount })
 }
 
 pub fn parse_advancement_roll(input: &str) -> Result<AdvancementRoll, DiceParsingError> {
     let input = input.trim();
-    let amounts = crate::parser::dice::parse_single_amount(input)?;
+    let (amounts, unconsumed_input) = crate::parser::dice::parse_single_amount(input)?;
 
+    if unconsumed_input.len() == 0 {
         Ok(AdvancementRoll {
             existing_skill: amounts,
         })
+    } else {
+        Err(DiceParsingError::InvalidAmount)
+    }
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::parser::dice::{Amount, Element, Operator};
+    use crate::parser::dice::{Amount, DiceParsingError, Element, Operator};
+
+    #[test]
+    fn parse_modifier_rejects_bad_value() {
+        let modifier = parse_modifier("qqq");
+        assert!(matches!(modifier, Err(DiceParsingError::InvalidModifiers)))
+    }
+
+    #[test]
+    fn parse_modifier_accepts_one_bonus() {
+        let modifier = parse_modifier("b");
+        assert!(matches!(modifier, Ok(DiceRollModifier::OneBonus)))
+    }
+
+    #[test]
+    fn parse_modifier_accepts_two_bonus() {
+        let modifier = parse_modifier("bb");
+        assert!(matches!(modifier, Ok(DiceRollModifier::TwoBonus)))
+    }
+
+    #[test]
+    fn parse_modifier_accepts_two_penalty() {
+        let modifier = parse_modifier("pp");
+        assert!(matches!(modifier, Ok(DiceRollModifier::TwoPenalty)))
+    }
+
+    #[test]
+    fn parse_modifier_accepts_one_penalty() {
+        let modifier = parse_modifier("p");
+        assert!(matches!(modifier, Ok(DiceRollModifier::OnePenalty)))
+    }
+
+    #[test]
+    fn parse_modifier_accepts_normal() {
+        let modifier = parse_modifier("");
+        assert!(matches!(modifier, Ok(DiceRollModifier::Normal)))
+    }
+
+    #[test]
+    fn parse_modifier_accepts_normal_unaffected_by_whitespace() {
+        let modifier = parse_modifier("         ");
+        assert!(matches!(modifier, Ok(DiceRollModifier::Normal)))
+    }
 
     #[test]
     fn regular_roll_accepts_single_number() {
@@ -72,7 +107,7 @@ mod tests {
 
     #[test]
     fn regular_roll_accepts_two_bonus() {
-        let result = parse_regular_roll("bb:60");
+        let result = parse_regular_roll("60 bb");
         assert!(result.is_ok());
         assert_eq!(
             DiceRoll {
@@ -88,7 +123,7 @@ mod tests {
 
     #[test]
     fn regular_roll_accepts_one_bonus() {
-        let result = parse_regular_roll("b:60");
+        let result = parse_regular_roll("60 b");
         assert!(result.is_ok());
         assert_eq!(
             DiceRoll {
@@ -104,7 +139,7 @@ mod tests {
 
     #[test]
     fn regular_roll_accepts_two_penalty() {
-        let result = parse_regular_roll("pp:60");
+        let result = parse_regular_roll("60 pp");
         assert!(result.is_ok());
         assert_eq!(
             DiceRoll {
@@ -120,7 +155,7 @@ mod tests {
 
     #[test]
     fn regular_roll_accepts_one_penalty() {
-        let result = parse_regular_roll("p:60");
+        let result = parse_regular_roll("60 p");
         assert!(result.is_ok());
         assert_eq!(
             DiceRoll {
@@ -140,21 +175,21 @@ mod tests {
         assert!(parse_regular_roll("   60").is_ok());
         assert!(parse_regular_roll("   60    ").is_ok());
 
-        assert!(parse_regular_roll("bb:60     ").is_ok());
+        assert!(parse_regular_roll("60bb     ").is_ok());
-        assert!(parse_regular_roll("   bb:60").is_ok());
+        assert!(parse_regular_roll("   60 bb").is_ok());
-        assert!(parse_regular_roll("   bb:60    ").is_ok());
+        assert!(parse_regular_roll("   60   bb    ").is_ok());
 
-        assert!(parse_regular_roll("b:60     ").is_ok());
+        assert!(parse_regular_roll("60b     ").is_ok());
-        assert!(parse_regular_roll("   b:60").is_ok());
+        assert!(parse_regular_roll("   60 b").is_ok());
-        assert!(parse_regular_roll("   b:60    ").is_ok());
+        assert!(parse_regular_roll("   60  b    ").is_ok());
 
-        assert!(parse_regular_roll("pp:60     ").is_ok());
+        assert!(parse_regular_roll("60pp     ").is_ok());
-        assert!(parse_regular_roll("   pp:60").is_ok());
+        assert!(parse_regular_roll("   60 pp").is_ok());
-        assert!(parse_regular_roll("   pp:60    ").is_ok());
+        assert!(parse_regular_roll("   60 pp   ").is_ok());
 
-        assert!(parse_regular_roll("p:60     ").is_ok());
+        assert!(parse_regular_roll("60p     ").is_ok());
-        assert!(parse_regular_roll("   p:60").is_ok());
+        assert!(parse_regular_roll("   60p  ").is_ok());
-        assert!(parse_regular_roll("   p:60    ").is_ok());
+        assert!(parse_regular_roll("   60  p    ").is_ok());
     }
 
     #[test]

dicebot/src/parser/dice.rs

@@ -178,20 +178,15 @@ pub fn parse_amounts(input: &str) -> ParseResult<(Vec<Amount>, &str)> {
 /// Parse an expression that expects a single number or variable. No
 /// operators are allowed. This function is common to systems that
 /// don't deal with XdY rolls. Currently. this function does not
-/// support parsing negative numbers.
+/// support parsing negative numbers. Returns the parsed amount and
-pub fn parse_single_amount(input: &str) -> ParseResult<Amount> {
+/// any unconsumed input (useful for dice roll modifiers).
+pub fn parse_single_amount(input: &str) -> ParseResult<(Amount, &str)> {
     // TODO add support for negative numbers, as technically they
     // should be allowed.
     let input = input.trim();
     let mut parser = first_amount_parser().map(|amount: ParseResult<Amount>| amount);
-
     let (result, rest) = parser.parse(input)?;
-
+    Ok((result?, rest))
-    if rest.len() == 0 {
-        result
-    } else {
-        Err(DiceParsingError::UnconsumedInput)
-    }
 }
 
 #[cfg(test)]
@@ -204,10 +199,13 @@ mod parse_single_amount_tests {
         assert!(result.is_ok());
         assert_eq!(
             result.unwrap(),
+            (
                 Amount {
                     operator: Operator::Plus,
                     element: Element::Variable("abc".to_string())
-            }
+                },
+                ""
+            )
         )
     }
 
@@ -231,23 +229,14 @@ mod parse_single_amount_tests {
         assert!(result.is_ok());
         assert_eq!(
             result.unwrap(),
+            (
                 Amount {
                     operator: Operator::Plus,
                     element: Element::Number(1)
-            }
+                },
+                ""
+            )
         )
-    }
-
-    #[test]
-    fn parse_multiple_elements_test() {
-        let result = parse_single_amount("1+abc");
-        assert!(result.is_err());
-
-        let result = parse_single_amount("abc+1");
-        assert!(result.is_err());
-
-        let result = parse_single_amount("-1-abc");
-        assert!(result.is_err());
     }
 }