schlosrat / token-health

FoundryVTT module
5 stars 10 forks source link

Additional max hp pool & numpad enter key #49

Closed mech-tools closed 2 years ago

mech-tools commented 3 years ago

Fix for https://github.com/schlosrat/token-health/issues/48 + numpad enter key

mech-tools commented 3 years ago

@schlosrat Hey, hope you are well. Is this OK with you? Can you review and hopefully merge this ?

schlosrat commented 3 years ago

@schlosrat Hey, hope you are well. Is this OK with you? Can you review and hopefully merge this ?

Hey DarKDinDoN, I see you've been busy solving this. I've been a bit swamped lately but will take a look soon. I want to make sure I understand the fix and that it doesn't collide with some other stuff I've been doing. Thanks for your effort. I'll get this in soon.

schlosrat commented 3 years ago

@DarKDinDoN, I've folded in your changes dealing with ALT_MAX_HITPOINTS_ATTRIBUTE_1 and ALT_MAX_HITPOINTS_ATTRIBUTE_2. Sorry for the delay, this was complicated by collisions with some other work I was doing to fix how settings get done.

Please try out 0.4.4 to make sure the temporary hp part is working correctly and to get your branch alined with how settings are done now (safe to ignore any code in the "default" function of settings - that's on the way out).

I've just started looking at the numpad functionality. My initial concern there is that the existing hotkey capability allows customization at the user level and so it's quite possible that some other key besides Enter could be set by a user. This could lead to confusion since the numpad settings are not configurable.

Perhaps a better approach is to optionally allow configuration of a second set of keys? That way users that want it can have the four basic functions triggered with two sets of options (which would work I think for both Enter keys), and those that are fine with just one option can pick what they like and leave it at that.

Your thoughts?

mech-tools commented 3 years ago

@schlosrat Hey, Thx for the review. Alt hitpoints are working great!

As for the numpad Enter key, you are right. Better not making things confusing for users. It was more about qol than a real feature.