Skip to content

Very early proof of concept for adding typed damage multipliers to system.traits.dm - #7289

Open
roth-michael wants to merge 1 commit into
foundryvtt:6.0.xfrom
roth-michael:damage-multiplier
Open

Very early proof of concept for adding typed damage multipliers to system.traits.dm#7289
roth-michael wants to merge 1 commit into
foundryvtt:6.0.xfrom
roth-michael:damage-multiplier

Conversation

@roth-michael

@roth-michael roth-michael commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Closes #4917, if we go this direction.
Very basic implementation of "what if we just add multiply under traits.dm?"
Besides just generally lacking some polish (there's a few areas for code deduplication, if nothing else), the main two pieces I'd call out as potentially troublesome are:

  • The existing trait infrastructure doesn't really support this use case. Illustrated in the DamagesConfig TODOs: I added a not-currently-typed hint and field to the SelectChoices being created so that they can be different for dm.multiply vs dm.amount. A better solution would also be a more involved solution, either registering them as different traits (maybe we should) or introducing logic in SelectChoices and in trait.mjs for scenarios like this one. Of which I think there are probably 0, not including this one.
  • When would this multiplication happen, ideally? Right now I have it being applied directly after dm.amount modification. I think any decision here will be a bit of a guess, since there isn't really a concept of arbitrary damage multiplication. Best we could do probably is determine how something like absorption would be intended to work in a scenario where a creature has resistance to all damage types, but also absorption of, say, fire damage. Should they absorb the non-resisted amount, or the post-resist amount, etc etc.

Also entirely possible that this method of implementation is not one that you guys wanna go with. The "hard" part of this PR was just making it play nice with the sheet UI, so nothing much would be lost if so.

Edit: unmarking as draft since in theory this is fully functional as-is; I just knew it'd need some cleanup which made me want to mark it draft, but there's nothing more I feel inclined to do on it without feedback on direction.

@roth-michael
roth-michael marked this pull request as ready for review July 30, 2026 14:38
@Fyorl Fyorl added the strategic Requires sign-off from a core Foundry VTT developer before merge. label Jul 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: low strategic Requires sign-off from a core Foundry VTT developer before merge. system: combat

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature request] Damage absorption

3 participants