Error selecting item grants during advancement.

[mark-clanfalconer]

Steps to Reproduce

1. Create a playbook

2. Add an item grant type 'Advancement', level 0, no repeat
   2a. Add some move items to the item grant

3. Add a second item grant, type 'Advancement', level 1, repeating
   3a. Add some move items to the grant, some of which are in the other item grant.

4. Create an actor.

5. Select the created playbook.

6. On the item grant dialog. select one of the moves and choose OK.

7. Advance the actor.

8. On the item grant dialog, select one of the moves and choose OK.

Expected behaviour

On the initial selection, a move is added to the actor
On advancement, a move is added to the actor

Actual behaviour

On the initial selection, a move is added to the actor
On advancement, a selected move is not added to the actor.
The console shows

item.js:425 Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'localeCompare')
    at _sortItemAdvancement (item.js:425:17)
    at Array.sort (<anonymous>)
    at item.js:414:49
    at Array.forEach (<anonymous>)
    at ItemPbta._preUpdate (item.js:412:31)
    at #preUpdateDocumentArray (foundry.mjs:58740:39)
    at async ClientDatabaseBackend._updateDocuments (foundry.mjs:58692:5)

Added a zip of a compendium with a playbook that exhibits the problem.
testadvancement.zip

[mark-clanfalconer]

	async _preUpdate(changed, options, user) {
		if (this.type === "playbook") {
			if (Object.keys(changed?.system?.choiceSets ?? {}).length) {
				if (!Array.isArray(changed.system.choiceSets)) {
					changed.system.choiceSets = Object.values(changed.system.choiceSets);
				}
				changed.system.choiceSets.forEach((cs) => {
					if (cs.choices && Object.keys(cs.choices).length) {
						if (Array.isArray(cs.choices)) cs.choices.sort(this._sortItemAdvancement); // <== this line...
						else cs.choices = Object.values(cs.choices).sort(this._sortItemAdvancement);
					}
				});
			}
		}
		await super._preUpdate(changed, options, user);
	}

	_sortItemAdvancement(a, b) {
		if (a.advancement - b.advancement) return a.advancement - b.advancement;
		return a.name.localeCompare(b.name); // <== ...gets to this line, which fails
	}

I have checked in my compendium and all the moves have a name property

[mark-clanfalconer]

Adding debug to the code (item.js)

	async _preUpdate(changed, options, user) {
		if (this.type === "playbook") {
			if (Object.keys(changed?.system?.choiceSets ?? {}).length) {
				if (!Array.isArray(changed.system.choiceSets)) {
					changed.system.choiceSets = Object.values(changed.system.choiceSets);
				}
				changed.system.choiceSets.forEach((cs) => {
					if (cs.choices && Object.keys(cs.choices).length) {
                		                console.debug(`==PRE_UPDATE== | ${JSON.stringify(cs)}`);
						if (Array.isArray(cs.choices)) cs.choices.sort(this._sortItemAdvancement);
						else cs.choices = Object.values(cs.choices).sort(this._sortItemAdvancement);
					}
				});
			}
		}
		await super._preUpdate(changed, options, user);
	}

	_sortItemAdvancement(a, b) {
		console.debug(`==SORT_ITEMS== | ${JSON.stringify(a)}:${JSON.stringify(b)}`);
		if (a.advancement - b.advancement) return a.advancement - b.advancement;
		return a.name.localeCompare(b.name);
	}

Gives

==PRE_UPDATE==

{
    "title": "Choice on creation",
    "desc": "Pick 2:",
    "type": "multi",
    "repeatable": false,
    "choices": [
        {
            "uuid": "Compendium.world.testadvancement.Item.10RLdiRaqX6g5jhC",
            "img": "icons/svg/aura.svg",
            "granted": true,
            "advancement": 0
        },
        {
            "uuid": "Compendium.world.testadvancement.Item.HmN7CjJivMNTS3ej",
            "img": "icons/svg/aura.svg",
            "granted": false,
            "advancement": 0
        }
    ],
    "grantOn": 1,
    "granted": true,
    "advancement": 0
}

==SORT_ITEMS==

{
    "uuid": "Compendium.world.testadvancement.Item.HmN7CjJivMNTS3ej",
    "img": "icons/svg/aura.svg",
    "granted": false,
    "advancement": 0
}

{
    "uuid": "Compendium.world.testadvancement.Item.10RLdiRaqX6g5jhC",
    "img": "icons/svg/aura.svg",
    "granted": true,
    "advancement": 0
}

so the update is not including the items names.

Not sure what this pre-update is trying to do though.

[mark-clanfalconer]

When a character is created and the playbook is selected, item.js async handleChoices(data) is returning

[
    {
        "title": "Choice on creation",
        "desc": "Pick 2:",
        "grantOn": 1,
        "advancement": 0,
        "repeatable": false,
        "choices": [
            {
                "uuid": "Compendium.world.testadvancement.Item.10RLdiRaqX6g5jhC",
                "img": "icons/svg/aura.svg",
                "granted": true,
                "advancement": 0,
                "name": "Move1",
                "desc": ""
            },
            {
                "uuid": "Compendium.world.testadvancement.Item.HmN7CjJivMNTS3ej",
                "img": "icons/svg/aura.svg",
                "granted": false,
                "advancement": 0,
                "name": "Move2",
                "desc": ""
            }
        ],
        "type": "multi",
        "granted": true
    },
    {
        "title": "Different choices on advance",
        "desc": "Increase a stat or pick 1:",
        "grantOn": 1,
        "advancement": 1,
        "repeatable": true,
        "choices": [
            {
                "uuid": "Compendium.world.testadvancement.Item.HmN7CjJivMNTS3ej",
                "img": "icons/svg/aura.svg",
                "granted": false,
                "advancement": 0
            },
            {
                "uuid": "Compendium.world.testadvancement.Item.MkLMIBp0idhcMnIC",
                "img": "icons/svg/aura.svg",
                "granted": false,
                "advancement": 0
            }
        ],
        "type": "multi",
        "granted": false
    }
]

but is not calling _preUpdate

When choosing the first advancement, handleChoices returns

[
    {
        "title": "Choice on creation",
        "desc": "Pick 2:",
        "type": "multi",
        "repeatable": false,
        "choices": [
            {
                "uuid": "Compendium.world.testadvancement.Item.10RLdiRaqX6g5jhC",
                "img": "icons/svg/aura.svg",
                "granted": true,
                "advancement": 0
            },
            {
                "uuid": "Compendium.world.testadvancement.Item.HmN7CjJivMNTS3ej",
                "img": "icons/svg/aura.svg",
                "granted": false,
                "advancement": 0
            }
        ],
        "grantOn": 1,
        "granted": true,
        "advancement": 0
    },
    {
        "title": "Different choices on advance",
        "desc": "Increase a stat or pick 1:",
        "type": "multi",
        "repeatable": true,
        "choices": [
            {
                "uuid": "Compendium.world.testadvancement.Item.HmN7CjJivMNTS3ej",
                "img": "icons/svg/aura.svg",
                "granted": true,
                "advancement": 0,
                "name": "Move2",
                "desc": ""
            },
            {
                "uuid": "Compendium.world.testadvancement.Item.MkLMIBp0idhcMnIC",
                "img": "icons/svg/aura.svg",
                "granted": false,
                "advancement": 0,
                "name": "Move3",
                "desc": ""
            }
        ],
        "grantOn": 1,
        "granted": true,
        "advancement": 1
    }
]

and calls _preUpdate, which fails on the first choiceset

[mark-clanfalconer]

-preUpdate is being called to update the embedded playbook itself, when the choices get fully populated, and to modify the embedded playbooks choicesets 'granted' status when the choices dialog is being shown, when they don't always get fully populated.

[mark-clanfalconer]

The problem is

1. Choice sets (Item grants) can be edited on a playbook item. I have not explored this but assume this fully populates choices. This is where the _preUpdate is trying to sort the edited choices to put them in order (I think)
2. You can also edit choice sets on an playbook contained in an actor. I assume this works the same as editing a playbook item above, and calls _preUpdate with fully populated choices.
3. Choice sets can have their 'granted' status changed in response to selecting a playbook for an actor. In this case, the actors contained playbook is being created (to add to the actors items collection) while the dialog is shown, so _preUpdate is not called.
4. When an actor is advanced, the contained playbook is updated to mark choice sets and choices as 'granted'. In this case, choices may not be fully populated, and _preUpdate fails.

Item.js, line 324

	async handleChoices(data) {
		if (data.system?.choiceSets?.length > 0) {
			for (const choiceSet of data.system.choiceSets) {
				const { advancement, choices, desc, granted, repeatable, title } = choiceSet;
				// ==== this line skips population, breaking _preUpdate ====
				if (advancement > this.parent.advancements || (granted && !repeatable)) continue; 
				const validChoices = (await Promise.all(
					choices.map(async (c) => {
						const item = await fromUuid(c.uuid);
						c.name = item.name;
						c.tags = item.system.tags;
						c.desc = await foundry.applications.ux.TextEditor.implementation.enrichHTML(item.system.description, {
							secrets: this.actor.isOwner,
							rollData: this.actor.getRollData(),
							relativeTo: this.actor
						});

[mark-clanfalconer]

I suggest Item.js _preUpdate is changed to

	async _preUpdate(changed, options, user) {
		if (this.type === "playbook") {
			if (Object.keys(changed?.system?.choiceSets ?? {}).length) {
				if (!Array.isArray(changed.system.choiceSets)) {
					changed.system.choiceSets = Object.values(changed.system.choiceSets);
				}
				changed.system.choiceSets.forEach((cs) => {
					if (cs.choices && Object.keys(cs.choices).length) {
						if (!Array.isArray(cs.choices)) {
							cs.choices = Object.values(cs.choices);
						}
					    // choiceSets can be updated on a playbook, when they are fully populated and we want to sort them,
						// or choiceSets 'granted' properties can be modified when a choice dialog has been shown,
						// where they might not be fully populated, but we don't need to sort them again.
						if (cs.choices[0].name) cs.choices.sort(this._sortItemAdvancement)
					}
				});
			}
		}
		await super._preUpdate(changed, options, user);
	}

[mark-clanfalconer]

Probably a duplicate of #273