Bonus keys for accessory and range type
Closed this issue · 6 comments
I expect @draconas1 will have the answer to this, but I wanted to file it here to keep it from getting lost in Discord convos: how hard would it be to expand the power.damage.* and power.attack.* keys to include some more common bonus clauses like the accessory used (weapon, implement, melee weapon, ranged weapon) and power range (melee, close, ranged, area)? It's come up several times on Discord that somebody is looking to implement a bonus to "all weapon attacks" or "all implement attacks" or the like, so I think it's worth the effort unless there's some big roadblock.
I get that selecting for more than one condition (e.g. "arcane implement attacks") is too big a task for now, but the info for these specific subsets seems to be more or less readily available. The hardest bit I can see is that for powers with "melee or ranged" we would have to check against the weapon actually being used as well as the power spec, but it looks like that data is present (for the sake of weapon groups and stuff) so maybe not too hard?
I've looked into it a bit myself, trying to work out where I need to look by checking the PRs where it was implemented. I wound up mucking about with _addKeywords in helper.js, but I've gotten no results (including not breaking anything) so there's clearly something I'm missing. I'd appreciate any insight you can give into where I should look <:)
It would be totally possible, you would just need to disambiguate somehow between "implement weapon type required" and "is using a weapon with the implement keyword" (the current behaviour). And then to code around translating between that bonus keyword and the value (this is why I force people to use the inner keys for all the damage, it makes my mapping work very simple)
At the point that we do the calculation, we have both the power and the selected weapon objects to hand.
Let me find where it is before I go to bed, there is a single helper method that sorts all of them. There is an option under the system options to make it log extensively what it is doing which you will probably find helpful.
You want : applyEffects() in Helper.js You were close, since its calling addKeyworks, that's just a helper to grab keys out of one of the maps.
I don't think there will be a conflict by adding range type keys, as said, you are going to have to figure out a new keyword to disambiguate required weapon type from the actual weapon keyword, then add that into suitableKeywords around about line 187
Thank you! I'll see what I can do :)
Okay, looks like I misspoke <XD It was in fact applyEffects() that I was messing with. Good news though, that means I actually had the code right :) I thought it was failing because the keys weren't showing up in autocomplete (Autocomplete Inline Properties) but with the log info on I can see that it's all working when I enter the keys manually. Thanks for pointing me at that setting!
So apparently what I really should have asked is what makes the keys show up in Autocomplete Inline Properties. I was assuming it queries them from the system somehow, but is it actually just defined per system in that module, or something?
So I've been thinking about it, and is there a reason to not use the power's tool requirement value for something like the implement keyword? It seems to me that, since rolling attack/damage will fail if you have no suitable tool equipped, it should be fine to let the power determine that keyword. Not sure if I'm missing something, but I haven't been able to think of any corner cases.
So I've been thinking about it, and is there a reason to not use the power's tool requirement value for something like the implement keyword?
No reason that I can think of, provided we make it clear the difference between .imp (the keyword on the weapon) and attack power where you have specified implement is required.
Though specific case note, implement attacks do not actually require an implement equipped, because 4E is a silly system and the Foundry implementation respects its choices to be silly. (I suppose technically it's the reverse, it should respect my fighters choice to forgo weapons and failing to find one simply use the stats for unarmed strike as the W instead of forcing me to equip something)
AutoComplete is done in my tools module: https://github.com/draconas1/foundry-4e-tools/blob/main/module/integrations/autocomplete-inline-properties.js.
It's a pretty simple integration that just iterates the config maps, we would just add some extra keywords to the keywords collection and will just pick them up.
So I've been thinking about it, and is there a reason to not use the power's tool requirement value for something like the implement keyword?
No reason that I can think of, provided we make it clear the difference between
.imp(the keyword on the weapon) andattack power where you have specified implement is required.
Gotcha. I did notice also that if you enter ".implement" it corresponds with ".imp" too, so I distanced it by using ".usesImplement" and all seems safe.
Though specific case note, implement attacks do not actually require an implement equipped, because 4E is a silly system and the Foundry implementation respects its choices to be silly. (I suppose technically it's the reverse, it should respect my fighters choice to forgo weapons and failing to find one simply use the stats for unarmed strike as the W instead of forcing me to equip something)
Oh yeah! I had totally forgotten about "naked" implement attacks. Though I could have sworn in testing I couldn't get an implement power to fire off without having one equipped. Either way though I think it's fine, since the power still has the keyword regardless of what's equipped, and that's what bonuses are normally looking for.
AutoComplete is done in my tools module: https://github.com/draconas1/foundry-4e-tools/blob/main/module/integrations/autocomplete-inline-properties.js.
It's a pretty simple integration that just iterates the config maps, we would just add some extra keywords to the keywords collection and will just pick them up.
Ah, of course! I knew I remembered it being your implementation somehow. Okay then, once I get everything set up I'll offer an updated list for your module too :)