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 Armor json data #9 #33

Merged
merged 3 commits into from
Nov 10, 2019
Merged

Add Armor json data #9 #33

merged 3 commits into from
Nov 10, 2019

Conversation

gbalbuena
Copy link
Contributor

@gbalbuena gbalbuena commented Oct 28, 2019

This is the armor json file that I'm using to calculate ac, was thinking could be valuable with some corrections.

@gbalbuena gbalbuena changed the title armor: add json data Add Armor json data #9 Oct 28, 2019
"ac": 14,
"cost": "400 gp",
"weight": "20 lb.",
"modifier": [ "dexterity" ],
Copy link
Collaborator

Choose a reason for hiding this comment

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

On these guys I think this is supposed to be dex (max 2). Is there any way to indicate that here?

Copy link
Contributor Author

@gbalbuena gbalbuena Oct 29, 2019

Choose a reason for hiding this comment

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

what do you think about

"modifier_max": 2

or

"modifier": [ { "ability": "dexterity", "max": 2 } ],
"modifier": [ { "ability": "dexterity", "max": null } ], // when no max

Copy link
Contributor Author

@gbalbuena gbalbuena Oct 29, 2019

Choose a reason for hiding this comment

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

This is the code to calculate ac using that data source

https://github.com/gbalbuena/dndcli/blob/master/src/core/index.js#L19-L67

data/WOTC_5e_SRD_v5.1/armor.json Outdated Show resolved Hide resolved
@augustjohnson
Copy link
Collaborator

Hey guys, just so you know, I took this general file and after building a model and import process that would work, I adjusted it (quite a bit actually). You can see my work over in PR #38. Technically that work completely supercedes this PR, but it absolutely was based on this file.

@gbalbuena
Copy link
Contributor Author

Hey guys, just so you know, I took this general file and after building a model and import process that would work, I adjusted it (quite a bit actually). You can see my work over in PR #38. Technically that work completely supercedes this PR, but it absolutely was based on this file.

Good work!, we haven't finalize the structure, I'm about to send and update around the structure of the data based on @eepMoody comments

@gbalbuena
Copy link
Contributor Author

please let me know if any change is required to merge this, so far I'm using it for my ac formulas and It's working as expected, you might need to do changes for your requirements or keep it as it is, we can always change and adapt

@eepMoody eepMoody merged commit d9233c8 into open5e:master Nov 10, 2019
@gbalbuena
Copy link
Contributor Author

@eepMoody Legendary! ⚔️

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.

3 participants