Closed SuperPrower closed 1 year ago
Thanks for finding this and putting in the effort to do a PR to fix it. The timing on it is a little unfortunate though, as @jalensailin is right now in the middle of a big rewrite of the roll code (specifically because a lot of it is pretty messy leading to issues like this one). He was asking me about the inhuman stuff the other day, so he might be aware of it already. @jalensailin, can you make sure this works properly in your refactored code?
yes this does work in my branch jalenml/refactor-rolls
thanks for the PR! As explained, I have a more definitive fix for this included in a larger refactor of the roll system in general. this will all be addressed in that rewrite, so we will close this PR
Thanks to you both for digging into my janky code haha. As a heads up, it's possible this may not be fixed in the next build of the system depending on when I cut it. But if not, it should be in the very next version which should follow fairly shortly thereafter.
Hi, just noticed something. When checking whether the stat roll is Inhuman, module checks if the rolled skill is a stat. However, the name passed looks like STATx5 (i.e. STRx5, CONx5), while
skillIsStatTest
function checks just for the stat name (i.e. STR, CON). Because of this, Inhuman rolls aren't properly detected. This PR fixes this.There is no need for
.toUpperCase()
for theskillName
in this function, since theactor-sheet.js:_onRoll()
function already converts the stat name to upper case.With this change, the rolls correctly detect crits for Inhuman rolls.