Add GrantItem RE for Raise a Shield to Shields of the Spirit spell effect #18343
Conversation
Update spell-effect-shields-of-the-spirit.json Added GrantItem Rule Element to the Shields of the Spirit spell effect that applies "Effect: Raise a Shield"
| }, | ||
| { | ||
| "key": "GrantItem", | ||
| "uuid": "Compendium.pf2e.equipment-effects.Item.2YgXoHvJfrDHucMr", |
There was a problem hiding this comment.
This uuid should have been converted on extract to: "Compendium.pf2e.equipment-effects.Item.Effect: Raise a Shield".
I suspect this was hand-built rather than extracted, as the keys aren't sorted.
Additionally, the effect description should probably be updated.
There was a problem hiding this comment.
You're right I totally forgot to change the UUID to the named version. I'll fix that (pending guidance on the below to do it all at once)
In what order should the keys be sorted? They are currently alphabetical.
The effect description is what's read in the effect on an ally in the aura, who do not get the benefits of the raised shield (until the Greater Security feat which AFAIK is covered by the separate spell effect shields-of-the-spirit-security). It does add that description to the Champion as well even though the Status Bonus doesn't apply to them. Should I figure out how change the description for what's on the Champion vs their allies? I didn't do that since that wasn't done on the effect originally.
I did make this by adding the RE within Foundry and then exporting the JSON to copy the new code into the base effect. How should I do it for something like this? Sorry this is my first time contributing to a big project like this (and using GitHub) so I don't know all the etiquette.
There was a problem hiding this comment.
In what order should the keys be sorted? They are currently alphabetical.
Alphabetical would be correct. For example predicate should sorted between key and uuid. This will be automatically handled by the extractor.
The effect description is what's read in the effect on an ally in the aura, who do not get the benefits of the raised shield (until the Greater Security feat which AFAIK is covered by the separate spell effect shields-of-the-spirit-security). It does add that description to the Champion as well even though the Status Bonus doesn't apply to them. Should I figure out how change the description for what's on the Champion vs their allies? I didn't do that since that wasn't done on the effect originally.
The effect is used for both, so try to succinctly describe both. It might be challenging. :)
I did make this by adding the RE within Foundry and then exporting the JSON to copy the new code into the base effect. How should I do it for something like this? Sorry this is my first time contributing to a big project like this (and using GitHub) so I don't know all the etiquette.
Thanks for contributing! Check out wiki.
There was a problem hiding this comment.
Ahh alphabetical within each one, I see.
Should I close this PR and try again with the normal workflow, or try to salvage this one manually?
Per Simon Ward's suggestions: • Updated the effect's description, explaining what's granted to the Champion and what's granted to their Allies. • Alphabetized the keys in the new rule element • Fixed the UUID to the named version
| @@ -4,7 +4,7 @@ | |||
| "name": "Spell Effect: Shields of the Spirit", | |||
| "system": { | |||
| "description": { | |||
| "value": "<p>Granted by @UUID[Compendium.pf2e.spells-srd.Item.Shields of the Spirit]</p>\n<p>You gain a +1 status bonus to AC.</p>" | |||
| "value": "<p>Granted by @UUID[Compendium.pf2e.spells-srd.Item.Shields of the Spirit]</p><p><strong>Champion:</strong> You Raise your Shield. The effect ends at the start of your next turn or when you're no longer raising your shield.</p><p><strong>Ally:</strong> You gain a +1 status bonus to AC while in the Champion's aura. When an enemy attacks you, they take 1d4 spirit damage.</p>" | |||
There was a problem hiding this comment.
We typically omit the duration as it's in the structured data. Also, the description should only include what the effect actually does, so omit the mention of spirit damage.
There was a problem hiding this comment.
Okay, done! I wasn't sure about the duration since the part of lowering the shield isn't automated. Similarly wasn't sure the extent to which they should have reminders in them.
Removed mention of the duration/end conditions from the Champion section, and removed mention of the spirit damage from the Ally section.
Update spell-effect-shields-of-the-spirit.json
Added GrantItem Rule Element to the Shields of the Spirit spell effect that applies "Effect: Raise a Shield"