Larkinabout / fvtt-custom-dnd5e

11 stars 7 forks source link

[BUG] Problems in recalculate damage in dnd5e 3.3 (and possibly earlier) when tempHP are present. #83

Closed tposney closed 4 days ago

tposney commented 1 month ago
function recalculateDamage (actor, amount, updates, options) {
    const hpMax = actor?.system?.attributes?.hp?.max ?? 0
    const hpTemp = actor?.system?.attributes?.hp?.temp ?? 0
    const hpValue = actor?.system?.attributes?.hp?.value ?? 0
    const newHpTemp = amount > 0 ? Math.max(hpTemp - amount, 0) : 0
    const startHp = (getSetting(CONSTANTS.HIT_POINTS.SETTING.NEGATIVE_HP_HEAL_FROM_ZERO)) ? 0 : hpValue
    const newHpValue = amount > 0
        ? hpValue - (amount - hpTemp)
        : Math.min(startHp - amount, hpMax)

First problem seems to be that if amount is 0 or negative, newTempHP is always 0, which ignores application of tempHP healing and overwrites any existing tempHP to 0.

Second problem: If actor has 50 tempHP and 250/500 hp, amount = 20 Then newHPTemp = 50 - 20 = 30 (correct) newHPValue = 250 - (20 - 50) = 280 (incorrect)

I think the following code resolves the issues.

    const hpMax = actor?.system?.attributes?.hp?.max ?? 0
    const hpTemp = actor?.system?.attributes?.hp?.temp ?? 0
    const hpValue = actor?.system?.attributes?.hp?.value ?? 0
    const newHpTemp = amount > 0 ? Math.max(hpTemp - amount, 0) : (updates["system.attribute.hp.temp"] ?? hpTemp);
    const startHp = (getSetting(CONSTANTS.HIT_POINTS.SETTING.NEGATIVE_HP_HEAL_FROM_ZERO)) ? 0 : hpValue
    const newHpValue = amount > 0
        ? hpValue - (amount - Math.mind(amount,  hpTemp))
        : Math.min(startHp - amount, hpMax)

Versions:

Additional context I came across this when looking at midi-qol damage application problems, but have done these tests without midi/dae active.

Larkinabout commented 1 month ago

Thanks for looking into this and figuring out how to solve it.

tposney commented 3 weeks ago

I'm afraid I screwed this up which became apparent when I switched midi to strictly follow the dnd5e way of doing damage updates. Basically I'd assumed that amount was the damage after applying tempHP which it isn't it's the total damage. Here's a corrected recalculateDamage which seems to work

function recalculateDamage (actor, amount, updates, options) {
  const hpMax = Math.floor(actor?.system?.attributes?.hp?.max ?? 0);
  const hpTemp = updates["system.attributes.hp.temp"] ?? 0;
  const startHP = actor?.system?.attributes?.hp?.value ?? 0;
  const updatedHP = updates["system.attributes.hp.value"] ?? startHP;
  // How much damage was applied to the actor's hp - after temp hp was applied
  const appliedDamage = Math.max(0, startHP - (updates["system.attributes.hp.value"] ?? startHP)); 
  // how much temp damage appled to the new hpTemp value
  const newAppliedTemp = Math.min(hpTemp, appliedDamage, hpMax - updatedHP);
  const newHpTemp = hpTemp - newAppliedTemp;
  const newHpValue = Math.max(0, updatedHP + newAppliedTemp);

  updates['system.attributes.hp.temp'] = newHpTemp
  updates['system.attributes.hp.value'] = newHpValue
}
Larkinabout commented 3 weeks ago

The updated code unfortunately breaks the application of negative HP, which is the only real intention for this module to recalculate the damage. I think I need to fully understand what's going wrong for users as I've tested with the code pasted below without Midi enabled and it is seemingly doing what I expect. It may just be that I'm not applying the correct use cases and it is broken with or without Midi enabled. I'll do some testing generally and with Midi enabled.

function recalculateDamage (actor, amount, updates, options) {
    const hpMax = actor?.system?.attributes?.hp?.max ?? 0
    const hpTemp = actor?.system?.attributes?.hp?.temp ?? 0
    const hpValue = actor?.system?.attributes?.hp?.value ?? 0
    const startHp = (getSetting(CONSTANTS.HIT_POINTS.SETTING.NEGATIVE_HP_HEAL_FROM_ZERO)) ? 0 : hpValue
    const newHpValue = amount > 0
        ? hpValue - (amount - hpTemp)
        : Math.min(startHp - amount, hpMax)

    updates['system.attributes.hp.value'] = newHpValue
}