Skip to content

Conversation

@ASTRELION
Copy link
Contributor

@ASTRELION ASTRELION commented Jan 13, 2024

What does this do?

Some of these strings would contain "...damage from nonmagical..." or "...from nonmagical...", so this PR removes "damage" from every string to be consistent with the rest.

Also, the Imp had a completely different format of "nonmagical/nonsilver" instead of the usual "nonmagical not made with silver".

Finally, I made the Archmage's resistance from Stoneskin more like the rest of the resistances' formats.

Some strings have "nonmagical attacks not made with silvered weapons" and some have "nonmagical weapons that aren't silvered" and those are unchanged. I wasn't sure if there was a practical difference between them, but if there isn't, I'd be happy to normalize those, too.

Let me know if I missed any!

Force push was to correct misspelled commit message.

How was it tested?

Built locally.

Is there a Github issue this is resolving?

No.

Did you update the docs in the API? Please link an associated PR if applicable.

N/A

Here's a fun image for your troubles

random photo - update me

@ASTRELION ASTRELION changed the title Normalize resistnace/immunity/vulnerability strings fix: Normalize resistnace/immunity/vulnerability strings Jan 13, 2024
@ASTRELION ASTRELION changed the title fix: Normalize resistnace/immunity/vulnerability strings fix: Normalize resistance/immunity/vulnerability strings Jan 13, 2024
@bagelbits
Copy link
Collaborator

@ASTRELION Can you list the additional strings that you're not sure of and how you might group them? As it stands, thanks for doing this, I think it was necessary.

@ASTRELION
Copy link
Contributor Author

@ASTRELION Can you list the additional strings that you're not sure of and how you might group them? As it stands, thanks for doing this, I think it was necessary.

For example, there are 4 instances of something like:
"bludgeoning, piercing, and slashing from nonmagical attacks not made with silvered weapons"

And 23 instances of something like:
"bludgeoning, piercing, and slashing from nonmagical weapons that aren't silvered"

Looking at the SRD, it does seem like they actually use those different strings. So, maybe it should just stay that way in the API.

@bagelbits
Copy link
Collaborator

@ASTRELION I think that they're functionally the same so I don't mind rewriting them for consistency.

@fergcb What do you think?

@fergcb
Copy link
Member

fergcb commented Jan 22, 2024

@fergcb What do you think?

Like you say, they're functionally the same. I'm in favour of this change - it'll make things nicer for computers, and it won't change the meaning for humans.

@ASTRELION
Copy link
Contributor Author

Are they actually the same? Couldn't it be interpreted that "bludgeoning, piercing, and slashing from nonmagical attacks not made with silvered weapons" would be resistant to an unarmed, bludgeoning attack but, "bludgeoning, piercing, and slashing from nonmagical weapons that aren't silvered" would not be resistant to an unarmed, bludgeoning attack since unarmed isn't a weapon. Or does an unarmed attack count as a weapon? Maybe there are better examples, but this was the first I thought of.

@bagelbits
Copy link
Collaborator

@ASTRELION As far as I am aware, they are the same. An unarmed attack is a melee weapon attack.

@ASTRELION
Copy link
Contributor Author

Sorry for the delay, this should be ready to merge now.

The last commit changes all 4 of the outlying "nonmagical attacks not made with silvered weapons" strings to "nonmagical weapons that aren't silvered".

@bagelbits bagelbits merged commit 1e7fa28 into 5e-bits:main Mar 4, 2024
@github-actions
Copy link
Contributor

github-actions bot commented Mar 4, 2024

🎉 This PR is included in version 3.3.3 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

markpol pushed a commit to markpol/5e-database that referenced this pull request May 19, 2024
* Normalize resistance/immunity/vulnerability strings

* Normalize non-silver weapon immunities
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants