schlosrat / token-health

FoundryVTT module
5 stars 10 forks source link

Feature Request: Option to apply damage to targeted enemies instead of selected #10

Closed Norc closed 3 years ago

Norc commented 4 years ago

I saw a suggestion by cyanomys on the Foundry Discord and thought it was a great idea. He/she would like to be able to have Token Health work with targeted tokens.

I wouldn't want to lose the option to target selected tokens, though. Maybe add two new keybinds: damage targeted and Damage Selected? (one would be the normal keys, the others would be a Modifier plus that key, like enter vs. Shift-enter)? And a checkbox to enable/disable each. I'm thinking something like;

image

Please excuse l33t MS Paint skills, just trying to get my idea across. Thanks!!

tonifisler commented 4 years ago

This is something I can implement, yeah!

I'm not using the targeting system though, so I want to really understand how those who do would use that feature.

I was thinking I could add a checkbox inside the dialog itself to choose to apply damage/healing to the targeted tokens and/or selected tokens. I think it's better if we avoid adding any more keyboard shortcuts that the user has to learn to first summon the dialog. Maybe we can add Shift + Enter or something when validating the entered value. That way a little help text can be displayed and it's easier to grasp for new users.

Dunno if my idea and explaining are making more sense than your MS Paint! 😆

tonifisler commented 4 years ago

To quote cyanomys:

Thanks for the response! I like the idea of allowing targeted tokens to be updated because then players can apply the damage while I move on to the next thing. We wouldn't want selected tokens to also be updated when targeted tokens are targeted, because then the players might damage themselves at the same time. But, when they just have their own token selected, the damage should apply to them. If that makes sense.

Norc commented 4 years ago

Yeah, whatever you think makes the most sense! and the keyboard shortcut real-estate is indeed limited, so whatever makes sense. I'd be happy to mess with a beta version and give feedback on your rough-sketch.

How I use targeting is as follows: I have a selected token who is attacking a target. I use a keyboard macro for their attack. The Minor Quality of Life module then automatically checks to see if the attack hits or misses, but often times the automatic calculation is wrong, and I need to "fix" the damage done by healing or doing extra damage, and I use Token Health to do that. however, that means I need to select off the attacker to the target before I apply Token Health... and I often forget., applying the effect to the wrong token.

In other cases, I'll use Token health to damage/heal outside of a direct attack (particularly heal), and I'll do that using Selected target usually, or I might just select a target quick to apply a health adjustment to them.

So, long story short, I use selection to apply damage/healing in some situations, and I would also be able to use Targeting INSTEAD of selected in other situations.

Thanks again for being awesome. and for your kick-butt module...

AJAnderson commented 4 years ago

Love this module, my first thought when I tried to use it was why doesn't this work on targetted enemies? I implemented a version that works on targetted enemies rather than selected, but I didn't make it an optional thing (that's a little beyond me).

Happy to supply the (very minor 3-line) patch if needed.

tonifisler commented 4 years ago

Hey thanks @AJAnderson ! You're very welcome to open a Pull Request (or just paste the lines here), that would be a great help. Sorry I haven't got a lot of time lately to implement or fix issues reported here (apart from breaking stuff due to other modules updates).

AJAnderson commented 4 years ago

I went ahead and did it, hope it works for you! 😊

On Sat, Jun 6, 2020, 00:18 Toni Fisler notifications@github.com wrote:

Hey thanks @AJAnderson https://github.com/AJAnderson ! You're very welcome to open a Pull Request (or just paste the lines here), that would be a great help. Sorry I haven't got a lot of time lately to implement or fix issues reported here (apart from breaking stuff due to other modules updates).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tonifisler/foundry-token-health/issues/10#issuecomment-639446607, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABF3UENQT6DF4PP5DLYN5ALRVDPCDANCNFSM4M5HE27A .

AJAnderson commented 4 years ago

I logged a pull request for this feature. Let me know if you'd like me to tidy it up or fix anything.