Skip to content

Add expiry events to DurationData, apply duration to enchantments - #7267

Open
arbron wants to merge 1 commit into
6.0.xfrom
activity/expiry
Open

Add expiry events to DurationData, apply duration to enchantments#7267
arbron wants to merge 1 commit into
6.0.xfrom
activity/expiry

Conversation

@arbron

@arbron arbron commented Jul 18, 2026

Copy link
Copy Markdown
Collaborator

Adds duration.expiry to activity and spell data. Does not hide the duration field for duration-less expiry events because you may need to keep the duration data (for example, Shield has a duration of 1 Round and the expiry event of "Start of Source's Next Turn").

Changes how duration overrides are handled for applied effects by moving the logic into the activity itself, to give activities a chance to fully modify the effect being applied. In the process this also applies those same changes to applied enchantments.

Adds `duration.expiry` to activity and spell data. Does not hide
the duration field for duration-less expiry events because you may
need to keep the duration data (for example, Shield has a duration
of 1 Round and the expiry event of "Start of Source's Next Turn").

Changes how duration overrides are handled for applied effects by
moving the logic into the activity itself, to give activities a
chance to fully modify the effect being applied. In the process
this also applies those same changes to applied enchantments.
@arbron
arbron force-pushed the activity/expiry branch from 2b8fa80 to 781556a Compare July 20, 2026 16:49
@arbron arbron mentioned this pull request Aug 14, 2026
42 tasks

@Fyorl Fyorl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This hint reads a bit weirdly for spell details. Maybe just no hint in this case to match the other fields.

Image

case "second": units = "seconds"; break;
case "minute": units = "minutes"; break;
case "hour": units = "hours"; break;
case "day": units = "days" ; break;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
case "day": units = "days" ; break;
case "day": units = "days"; break;

const general = _loc("DND5E.EFFECT.Expiry.Group.General");
const specific = _loc("DND5E.EFFECT.Expiry.Group.Specific");
return [
...Object.entries(ActiveEffect5e.EXPIRY_EVENTS).map(([value, l]) => ({ value, label: _loc(l), group: general })),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
...Object.entries(ActiveEffect5e.EXPIRY_EVENTS).map(([value, l]) => ({ value, label: _loc(l), group: general })),
...Object.entries(this.EXPIRY_EVENTS).map(([value, l]) => ({ value, label: _loc(l), group: general })),

*/
expirySupportsDuration(expiry=this.duration.expiry) {
return !this.constructor.PSEUDO_EXPIRIES.has(expiry) && !this.constructor.DURATIONLESS_EXPIRIES.has(expiry);
return ActiveEffect5e.expirySupportsDuration(expiry);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
return ActiveEffect5e.expirySupportsDuration(expiry);
return this.constructor.expirySupportsDuration(expiry);

};
}
context.expiryEvents = Object.fromEntries(
ActiveEffect.implementation.expiryOptions.map(({ value, ...data }) => [value, data])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do these need to be alphabetised?

Comment thread module/config.mjs
/**
* System-specific "expiry" choices which do not require registration or custom expiry events, and instead
* are handled dynamically in `ActiveEffect#isExpiryEvent`.
* @type {Record<string>}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
* @type {Record<string>}
* @type {Record<string, string>}

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.

2 participants