pafvel / dragonbane

Other
14 stars 6 forks source link

Professions can't add all associated skills #30

Closed Akaito closed 8 months ago

Akaito commented 8 months ago

Due to character-sheet.js line 1395, professions fail to find core and weapon skills when dropped onto character sheets.

I only have the system installed. Not the for-purchase core rules or other modules; just to see if I like the FVTT system enough to purchase the core rules module. I've manually made all skills the Artisan profession lists. But when I drop the Artisan profession onto a character sheet, it fails to add the core and weapon skills to it, because they aren't of type "secondary" or "magic".

Suggest reworking the skill lookup so it's instead of an "ensureExists()" sort of style. Check if the Actor already has the skill item. If not, add it. Pay no mind to skill type. Rather than assuming something else has already added a skill if it's of the "core" or "weapon" type. This would also make the system more flexible in supporting homebrew skill sets.

pafvel commented 8 months ago

All Core and Weapon skills in the world (if you don't have the core module) should have been added to the character when it was created. Your problem suggests that you either deleted them from the character, or you created the character before you created the skills. If that's not the case, then that's a bug (please send detailed repro steps - I just tested it and it worked)

Let's say the skills are added as you suggest. Then you need to add all remaining Core and Weapon skills. This is easiest done by dragging and dropping a folder containing all the skills onto the character, resulting in duplicates of the Core and Weapons skills that were previously added by the Profession. If I implement your suggestion, I would also have to prevent duplicates from being added when doing like this. Currently, this is handled by built in Foundry functionality.

Currently I'm not convinced that it's really a problem worth solving, but if I change my mind I need to solve both issues.