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

Allow for "spellcasting" as the chosen ability in enrichers #4883

Open
wants to merge 1 commit into
base: 4.2.x
Choose a base branch
from

Conversation

krbz999
Copy link
Contributor

@krbz999 krbz999 commented Dec 15, 2024

We had a use case for a lot of "make a saving throw with your spellcasting ability" in adventures.

image

@arbron arbron added this to the D&D5E 4.2.0 milestone Dec 27, 2024
Copy link
Collaborator

@arbron arbron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The _onHoverPassive method in tooltips.mjs will also need adjustment in case someone decides to do a passive Spellcasting (Arcana) enricher.

module/documents/actor/actor.mjs Outdated Show resolved Hide resolved
@krbz999 krbz999 force-pushed the spellcasting-d20-rolls branch from 4e5677a to fd2c7cb Compare December 28, 2024 11:27
@krbz999 krbz999 requested a review from arbron December 28, 2024 11:29
@krbz999 krbz999 changed the title Allow for "spellcasting" as the chosen ability in enrichers and d20 roll methods Allow for "spellcasting" as the chosen ability in enrichers Dec 28, 2024
@arbron arbron requested a review from Fyorl December 28, 2024 21:31
@krbz999 krbz999 mentioned this pull request Jan 17, 2025
19 tasks
Copy link
Contributor

@Fyorl Fyorl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer spell over spellcasting (should probably support both).

We had a use case for a lot of "make a saving throw with your spellcasting ability" in adventures.

Do you have more context on this? I ask because we're using the creature's default spellcasting ability modifier here, which will not generally be correct for multi-classed characters and/or characters that receive spells via species traits. So I'm interested in understanding how this content disambiguates. Is it a saving throw that is only made during the casting of another spell, or to remove a spell or something?

@krbz999
Copy link
Contributor Author

krbz999 commented Jan 20, 2025

Scouring through the material, there are no specifics wrt multiclassed characters or anything else.

One example is a saving throw forced by a monster on targets and they "make a saving throw using their spellcasting ability," but no further distinction. Other examples are "[...] cast this spell and succeed on a DC 19 ability check using their spellcasting ability."

@arbron arbron modified the milestones: D&D5E 4.2.0, D&D5E 4.3.0 Jan 20, 2025
@Fyorl
Copy link
Contributor

Fyorl commented Jan 28, 2025

"[...] cast this spell and succeed on a DC 19 ability check using their spellcasting ability."

This one makes sense. Any spell you cast has an associated spellcasting ability.

One example is a saving throw forced by a monster on targets and they "make a saving throw using their spellcasting ability," but no further distinction.

This one I don't understand. Multi-classed characters aside, what if the target is a Monk? They don't have a spellcasting ability.

In the system we happen to have a default spellcasting ability which we can use for this enricher, but the concept of a default spellcasting ability is more of a legacy implementation holdover. In game terms, it's mostly only applicable to NPCs. PCs don't have default spellcasting abilities. The spellcasting ability for any given spell depends entirely on the source of the spell, which may be a class Spellcasting feature, a feat, a species feature, etc.

The DMG offers some guidance wrt. spells cast from magic items:

A magic item may require the user to use their own spellcasting ability when casting a spell from the item. If the user has more than one spellcasting ability, the user chooses which one to use with the item. If the user doesn’t have a spellcasting ability, their spellcasting ability modifier is +0 for the item, and the user’s Proficiency Bonus applies.

So we could potentially derive a default spellcasting ability for creatures that don't have one, using these rules. But these are clearly intended for magic items only, so this concept would be our own invention.

I guess I'm mostly unsure if this implementation of the enricher actually covers the intended use case here. Am I perhaps misinterpreting these examples? It's hard to say with these small snippets.

@krbz999
Copy link
Contributor Author

krbz999 commented Jan 31, 2025

So we could potentially derive a default spellcasting ability for creatures that don't have one, using these rules. But these are clearly intended for magic items only, so this concept would be our own invention.

I guess I'm mostly unsure if this implementation of the enricher actually covers the intended use case here. Am I perhaps misinterpreting these examples? It's hard to say with these small snippets.

I think we agree on the examples. Though I'll say there are, far as I remember, some magic items that use Intelligence in the absence of a dedicated spellcasting ability. Maybe that is a sensible fallback.

I've asked for clarification on the examples, but it may be a ways out. I would think, however, that it would be whichever is the highest modifier in case there are multiple.

But we do need some way to have spellcasting abilities tracked easily from other sources than classes (at least I don't think there is any property for this on a race or background item?).

@Fyorl
Copy link
Contributor

Fyorl commented Feb 3, 2025

I think we agree on the examples. Though I'll say there are, far as I remember, some magic items that use Intelligence in the absence of a dedicated spellcasting ability. Maybe that is a sensible fallback.

Do you have any examples? Again though, anything we do here would just be our own invention and not in-line with the game's rules.

But we do need some way to have spellcasting abilities tracked easily from other sources than classes (at least I don't think there is any property for this on a race or background item?).

For non-spells we can track sources via advancement only at the moment. More modern feats and species typically allow you to choose the spellcasting ability when you gain the spell, in which case they would be baked into the spell's activity configuration if it features an attack or save. So we probably need a more reliable way to set and retrieve that choice.

The Dispel Magic example from #5092 is a good example, because if you use this enricher in the item's description to retrieve attributes.spellcasting, it will be wrong in cases where the character got the spell from a feat/species, or:

  1. The character has multi-classed two spellcasting classes with different spellcasting abilities,
  2. They have Dispel Magic from the class that is not set as their primary spellcasting ability class.

Maybe that's ok because (1) is pretty rare, and so both (1) & (2) maybe almost never happens. But we do know each spell's source so it is possible to get the correct result here.

We need to make sure this is the final form of the enricher and that it doesn't need additional parameters/syntax that can't be easily added later, since we can't migrate enricher syntax very easily once it has been widely embedded in text content.

@krbz999
Copy link
Contributor Author

krbz999 commented Feb 3, 2025

Well maybe it boils down to two cases:

  • Non-contextual enricher, where the spellcasting ability is just the highest of all the actor's spellcasting abilities.
  • Contextual enricher, such as a check in the item description of Dispel Magic, where the spellcasting ability should be equal to the spellcasting ability of the spell.

Adjustments to how spellcasting is retrieved, though, I think is beyond this PR. Maybe this should be marked as blocked until then?

@Fyorl Fyorl added the blocked label Feb 3, 2025
@arbron arbron linked an issue Feb 3, 2025 that may be closed by this pull request
@arbron arbron mentioned this pull request Feb 3, 2025
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add spellcasting modifier as a /check enricher option
3 participants