projectmoon
/
tenebrous-dicebot
1
0
Fork 1
Code Issues 12 Pull Requests 1 Projects Releases 10 Wiki Activity

Keep/Drop Function #92

Manually merged
kg333 merged 12 commits from kg333/tenebrous-dicebot:keep_drop_function into master 2021-09-26 14:05:57 +00:00
Conversation 2 Commits 12 Files Changed 4 +37 -2
1 changed files with 37 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 1860eaf378 - Show all commits

39
dicebot/src/basic/parser.rs
View File

@ -33,10 +33,22 @@ enum Sign {
// Parse a dice expression. Does not eat whitespace
fn parse_dice(input: &str) -> IResult<&str, Dice> {
let (input, (count, _, sides)) = tuple((digit1, tag("d"), digit1))(input)?;
// parse main dice expression
let (mut input, (count, _, sides)) = tuple((digit1, tag("d"), digit1))(input)?;
// check for keep expression (i.e. D&D 5E Advantage)
let keep;
kg333 marked this conversation as resolved Outdated
projectmoon commented 2021-09-17 06:51:15 +00:00
Outdated
Review
Copy Link

I think it is better to document what the keep expression actually is here, so tacking on "1dXkY" would be useful.

And since pattern matches are expressions, I think we can rewrite these two match blocks. Something like this:

let (keep, input) = match tuple::<&str, _, (_, _), _>((tag("k"), digit1))(input) {
    // if ok, keep expression is present
    Ok(r) => (r.0, r.1.1),
    // otherwise absent and keep all dice
    Err(_) => ("".to_string(), count)
};

The same should be doable for the drop expression. I think this way you can also get rid of the mut on input, because let in rust is not just a variable assignment. It actually creates a whole new variable binding.

Note: I haven't tested the above, but it should work. Or some variation of it should work.

I think it is better to document what the keep expression actually is here, so tacking on "1dXkY" would be useful. And since pattern matches are expressions, I think we can rewrite these two match blocks. Something like this: ```rust let (keep, input) = match tuple::<&str, _, (_, _), _>((tag("k"), digit1))(input) { // if ok, keep expression is present Ok(r) => (r.0, r.1.1), // otherwise absent and keep all dice Err(_) => ("".to_string(), count) }; ``` The same should be doable for the drop expression. I think this way you can also get rid of the `mut` on input, because `let` in rust is not just a variable assignment. It actually creates a whole new variable binding. Note: I haven't tested the above, but it should work. Or some variation of it should work.
match tuple::<&str, _, (_, _), _>((tag("k"), digit1))(input) {
// if ok, keep expression is present
Ok(r) => {
kg333 marked this conversation as resolved Outdated
projectmoon commented 2021-09-22 19:53:33 +00:00
Outdated
Review
Copy Link

Rust supports tuple destructuring in pattern matching, which may be useful here and the other match:

Ok((rest, (_, keep_amount)) => ...

rest being the rest of the input in this case.

Rust supports tuple destructuring in pattern matching, which may be useful here and the other match: ```rust Ok((rest, (_, keep_amount)) => ... ``` `rest` being the rest of the input in this case.
kg333 commented 2021-09-25 02:58:04 +00:00
Outdated
Review
Copy Link

Ooh, that's much more human-readable, thanks!

Ooh, that's much more human-readable, thanks!
input = r.0;
keep = r.1.1;
}
// otherwise absent and keep all dice
Err(_) => keep = count,
};
Ok((
input,
Dice::new(count.parse().unwrap(), sides.parse().unwrap(), count.parse().unwrap()),
Dice::new(count.parse().unwrap(), sides.parse().unwrap(), keep.parse().unwrap()),
))
}
@ -111,6 +123,8 @@ mod tests {
assert_eq!(parse_dice("2d4"), Ok(("", Dice::new(2, 4, 2))));
assert_eq!(parse_dice("20d40"), Ok(("", Dice::new(20, 40, 20))));
assert_eq!(parse_dice("8d7"), Ok(("", Dice::new(8, 7, 8))));
assert_eq!(parse_dice("2d20k1"), Ok(("", Dice::new(2, 20, 1))));
assert_eq!(parse_dice("100d10k90"), Ok(("", Dice::new(100, 10, 90))));
}
#[test]
@ -119,6 +133,10 @@ mod tests {
parse_element(" \t\n\r\n 8d7 \n"),
Ok((" \n", Element::Dice(Dice::new(8, 7, 8))))
);
assert_eq!(
parse_element(" \t\n\r\n 3d20k2 \n"),
Ok((" \n", Element::Dice(Dice::new(3, 20, 2))))
);
assert_eq!(
parse_element(" \t\n\r\n 8 \n"),
Ok((" \n", Element::Bonus(8)))
@ -142,6 +160,13 @@ mod tests {
SignedElement::Negative(Element::Dice(Dice::new(8, 4, 8)))
))
);
assert_eq!(
parse_signed_element(" \t\n\r\n- 8d4k4 \n"),
Ok((
" \n",
SignedElement::Negative(Element::Dice(Dice::new(8, 4, 4)))
))
);
assert_eq!(
parse_signed_element(" \t\n\r\n+ 8d4 \n"),
Ok((
@ -162,6 +187,16 @@ mod tests {
)))])
))
);
assert_eq!(
parse_element_expression("\t2d20k1 + 5"),
Ok((
"",
ElementExpression(vec![
SignedElement::Positive(Element::Dice(Dice::new(2, 20, 1))),
SignedElement::Positive(Element::Bonus(5)),
])
))
);
assert_eq!(
parse_element_expression(" - 8d4 \n "),
Ok((