dice-notation-java icon indicating copy to clipboard operation
dice-notation-java copied to clipboard

consider allowing multiplication and division

Open danlangford opened this issue 6 years ago • 13 comments

sometimes, though usually its rare, an RPG will ask for multiplication or division. Here is an example from the freely available "Basic Rules" of Dungeons & Dragons 5th Edition:

Starting Wealth

Class Funds
Cleric 5d4 × 10 gp
Fighter 5d4 × 10 gp
Rogue 4d4 × 10 gp
Wizard 4d4 × 10 gp

would you consider allowing multiplication (poss chars: x, *, ×) and division (poss chars: /, ÷) i personally prefer the charcaters * and /.

i would be willing to take a look at adding the necessary operation classes. I wanted to just bounce it off you first in case you have already ruled it out

danlangford avatar Jan 07 '19 21:01 danlangford

I'll take a look into it. This would require extending the binary operation expression.

Bernardo-MG avatar Jan 07 '19 21:01 Bernardo-MG

Once parsed these operations should apply an order of preference.

This can be verified with these test cases on the final result:

1*2-3=-1
1-2*3=-5

If the correct order of operations is not applied then the second case will return '-3'.

Bernardo-MG avatar Jan 07 '19 22:01 Bernardo-MG

ya i played with it a little bit. my code currently fails that second "order of operation" test you suggested up there.

danlangford avatar Jan 07 '19 23:01 danlangford

I remember that I had to change the parse tree structure, making it left-handed, for some reason related to the order in which the operations were performed.

But until now I only had additions and substractions.

I'll take a deeper look as soon as I can.

Bernardo-MG avatar Jan 08 '19 06:01 Bernardo-MG

The first test case was invalid. It gives the same result if it works or it fails.

These two are better expressions (the second one is the same as before):

2*3-3=3
1-2*3=-5
2*3-3
On success: 3
On failure: 0
1-2*3
On success: -5
On failure: -3

Bernardo-MG avatar Jan 08 '19 07:01 Bernardo-MG

ITDefaultDiceSetsTransformerParser includes end to end tests, but these are too simple.

I'll add some tests to verify the existing binary operations too:

1-2+3
On success: 2
On failure: -4

The thing in this test case is that the parser always returns positive values, and makes use of the subtraction binary operation to transform them into negative values.

'-2' is '0-2', and '1-2' is not the same as '1 + -2'.

Bernardo-MG avatar Jan 08 '19 10:01 Bernardo-MG

The division symbol '/' is giving some problems.

ANTLR manages to build a valid tree from any expression using divisions, but the parser is unable to parse the binary operation. It seems that the '/' symbol is breaking the parse tree visitor.

Bernardo-MG avatar Jan 12 '19 00:01 Bernardo-MG

I'm correcting myself, it seems I didn't pay attention to what the expression listener was doing or the exceptions.

Actually it seems like it tries to build the binary expression twice, and the second time it is missing the operands, as they were already used to build an expression.

I'll take another look and try to fix it as soon as I can.

Bernardo-MG avatar Jan 12 '19 16:01 Bernardo-MG

No need for a better look. I was matching the wrong operator when building divisions.

There are two problems left to solve:

  • Order of operators, for example multiplications have priority over additions
  • Float values resulting from divisions, such as '3/2=1.5'

Bernardo-MG avatar Jan 12 '19 16:01 Bernardo-MG

Added issues #44 and #43 to tackle these problems.

Bernardo-MG avatar Jan 14 '19 07:01 Bernardo-MG

Added support for multiplications and divisions. The code is in develop.

Float values are not supported yet. This means that divisions will always return integers.

Bernardo-MG avatar Jan 20 '19 21:01 Bernardo-MG

@danlangford I've just released v1.4.0 which supports multiplications and integer divisions.

Bernardo-MG avatar Jan 21 '19 20:01 Bernardo-MG

thank you. we will give it a spin.

danlangford avatar Jan 29 '19 17:01 danlangford