converting advancement to dynamic list skills

system/scripts/actor.js

@@ -216,38 +216,51 @@ export class ActorL5r5e extends Actor {
      * @private
      */
     async _updateActorFromAdvancement(item, isAdd) {
-        if (item && item.type === "advancement") {
+        if (!item || item.type !== "advancement") {
-            const actor = foundry.utils.duplicate(this.system);
+            return;
-            const itemData = item.system;
+        }
-            if (itemData.advancement_type === "ring") {
+
-                // Ring
+        const actorSystem = foundry.utils.duplicate(this.system);
-                if (isAdd) {
+        const itemData = item.system;
-                    actor.rings[itemData.ring] = Math.min(9, actor.rings[itemData.ring] + 1);
+        if (itemData.advancement_type === "ring") {
-                } else {
+            // Ring
-                    actor.rings[itemData.ring] = Math.max(1, actor.rings[itemData.ring] - 1);
+            if (isAdd) {
-                }
+                actorSystem.rings[itemData.ring] = Math.min(9, actorSystem.rings[itemData.ring] + 1);
             } else {
-                // Skill
+                actorSystem.rings[itemData.ring] = Math.max(1, actorSystem.rings[itemData.ring] - 1);
-                const skillCatId = CONFIG.l5r5e.skills.get(itemData.skill);
-                if (skillCatId) {
-                    if (isAdd) {
-                        actor.skills[skillCatId][itemData.skill] = Math.min(
-                            9,
-                            actor.skills[skillCatId][itemData.skill] + 1
-                        );
-                    } else {
-                        actor.skills[skillCatId][itemData.skill] = Math.max(
-                            0,
-                            actor.skills[skillCatId][itemData.skill] - 1
-                        );
-                    }
-                }
             }
 
             // Update Actor
             await this.update({
-                system: foundry.utils.diffObject(this.system, actor),
+                system: foundry.utils.diffObject(this.system, actorSystem),
             });
+
+        } else {
+            // Skill
+            let skillItem = await fromUuid(itemData.skill); // Skill itemUuid
+            if (!skillItem) {
+                console.warn("L5R5E | Unknown skill item uuid", itemData.skill);
+                return;
+            }
+            // Out of actor item ?
+            if (!skillItem.actor || skillItem.actor._id !== this._id) {
+                const checkItem = this.items.getName(skillItem.name);
+                if (checkItem) {
+                    skillItem = checkItem;
+                } else {
+                    throw new Error(`Unable to find "${skillItem.name}" on this actor`);
+                }
+            }
+
+            const newRank = isAdd
+                ? Math.min(9, skillItem.system.rank + 1)
+                : Math.max(0, skillItem.system.rank - 1);
+            if (skillItem.system.rank === newRank) {
+                return;
+            }
+
+            // Update Item
+            await skillItem.update({ "system.rank": newRank });
         }
     }
 

system/scripts/actors/base-character-sheet.js

@@ -133,146 +133,151 @@ export class BaseCharacterSheetL5r5e extends BaseSheetL5r5e {
      * @param {DragEvent} event
      */
     async _onDrop(event) {
-        // *** Everything below here is only needed if the sheet is editable ***
+        try {
-        if (!this.isEditable || this.actor.system.soft_locked) {
+            // *** Everything below here is only needed if the sheet is editable ***
-            console.log("L5R5E | This sheet is not editable");
+            if (!this.isEditable || this.actor.system.soft_locked) {
-            return;
+                console.log("L5R5E | This sheet is not editable");
-        }
-
-        // Check item type and subtype
-        const item = await game.l5r5e.HelpersL5r5e.getDragnDropTargetObject(event);
-        if (!item || !["Item", "JournalEntry"].includes(item.documentName) || item.type === "property") {
-            console.log(`L5R5E | Wrong subtype ${item?.type}`, item);
-            return;
-        }
-
-        // Specific curriculum journal drop
-        if (item.documentName === "JournalEntry") {
-            // npc does not have this
-            if (!this.actor.system.identity?.school_curriculum_journal) {
-                console.log("L5R5E | NPC won't go to school :'(");
-                return;
-            }
-            this.actor.system.identity.school_curriculum_journal = {
-                id: item._id,
-                name: item.name,
-                pack: item.pack || null,
-            };
-            await this.actor.update({
-                system: {
-                    identity: {
-                        school_curriculum_journal: this.actor.system.identity.school_curriculum_journal,
-                    },
-                },
-            });
-            return;
-        }
-
-        // Dropped an item with same "id" as one owned
-        if (this.actor.items) {
-            // Exit if we already owned exactly this id (drag a personal item on our own sheet)
-            if (
-                this.actor.items.some((embedItem) => {
-                    // Search in children
-                    if (embedItem.items instanceof Map && embedItem.items.has(item._id)) {
-                        return true;
-                    }
-                    return embedItem._id === item._id;
-                })
-            ) {
-                console.log("L5R5E | This element has been ignored because it already exists in this actor", item.uuid);
                 return;
             }
 
-            // Add quantity instead if they have (id is different so use type and name)
+            // Check item type and subtype
-            if (item.system.quantity) {
+            const item = await game.l5r5e.HelpersL5r5e.getDragnDropTargetObject(event);
-                const tmpItem = this.actor.items.find(
+            if (!item || !["Item", "JournalEntry"].includes(item.documentName) || item.type === "property") {
-                    (embedItem) => embedItem.name === item.name && embedItem.type === item.type
+                console.log(`L5R5E | Wrong subtype ${item?.type}`, item);
-                );
+                return;
-                if (tmpItem && this._modifyQuantity(tmpItem.id, 1)) {
+            }
+
+            // Specific curriculum journal drop
+            if (item.documentName === "JournalEntry") {
+                // npc does not have this
+                if (!this.actor.system.identity?.school_curriculum_journal) {
+                    console.log("L5R5E | NPC won't go to school :'(");
                     return;
                 }
-            }
+                this.actor.system.identity.school_curriculum_journal = {
-        }
+                    id: item._id,
-
+                    name: item.name,
-        // Can add the item - Foundry override cause props
+                    pack: item.pack || null,
-        const allowed = Hooks.call("dropActorSheetData", this.actor, this, item);
+                };
-        if (allowed === false) {
+                await this.actor.update({
-            return;
+                    system: {
-        }
+                        identity: {
-
+                            school_curriculum_journal: this.actor.system.identity.school_curriculum_journal,
-        let itemData = item.toObject(true);
+                        },
-
+                    },
-        // Item subtype specific
+                });
-        switch (itemData.type) {
-            case "army_cohort":
-            case "army_fortification":
-                console.warn("L5R5E | Army items are not allowed", item?.type, item);
                 return;
+            }
 
-            case "advancement":
+            // Dropped an item with same "id" as one owned
-                // Specific advancements, remove 1 to selected ring/skill
+            if (this.actor.items) {
-                await this.actor.addBonus(item);
+                // Exit if we already owned exactly this id (drag a personal item on our own sheet)
-                break;
+                if (
-
+                    this.actor.items.some((embedItem) => {
-            case "title":
+                        // Search in children
-                // Generate new Ids for the embed items
+                        if (embedItem.items instanceof Map && embedItem.items.has(item._id)) {
-                await item.generateNewIdsForAllEmbedItems();
+                            return true;
-
+                        }
-                // Add embed advancements bonus
+                        return embedItem._id === item._id;
-                for (let [embedId, embedItem] of item.system.items) {
+                    })
-                    if (embedItem.type === "advancement") {
+                ) {
-                        await this.actor.addBonus(embedItem);
+                    console.log("L5R5E | This element has been ignored because it already exists in this actor", item.uuid);
-                    }
+                    return;
                 }
 
-                // refresh data
+                // Add quantity instead if they have (id is different so use type and name)
-                itemData = item.toObject(true);
+                if (item.system.quantity) {
-                break;
+                    const tmpItem = this.actor.items.find(
-
+                        (embedItem) => embedItem.name === item.name && embedItem.type === item.type
-            case "skill":
+                    );
-                itemData.system.rank = 0;
+                    if (tmpItem && this._modifyQuantity(tmpItem.id, 1)) {
-                itemData.system.modifier = 0;
-                break;
-
-            case "technique":
-                // School_ability and mastery_ability, allow only 1 per type
-                if (CONFIG.l5r5e.techniques.get(itemData.system.technique_type)?.type === "school") {
-                    if (
-                        Array.from(this.actor.items).some((e) => {
-                            return e.type === "technique" && e.system.technique_type === itemData.system.technique_type;
-                        })
-                    ) {
-                        ui.notifications.info(game.i18n.localize("l5r5e.techniques.only_one"));
                         return;
                     }
-
-                    // No cost for schools
-                    itemData.system.xp_cost = 0;
-                    itemData.system.xp_used = 0;
-                    itemData.system.in_curriculum = true;
-                } else {
-                    // Check if technique is allowed for this character
-                    // if (!game.user.isGM && !this.actor.system.techniques[itemData.system.technique_type]) {
-                    //     ui.notifications.info(game.i18n.localize("l5r5e.techniques.not_allowed"));
-                    //     return;
-                    // }
-
-                    // Verify cost
-                    itemData.system.xp_cost =
-                        itemData.system.xp_cost > 0 ? itemData.system.xp_cost : CONFIG.l5r5e.xp.techniqueCost;
-                    itemData.system.xp_used = itemData.system.xp_cost;
                 }
-                break;
+            }
-        }
 
-        // Modify the bought at rank to the current actor rank
+            // Can add the item - Foundry override cause props
-        if (itemData.system.bought_at_rank !== undefined && this.actor.system.identity?.school_rank) {
+            const allowed = Hooks.call("dropActorSheetData", this.actor, this, item);
-            itemData.system.bought_at_rank = this.actor.system.identity.school_rank;
+            if (allowed === false) {
-        }
+                return;
+            }
 
-        // Finally create the embed
+            let itemData = item.toObject(true);
-        return this.actor.createEmbeddedDocuments("Item", [itemData]);
+
+            // Item subtype specific
+            switch (itemData.type) {
+                case "army_cohort":
+                case "army_fortification":
+                    console.warn("L5R5E | Army items are not allowed", item?.type, item);
+                    return;
+
+                case "advancement":
+                    // Specific advancements, add x to selected ring/skill
+                    await this.actor.addBonus(item);
+                    break;
+
+                case "title":
+                    // Generate new Ids for the embed items
+                    await item.generateNewIdsForAllEmbedItems();
+
+                    // Add embed advancements bonus
+                    for (let [embedId, embedItem] of item.system.items) {
+                        if (embedItem.type === "advancement") {
+                            await this.actor.addBonus(embedItem);
+                        }
+                    }
+
+                    // refresh data
+                    itemData = item.toObject(true);
+                    break;
+
+                case "skill":
+                    itemData.system.rank = 0;
+                    itemData.system.modifier = 0;
+                    break;
+
+                case "technique":
+                    // School_ability and mastery_ability, allow only 1 per type
+                    if (CONFIG.l5r5e.techniques.get(itemData.system.technique_type)?.type === "school") {
+                        if (
+                            Array.from(this.actor.items).some((e) => {
+                                return e.type === "technique" && e.system.technique_type === itemData.system.technique_type;
+                            })
+                        ) {
+                            ui.notifications.info(game.i18n.localize("l5r5e.techniques.only_one"));
+                            return;
+                        }
+
+                        // No cost for schools
+                        itemData.system.xp_cost = 0;
+                        itemData.system.xp_used = 0;
+                        itemData.system.in_curriculum = true;
+                    } else {
+                        // Check if technique is allowed for this character
+                        // if (!game.user.isGM && !this.actor.system.techniques[itemData.system.technique_type]) {
+                        //     ui.notifications.info(game.i18n.localize("l5r5e.techniques.not_allowed"));
+                        //     return;
+                        // }
+
+                        // Verify cost
+                        itemData.system.xp_cost =
+                            itemData.system.xp_cost > 0 ? itemData.system.xp_cost : CONFIG.l5r5e.xp.techniqueCost;
+                        itemData.system.xp_used = itemData.system.xp_cost;
+                    }
+                    break;
+            }
+
+            // Modify the bought at rank to the current actor rank
+            if (itemData.system.bought_at_rank !== undefined && this.actor.system.identity?.school_rank) {
+                itemData.system.bought_at_rank = this.actor.system.identity.school_rank;
+            }
+
+            // Finally create the embed
+            return this.actor.createEmbeddedDocuments("Item", [itemData]);
+
+        } catch (ex) {
+            console.warn("L5R5E |", ex.message);
+        }
     }
 
     /** @inheritdoc */

system/scripts/config.js

@@ -47,7 +47,7 @@ L5R5E.techniques.set("title_ability", { type: "title", displayInTypes: false });
 L5R5E.techniques.set("specificity", { type: "custom", displayInTypes: false });
 
 // *** SkillId - CategoryId ***
-// TODO @deprecated hardcoded skills
+// Hardcoded skills are still required for compatibility (migration & olds worlds)
 L5R5E.skills = new Map();
 L5R5E.skills.set("aesthetics", "artisan");
 L5R5E.skills.set("composition", "artisan");

system/scripts/helpers.js

@@ -90,7 +90,7 @@ export class HelpersL5r5e {
      * @return {{cat: any, id: any, label: *}[]}
      */
     static getSkillsList(useGroup = false) {
-        console.warn('@deprecated hardcoded skills - helpers.getSkillsList()'); // TODO @deprecated hardcoded skills
+        console.warn('@deprecated hardcoded skills - helpers.getSkillsList() - Use getSkillsItemsList() + splitSkillByCategory() instead'); // TODO @deprecated hardcoded skills
 
         if (!useGroup) {
             return Array.from(CONFIG.l5r5e.skills).map(([id, cat]) => ({

system/scripts/item.js

@@ -242,7 +242,11 @@ export class ItemL5r5e extends Item {
         if (addBonusToActor) {
             const actor = this.actor;
             if (item instanceof Item && actor instanceof Actor) {
-                actor.addBonus(item);
+                try {
+                    await actor.addBonus(item);
+                } catch (ex) {
+                    console.warn("L5R5E |", ex.message);
+                }
             }
         }
 

system/scripts/items/advancement-sheet.js

@@ -24,7 +24,7 @@ export class AdvancementSheetL5r5e extends ItemSheetL5r5e {
         const sheetData = await super.getData(options);
 
         sheetData.data.subTypesList = AdvancementSheetL5r5e.types;
-        sheetData.data.skillsList = game.l5r5e.HelpersL5r5e.getSkillsList(true);
+        sheetData.data.skillsList = game.l5r5e.HelpersL5r5e.splitSkillByCategory(await game.l5r5e.HelpersL5r5e.getSkillsItemsList(this.actor));
 
         return sheetData;
     }
@@ -82,33 +82,45 @@ export class AdvancementSheetL5r5e extends ItemSheetL5r5e {
         let name = this.object.name;
         let img = this.object.img;
 
+        const getLocalItemByUuid = async (uuid) => {
+            const item = await fromUuid(uuid);
+            if (!item) {
+                return null;
+            }
+            if (item?.actor?._id === this.actor._id) {
+                return item;
+            }
+            return this.items.getName(item.name);
+        }
+        const skillItemNew = newChoice.skill ? (await getLocalItemByUuid(newChoice.skill)) : null;
+        const skillItemOld = oldChoice.skill ? (await getLocalItemByUuid(oldChoice.skill)) : null;
+
         // Modify image to reflect choice
         if (newChoice.ring) {
             name = game.i18n.localize(`l5r5e.rings.${newChoice.ring}`) + "+1";
             img = `systems/l5r5e/assets/icons/rings/${newChoice.ring}.svg`;
-        } else if (newChoice.skill) {
+
-            name =
+        } else if (newChoice.skill && skillItemNew) {
-                game.i18n.localize(`l5r5e.skills.${CONFIG.l5r5e.skills.get(newChoice.skill)}.${newChoice.skill}`) +
+            name = skillItemNew.name +"+1";
-                "+1";
             img = `systems/l5r5e/assets/dices/default/skill_blank.svg`;
         }
 
         // Object embed in actor ?
         const actor = this.document.actor;
         if (actor) {
+            if (newChoice.skill && !skillItemNew.actor) {
+                ui.notifications.warn(`Unable to find "${skillItemNew?.name}" on this actor`);
+                return;
+            }
+
             const actorData = foundry.utils.duplicate(actor.system);
-            let skillCatId = null;
 
             // Old choices
             if (oldChoice.ring) {
                 actorData.rings[oldChoice.ring] = Math.max(1, actorData.rings[oldChoice.ring] - 1);
             }
-            if (oldChoice.skill) {
+            if (oldChoice.skill && skillItemOld) {
-                skillCatId = CONFIG.l5r5e.skills.get(oldChoice.skill);
+                await skillItemOld.update({ "system.rank": Math.max(0, skillItemOld.system.rank - 1) });
-                actorData.skills[skillCatId][oldChoice.skill] = Math.max(
-                    0,
-                    actorData.skills[skillCatId][oldChoice.skill] - 1
-                );
             }
 
             // new choices
@@ -119,15 +131,11 @@ export class AdvancementSheetL5r5e extends ItemSheetL5r5e {
                     game.i18n.localize(`l5r5e.rings.${newChoice.ring}`) +
                     ` +1 (${actorData.rings[newChoice.ring] - 1} -> ${actorData.rings[newChoice.ring]})`;
             }
-            if (newChoice.skill) {
+            if (newChoice.skill && skillItemNew) {
-                skillCatId = CONFIG.l5r5e.skills.get(newChoice.skill);
+                const newRank = Math.min(9, skillItemNew.system.rank + 1);
-                actorData.skills[skillCatId][newChoice.skill] = actorData.skills[skillCatId][newChoice.skill] + 1;
+                await skillItemNew.update({ "system.rank": newRank });
-                xp_used = actorData.skills[skillCatId][newChoice.skill] * CONFIG.l5r5e.xp.skillCostMultiplier;
+                xp_used = newRank * CONFIG.l5r5e.xp.skillCostMultiplier;
-                name =
+                name = `${skillItemNew.name} +1 (${newRank - 1} -> ${newRank})`;
-                    game.i18n.localize(`l5r5e.skills.${skillCatId}.${newChoice.skill}`) +
-                    ` +1 (${actorData.skills[skillCatId][newChoice.skill] - 1} -> ${
-                        actorData.skills[skillCatId][newChoice.skill]
-                    })`;
             }
 
             // Update Actor
@@ -141,7 +149,7 @@ export class AdvancementSheetL5r5e extends ItemSheetL5r5e {
             name: name,
             img: img,
             system: {
-                xp_used: xp_used,
+                xp_used,
             },
         });
 

system/scripts/items/technique-sheet.js

@@ -160,18 +160,6 @@ export class TechniqueSheetL5r5e extends ItemSheetL5r5e {
             }
         });
 
-        // List skill (not include in cat)
+        return [...unqCatList];
-        const unqSkillList = new Set();
-        skillList.forEach((s) => {
-            s = s?.trim();
-            if (!!s && CONFIG.l5r5e.skills.has(s)) {
-                const cat = CONFIG.l5r5e.skills.get(s);
-                if (!unqCatList.has(cat)) {
-                    unqSkillList.add(s);
-                }
-            }
-        });
-
-        return [...unqCatList, ...unqSkillList];
     }
 }

system/templates/items/advancement/advancement-sheet.html

@@ -32,7 +32,7 @@
                         {{#each data.skillsList as |skills catId|}}
                         <optgroup label="{{localizeSkill catId 'title'}}">
                             {{#each skills as |obj|}}
-                            <option value="{{obj.id}}">{{obj.label}}</option>
+                            <option value="{{obj.uuid}}">{{obj.name}}</option>
                             {{/each}}
                         </optgroup>
                         {{/each}}