-
-
Notifications
You must be signed in to change notification settings - Fork 360
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
Equipment category schema rationalization #397
Comments
I actually think that kind of makes sense? I'm not even sure where the concept of We play a little fast and loose with breaking changes. Probably more than we should. If you want to do this right, I'd break this PR into two separate PR.
This allows us to make a deprecation announcement between them and space out when we merge them. |
To be clear, I think we should definitely do this. |
I'll get there at some point if you (or someone else) doesn't beat me to it. I'm not working on integrating SRD data sets right now, so further data cleanups have been bumped to the back burner for me, but you'll know next time I do. 🙂 |
Sounds great! I'm sure someone will get to it! |
It appears that "arrow" equipment which might be a special case but it's in 3 different categories. "adventuring-gear", "ammunition", and "standard-gear". So I would actually suggest that "equipment_category" becomes a list instead of just adding a "equipment_subcategory". |
@crazyfox55 That's an interesting suggestion. I think that makes a kind of sense. Like a list of tags. I'd love to see that in a PR. |
@MikkelPaulson I had an idea related to this in another issue where it might make more sense to combine these like so: "equipment_categories": [{
"index": "weapon",
"name": "Weapon",
"url": "/api/equipment-categories/weapon"
}, {
"index": "martial-melee-weapons",
"name": "Martial Melee Weapons",
"url": "/api/equipment-categories/martial-melee-weapons"
}, {
"index": "martial-weapons",
"name": "Martial Weapons",
"url": "/api/equipment-categories/martial-weapons"
}, {
"index": "melee-weapons",
"name": "Melee Weapons",
"url": "/api/equipment-categories/melee-weapons"
}], |
@bagelbits If we did this, and we waited for a second PR to eliminate the old fields, then we'd have something like the following, right? {
"index": "club",
"name": "Club",
"equipment_category": {
"index": "weapon",
"name": "Weapon",
"url": "/api/equipment-categories/weapon"
},
"equipment_categories": [{
"index": "weapon",
"name": "Weapon",
"url": "/api/equipment-categories/weapon"
}, {
"index": "simple-melee-weapons",
"name": "Simple Melee Weapons",
"url": "/api/equipment-categories/simple-melee-weapons"
}, {
"index": "simple-weapons",
"name": "Simple Weapons",
"url": "/api/equipment-categories/simple-weapons"
}, {
"index": "melee-weapons",
"name": "Melee Weapons",
"url": "/api/equipment-categories/melee-weapons"
}],
"weapon_category": "Simple",
"weapon_range": "Melee",
"category_range": "Simple Melee",
"cost": {
"quantity": 1,
"unit": "sp"
},
"damage": {
"damage_dice": "1d4",
"damage_type": {
"index": "bludgeoning",
"name": "Bludgeoning",
"url": "/api/damage-types/bludgeoning"
}
},
"range": {
"normal": 5
},
"weight": 2,
"properties": [
{
"index": "light",
"name": "Light",
"url": "/api/weapon-properties/light"
},
{
"index": "monk",
"name": "Monk",
"url": "/api/weapon-properties/monk"
}
],
"url": "/api/equipment/club"
}, The initial PR would look pretty clunky with both an "equipment_category" and an "equipment_categories" field. Would we be okay with this for the initial PR? |
I think it's not that clunky it's like the primary equipment category and then the set of secondary categories. I have not thought of all of the downstream effects this change would have. Consumers of the data would all need to change to support a list of categories. |
I agree this will be quite the change for consumers. We should definitely depreciate the old format first. As long as the above code looks acceptable, I think I'll get cracking on a build. |
@mpeck451 I'm not sure your example removed any old fields. But that is what the interim state would look like. I assume the final result after the second PR would just be: {
"index": "club",
"name": "Club",
"equipment_categories": [{
"index": "weapon",
"name": "Weapon",
"url": "/api/equipment-categories/weapon"
}, {
"index": "simple-melee-weapons",
"name": "Simple Melee Weapons",
"url": "/api/equipment-categories/simple-melee-weapons"
}, {
"index": "simple-weapons",
"name": "Simple Weapons",
"url": "/api/equipment-categories/simple-weapons"
}, {
"index": "melee-weapons",
"name": "Melee Weapons",
"url": "/api/equipment-categories/melee-weapons"
}],
"cost": {
"quantity": 1,
"unit": "sp"
},
"damage": {
"damage_dice": "1d4",
"damage_type": {
"index": "bludgeoning",
"name": "Bludgeoning",
"url": "/api/damage-types/bludgeoning"
}
},
"range": {
"normal": 5
},
"weight": 2,
"properties": [
{
"index": "light",
"name": "Light",
"url": "/api/weapon-properties/light"
},
{
"index": "monk",
"name": "Monk",
"url": "/api/weapon-properties/monk"
}
],
"url": "/api/equipment/club"
}, |
There's a general schema for links to other entities that looks like this:
But most of the subcategories in 5e-SRD-Equipment.json look like this instead:
The one exception to this is
gear_category
:My initial thought was that the categories that were represented as simple strings weren't found in 5e-SRD-Equipment-Categories.json, but nope:
I'm not sure what the policy is for breaking changes.
gear_category
,armor_category
,vehicle_category
,tool_category
,weapon_category
) appear at first glance to be mutually exclusive, can this be consolidated as anequipment_subcategory
field?The text was updated successfully, but these errors were encountered: