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 +11 -19
1 changed files with 11 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit f904e3a948 - Show all commits

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

@ -31,33 +31,25 @@ enum Sign {
Minus,
}
// Parse a dice expression. Does not eat whitespace
/// Parse a dice expression. Does not eat whitespace
fn parse_dice(input: &str) -> IResult<&str, Dice> {
// parse main dice expression
let (mut input, (count, _, sides)) = tuple((digit1, tag("d"), digit1))(input)?;
let (input, (count, _, sides)) = tuple((digit1, tag("d"), digit1))(input)?;
// check for keep expression (i.e. D&D 5E Advantage)
let keep;
match tuple::<&str, _, (_, _), _>((tag("k"), digit1))(input) {
// check for keep expression to keep highest dice (2d20k1)
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.
let (keep, input) = match tuple::<&str, _, (_, _), _>((tag("k"), digit1))(input) {
// if ok, keep expression is present
Ok(r) => {
input = r.0;
keep = r.1.1;
}
Ok(r) => (r.1.1, r.0),
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!
// otherwise absent and keep all dice
Err(_) => keep = count,
Err(_) => (input, "")
};
// check for drop expression (i.e. D&D 5E Disadvantage)
let drop;
match tuple::<&str, _, (_, _), _>((tag("d"), digit1))(input) {
// if ok, drop expression is present
Ok(r) => {
input = r.0;
drop = r.1.1;
}
// 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(r) => (r.1.1, r.0),
// otherwise absent and keep all dice
Err(_) => drop = "0",
Err(_) => (input, "")
};
let count: u32 = count.parse().unwrap();