Adapt Archetype schema to allow for specifying level for talents #540
Conversation
aaclayton
left a comment
aaclayton
left a comment
There was a problem hiding this comment.
Looking good. A couple requests!
| return { | ||
| uuid, | ||
| name: talent.name, | ||
| img: talent.img, | ||
| description: await CONFIG.ux.TextEditor.enrichHTML(talent.system.description), | ||
| tags: talent.getTags(), | ||
| item: await talent.renderInline({showRemove: this.isEditable}) | ||
| } |
There was a problem hiding this comment.
Maybe I'm missing something, but i think we shouldn't need to do all this work and call talent.renderInline(). Don't we only care about the inline render now and we can ignore the other stuff like manual description enrichment?
There was a problem hiding this comment.
Good point - cleaned this up both here and in CrucibleActorDetailsItemSheet.
| const data = CONFIG.ux.TextEditor.getDragEventData(event); | ||
| const talents = this.document.system.talents; | ||
| if ( (data.type !== "Item") || talents.has(data.uuid) ) return; | ||
| const hasLeveledTalents = this.document.type === "archetype"; |
There was a problem hiding this comment.
For the very short term, can we leave a // TODO [issue number here] anywhere that we have to logically condition on whether we have leveled talents or not so we can identify all the places that we need to circle back to later?
| const grantingTalents = cls.grantingTalents[component]; | ||
| if ( grantingTalents.some(({uuid}) => talents.has(uuid)) ) continue; | ||
| const minTalent = grantingTalents.reduce((currMin, t) => (currMin.tier < t.tier) ? currMin : t).uuid; | ||
| requisiteTalents.push(minTalent); | ||
| if ( grantingTalents.some(({uuid}) => talents.some(t => t.item === uuid)) ) continue; | ||
| const minTalent = grantingTalents.reduce((currMin, t) => (currMin.tier < t.tier) ? currMin : t); |
There was a problem hiding this comment.
This could be a worthwhile helper method on the spellcraft component class, something like:
const reqTalent = await cls.getGrantingTalent(component);There was a problem hiding this comment.
Added - I do think it probably makes sense to create some base spellcraft component class they all inherit from, but probably not in this PR.
| let deleteItemIds = new Set(); | ||
| for ( const uuid of (existing?.talents || []) ) { | ||
| const talentId = foundry.utils.parseUuid(uuid)?.documentId; | ||
| const talentId = foundry.utils.parseUuid(uuid.item ?? uuid)?.documentId; |
There was a problem hiding this comment.
Let's tag places where we have to do this with a // TODO [issue number here]
| if ( source.details?.archetype ) crucible.api.models.CrucibleArchetypeItem.migrateData(source.details.archetype); | ||
| /** @deprecated since 0.7.3 */ | ||
| if ( source.details?.taxonomy ) crucible.api.models.CrucibleArchetypeItem.migrateData(source.details.taxonomy); | ||
| if ( source.details?.taxonomy ) crucible.api.models.CrucibleTaxonomyItem.migrateData(source.details.taxonomy); |
| talents: new fields.ArrayField(new fields.SchemaField({ | ||
| item: new fields.DocumentUUIDField({type: "Item"}), | ||
| level: new fields.NumberField({required: true, nullable: true, integer: true, initial: null}) | ||
| })), |
There was a problem hiding this comment.
We could consider using a TypedObjectField here where the keys are the talent UUIDs and the values are the required levels (or maybe better a schema field if we anticipate other potential metadata). This would have the side effect of avoiding duplicates in the array since the UUID keys would be unique.
| talents: new fields.ArrayField(new fields.SchemaField({ | |
| item: new fields.DocumentUUIDField({type: "Item"}), | |
| level: new fields.NumberField({required: true, nullable: true, integer: true, initial: null}) | |
| })), | |
| talents: new fields.TypedObjectField(new fields.SchemaField({ | |
| level: new fields.NumberField({required: true, nullable: true, integer: true, initial: null}) | |
| }), {validateKey: itemUuidField.validate}), |
aaclayton
added
the
pr:pending
2 participants
So far, just modifying the schema and making any required code changes so that everything works as it already did.
uuid.item ?? uuidis awful each time it comes up, rest assured it's a temporary measure, since everywhere it's used we'll be needing new explicit logic to handle thelevelaspect of things (in a follow-up PR).