Stendarpaval / mob-attack-tool

A module for Foundry VTT that offers a tool for handling mob attacks in the dnd5e system.
GNU General Public License v3.0
7 stars 14 forks source link

Arrow buttons not working in the pop-up #34

Closed caewok closed 3 years ago

caewok commented 3 years ago

Setup Foundry 0.8.8, dnd5e 1.3.6, Mob Attack 0.3.2

Issue In the pop-up dialog to set the Mob Attack, the up/down arrows for the number boxes do not work. (They do nothing.) Entering numbers manually does work.

Console Error The console displays this error:

Uncaught (in promise) TypeError: numAttack.isNaN is not a function activateListeners https://vtt.planeteternium.com/modules/mob-attack-tool/scripts/mobAttackTool.js:398 jQuery 8 activateListeners https://vtt.planeteternium.com/modules/mob-attack-tool/scripts/mobAttackTool.js:376 _render https://vtt.planeteternium.com/scripts/foundry.js:2070 _render https://vtt.planeteternium.com/scripts/foundry.js:2729 render https://vtt.planeteternium.com/scripts/foundry.js:2004 mobAttackTool https://vtt.planeteternium.com/modules/mob-attack-tool/scripts/mobAttackTool.js:42 onClick https://vtt.planeteternium.com/modules/mob-attack-tool/scripts/mobAttackTool.js:16 _onClickTool https://vtt.planeteternium.com/scripts/foundry.js:27425 jQuery 9 activateListeners https://vtt.planeteternium.com/scripts/foundry.js:27381 _render https://vtt.planeteternium.com/scripts/foundry.js:2070 render https://vtt.planeteternium.com/scripts/foundry.js:2004 _onClickTool https://vtt.planeteternium.com/scripts/foundry.js:27435 jQuery 9 activateListeners https://vtt.planeteternium.com/scripts/foundry.js:27381 _render https://vtt.planeteternium.com/scripts/foundry.js:2070 render https://vtt.planeteternium.com/scripts/foundry.js:2004 _onClickTool https://vtt.planeteternium.com/scripts/foundry.js:27435 jQuery 2 mobAttackTool.js:398:18

Possible solution In the file mobAttackTool.js, consider changing numAttack.isNaN() to Number.isNaN(numAttack). (See, e.g., this description as to why.)

This may be more treating the symptom than a solution, however, because I would have thought that numAttack would be a number already. This may be caused by a different bug as well, but switching the Number.isNaN will still probably help in edge cases, and avoids throwing an error.

Thanks!

Stendarpaval commented 3 years ago

Huh, how curious. I forgot to test that function, and can indeed replicate this bug. Very strange, though. Number.isNaN(numAttack) does fix it, but yes numAttack is the result of a parseInt() so it should be a number, or be undefined if something in there went wrong.

I'll look into it, might push a small patch soon too. Thanks for the report!

Edit: Oh wait. I never properly implemented the isNaN() check in the first place. It should have been isNaN(numAttack), whereas I did numAttack.isNaN(). It just never triggered an error before, for some reason.

Stendarpaval commented 3 years ago

This bug should be fixed as of the newly released v0.3.3. Please let me know if the fix works for you, so we can close this issue too :)

caewok commented 3 years ago

Fixed. Thanks!