Type hints have been standardized to use generics as of 3.9 with backwards capability to 3.7 by importing from __futures import annotations

We can change a number of areas to remove Dict and List notations. However, I believe we may be stuck with Union? Will have to research further to ensure this is correct.

Add Pi and E constants to be able to be used in expressions!

There are examples generated that show the scripts running in a command line using the Typed JS library that look super sweet. Would be nice to have that in the readme front and center to give people an idea of what this is and how to use it.

Add the comparison operations for dice rolls.

Example:

roll 4d6>3
4d6: [ 3, 2, 4, 6 ]
 >3: 2
result: 2

Add the ability to use square root in expressions.

Add the operators for sin, cos, and tan in addition to what's already available.

Currently sqrt is set up so that it doesn't require parens. These would likely be the same I'm feeling where the parens are optional.

Add an option to always roll the highest amount given a dice roll.

example:
roll 1d6 --max == 6

Problem:
Currently our verbose output only contains the rolls that were made. However, this does not give a full picture of what is going on under the hood and will make things hard to test as we are going to edit the rolls with things like the keep, re-roll, and drop notations.

Solution:
I believe that the full history should be saved throughout a dice roll. While we are parsing, it can save the step that it is at and what the current state was as well as the operation that was taking place.

Example:

roll 4d20K3 -v

# output:
Rolled: 4d20: [ 2, 13, 17, 10 ]
Keep: K3: [ 13, 17, 10 ]

Considerations:

• Should we also do this for the mathematical operations? I think it might be difficult or noisy if we do, but, for the full picture, it's likely necessary.
• This will also help with testing because then we can compare the output with what we expect to ensure that we did not just get lucky, it actually performed the order of operations correctly at all points.
• One helpful thing that we could do for this if it is implemented is overloading the str method of EvaluationResults to give the verbose output of the history of the roll as well.

When checking for recursion issues using pyparsing's built-in method, it flags as there being one.

What I found is that this happens whenever we have the dice notation supplied for both one and two operands being available. This likely means that there is a better way to do what we're doing, potentially moving away from the inFixNotation or adding an optional number for the left operand in the 1 operand statement and removing the other.

Currently the reason why we're tightly coupled with roll and click is because parsley doesn't allow passing of variables when it is parsing strings. After we decouple, we need to be able to come full circle and still have this information outputted so that click can still use it. Right now we're using a global variable and handing it off that way and it's just not good.

For the people that would use this, I think that there is a group that would use it to play games and want to reroll things that they've already done before and give it a name, like "attack" and "mace_damage". Having the ability to save it into roll could help cut down on typing.

Examples:

# Saving named dice info
roll add attack 1d20 + 5

# Deleting named dice info
roll del attack

# Using named dice info
roll attack

When the number of dice to roll are supplied as a floating point number, it is currently messing up both the range (changing it from whatever it is to 1 to the max value). The chances of each output are thus changed from their normal distribution to an equal chance for all, making outliers far more likely than they otherwise would be.

Add the ability to denote that a dice should be rolled and only a number of highest or lowest rolls should be kept.

(Dice) K/k [count]

2d20K - Keep the highest roll out of the two.

4d6K3 - Keep the highest three rolls out of four. (Like for stat generation for D&D)

2d20k - Keep the lowest roll out of the two.

4d6k2 - Keep the lowest two rolls out of four.

Some of the issues that we have, including adding to pypi, using ci/cd pipelines, etc, are all things that would be fixed if we were to switch to using the hyper modern cookie cutter project starter. This should give us some bonus goodies as well, including better documentation using sphinx.

Concerns:

• We need to have the project all updated on the main branch before we do this. There is currently some code changes floating on a separate branch that needs to be dealt with first, either ignored or finished and merged.

Issues that this could aid in addressing or address outright: #40, #39, #28, and #20

As stated in #3, there was another way that we could've handled the evaluation that would have allowed us to uncouple from click. I propose that we use that method now with pyparsing as during the parsing step one can give a function for how to handle the parsed portions as they are parsed. This allows us to move away from the messy evaluate function. As it stands currently, it's not pretty to look at. There are parts that have to be deeply couched and the typechecks are worrisome, especially for the functions as the operators are currently mostly x: Union[int, float], y: Union[int, float], but that isn't the case for all and there are nested if-statements to ensure types before it uses them. Still, mypy is unhappy with this set up.

Pros:

• Type checking should be happy and the errors that mypy are raising should go away.
• This should also end up being cleaner code as lambdas that call each operation specifically should be much easier to grok.

Cons:

• Requires a rewrite of a substantial amount of the diceparser, including a lot of filling out for the EvaluationResults class that will need a lot of operation overloading to make everything pretty.
• This method I do not believe will be compatible with the new parser that is introduced in 3.10 which we are considering in #35. This issue should be a year or two in the future however, so new things may come to make this issue moot (hopefully). We cannot be the only ones that would want that type of functionality.

I am currently using a bunch of linters on my vim by default using a different environment which has led me to put some extra dotfiles into the project to quiet the noise. I need to take a look at all of what's present and get it down to what I'm actually using for the project. I believe some of the configuration files may also not be necessary and require looking in to.

In order to fix this properly, poetry needs to be used first and implemented correctly.

One of the main things that I stated were important to me going into this project is that it should be fast. While I have attempted to code in such a way that it makes the project efficient, I do not currently have any tests set up specifically for capturing how fast different aspects of the code work.

Proposed additional tests:

• Test the overall speed of the project from import all the way to getting a dice roll
• Test the underlying rolling functions
• Test the functions when called through Click

Considerations:

• It is likely going to be difficult to test how quick these things are as a CLI command because likely a good portion of the time that is spent is by Python itself getting everything loaded such that it can run our code.
  • I think though that, since we cannot really change that fact, we should really focus on the time that it takes to go from importing our code all the way to rolling a dice as our metric since that's what we control.

Why we should do this:

• I think that this is going to be leveraged heavily on whether or not changing certain elements of the code to Cython (#29) is beneficial and will allow us to better state exactly how beneficial it actually is rather than simply "it seems faster", especially as that feeling may be in part due to expectations and not based in reality.

How:

• I think this can be done via pytest using something like the built-in timeit library. We can probably do warnings if code gets slower than it currently is and completely fail the code if it takes too much time.
  • The concept of too much time and whether something is slower than the current time are both things that I will likely have to figure out through testing. I want the rolls to generally feel instantaneous. For websites to feel that way, they generally have to be under 700ms from memory.

Attempt to increase the speed of the script by converting the underlying parser/evaluator to Citron for a hopefully decent speed boost. Current tests take ~0.50ms to complete. I wonder if I can halve that time.

Add a notation to dice rolls that allows for a dice to be rolled, and, if it lands on a specific value, the initial value is kept and the dice is re-rolled. The notation should allow for the lowercase variant to re-roll only one and the uppercase should re-roll as many times as the number is hit.

This is different from the re-roll notation in that it keeps the initial value and adds it towards the total for the roll.

Examples:

# Example 1
roll 1d6p6
Rolled 6
Rolled 3
Total: 9

# Example 2
roll 1d6p6
Rolled 6
Rolled 6
Total: 12

# Example 3
roll 1d6P6
Rolled 6
Rolled 3
Total: 9

# Example 4
roll 1d6P6
Rolled 6
Rolled 6
Rolled 2
Total: 14

# Example 5
roll 1d6P6
Rolled 6
Rolled 6
Rolled 6
Rolled 5
Total: 23

It would be great if this were also uploaded to PyPI. I know that there are a number of dice rollers already present, but I think that, with the large number of features that are present in mine and how rigorously I have tested, that it would be worthwhile to add to the ecosystem.

Add the ability to interpret variables.

Example:
roll 4x = 8 == 2

Add the ability to always roll the lowest amount possible for a roll.

Example:
roll 1d6 --min == 1

❯ roll 4d6X1d2 -v
Rolled: 4d6: [5, 3, 4, 2]
Rolled: 1d2: [1]
Adding: 14 + 1 = 15
Dropping highest: 1: [2, 3, 4]
9

This is very odd. The numbers aren't even being added together in the end.

Problem:
Pyparsing states that the usual method for parsing statements is with the Lex/Yacc method. This may have the potential for a speed increase, even with the pure-python implementation PLY.

Solution:
Research this method, attempt to create a small prototype and try it on ((((((((((1 + 1)))))))))) to see whether it's quicker. If this is determined to be meaningfully faster, then switch over to using it.

Considerations:

• Will this really be a meaningful speed increase? Will have to ensure that this is worthwhile in #41
• We are already considering using the new python pattern matching notation in #35, will this be faster?

Things like 4d6K1d2 are not happening from left-to-right. Right now, this is the order that they're being handled:

❯ roll 4d6K1d2 -v
Rolled: 4d6: [6, 2, 5, 6]
Rolled: 1d2: [1]
Adding: 19 + 1 = 20
Keeping highest: 1: [6]
6

The expected order would be:
Roll 4d6
Keep the highest 1
Roll the highest 1d6 roll's worth of d2

Right now the probabilities haven't been properly considered. Do we have equal chance of getting all numbers in a 1d20 roll? If random.randint is not cryptographically secure, does that mean that will affect our probabilities enough to be worthwhile to switch to something that is? One of the issues is that cryptographically secure pseudorandom number generators would either require using the operating system's one, something that is not guaranteed on emulated systems like Termux for android. Maybe have the os version and then fall back to the random module if it's not available?

Add the ability to give a description for a given roll to be printed along with the roll itself.

Something like:
roll 1d20 --description "Initiative"

Output:
Rolling Initiative: 8

There are no sections and it would be nice to at least include an installation section.

I don't know that this is strictly necessary but it would probably make sense to do so.

Due to how parsley works, in order to have more information available to be printed using click, I had to couple things tighter than they really ought to be. This becomes an issue because I believe that the roll library cannot be imported and used by itself without weird output. It'll also have a messy namespace with click stuff inside of it.

In addition to the standard * operator, multiplication can be implicitly stated using parens. e.g. 2(1 + 3).

I noticed that the current implementation is cycling through and calling the calculate function without anything needing to be calculated:

:!python3 roll/roll.py                                                           
Start: 1, pairs: []
Start: 20, pairs: []
Start: 1, pairs: [('d', 20)]
Start: 8, pairs: []
Start: 8, pairs: []
8

I believe this is because it's firing calculate on each individually parsed chunk instead of only after a left and right side is identified with an operation.

With the typeguard jobs kicked off, the vast majority of our tests fail. This is due to slight type mismatches due to the Pyparsing library having their own types.

I think that the thing that's going to cause the most problems however is going to be the actual Parse method itself as it is now a ParseResults object, which is a List[Any], but in actuality is List[string, int, float, EvaluationResults], e.g. [1, "+", 2.5].

This is going to be a headache and a half to deal with and I am really worried because I have done stuff like check types ahead of time and raise exceptions if types are not what we expect (however, they should always be what we expect and it's impossible to my knowledge to test without mocking the function and having bad returned values). We may be able to get away with a one-liner so that we can still hit 100% code coverage with something like: if not isinstance(var, (type, type)): raise Exception, but I would like to stay away from that sort of thing if possible because I do not know if Black will like it and it's a pretty hacky workaround.

Another way we can insure types is by having a helper static method in our parser that takes input and ensures it's what we're expecting. That would be testable to cause it to throw, allowing us our 100%.

Feature request to add a command line flag, like -p/--probabilities, that will instead of rolling for a value, will output a list of all possible values and their percentage chance.

I.e.

roll -p 1d5
1: 0.20
2: 0.20
3: 0.20
4: 0.20
5: 0.20

Add the ability to denote that a dice roll should be excluded if it is the highest or lowest value.

(Dice) X/x [count]

2d20X - Drop the highest value (like disadvantage in D&D 5e.)

2d20x - Drop the lowest value (like advantage in D&D 5e.)

4d6X3 - Drop the highest 3 values

5d20x4 - Drop the lowest 4 values.

Considerations:
In the event that we are asked to drop more dice than are available, should we throw an exception or simply give back 0? I'm leaning toward 0.

Just realized I wasn't testing for parens within parens and had a good hour or two in bed of just pondering whether this was going to be an issue or not.

The main fear here is that it's not being greedy in its assessment of parens. It needs to make sure that it is finding encapsulating pairs instead of the first ones it sees. Otherwise ( 2 + ( 8 / 4 ) * 3 ) will error out believing that ( 2 + ( 8 / 4 ) to be one paren pair start and end point and that the inner portion is missing a closing paren.

One thing that I have been mulling over is a refactor for the DiceParser
anyway that would allow me to remove a considerable amount of reused
boiler plate code. We have the initial get-left-hand-value and then the
for-loop that then allows us to use the next operation and
left-hand-value, but that means then that we should make functions to
handle those operations and use it inside of a helper method that does
that heavy lifting only in one place.

We are currently showing out of order parsing for the given expression. This does not appear to cause any issues with the end result however, but it does cause the verbose output to show incorrect information.

Currently, using verbose output, we get:

❯ roll "3+2-1*7" -v
Adding: 3 + 2 = 5
Multiplying: 1 * 7 = 7
Subtracting: 5 - 7 = -2
-2

The expected output would have the addition happening after the multiplication.

This I feel is not a feature request but a bug as it is a pretty basic math operation, not an added dice notation feature.

Add the ability to state that there should not be any duplicates rolled.

Examples:
4d4u -> [ 2, 4, 1, 3 ]

Problem:
Some of the tests could be rewritten better. Currently some are parameterized and others aren't. We also are capturing errors without specifying what errors we're supposed to be capturing.

Solution:

• Go through and state what errors are thrown for which tests
• Parameterize the tests that need it (Like modulus)

Considerations:

• The errors may change in the future when we do better error handling that's more human friendly.

Problem:
Currently we allow dice to be given in the <num_dice>D<num_sides> format. The number of sides can be given as a floating-point number. What I have previously taken this to mean is that there are n-1 normal sized sides and the nth side is reduced in size and has a lower probability of being rolled. I do not believe this is the best way of handling this and does not give the user very many options. If it's the case with a D20 for instance, 1-19 would have equal weight and the 20th side would be differently weighted. What about the cases where half of the sides are differently weighted? How about all even numbered sides?

Solution:
Add in a notation that allows the user to stipulate which sides that should have unequal weight.

Example:

# Sides 1-10 are half the weight they normally would be
D20[1-10:0.5]

# Sides 1, 5, and 10 are 10% as likely to show as the other sides
D10[1, 5, 10:0.1]

# The weights are staggered such that the lowest roll is much more likely than the highest
D6[2: 0.9, 3: 0.7, 4: 0.5, 5: 0.3, 6: 0.1]

Considerations:

• This is going to get difficult, especially if the user gives impossible-to-distribute weights. (What would it mean if half are half as likely and the other half are twice as likely? Does that mean the first are 1/3rd as likely, the latter 2/3rds? Should this be an error?)
• What if the user specifies a side to add weight to that is outside of the range that is being rolled? (D6[20:0.5])
• There likely is a way to do this already that uses some statistics wizardry with the already available dice notation elements, I'm just not aware of them.

There are currently two issues that I think are probably related. Right now, with our github actions, it's generating a coverage report that collects all six of our test run info into a single report and uses that complete coverage info in order to base our coverage percentage on.

However, what is happening right now is that it is saying it cannot find some of the source files, first the roll-cli/init.py file, then the roll-cli/parser/diceparser.py file. Based on the limited research that I did, I think it might be from the test suites not clearing out pyc files? Either way, the maintainer suggests ignoring them and seeking their help if the problem persists.

I have already opened an issue with the template because I thought that it was due to how the coverage git action script set up the runs, but I cannot be certain that that's to blame.

There is also the problem of our abysmal code coverage percentage that I believe is tied to the above issue. There's likely a misconfiguration that's causing both is my guess.

Completion steps:

• Ensure that we can run the coverage job without ignoring coverage errors
• Our coverage percentage is back to >=95%

Brand new PEP 634 -- Structural Pattern Matching specification came out that may make pyparsing unnecessary and so that we can use the Python standard library for our parsing needs.

Pros:

• Offers a way to remove yet another dependency which reduces the potential for issues down the line if pyparsing does not get updates, both in terms of functionality and also malicious code threats.
• Potentially faster as we will not have to import outside code.
• Reduces overall footprint of the project on user's computers.

Cons:

• Requires yet another rewrite of underlying parser logic. The third time is the charm, but it's going to be even more of a pain now that additional features have been incorporated.
• Available starting at Python v3.10. This means that this spec is going to be out of reach of our general target audience for quite some time as we are currently shooting for 3.6-3.8. Ubuntu doesn't even have 3.9 officially yet.

CI/CD pipelines are a great way to get things done automagically for the project that do not require anything outside of the normal workflow.

Things that should be considered to be added:

• Automatic changelog generation using the Conventional Commits standard
• Black the Python code
• Release generation
  • This is going to require a restructuring of the project. No more PRs directly into master, we'll have to create a development branch using the gitflow method.
• PyPI publishing
• Code quality check
• Linting

Things we're currently using it for:

• Automated testing across the Python versions we state the project is good for.

I've had good experiences doing dependency management using this tool: https://python-poetry.org/

I created this before I really got into poetry, so I think this would benefit from being ported to it as it will help manage things like auto-generation of a python environment with the required dependencies as well as creating releases.

Add the ability to denote that a dice should be rerolled under certain conditions:

(Dice) R/r [number]

4d6r1 - Roll 4 d6 dice, reroll a die once if it lands on 1

4d6R1 - Roll 4 d6 dice, reroll a die every time if it lands on 1

Considerations:
1d1R1 could be an infinite loop. Must either have this as an error case or simply return 1 as the only possibility.

It would be nice if the error handler can output nicely formatted messages to point directly to the operation that caused the parser not to be able to work.

Example:

An exception occurred attempting to parse the input:
1 +
  ^
The addition operator expects a left and a right number.

Our project could do with some documentation. One way to create docs is by using Sphinx. This will allow us to keep docs in terms of the functionality that is available and what expected outputs are and examples of how to use different operators.

Here is a how-to guide: https://pythonhosted.org/an_example_pypi_project/sphinx.html
Here's a Medium article: https://medium.com/@richdayandnight/a-simple-tutorial-on-how-to-document-your-python-project-using-sphinx-and-rinohtype-177c22a15b5b

Currently the algorithm for rolling a dice takes a given number of dice to roll and a number of sides for the dice and creates that many random numbers and sums them all up.

Example:
3d6 == Sum of 3 random numbers between range 1 to 6.

Instead, as this can be extremely inefficient at higher rolls, we should do a weighted distribution and randomly choose.

Example:
10d6 == A number between 1 and 6 where the entries are weighted to account for the fact that we are rolling 10 dice.