5e-bits / 5e-database

Database for the D&D 5th Edition API
http://dnd5eapi.co/
MIT License
723 stars 355 forks source link

Non-distinct Feature Indices #149

Closed jtompkins84 closed 4 years ago

jtompkins84 commented 4 years ago

There are features with non-distinct indices, which leads to URLs referenced by classes and levels routing to the wrong object, e.g. /api/features/choose-expertise-1 will return the feature information for bards, but not for rogue, because both rogue and bard have a feature choice at some level with index choose-expertise-1. This means there is no valid route to get information about choose-expertise-1 for the rogue class, only bard.

All non-distinct feature indices are listed below:

choose-additional-eldritch-invocation, choose-additional-metamagic, choose-expertise-1, choose-expertise-2, choose-fighting-style, evasion, expertise-acrobatics, expertise-animal-handling, expertise-arcana, expertise-athletics, expertise-deception, expertise-history, expertise-insight, expertise-intimidation, expertise-investigation, expertise-medicine, expertise-nature, expertise-perception, expertise-performance, expertise-persuasion, expertise-religion, expertise-sleight-of-hand, expertise-stealth, expertise-survival, extra-attack, fast-movement, fighting-style-archery, fighting-style-defense, fighting-style-dueling, fighting-style-great-weapon-fighting, fighting-style-protection, fighting-style-two-weapon-fighting, lands-stride, ranger-ability-score-improvement-4, timeless-body, unarmored-defense

Cleanest solution would probably be to prefix these indices with the index of class they are associated with, and update all URLs referencing this data. This approach would not require any changes in the API code.

There are a few non-distinct feature index which references 2 features of the same class at different levels, e.g. fast-movement for the barbarian class. Only one of these features can be reached in the API currently.

bagelbits commented 4 years ago

Heck. I thought I caught all of these but as always, quite a few fell through the cracks when I switch from number to string indices. I think prefixing with class is a sane and reasonable solution. Though I'm a little amused that ranger-ability-score-improvement-4 found it in there?

For the features of the same class, I've been postfixing with an integer but I'm not 100% sure if that's the right approach.

bagelbits commented 4 years ago

@jtompkins84 I've opened a PR that should address all of these. Please take a look to see if it does. I'll try to merge this in sometime tomorrow.