Skip to content
This repository was archived by the owner on Sep 17, 2025. It is now read-only.

Commit dce5f9b

Browse files
adrianocolatom-wolfe
adrianocola
authored and
tom-wolfe
committed
Exploding/rerolls don't automatically execute for highest/lowest value if a condition is specified. Fixed infinite loops that could happen if exploding/reroll conditions included all dice faces. (#22)
1 parent 440b7ed commit dce5f9b

File tree

3 files changed

+286
-8
lines changed

3 files changed

+286
-8
lines changed

‎spec/interpreter/dice-interpreter.evaluate.explode.spec.ts‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,5 +58,134 @@ describe('DiceInterpreter', () => {
5858

5959
expect(dice.getChildCount()).toBe(5);
6060
});
61+
it('evaluates an exploding dice with equal condition (4d6!=3).', () => {
62+
const exp = Ast.Factory.create(Ast.NodeType.Explode)
63+
.setAttribute('compound', false)
64+
.setAttribute('penetrate', false);
65+
66+
const dice = Ast.Factory.create(Ast.NodeType.Dice);
67+
dice.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', 4));
68+
dice.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', 6));
69+
70+
const equal = Ast.Factory.create(Ast.NodeType.Equal);
71+
equal.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', 3));
72+
73+
exp.addChild(dice);
74+
exp.addChild(equal);
75+
76+
const mockList = new MockListRandomProvider();
77+
mockList.numbers.push(
78+
1, 2, 4, 3, // This 3 should get exploded into the next.
79+
6 // Highest dice face should not automatically explode
80+
);
81+
82+
const interpreter = new Interpreter.DiceInterpreter(null, mockList);
83+
const errors: Interpreter.InterpreterError[] = [];
84+
expect(interpreter.evaluate(exp, errors)).toBe(16);
85+
86+
expect(dice.getChildCount()).toBe(5);
87+
});
88+
it('evaluates not exploding highest dice value if specified a condition (4d6!<=2).', () => {
89+
const exp = Ast.Factory.create(Ast.NodeType.Explode)
90+
.setAttribute('compound', false)
91+
.setAttribute('penetrate', false);
92+
93+
const dice = Ast.Factory.create(Ast.NodeType.Dice);
94+
dice.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', 4));
95+
dice.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', 6));
96+
97+
const less = Ast.Factory.create(Ast.NodeType.LessOrEqual);
98+
less.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', 2));
99+
100+
exp.addChild(dice);
101+
exp.addChild(less);
102+
103+
const mockList = new MockListRandomProvider();
104+
mockList.numbers.push(
105+
4, 3, 5, 2, // This 2 should get exploded into the next.
106+
6 // Highest dice face should not automatically explode
107+
);
108+
109+
const interpreter = new Interpreter.DiceInterpreter(null, mockList);
110+
const errors: Interpreter.InterpreterError[] = [];
111+
expect(interpreter.evaluate(exp, errors)).toBe(20);
112+
113+
expect(dice.getChildCount()).toBe(5);
114+
});
115+
it('errors if condition includes all dice faces (4d6!>=1).', () => {
116+
const exp = Ast.Factory.create(Ast.NodeType.Explode)
117+
.setAttribute('compound', false)
118+
.setAttribute('penetrate', false);
119+
120+
const dice = Ast.Factory.create(Ast.NodeType.Dice);
121+
dice.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', 4));
122+
dice.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', 6));
123+
124+
const greater = Ast.Factory.create(Ast.NodeType.GreaterOrEqual);
125+
greater.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', 1));
126+
127+
exp.addChild(dice);
128+
exp.addChild(greater);
129+
130+
const mockList = new MockListRandomProvider();
131+
mockList.numbers.push(
132+
1, 2, 3, 4,
133+
);
134+
135+
const interpreter = new Interpreter.DiceInterpreter(null, mockList);
136+
const errors: Interpreter.InterpreterError[] = [];
137+
interpreter.evaluate(exp, errors);
138+
expect(errors.length).toBeGreaterThanOrEqual(1);
139+
});
140+
it('errors if condition includes all dice faces (4d6!<7).', () => {
141+
const exp = Ast.Factory.create(Ast.NodeType.Explode)
142+
.setAttribute('compound', false)
143+
.setAttribute('penetrate', false);
144+
145+
const dice = Ast.Factory.create(Ast.NodeType.Dice);
146+
dice.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', 4));
147+
dice.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', 6));
148+
149+
const less = Ast.Factory.create(Ast.NodeType.Less);
150+
less.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', 7));
151+
152+
exp.addChild(dice);
153+
exp.addChild(less);
154+
155+
const mockList = new MockListRandomProvider();
156+
mockList.numbers.push(
157+
1, 2, 3, 4,
158+
);
159+
160+
const interpreter = new Interpreter.DiceInterpreter(null, mockList);
161+
const errors: Interpreter.InterpreterError[] = [];
162+
interpreter.evaluate(exp, errors);
163+
expect(errors.length).toBeGreaterThanOrEqual(1);
164+
});
165+
it('errors if condition includes all dice faces (4d1!=1).', () => {
166+
const exp = Ast.Factory.create(Ast.NodeType.Explode)
167+
.setAttribute('compound', false)
168+
.setAttribute('penetrate', false);
169+
170+
const dice = Ast.Factory.create(Ast.NodeType.Dice);
171+
dice.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', 4));
172+
dice.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', 1));
173+
174+
const Equal = Ast.Factory.create(Ast.NodeType.Equal);
175+
Equal.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', 1));
176+
177+
exp.addChild(dice);
178+
exp.addChild(Equal);
179+
180+
const mockList = new MockListRandomProvider();
181+
mockList.numbers.push(
182+
1, 1, 1, 1
183+
);
184+
185+
const interpreter = new Interpreter.DiceInterpreter(null, mockList);
186+
const errors: Interpreter.InterpreterError[] = [];
187+
interpreter.evaluate(exp, errors);
188+
expect(errors.length).toBeGreaterThanOrEqual(1);
189+
});
61190
});
62191
});

‎spec/interpreter/dice-interpreter.evaluate.reroll.spec.ts‎

Lines changed: 119 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ describe('DiceInterpreter', () => {
2929
expect(interpreter.evaluate(exp, errors)).toBe(21);
3030
expect(dice.getChildCount()).toBe(4);
3131
});
32-
it('evaluates a rerolling dice (4d6ro<3).', () => {
32+
it('evaluates rerolling once dice (4d6ro<3).', () => {
3333
const exp = Ast.Factory.create(Ast.NodeType.Reroll)
3434
.setAttribute('once', true);
3535

@@ -54,7 +54,7 @@ describe('DiceInterpreter', () => {
5454
expect(interpreter.evaluate(exp, errors)).toBe(18);
5555
expect(dice.getChildCount()).toBe(4);
5656
});
57-
it('evaluates rerolling dice (4d6r).', () => {
57+
it('evaluates rerolling dice no condition (4d6r).', () => {
5858
const exp = Ast.Factory.create(Ast.NodeType.Reroll)
5959
.setAttribute('once', false);
6060

@@ -74,7 +74,7 @@ describe('DiceInterpreter', () => {
7474
expect(interpreter.evaluate(exp, errors)).toBe(21);
7575
expect(dice.getChildCount()).toBe(4);
7676
});
77-
it('evaluates a rerolling dice no condition (4d6ro).', () => {
77+
it('evaluates a rerolling once dice no condition (4d6ro).', () => {
7878
const exp = Ast.Factory.create(Ast.NodeType.Reroll)
7979
.setAttribute('once', true);
8080

@@ -94,6 +94,56 @@ describe('DiceInterpreter', () => {
9494
expect(interpreter.evaluate(exp, errors)).toBe(15);
9595
expect(dice.getChildCount()).toBe(4);
9696
});
97+
it('evaluates rerolling dice equal condition (4d6r=3).', () => {
98+
const exp = Ast.Factory.create(Ast.NodeType.Reroll)
99+
.setAttribute('once', false);
100+
101+
const dice = Ast.Factory.create(Ast.NodeType.Dice);
102+
dice.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', 4));
103+
dice.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', 6));
104+
105+
const equal = Ast.Factory.create(Ast.NodeType.Equal);
106+
equal.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', 3));
107+
108+
exp.addChild(dice);
109+
exp.addChild(equal);
110+
111+
const mockList = new MockListRandomProvider();
112+
mockList.numbers.push(
113+
6, 5, 6, 3, // This 3 should get re-rolled into a 1
114+
1 // Lowest dice faces (1) should not be automatically rerolled
115+
);
116+
117+
const interpreter = new Interpreter.DiceInterpreter(null, mockList);
118+
const errors: Interpreter.InterpreterError[] = [];
119+
expect(interpreter.evaluate(exp, errors)).toBe(18);
120+
expect(dice.getChildCount()).toBe(4);
121+
});
122+
it('evaluates not rerolling smallest dice value if specified a condition (4d6r>=5).', () => {
123+
const exp = Ast.Factory.create(Ast.NodeType.Reroll)
124+
.setAttribute('once', false);
125+
126+
const dice = Ast.Factory.create(Ast.NodeType.Dice);
127+
dice.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', 4));
128+
dice.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', 6));
129+
130+
const greater = Ast.Factory.create(Ast.NodeType.GreaterOrEqual);
131+
greater.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', 6));
132+
133+
exp.addChild(dice);
134+
exp.addChild(greater);
135+
136+
const mockList = new MockListRandomProvider();
137+
mockList.numbers.push(
138+
2, 4, 3, 6, // This 6 should get re-rolled into a 1
139+
1 // Lowest dice faces (1) should not be automatically rerolled
140+
);
141+
142+
const interpreter = new Interpreter.DiceInterpreter(null, mockList);
143+
const errors: Interpreter.InterpreterError[] = [];
144+
expect(interpreter.evaluate(exp, errors)).toBe(10);
145+
expect(dice.getChildCount()).toBe(4);
146+
});
97147
it('errors on an invalid condition (4d6ro[dice]).', () => {
98148
const exp = Ast.Factory.create(Ast.NodeType.Reroll)
99149
.setAttribute('once', true);
@@ -112,6 +162,72 @@ describe('DiceInterpreter', () => {
112162
const mockList = new MockListRandomProvider();
113163
mockList.numbers.push(6, 5, 6, 2, 1);
114164

165+
const interpreter = new Interpreter.DiceInterpreter(null, mockList);
166+
const errors: Interpreter.InterpreterError[] = [];
167+
interpreter.evaluate(exp, errors);
168+
expect(errors.length).toBeGreaterThanOrEqual(1);
169+
});
170+
it('errors if condition includes all dice faces (4d6r<7).', () => {
171+
const exp = Ast.Factory.create(Ast.NodeType.Reroll)
172+
.setAttribute('once', false);
173+
174+
const dice = Ast.Factory.create(Ast.NodeType.Dice);
175+
dice.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', 4));
176+
dice.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', 6));
177+
178+
const less = Ast.Factory.create(Ast.NodeType.Less);
179+
less.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', 7));
180+
181+
exp.addChild(dice);
182+
exp.addChild(less);
183+
184+
const mockList = new MockListRandomProvider();
185+
mockList.numbers.push(3, 4, 2, 1);
186+
187+
const interpreter = new Interpreter.DiceInterpreter(null, mockList);
188+
const errors: Interpreter.InterpreterError[] = [];
189+
interpreter.evaluate(exp, errors);
190+
expect(errors.length).toBeGreaterThanOrEqual(1);
191+
});
192+
it('errors if condition includes all dice faces (4d6r>=1).', () => {
193+
const exp = Ast.Factory.create(Ast.NodeType.Reroll)
194+
.setAttribute('once', false);
195+
196+
const dice = Ast.Factory.create(Ast.NodeType.Dice);
197+
dice.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', 4));
198+
dice.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', 6));
199+
200+
const moreOrEqual = Ast.Factory.create(Ast.NodeType.GreaterOrEqual);
201+
moreOrEqual.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', 1));
202+
203+
exp.addChild(dice);
204+
exp.addChild(moreOrEqual);
205+
206+
const mockList = new MockListRandomProvider();
207+
mockList.numbers.push(3, 4, 2, 1);
208+
209+
const interpreter = new Interpreter.DiceInterpreter(null, mockList);
210+
const errors: Interpreter.InterpreterError[] = [];
211+
interpreter.evaluate(exp, errors);
212+
expect(errors.length).toBeGreaterThanOrEqual(1);
213+
});
214+
it('errors if condition includes all dice faces (4d1r=1).', () => {
215+
const exp = Ast.Factory.create(Ast.NodeType.Reroll)
216+
.setAttribute('once', false);
217+
218+
const dice = Ast.Factory.create(Ast.NodeType.Dice);
219+
dice.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', 4));
220+
dice.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', 1));
221+
222+
const equal = Ast.Factory.create(Ast.NodeType.Equal);
223+
equal.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', 1));
224+
225+
exp.addChild(dice);
226+
exp.addChild(equal);
227+
228+
const mockList = new MockListRandomProvider();
229+
mockList.numbers.push(3, 4, 2, 1);
230+
115231
const interpreter = new Interpreter.DiceInterpreter(null, mockList);
116232
const errors: Interpreter.InterpreterError[] = [];
117233
interpreter.evaluate(exp, errors);

‎src/interpreter/dice-interpreter.class.ts‎

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -196,23 +196,31 @@ export class DiceInterpreter implements Interpreter<DiceResult> {
196196
if (!this.expectChildCount(expression, 1, errors)) { return 0; }
197197
const dice = this.findDiceOrGroupNode(expression, errors);
198198
if (!dice) { return 0; }
199-
let condition: Ast.ExpressionNode;
200199
const penetrate = expression.getAttribute('penetrate');
200+
201+
const sides = dice.getAttribute('sides');
202+
203+
let condition: Ast.ExpressionNode;
201204
if (expression.getChildCount() > 1) {
202205
condition = expression.getChild(1);
206+
if (this.wouldRollAgainForever(dice, condition, errors)) {
207+
return 0;
208+
}
209+
} else {
210+
condition = Ast.Factory.create(Ast.NodeType.Equal);
211+
condition.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', sides));
203212
}
204213

205214
this.evaluate(dice, errors);
206215

207216
const newRolls: Ast.ExpressionNode[] = [];
208-
209217
let total = 0;
210-
const sides = dice.getAttribute('sides');
218+
211219
dice.forEachChild(die => {
212220
if (!die.getAttribute('drop')) {
213221
let dieValue = this.evaluate(die, errors);
214222
total += dieValue;
215-
while (condition && this.evaluateComparison(dieValue, condition, errors) || dieValue === sides) {
223+
while (condition && this.evaluateComparison(dieValue, condition, errors)) {
216224
die = this.createDiceRoll(sides, errors);
217225
dieValue = this.evaluate(die, errors);
218226
if (penetrate) { dieValue -= 1; }
@@ -315,6 +323,12 @@ export class DiceInterpreter implements Interpreter<DiceResult> {
315323

316324
if (expression.getChildCount() > 1) {
317325
condition = expression.getChild(1);
326+
if (this.wouldRollAgainForever(dice, condition, errors)) {
327+
return 0;
328+
}
329+
} else {
330+
condition = Ast.Factory.create(Ast.NodeType.Equal);
331+
condition.addChild(Ast.Factory.create(Ast.NodeType.Number).setAttribute('value', 1));
318332
}
319333

320334
this.evaluate(dice, errors);
@@ -324,7 +338,7 @@ export class DiceInterpreter implements Interpreter<DiceResult> {
324338
dice.forEachChild(die => {
325339
if (!die.getAttribute('drop')) {
326340
let dieValue = this.evaluate(die, errors);
327-
while (condition && this.evaluateComparison(dieValue, condition, errors) || dieValue === 1) {
341+
while (condition && this.evaluateComparison(dieValue, condition, errors)) {
328342
dieValue = this.createDiceRollValue(sides, errors);
329343
if (once) { break; }
330344
}
@@ -490,4 +504,23 @@ export class DiceInterpreter implements Interpreter<DiceResult> {
490504
}
491505
return this.random.numberBetween(minValue, maxValue);
492506
}
507+
508+
private wouldRollAgainForever(dice: Ast.ExpressionNode, expression: Ast.ExpressionNode, errors: InterpreterError[]): boolean {
509+
const sides = dice.getAttribute('sides');
510+
const value = expression.getChild(0).getAttribute('value');
511+
let wouldRunForever = false;
512+
switch (expression.type) {
513+
case Ast.NodeType.Equal: wouldRunForever = sides === 1 && value === 1; break;
514+
case Ast.NodeType.Greater: wouldRunForever = value < 1; break;
515+
case Ast.NodeType.GreaterOrEqual: wouldRunForever = value <= 1; break;
516+
case Ast.NodeType.Less: wouldRunForever = value > sides; break;
517+
case Ast.NodeType.LessOrEqual: wouldRunForever = value >= sides;
518+
}
519+
520+
if (wouldRunForever) {
521+
errors.push(new InterpreterError('Condition to roll again includes all dice faces and would run forever.', expression));
522+
}
523+
524+
return wouldRunForever;
525+
}
493526
}

0 commit comments

Comments
 (0)