Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#2472] Change ammo logic for attack and damage rolls #2525

Merged
merged 2 commits into from
Nov 1, 2023

Conversation

krbz999
Copy link
Contributor

@krbz999 krbz999 commented Oct 26, 2023

Instead of a damage roll requiring a preceding attack roll to gain the benefit of 'ammo damage', the damage roll simply checks if the item has ammo and can attack. The attack roll simply serves to consume ammo, nothing more.

There are some additional benefits to this.

  • If one user made the attack roll, another user can still gain the benefit of the ammo on the damage roll, in that weird edge case of two owners of one actor, where before the addition of _ammo to the item was local only.
  • Users can make all their attack rolls first, and then roll all the damage rolls for easier calculation.
  • Users can "redo" a damage roll that was meant to be a critical hit, without having to make another attack roll first.

@arbron
Copy link
Collaborator

arbron commented Oct 26, 2023

So the issue with this is that ammo's quantity is consumed on attack, which also means that ammo might have 0 uses remaining or the item might be fully deleted by the time damage is rolled (if delete on empty is checked), resulting in no ammo being available to calculate damage bonuses.

@krbz999
Copy link
Contributor Author

krbz999 commented Oct 26, 2023

So the issue with this is that ammo's quantity is consumed on attack, which also means that ammo might have 0 uses remaining or the item might be fully deleted by the time damage is rolled (if delete on empty is checked), resulting in no ammo being available to calculate damage bonuses.

Ammo does not get destroyed.

@Fyorl Fyorl added this to the D&D5E 2.4.0 milestone Nov 1, 2023
@Fyorl Fyorl merged commit 0837724 into foundryvtt:2.4.x Nov 1, 2023
@krbz999 krbz999 deleted the 2472 branch November 1, 2023 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants