TheLastScrub / delta-green-foundry-vtt-system-unofficial

A Foundry VTT game system for Delta Green: The RPG! This is a fan made work that is unaffiliated with Shane Ivey or Arc Dream Publishing, published under the Delta Green Community license. http://www.delta-green.com https://foundryvtt.com/
MIT License
25 stars 23 forks source link

Fix Inhuman roll check for stat #84

Closed SuperPrower closed 10 months ago

SuperPrower commented 10 months ago

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 the skillName in this function, since the actor-sheet.js:_onRoll() function already converts the stat name to upper case.

With this change, the rolls correctly detect crits for Inhuman rolls.

TheLastScrub commented 10 months 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?

jalensailin commented 10 months ago

yes this does work in my branch jalenml/refactor-rolls

jalensailin commented 10 months ago

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

TheLastScrub commented 10 months ago

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.