Changing Critical Rolls to Display Amounts instead of differences

[laclark93]

Show the actual amounts rolled for critical rolls instead of calculating differences as mentioned in #223. This will still need to be refactored for use with forced critical rolls but makes critical rolls a lot more straight forward.
@CarlosFdez I'm not sure if this is what you meant but please give me any feedback or change anything around.

[CarlosFdez]

So rollCrit should probably be isCrit.

So there's actually a tricky thing involving handling roll formulas this way. So in the "dual roll" mode, if EITHER d20 roll procs a crit, it will show crit damage. If you click the apply damage button, it asks if you want to apply crit. If "no" is selected, it will need to apply the original roll rather than a maximized base roll. This is why the modes that maximize base moved the difference to the crit roll, because it needed a way to remember the base roll for non-crits.

It needs a way to display or handle that use case before it can be used, unfortunately, since despite not using it myself, a draw for Better Rolls is that dual roll mode.

One thought is to allow "downgrading" a crit with a - button to force not crit. Another thought is that if the mode is dual, it will show {base} OR {altered-base} + {crit} crit! Not quite sure yet.

[CarlosFdez]

this can be return { baseRoll, critRoll };

[laclark93]

does it make sense to keep the naming of the variables for clarity? Is there anyone you could message me on discord at N0obassassin#2047or email me at [email protected]?

[CarlosFdez]

I am Supe on the foundry discord. And yeah, variables mostly seem clear. Clear is important.

[CarlosFdez]

Thinking a bit more on it and yeah, the tricky thing would be the user interface and handling the apply damage thing (as mentioned), but for the code listed, the only thing that would change is that there would also be an originalRoll property added to the result of constructDamageRoll(). Updates would be needed for chat-message.js (so that apply crit damage set to "no" uses originalRoll instead of baseRoll), and maybe renderer.js to somehow show originalRoll if it differs from baseRoll.

[laclark93]

Added another commit to make the recommended changes