src/cthulhu/dice.rs

@@ -1,14 +1,14 @@
+use crate::context::Context;
+use crate::dice::calculate_dice_amount;
 use crate::error::{BotError, DiceRollingError};
-use crate::parser::{Amount, Element};
+use crate::parser::Amount;
-use crate::{context::Context, db::variables::UserAndRoom};
-use crate::{dice::calculate_single_die_amount, parser::DiceParsingError};
 use std::convert::TryFrom;
 use std::fmt;
 
 /// A planned dice roll.
 #[derive(Clone, Debug, PartialEq)]
 pub struct DiceRoll {
-    pub amount: Amount,
+    pub amounts: Vec<Amount>,
     pub modifier: DiceRollModifier,
 }
 
@@ -188,7 +188,7 @@ impl fmt::Display for RolledDice {
 pub struct AdvancementRoll {
     /// The amount (0 to 100) of the existing skill. We must beat this
     /// target number to advance the skill, or roll above a 95.
-    pub existing_skill: Amount,
+    pub existing_skill: Vec<Amount>,
 }
 
 impl fmt::Display for AdvancementRoll {
@@ -251,25 +251,10 @@ impl fmt::Display for RolledAdvancement {
     }
 }
 
-/// This is a trait so we can inject controlled dice rolls in unit
-/// tests.
 trait DieRoller {
     fn roll(&mut self) -> u32;
 }
 
-/// Macro to determine if an Amount is a variable.
-macro_rules! is_variable {
-    ($existing_skill:ident) => {
-        matches!(
-            $existing_skill,
-            Amount {
-                element: Element::Variable(_),
-                ..
-            }
-        );
-    };
-}
-
 ///A version of DieRoller that uses a rand::Rng to roll numbers.
 struct RngDieRoller<R: rand::Rng>(R);
 
@@ -358,39 +343,23 @@ fn roll_advancement_dice<R: DieRoller>(target: u32, roller: &mut R) -> RolledAdv
 pub async fn regular_roll(
     roll_with_ctx: &DiceRollWithContext<'_>,
 ) -> Result<ExecutedDiceRoll, BotError> {
-    let target = calculate_single_die_amount(&roll_with_ctx.0.amount, roll_with_ctx.1).await?;
+    let target = calculate_dice_amount(&roll_with_ctx.0.amounts, roll_with_ctx.1).await?;
     let target = u32::try_from(target).map_err(|_| DiceRollingError::InvalidAmount)?;
 
     let mut roller = RngDieRoller(rand::thread_rng());
     let rolled_dice = roll_regular_dice(&roll_with_ctx.0.modifier, target, &mut roller);
 
     Ok(ExecutedDiceRoll {
-        target,
+        target: target,
         modifier: roll_with_ctx.0.modifier,
         roll: rolled_dice,
     })
 }
 
-fn update_skill(ctx: &Context, variable: &str, value: u32) -> Result<(), BotError> {
-    use std::convert::TryInto;
-    let value: i32 = value.try_into()?;
-    let key = UserAndRoom(ctx.username, ctx.room_id().as_str());
-    ctx.db.variables.set_user_variable(&key, variable, value)?;
-    Ok(())
-}
-
-fn extract_variable(amount: &Amount) -> Result<&str, DiceParsingError> {
-    match amount.element {
-        Element::Variable(ref varname) => Ok(&varname[..]),
-        _ => Err(DiceParsingError::WrongElementType),
-    }
-}
 pub async fn advancement_roll(
     roll_with_ctx: &AdvancementRollWithContext<'_>,
 ) -> Result<ExecutedAdvancementRoll, BotError> {
-    let existing_skill = &roll_with_ctx.0.existing_skill;
+    let target = calculate_dice_amount(&roll_with_ctx.0.existing_skill, roll_with_ctx.1).await?;
-    let target = calculate_single_die_amount(existing_skill, roll_with_ctx.1).await?;
-
     let target = u32::try_from(target).map_err(|_| DiceRollingError::InvalidAmount)?;
 
     if target > 100 {
@@ -399,12 +368,6 @@ pub async fn advancement_roll(
 
     let mut roller = RngDieRoller(rand::thread_rng());
     let roll = roll_advancement_dice(target, &mut roller);
-
-    if roll.successful && is_variable!(existing_skill) {
-        let variable_name: &str = extract_variable(existing_skill)?;
-        update_skill(roll_with_ctx.1, variable_name, roll.new_skill_amount())?;
-    }
-
     Ok(ExecutedAdvancementRoll { target, roll })
 }
 
@@ -450,37 +413,13 @@ mod tests {
         }
     }
 
-    #[test]
-    fn extract_variable_gets_variable_name() {
-        let amount = Amount {
-            operator: Operator::Plus,
-            element: Element::Variable("abc".to_string()),
-        };
-
-        let result = extract_variable(&amount);
-
-        assert!(result.is_ok());
-        assert_eq!(result.unwrap(), "abc");
-    }
-
-    #[test]
-    fn extract_variable_fails_on_number() {
-        let result = extract_variable(&Amount {
-            operator: Operator::Plus,
-            element: Element::Number(1),
-        });
-
-        assert!(result.is_err());
-        assert!(matches!(result, Err(DiceParsingError::WrongElementType)));
-    }
-
     #[tokio::test]
     async fn regular_roll_rejects_negative_numbers() {
         let roll = DiceRoll {
-            amount: Amount {
+            amounts: vec![Amount {
                 operator: Operator::Plus,
                 element: Element::Number(-10),
-            },
+            }],
             modifier: DiceRollModifier::Normal,
         };
 
@@ -506,10 +445,10 @@ mod tests {
     #[tokio::test]
     async fn advancement_roll_rejects_negative_numbers() {
         let roll = AdvancementRoll {
-            existing_skill: Amount {
+            existing_skill: vec![Amount {
                 operator: Operator::Plus,
                 element: Element::Number(-10),
-            },
+            }],
         };
 
         let db = Database::new_temp().unwrap();
@@ -534,10 +473,10 @@ mod tests {
     #[tokio::test]
     async fn advancement_roll_rejects_big_numbers() {
         let roll = AdvancementRoll {
-            existing_skill: Amount {
+            existing_skill: vec![Amount {
                 operator: Operator::Plus,
                 element: Element::Number(3000),
-            },
+            }],
         };
 
         let db = Database::new_temp().unwrap();
@@ -559,26 +498,6 @@ mod tests {
         ));
     }
 
-    #[test]
-    fn is_variable_macro_succeds_on_variable() {
-        let amount = Amount {
-            operator: Operator::Plus,
-            element: Element::Variable("abc".to_string()),
-        };
-
-        assert_eq!(is_variable!(amount), true);
-    }
-
-    #[test]
-    fn is_variable_macro_fails_on_number() {
-        let amount = Amount {
-            operator: Operator::Plus,
-            element: Element::Number(1),
-        };
-
-        assert_eq!(is_variable!(amount), false);
-    }
-
     #[test]
     fn regular_roll_succeeds_when_below_target() {
         //Roll 30, succeeding.

src/cthulhu/parser.rs

@@ -20,6 +20,7 @@ fn parse_modifier(input: &str) -> Result<DiceRollModifier, DiceParsingError> {
 //Make diceroll take a vec of Amounts
 //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();
 
@@ -30,13 +31,14 @@ pub fn parse_regular_roll(input: &str) -> Result<DiceRoll, DiceParsingError> {
     }?;
 
     let modifier = parse_modifier(modifiers_str)?;
-    let amount = crate::parser::parse_single_amount(amounts_str)?;
+    let amounts = crate::parser::parse_amounts(amounts_str)?;
-    Ok(DiceRoll { modifier, amount })
+
+    Ok(DiceRoll { modifier, amounts })
 }
 
 pub fn parse_advancement_roll(input: &str) -> Result<AdvancementRoll, DiceParsingError> {
     let input = input.trim();
-    let amounts = crate::parser::parse_single_amount(input)?;
+    let amounts = crate::parser::parse_amounts(input)?;
 
     Ok(AdvancementRoll {
         existing_skill: amounts,
@@ -55,32 +57,26 @@ mod tests {
         assert!(result.is_ok());
         assert_eq!(
             DiceRoll {
-                amount: Amount {
+                amounts: vec![Amount {
                     operator: Operator::Plus,
                     element: Element::Number(60)
-                },
+                }],
                 modifier: DiceRollModifier::Normal
             },
             result.unwrap()
         );
     }
 
-    #[test]
-    fn regular_roll_rejects_complex_expressions() {
-        let result = parse_regular_roll("3 + abc + bob - 4");
-        assert!(result.is_err());
-    }
-
     #[test]
     fn regular_roll_accepts_two_bonus() {
         let result = parse_regular_roll("bb:60");
         assert!(result.is_ok());
         assert_eq!(
             DiceRoll {
-                amount: Amount {
+                amounts: vec![Amount {
                     operator: Operator::Plus,
                     element: Element::Number(60)
-                },
+                }],
                 modifier: DiceRollModifier::TwoBonus
             },
             result.unwrap()
@@ -93,10 +89,10 @@ mod tests {
         assert!(result.is_ok());
         assert_eq!(
             DiceRoll {
-                amount: Amount {
+                amounts: vec![Amount {
                     operator: Operator::Plus,
                     element: Element::Number(60)
-                },
+                }],
                 modifier: DiceRollModifier::OneBonus
             },
             result.unwrap()
@@ -109,10 +105,10 @@ mod tests {
         assert!(result.is_ok());
         assert_eq!(
             DiceRoll {
-                amount: Amount {
+                amounts: vec![Amount {
                     operator: Operator::Plus,
                     element: Element::Number(60)
-                },
+                }],
                 modifier: DiceRollModifier::TwoPenalty
             },
             result.unwrap()
@@ -125,10 +121,10 @@ mod tests {
         assert!(result.is_ok());
         assert_eq!(
             DiceRoll {
-                amount: Amount {
+                amounts: vec![Amount {
                     operator: Operator::Plus,
                     element: Element::Number(60)
-                },
+                }],
                 modifier: DiceRollModifier::OnePenalty
             },
             result.unwrap()
@@ -136,7 +132,7 @@ mod tests {
     }
 
     #[test]
-    fn regular_roll_accepts_whitespace() {
+    fn regular_roll_accepts_whitespacen() {
         assert!(parse_regular_roll("60     ").is_ok());
         assert!(parse_regular_roll("   60").is_ok());
         assert!(parse_regular_roll("   60    ").is_ok());
@@ -171,10 +167,10 @@ mod tests {
         assert!(result.is_ok());
         assert_eq!(
             AdvancementRoll {
-                existing_skill: Amount {
+                existing_skill: vec![Amount {
                     operator: Operator::Plus,
                     element: Element::Number(60)
-                }
+                }]
             },
             result.unwrap()
         );
@@ -191,18 +187,41 @@ mod tests {
         assert!(result.is_ok());
         assert_eq!(
             AdvancementRoll {
-                existing_skill: Amount {
+                existing_skill: vec![Amount {
                     operator: Operator::Plus,
                     element: Element::Variable(String::from("abc"))
-                }
+                }]
             },
             result.unwrap()
         );
     }
 
     #[test]
-    fn advancement_roll_rejects_complex_expressions() {
+    fn advancement_roll_allows_complex_expressions() {
         let result = parse_advancement_roll("3 + abc + bob - 4");
-        assert!(result.is_err());
+        assert!(result.is_ok());
+        assert_eq!(
+            AdvancementRoll {
+                existing_skill: vec![
+                    Amount {
+                        operator: Operator::Plus,
+                        element: Element::Number(3)
+                    },
+                    Amount {
+                        operator: Operator::Plus,
+                        element: Element::Variable(String::from("abc"))
+                    },
+                    Amount {
+                        operator: Operator::Plus,
+                        element: Element::Variable(String::from("bob"))
+                    },
+                    Amount {
+                        operator: Operator::Minus,
+                        element: Element::Number(4)
+                    }
+                ]
+            },
+            result.unwrap()
+        );
     }
 }

src/dice.rs

@@ -5,17 +5,6 @@ use crate::error::DiceRollingError;
 use crate::parser::Amount;
 use crate::parser::Element as NewElement;
 use futures::stream::{self, StreamExt, TryStreamExt};
-use std::slice;
-
-/// Calculate the amount of dice to roll by consulting the database
-/// and replacing variables with corresponding the amount. Errors out
-/// if it cannot find a variable defined, or if the database errors.
-pub async fn calculate_single_die_amount(
-    amount: &Amount,
-    ctx: &Context<'_>,
-) -> Result<i32, BotError> {
-    calculate_dice_amount(slice::from_ref(amount), ctx).await
-}
 
 /// Calculate the amount of dice to roll by consulting the database
 /// and replacing variables with corresponding amounts. Errors out if

src/error.rs

@@ -75,9 +75,6 @@ pub enum BotError {
 
     #[error("too many commands or message was too large")]
     MessageTooLarge,
-
-    #[error("could not convert to proper integer type")]
-    TryFromIntError(#[from] std::num::TryFromIntError),
 }
 
 #[derive(Error, Debug)]

src/parser.rs

@@ -21,9 +21,6 @@ pub enum DiceParsingError {
 
     #[error("number parsing error (too large?)")]
     ConversionError,
-
-    #[error("unexpected element in expression")]
-    WrongElementType,
 }
 
 impl From<std::num::ParseIntError> for DiceParsingError {
@@ -36,7 +33,7 @@ type ParseResult<T> = Result<T, DiceParsingError>;
 
 /// A parsed operator for a number. Whether to add or remove it from
 /// the total amount of dice rolled.
-#[derive(Debug, PartialEq, Eq, Clone, Copy)]
+#[derive(Debug, PartialEq, Eq, Clone)]
 pub enum Operator {
     Plus,
     Minus,
@@ -151,7 +148,10 @@ 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.
+/// deal with XdY rolls. Support for that will be added later. Parsers
+/// utilzing this function should layer their own checks on top of
+/// this; perhaps they do not want more than one expression, or some
+/// other rules.
 pub fn parse_amounts(input: &str) -> ParseResult<Vec<Amount>> {
     let input = input.trim();
 
@@ -177,84 +177,9 @@ pub fn parse_amounts(input: &str) -> ParseResult<Vec<Amount>> {
     }
 }
 
-/// 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> {
-    // 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)
-    }
-}
-
 #[cfg(test)]
-mod parse_single_amount_tests {
+mod tests {
-    use super::*;
 
-    #[test]
-    fn parse_single_variable_test() {
-        let result = parse_single_amount("abc");
-        assert!(result.is_ok());
-        assert_eq!(
-            result.unwrap(),
-            Amount {
-                operator: Operator::Plus,
-                element: Element::Variable("abc".to_string())
-            }
-        )
-    }
-
-    // TODO add support for negative numbers in parse_single_amount
-    // #[test]
-    // fn parse_single_negative_number_test() {
-    //     let result = parse_single_amount("-1");
-    //     assert!(result.is_ok());
-    //     assert_eq!(
-    //         result.unwrap(),
-    //         Amount {
-    //             operator: Operator::Minus,
-    //             element: Element::Number(1)
-    //         }
-    //     )
-    // }
-
-    #[test]
-    fn parse_single_number_test() {
-        let result = parse_single_amount("1");
-        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());
-    }
-}
-
-#[cfg(test)]
-mod parse_many_amounts_tests {
     use super::*;
 
     #[test]