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

Revise item usage and consumption #2400

Merged
merged 33 commits into from
Oct 13, 2023

Conversation

krbz999
Copy link
Contributor

@krbz999 krbz999 commented Aug 1, 2023

The function Item5e#_getUsageConfig returns an object of what an item is able to do:

  • Create a template.
  • Consume a spell slot.
  • Consume limited uses/recharge.
  • Consume a resource.

The function Item5e#_getUsageConfigValues returns an object that depends on _getUsageConfig to determine what an item, by default, will do. This is the object that can be modified directly in Item5e#use. This also includes slotLevel, which is now always a string.

Whether an item then needs configuration is determined by if any of the default values are true (they are by default true if the item is able to fulfill their function).

If none are true, a prompt will not be forced. Any boxes that are possible but overridden to be false by default will still appear in the AbilityUseDialog, but unchecked; if the prompt is skipped (options.configureDialog being false) the initial values are used (possibly partially overridden).

Also added two properties in item.system.uses and item.system.target for #2463 which function with getUsageConfigValues.

@krbz999 krbz999 force-pushed the item-usage-and-scale-non-spells branch from fe4e036 to dd0115d Compare August 3, 2023 17:13
@krbz999 krbz999 marked this pull request as ready for review September 8, 2023 12:14
@Fyorl Fyorl changed the base branch from 2.3.x to 2.4.x September 8, 2023 15:55
@Fyorl Fyorl added this to the D&D5E 2.4.0 milestone Sep 8, 2023
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.

Good changes here, thank you for taking on this refactor. I think you've managed to streamline the logic and make the pipeline a lot easier to follow (at least for me). I have some suggestions for your consideration that we can discuss.

module/documents/item.mjs Outdated Show resolved Hide resolved
module/documents/item.mjs Outdated Show resolved Hide resolved
module/documents/item.mjs Show resolved Hide resolved
module/documents/item.mjs Outdated Show resolved Hide resolved
module/documents/item.mjs Outdated Show resolved Hide resolved
module/applications/item/ability-use-dialog.mjs Outdated Show resolved Hide resolved
module/documents/item.mjs Outdated Show resolved Hide resolved
module/data/item/templates/activated-effect.mjs Outdated Show resolved Hide resolved
module/documents/item.mjs Outdated Show resolved Hide resolved
module/documents/item.mjs Outdated Show resolved Hide resolved
krbz999 and others added 12 commits September 14, 2023 15:46
Co-authored-by: Kim Mantas <kim.mantas@gmail.com>
Co-authored-by: Kim Mantas <kim.mantas@gmail.com>
Co-authored-by: Kim Mantas <kim.mantas@gmail.com>
Co-authored-by: Kim Mantas <kim.mantas@gmail.com>
Co-authored-by: Kim Mantas <kim.mantas@gmail.com>
Co-authored-by: Kim Mantas <kim.mantas@gmail.com>
using null and bools
@krbz999 krbz999 requested a review from Fyorl September 14, 2023 14:47
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.

I caught a couple more things on re-review, but otherwise this looks pretty much there. I will try for a faster turn around on any comments/changes here. Apologies for the delay.

module/applications/item/ability-use-dialog.mjs Outdated Show resolved Hide resolved
module/applications/item/ability-use-dialog.mjs Outdated Show resolved Hide resolved
module/documents/item.mjs Show resolved Hide resolved
module/documents/item.mjs Outdated Show resolved Hide resolved
module/documents/item.mjs Show resolved Hide resolved
module/data/item/templates/activated-effect.mjs Outdated Show resolved Hide resolved
Zhell added 3 commits October 13, 2023 20:25
fix erroneous upcasting not looking for spell slot
fix lingering variable
@krbz999 krbz999 requested a review from Fyorl October 13, 2023 18:34
@Fyorl
Copy link
Contributor

Fyorl commented Oct 13, 2023

@krbz999 Any thoughts on this? Do you have a strong objection to it?

@Fyorl Fyorl merged commit ee225a1 into foundryvtt:2.4.x Oct 13, 2023
@krbz999 krbz999 deleted the item-usage-and-scale-non-spells branch October 20, 2023 19:23
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.

None yet

2 participants