avrae / d20

A fast, powerful, and extensible dice engine for D&D, d20 systems, and any other system that needs dice!
MIT License
122 stars 27 forks source link

Add support for percentile dice (Nd%) #2

Closed dalemyers closed 3 years ago

dalemyers commented 3 years ago

Summary

Percentile dice are used in various games which use dice (including other RPGs). This PR adds support for them. Adding support makes the d20 library more flexible and useful for these games.

Examples of usage: d%, 1d%, 10d%

Checklist

PR Type

dalemyers commented 3 years ago

I feel like this sounds like an even better way of handling this, but also that I have no idea how to do that :P I'll take a look though as I find the parsing here fascinating. Only just learned about Lark.

dalemyers commented 3 years ago

Ah, if I'm not mistaken, won't that be a breaking change though for anyone who uses the parsed expression?

dalemyers commented 3 years ago

I've got something that works, but it's less clear and has an API change (you'll do result.size.size or similar instead of just result.size). With the way I have now, there's no API change unless d% is rolled. Then you'll get % as the value instead of an integer. What do you think?

zhudotexe commented 3 years ago

Only just learned about Lark.

Super cool library, isn't it? 😄

won't that be a breaking change though for anyone who uses the parsed expression?

I was thinking the DiceSize object could implement an __eq__() method, e.g.

class DizeSize(Node):
    ...

    def __eq__(self, other):
        return self.size == other

... having just tinkered with an implementation of this for an hour, though, I don't think it's really worth it. It'd make more sense if there were other types of dice other than integer and percentile.

LGTM!

zhudotexe commented 3 years ago

If you want to pursue the DiceSize route further, here's my implementation: https://github.com/avrae/d20/commit/56205aa807368864a9ea1eaf144dd0ede7f9d006

dalemyers commented 3 years ago

Yeah, your implementation is similar (but more fully fledged) to my own foray into this. The one thing to be aware is that a few tests do break with this, since there is an implicit assumption about dice.size being the value, rather than dice.size.size or whatever.

It'd make more sense if there were other types of dice other than integer and percentile.

Yup. I came to the same conclusion. When I was playing with the grammar I wanted to ensure I expressed it as efficiently for future cases as possible but I couldn't think of anything that would match. There are plenty of special dice out there, but unless you start accepting something like 4d'game_whatever' then this isn't really suitable. Percentile dice cover the bases for basically every game that uses the d20 system already.

Thanks for merging!