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

Add advantage modes to abilities objects #4922

Open
wants to merge 3 commits into
base: 4.2.x
Choose a base branch
from

Conversation

krbz999
Copy link
Contributor

@krbz999 krbz999 commented Dec 25, 2024

(Built on top of #4890.)

Adds system.abilities.<abilityId>.check.roll and .save.roll objects.

Moves the derived abilities.<abilityId>.save value into .check/.save.value with a toString method for the deprecation period.

(Also removes the ability property from the attributes.death object.)

TODO:

  • Add the roll mode and min/max stuff to ability configs.
  • Hook up actually using the data in rolls.

@krbz999 krbz999 force-pushed the more-advantage-coverage++ branch from 5afcf18 to 0744b69 Compare December 25, 2024 00:12
@arbron arbron added this to the D&D5E 4.2.0 milestone Dec 25, 2024
@JPMeehan
Copy link
Contributor

Wasn't there a discussion that for AEs to work properly you probably should split dis & adv so that way the cancelling can be parsed correctly?

@krbz999
Copy link
Contributor Author

krbz999 commented Dec 26, 2024

Wasn't there a discussion that for AEs to work properly you probably should split dis & adv so that way the cancelling can be parsed correctly?

See the AdvantageModeField.

@JPMeehan
Copy link
Contributor

Those are properties on the shared model schema – does this not have collisions across multiple actors of the same type?

@krbz999
Copy link
Contributor Author

krbz999 commented Dec 26, 2024

Those are properties on the shared model schema – does this not have collisions across multiple actors of the same type?

I don't see why - the field is reset when initialized, and actors don't share the same instance of a DataField.

@JPMeehan
Copy link
Contributor

image

Fields are properties of the static class - myModel.schema accesses myModel.constructor.schema. That's why all of the field methods like _cast take in the data they're modifying.

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.

Seems like a good start 👍

module/data/actor/templates/common.mjs Outdated Show resolved Hide resolved
module/data/actor/templates/common.mjs Outdated Show resolved Hide resolved
…ove derived value `abilities.<x>.save` to `abilities.<x>.save.value`.
@krbz999 krbz999 force-pushed the more-advantage-coverage++ branch from da1c939 to bb31ffe Compare January 2, 2025 22:33
@krbz999 krbz999 marked this pull request as ready for review January 2, 2025 22:35
module/data/shared/roll-config-field.mjs Outdated Show resolved Hide resolved
module/data/shared/roll-config-field.mjs Outdated Show resolved Hide resolved
@krbz999 krbz999 force-pushed the more-advantage-coverage++ branch from c1c65af to e5feb4a Compare January 2, 2025 23:36
@arbron arbron requested a review from Fyorl January 2, 2025 23:49
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.

3 participants