Change modifier order in Cthulhu

dicebot/src/cofd/parser.rs

@@ -45,13 +45,13 @@ pub fn parse_modifiers(input: &str) -> Result<DicePoolModifiers, DiceParsingErro
     let (result, rest) = parser.parse(input)?;
 
     if rest.len() == 0 {
-        convert_to_info(&result)
+        convert_to_modifiers(&result)
     } else {
         Err(DiceParsingError::UnconsumedInput)
     }
 }
 
-fn convert_to_info(parsed: &Vec<ParsedInfo>) -> Result<DicePoolModifiers, DiceParsingError> {
+fn convert_to_modifiers(parsed: &Vec<ParsedInfo>) -> Result<DicePoolModifiers, DiceParsingError> {
     use ParsedInfo::*;
     if parsed.len() == 0 {
         Ok(DicePoolModifiers::default())
@@ -79,19 +79,8 @@ fn convert_to_info(parsed: &Vec<ParsedInfo>) -> Result<DicePoolModifiers, DicePa
 }
 
 pub fn parse_dice_pool(input: &str) -> Result<DicePool, BotError> {
-    //The "modifiers:" part is optional. Assume amounts if no modifier
-    //section found.
-    let split = input.split(":").collect::<Vec<_>>();
-    let (modifiers_str, amounts_str) = (match split[..] {
-        [amounts] => Ok(("", amounts)),
-        [modifiers, amounts] => Ok((modifiers, amounts)),
-        _ => Err(BotError::DiceParsingError(
-            DiceParsingError::UnconsumedInput,
-        )),
-    })?;
-
+    let (amounts, modifiers_str) = parse_amounts(input)?;
     let modifiers = parse_modifiers(modifiers_str)?;
-    let amounts = parse_amounts(&amounts_str)?;
     Ok(DicePool::new(amounts, modifiers))
 }
 
@@ -175,7 +164,7 @@ mod tests {
 
     #[test]
     fn dice_pool_number_with_quality() {
-        let result = parse_dice_pool("n:8");
+        let result = parse_dice_pool("8 n");
         assert!(result.is_ok());
         assert_eq!(
             result.unwrap(),
@@ -186,7 +175,7 @@ mod tests {
     #[test]
     fn dice_pool_number_with_success_change() {
         let modifiers = DicePoolModifiers::custom_exceptional_on(3);
-        let result = parse_dice_pool("s3:8");
+        let result = parse_dice_pool("8 s3");
         assert!(result.is_ok());
         assert_eq!(result.unwrap(), DicePool::easy_with_modifiers(8, modifiers));
     }
@@ -194,7 +183,7 @@ mod tests {
     #[test]
     fn dice_pool_with_quality_and_success_change() {
         let modifiers = DicePoolModifiers::custom(DicePoolQuality::Rote, 3);
-        let result = parse_dice_pool("rs3:8");
+        let result = parse_dice_pool("8 rs3");
         assert!(result.is_ok());
         assert_eq!(result.unwrap(), DicePool::easy_with_modifiers(8, modifiers));
     }
@@ -224,20 +213,20 @@ mod tests {
 
         let expected = DicePool::new(amounts, modifiers);
 
-        let result = parse_dice_pool("rs3:8+10-2+varname");
+        let result = parse_dice_pool("8+10-2+varname rs3");
         assert!(result.is_ok());
         assert_eq!(result.unwrap(), expected);
 
-        let result = parse_dice_pool("rs3:8+10-   2 + varname");
+        let result = parse_dice_pool("8+10-   2 + varname      rs3");
         assert!(result.is_ok());
         assert_eq!(result.unwrap(), expected);
 
-        let result = parse_dice_pool("rs3  :  8+ 10 -2 + varname");
+        let result = parse_dice_pool("8+ 10 -2 + varname           rs3");
         assert!(result.is_ok());
         assert_eq!(result.unwrap(), expected);
 
         //This one has tabs in it.
-        let result = parse_dice_pool("  r	s3  :	8	+ 10 -2 + varname");
+        let result = parse_dice_pool(" 	8	+ 10 -2 + varname	r	s3");
         assert!(result.is_ok());
         assert_eq!(result.unwrap(), expected);
     }

dicebot/src/commands/parser.rs

@@ -221,9 +221,9 @@ mod tests {
 
     #[test]
     fn pool_whitespace_test() {
-        parse_command("!pool ns3:8   ").expect("was error");
-        parse_command("   !pool ns3:8").expect("was error");
-        parse_command("   !pool ns3:8   ").expect("was error");
+        parse_command("!pool 8   ns3    ").expect("was error");
+        parse_command("   !pool 8 ns3").expect("was error");
+        parse_command("   !pool 8   ns3    ").expect("was error");
     }
 
     #[test]

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") {
-        Ok(DiceRollModifier::TwoBonus)
-    } else if input.ends_with("b") {
-        Ok(DiceRollModifier::OneBonus)
-    } else if input.ends_with("pp") {
-        Ok(DiceRollModifier::TwoPenalty)
-    } else if input.ends_with("p") {
-        Ok(DiceRollModifier::OnePenalty)
-    } else {
-        Ok(DiceRollModifier::Normal)
+    match input.trim() {
+        "bb" => Ok(DiceRollModifier::TwoBonus),
+        "b" => Ok(DiceRollModifier::OneBonus),
+        "pp" => Ok(DiceRollModifier::TwoPenalty),
+        "p" => Ok(DiceRollModifier::OnePenalty),
+        "" => Ok(DiceRollModifier::Normal),
+        _ => Err(DiceParsingError::InvalidModifiers),
     }
 }
 
@@ -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 (modifiers_str, amounts_str) = match input[..] {
-        [amounts] => Ok(("", amounts)),
-        [modifiers, amounts] => Ok((modifiers, amounts)),
-        _ => Err(DiceParsingError::UnconsumedInput),
-    }?;
-
+    let (amount, modifiers_str) = crate::parser::dice::parse_single_amount(input)?;
     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)?;
 
-    Ok(AdvancementRoll {
-        existing_skill: amounts,
-    })
+    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("   bb:60").is_ok());
-        assert!(parse_regular_roll("   bb:60    ").is_ok());
+        assert!(parse_regular_roll("60bb     ").is_ok());
+        assert!(parse_regular_roll("   60 bb").is_ok());
+        assert!(parse_regular_roll("   60   bb    ").is_ok());
 
-        assert!(parse_regular_roll("b:60     ").is_ok());
-        assert!(parse_regular_roll("   b:60").is_ok());
-        assert!(parse_regular_roll("   b:60    ").is_ok());
+        assert!(parse_regular_roll("60b     ").is_ok());
+        assert!(parse_regular_roll("   60 b").is_ok());
+        assert!(parse_regular_roll("   60  b    ").is_ok());
 
-        assert!(parse_regular_roll("pp:60     ").is_ok());
-        assert!(parse_regular_roll("   pp:60").is_ok());
-        assert!(parse_regular_roll("   pp:60    ").is_ok());
+        assert!(parse_regular_roll("60pp     ").is_ok());
+        assert!(parse_regular_roll("   60 pp").is_ok());
+        assert!(parse_regular_roll("   60 pp   ").is_ok());
 
-        assert!(parse_regular_roll("p:60     ").is_ok());
-        assert!(parse_regular_roll("   p:60").is_ok());
-        assert!(parse_regular_roll("   p:60    ").is_ok());
+        assert!(parse_regular_roll("60p     ").is_ok());
+        assert!(parse_regular_roll("   60p  ").is_ok());
+        assert!(parse_regular_roll("   60  p    ").is_ok());
     }
 
     #[test]

dicebot/src/parser/dice.rs

@@ -151,8 +151,9 @@ where
 /// should not have an operator, but every one after that should.
 /// Accepts expressions like "8", "10 + variablename", "variablename -
 /// 3", etc. This function is currently common to systems that don't
-/// deal with XdY rolls. Support for that will be added later.
-pub fn parse_amounts(input: &str) -> ParseResult<Vec<Amount>> {
+/// deal with XdY rolls. Support for that will be added later. Returns
+/// parsed amounts and unconsumed input (e.g. roll modifiers).
+pub fn parse_amounts(input: &str) -> ParseResult<(Vec<Amount>, &str)> {
     let input = input.trim();
 
     let remaining_amounts = many(amount_parser()).map(|amounts: Vec<ParseResult<Amount>>| amounts);
@@ -169,31 +170,23 @@ pub fn parse_amounts(input: &str) -> ParseResult<Vec<Amount>> {
             (amounts, results.1)
         })?;
 
-    if rest.len() == 0 {
-        // Any ParseResult errors will short-circuit the collect.
-        results.into_iter().collect()
-    } else {
-        Err(DiceParsingError::UnconsumedInput)
-    }
+    // Any ParseResult errors will short-circuit the collect.
+    let results: Vec<Amount> = results.into_iter().collect::<ParseResult<_>>()?;
+    Ok((results, rest))
 }
 
 /// 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.
-pub fn parse_single_amount(input: &str) -> ParseResult<Amount> {
+/// support parsing negative numbers. Returns the parsed amount and
+/// 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)?;
-
-    if rest.len() == 0 {
-        result
-    } else {
-        Err(DiceParsingError::UnconsumedInput)
-    }
+    Ok((result?, rest))
 }
 
 #[cfg(test)]
@@ -206,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())
-            }
+            (
+                Amount {
+                    operator: Operator::Plus,
+                    element: Element::Variable("abc".to_string())
+                },
+                ""
+            )
         )
     }
 
@@ -233,24 +229,15 @@ mod parse_single_amount_tests {
         assert!(result.is_ok());
         assert_eq!(
             result.unwrap(),
-            Amount {
-                operator: Operator::Plus,
-                element: Element::Number(1)
-            }
+            (
+                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());
-    }
 }
 
 #[cfg(test)]
@@ -263,20 +250,26 @@ mod parse_many_amounts_tests {
         assert!(result.is_ok());
         assert_eq!(
             result.unwrap(),
-            vec![Amount {
-                operator: Operator::Plus,
-                element: Element::Number(1)
-            }]
+            (
+                vec![Amount {
+                    operator: Operator::Plus,
+                    element: Element::Number(1)
+                }],
+                ""
+            )
         );
 
         let result = parse_amounts("10");
         assert!(result.is_ok());
         assert_eq!(
             result.unwrap(),
-            vec![Amount {
-                operator: Operator::Plus,
-                element: Element::Number(10)
-            }]
+            (
+                vec![Amount {
+                    operator: Operator::Plus,
+                    element: Element::Number(10)
+                }],
+                ""
+            )
         );
     }
 
@@ -295,20 +288,26 @@ mod parse_many_amounts_tests {
         assert!(result.is_ok());
         assert_eq!(
             result.unwrap(),
-            vec![Amount {
-                operator: Operator::Plus,
-                element: Element::Variable("asdf".to_string())
-            }]
+            (
+                vec![Amount {
+                    operator: Operator::Plus,
+                    element: Element::Variable("asdf".to_string())
+                }],
+                ""
+            )
         );
 
         let result = parse_amounts("nosis");
         assert!(result.is_ok());
         assert_eq!(
             result.unwrap(),
-            vec![Amount {
-                operator: Operator::Plus,
-                element: Element::Variable("nosis".to_string())
-            }]
+            (
+                vec![Amount {
+                    operator: Operator::Plus,
+                    element: Element::Variable("nosis".to_string())
+                }],
+                ""
+            )
         );
     }