-
Notifications
You must be signed in to change notification settings - Fork 232
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
[#342] Add new race type #2218
[#342] Add new race type #2218
Conversation
Is scale value a possible Advancement of the item type? |
It will be, but there need to be some changes to its logic because currently it assumes it is using the class level, rather than the character level. That will probably be better handled in a follow-up PR. |
If i'm understanding this correctly, each We already have EDIT - for reference, we continued this conversation in Discord. https://discord.com/channels/170995199584108546/670336046164213761/1082724174495223890 I understand the motives and I think they are sound, just to not keep this open question here! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Structure of the PR
Big feature changes like this would be much easier to review if we separated them into a structured sequence of individual PRs, for example like:
- Add Race data model
- Add Race configuration sheet
- Add Actor sheet integration and advancement support
- Compendium content changes
Having this be "all or nothing" makes it hard to review (at least for me) and it would be easier to build up one piece at a time with a checkpoint between each PR where the overall task is not complete but the system remains in working order.
Race Data Model
I'm not sure I agree with race ONLY having an identifier and advancement. There are some features that we know each race has based on the 5th edition style guide:
- Ability Score Increase
- Age
- Size
- Speed
- Senses
- Features (covered by advancement)
- Proficiencies (covered by advancement)
- Languages (covered by advancement?)
I know that some of these things will be covered by the advancement system, but it doesn't seem right to me that all of these are just advancement. We should draw a line between what are the fundamental attributes of a race in D&D vs. what are the things you get as you level up.
It feels to me like our Race data model should be a bit more nuanced than just shoving everything into advancement
.
Actor Data Model
I don't love having race handled as "just another item" where the actor could be assigned multiple races. For race
and background
I think a better design would be to add these to the Actor data model as a singleton data structure like system.details.race
and system.details.background
(names are negotiable). We would then prevent creating race
and background
items on the actor and instead populate these singleton data fields.
This is the approach used in crucible
and I think it's more effective than the proposed design here. See for example:
- Actor data model: https://github.com/foundryvtt/crucible/blob/master/module/data/hero.mjs#L32
- Singleton enforcement in
Item#_preCreate
: https://github.com/foundryvtt/crucible/blob/master/module/documents/item.mjs#L119
I plan on splitting this up.
I designed the existing
I figured this would just be part of the description because there isn't a particular mechanical implementation here.
I think this should be handled by an advancement because some races give players the option of choosing between small and medium, so it would be good to prompt them for that choice. I figure using the existing advancement system for presenting this choice would be better than designing a whole new system that also prompts the player when they select their race.
No races offer choices here, so we could handle this using a separate system without issue.
I suppose my question is do we want to copy the data over when the race/background are added, or do we want to keep them as items and create links to them inside the data structure. So I feel like copying over the data to the actor would lead to issues with the advancement data and implementation which is entirely based on advancements being on items. |
Note: This is being split into smaller PRs starting with #2503
This introduces a newrace
item type to the system. This type is very simple, just having the normal description fields, an identifier, and a set of advancements. Additional race data for things like creature type, movement, and senses will be handled by a newRaceAdvancement
.I have converted the races in the SRD over to the new type, removing the base race types in the process (so we have"Lightfoot Halfling"
, but no longer the generic"Halfling"
). I have also split up the dragonborn's breath weapon so each type has its own item for the player to select.