Allow Damaging Items to Override Graze Damage

[zithith]

Type
What type of pull request is this? (e.g., Bug fix, Feature, Refactor, etc.)

• Bug fix
• Feature
• Refactor
• Other (please describe):

Description
Allow damaging effects to specify a new formula for calculating graze damage value. This is mostly to satisfy the requirement for NPCs/Adversaries who can have values in official statblocks for grazing hits that don't follow the standard rules for attacks.

Related Issue
Closes #102 (WiP)

How Has This Been Tested?
Describe how you tested the changes and how others can replicate the testing steps.
In Progress

Screenshots (if applicable)
Add screenshots or GIFs that help illustrate the changes, especially if they affect the UI or user-facing aspects of the system.

Checklist:

• I have commented on my code, particularly in hard-to-understand areas.
• My changes do not introduce any new warnings or errors.
• My PR does not contain any copyrighted works that I do not have permission to use.
• I have tested my changes on Foundry VTT version: [insert version here].

Additional context
I will presume that this change won't add any sorts of flags or complexity such as a checkbox to add ability mod to the damage or the like. If GMs want an odd action that uses a mod in the grazing formula, then they can just use the @ property shorthand?

[zithith]

• to remove span styling

[stanavdb]

Can you drop a comment in #102 so that I can assign it to you?

[zithith]

update: https://discord.com/channels/1299110557689053264/1300257113766891533/1300257113766891533

[stanavdb]

If we're going to include the mod in the formula, shouldn't we just require that its actually included in the formula, instead of adding it like this? That would give more freedom for homebrewers, but it would make extracting the base damage harder.

[zithith]

I see where you're coming from, but the flipside of such flexibility is that all the standard items then have to actively declare it. Potentially we could achieve the same level of flexibility by providing a checkbox to add the attribute value and default it to on?
Seems like it's a fair assumption for the system to apply this core mechanic to damaging effects (unless this extra modifier only applies to attacks, and other damaging items don't apply this? In which case, default the checkbox to off).
Certainly if I was playing an early release of a system, I'd not be upset that it was making such an assumption even if it meant I couldn't model a particular item I was homebrewing.

[stanavdb]

We should either auto include the mod (as its currently set up to do), or not include it (so it needs to be explicitly added to the formula), anything in between is less clear imo. I think its fine if all standard items have to declare it, I believe that's what 5e does as well. And anyone homebrewing is going to look at the standard items to figure out how to set up their own stuff.
If we auto include the mod there's just certain things that we won't be able to model. I imagine there might be adversaries that can't be easily set up without it.

Also from just a general clarity point; If something is labelled "formula" it should be the formula. The formula shouldn't then be it plus something else. At least not without a hint telling you.

[MangoFVTT]

I agree with requiring the mod to be manually entered. It's how it's set up in other d20 systems as well

[zithith]

Disclaimer: I apologise if my tone in any of this sounds short or harsh (or overly wordy either...), I've just dealt with the mountain/molehill dilemma of getting my nearly 3 year old to bed and I am low on spoons for double checking my sentences. But I do want to find some time to update this with whichever approach we favour tomorrow, so want the discussion to continue. I mean anything said here to be constructive and honestly just see if I have missed something in my understanding.

If you guys prefer to set it to manual, that's fine. Thinking about it, whichever way we implement it, there will be a set of users who assume it's the other way or don't know and just roll something and see how it works - or look at pre-existing examples as you say - in which case it's an easy fix, especially if they have access to a compendium of such examples. I guess the expectation for damage values tend to be less for the base roll automation as it is for the basic skill roll; though it's nice if there's an easier way to have it magically do it with few clicks in my experience. And I still am unsure how well-versed the average VTT user is with roll formulas, especially where there's anything platform specific, without a helper/guide. It was doing macros and modding MapTool over 15 years ago that really made me consider programming as a career, and for about half the time since I've been involved with VTTs but I still find my players (who also GM other groups on VTTs) struggle to write out a fairly basic dice pool 🤷🏻‍♂️
If we do go this route however I question whether we need to bother setting a damage stat at all then? might as well just add all the actor attributes to the context the formula needs and then the user can put @spe or @str or whatever. As I don't know how else the attribute feeds in, and we're looking at reducing clicks.
Alternatively we keep automatic mod addition, but as I suggested above, make it toggleable from the item details window (i.e. a checkbox for "Ignore Modifier?" and have it default to on unless you're creating a weapon?).

either auto include the mod (as its currently set up to do), or not include it (so it needs to be explicitly added to the formula), anything in between is less clear imo.

Just for my clarity, I believe that the approach I have here is an implementation of auto-including the mod. It's not in the middle, I just chose to apply the mod on the roll here rather than in the apply damage step. I feel that made more logical sense as to how I think of the math when doing the rolls manually at an in-person table; it matches closer to how the skill/attack roll is calculated and also pulls similar logic into a closer location in the code, you don't have to go dig to find out where the mod is being added and we can just pass the value to apply to the damage application function from the chat cards rather than passing a flag, making the cards carry less game logic and can be more display-oriented.
If I've missed some implication that makes this different, or even wrong by the rules, I'm sorry I missed it, please let me know 👍🏻

[MangoFVTT]

The point IS to allow users who want to to add @spe or @str or whatever they want to the damage formula. By default, we should support @mod to add whatever attribute is used for the skill test, but we shouldn't be hardcoding it to ONLY allow that. A checkbox is fine, it just means if someone wants to skill test with STR then add INT to the damage (for example), they have to first remove the checkbox then add @int manually to the formula. Essentially, like @stanavdb said, the "formula" field becomes not actually the formula, but part of the formula. To me that's even more confusing to setup than just writing "1d6 + @mod" in the damage.

I do agree that the mod should be included in the actual damage roll object, and not just added later. As I've mentioned elsewhere, my ideal solution for damage rolls would be to have a nested graze damageroll object that can be called directly, so that when a user pressed apply damage we just go "ok full damage is selected, use the base damage roll" or "ok graze damage is selected, use the graze damage roll" and not have to be doing a bunch of logic checks in apply damage functions.

[zithith]

unless I'm mistaken by the item setup, we already do separate the damage attribute (red) from the attack one (yellow), and just set the default for the damage to use what is selected for the activation?

I don't know why we have a skill for the damage entry, but I suppose someone might need to use it for something later on.
My comment earlier was that if we don't want to add it automatically we should drop the select boxes in the damage area and just add instructions that explains how to access the attributes/skill ranks through @-notation in the formula; or we keep as is and just add a button to turn off the automatic addition (with @mod also being available as a shorthand for the attack modifier so that if you wanted to change both together it would be one input change later).

I was trying to get this changeset to end up with the nested roll as you suggested, how far off from what you are after am I? (assuming that the other discussion threads about how we generate the roll values doesn't change the structure of what's done so far, just the values)

[MangoFVTT]

The object structure atm seems fine to me and in line with how I expected it to be set up. I was just re-iterating that that should stay the case.

TBH i forgot that we have attribute drop downs in the damage details. I think the sensible thing would be to remove that and have @mod specifically refer to the attribute from the skill test. Then if someone wants to add custom attributes, they can just @ that specific attribute as normal.

[stanavdb]

The attribute dropdown is relevant with regards to graze damage. Surely we only want the mod to drop out for graze damage and not any other flat modifiers? This way the user can target what they want the @mod value to be, which is what gets dropped for graze, but still apply any other flat damage based on the roll data.

[zithith]

noises of consternation
you're right. Assuming we're decided on implementing graze as just "hit damage total - main attribute value". But if we're also deciding that the user has to manually call out adding the attribute, do we then insist they call out a graze override when they use an attrib other than that defined for the attack roll?
Or going even further, just ask them to define the graze formula all the time and not have a default for them to override?

N.B. this would need the discussion on finding a neat way to replicate the main damage rolls and expose that to the formula to have a solution

[stanavdb]

Maybe we should pass through a couple of different options, like:

• @damage.value - The total damage that was rolled, including mod
• @damage.unmodded - The total damage without the modifier
• @damage.dice - The total of all the dice (excludes any additions)

I'm not attached to the names, though I do think baseRoll isn't super clear.

[zithith]

I thought the baseRoll term might be a bit confusing, though I think whatever terminology we use it will require some level of explanation (hence I included it in the hint text). I do quite like the idea of storing it in an object with multiple options though...

[stanavdb]

Since we're passing the base roll results as the formula for the graze, will that end up showing up in the chat message? I wonder if it's perhaps better to pass the actual evaluated terms? Or the damage formula (without mod) but then apply the dice results to the dice terms?

[zithith]

Good thinking.
I guess the simple way would be to set the graze roll's dice property to that of the standard damage roll? That seems like it would be a solution about as simple as current? But I don't know if that actually works?
However I'm wondering if this is potentially just going to introduce unnecessary levels of branching, i.e. we'd need to first check if there isn't an override formula or the formula includes "the base roll string" then the formula used would have to get set based on which of those it was and the previous roll's dice inserted. Otherwise we just use the override formula as is?
I think the current approach covers the existing requirements and in conjunction with @MangoFVTT 's design discussions on discord, regardless of which damage pot you have selected for applying, when you expand the accordion to see the dice details it'll be showing both the default roll and the graze formula anyway.

[MangoFVTT]

Why not pass the full roll object as baseRoll, then take the terms of baseRoll, remove the mods, and do Roll.fromTerms() on the remainder to get another roll object that we can pass as grazeRoll? For non-override I mean.

[zithith]

Yeah, that sounds positive, I don't really know the Roll API well enough to remember what's possible.
So if we have someone who wants to write an override formula for like the cremlings, say, which is floor(@baseRoll/2) will the foundry engine pick up the total from the roll object correctly, or will it need to evaluate to a numeric term?
This could be alleviated slightly by the suggestion of providing a damage object with various options. One could be damage.roll (which is as you say) and the others are like damage.total or damage.dice as stan suggested, which are just number values that the users can refer to. Though this does then mean in those cases you couldn't give a dice breakdown. But is that the end of the world, per my previous comment?

[MangoFVTT]

If we don't have a dice breakdown we can just not show the tooltip, which is fine. Obviously most people would prefer to HAVE the tooltip always, but if you use the Roll API that shouldn't be an issue. It does all the logic of evaluating terms internally afaik, including Function terms like the one you gave