League-of-Foundry-Developers / torch

Simple torch module for Foundry VTT
10 stars 20 forks source link

Do not delete torches if we are using Light or Dancing Lights #9

Closed C-S-McFarland closed 2 years ago

C-S-McFarland commented 2 years ago

I noticed that one of my Cleric's in a DnD5e game was having Torches subtracted even though they have the Light spell. And it appears that while the returns in the useTorch() function break the loop, they do not skip the actual code that removes torches.

I think this should help repair that problem. At least, it seemed to fix it in my local game.

lupestro commented 2 years ago

I see where the original coding error came in. The actor.data.items.forEach((item) => { ... } had returns inside, but that only returns from the inner function, not returning from the entire useTorch() method. The intent is ambiguous, and the whole approach is a potential source of subtle bugs. I will initially apply your two-line fix, but there are other cases it doesn't cover, so I will earmark this method for more extensive rework that makes the intended behavior clearer. Thanks for drawing this to my attention.

lupestro commented 2 years ago

Tested - this solves this problem, but testing for the eight combinations of items with the five sets of user permissions revealed a couple other unrelated features. I'll merge this PR but will address at least one of the other issues before issuing a new release. Many thanks.