foundryvtt / dnd5e

An implementation of the 5th Edition game system for Foundry Virtual Tabletop (http://foundryvtt.com).
MIT License
308 stars 212 forks source link

null error in rollAttackV2 #4229

Closed gludington closed 2 days ago

gludington commented 4 days ago

The jsdocs for rollAttackV2 specify that AmmunicationUpdate can be null

 * @param {AmmunitionUpdate|null} data.ammoUpdate  Any updates related to ammo consumption for this attack.

However, it is used without a null check in the hook:

  const oldAmmoUpdate = [{ _id: ammoUpdate.id, "system.quantity": ammoUpdate.quantity }];
  Hooks.callAll("dnd5e.rollAttack", this.item, roll, oldAmmoUpdate);
  if ( oldAmmoUpdate[0] ) {
    ammoUpdate.id = oldAmmoUpdate[0]._id;
    ammoUpdate.quantity = foundry.utils.getProperty(oldAmmoUpdate[0], "system.quantity");
  }
}

This will break firing of the hook for consumers. One way to take a look at this is to enable the Automated Animations module, and cast Firebolt. This will call this hook with a null ammoUpdate argument, and the hook will break there. If you open your debug tools and change ammoUpdate to an empty object, the hook will fire (and the animation display).

gludington commented 4 days ago

referenced in PR here: https://github.com/foundryvtt/dnd5e/pull/4232