ElfFriend-DnD / foundryvtt-item-effects-to-chat-5e

A module which allows a GM to interact with effects on rolled items from chat.
MIT License
0 stars 2 forks source link

[BUG] Not working when consuming the last consumable use #5

Closed mech-tools closed 2 years ago

mech-tools commented 2 years ago

Hello,

When using the last of a consumable item and if that item has charges and "Destroy on empty" activated, the item gets destroyed on use (that's standard 5e behavior). image

Upon use, the effect is correctly printed in the chat, but the effect can't be used or attached to a token on the map. I think this is because the item no longer exists on the sheet. And apparently, the 5e system embed the item data in the chat card even if the item is destroyed, so the effect data should be retrievable.

akrigline commented 2 years ago

This one might be gnarly. The current handling in classes/chat.js which gets the effect data to apply to a token is in _onDragStart and _onClickApply.

Currently we only save the uuid of the effect in the chatmessage's flags and there is no reference to the chat message created by rolling an item because for all this module knows, it might not exist. We are powered by the Item5e.rollItem hook, which does provide the item data. If we were to rely on this other chat message's existence we would be opening ourselves up to the same problem but different (if the other chatmessage is deleted...same issue).

So I suspect we're going to have to change what we save in our chat message flags to include all of the data needed for the effect application at the time of item roll. That change will happen in classes/item.js during createListChatCard. Instead of only saving an array of effectUuids, we'll save the whole effect.toJSON(). Then we'll adjust our application methods to not lookup the uuid, but use the stored effect data.

That's not the worst thing in the world, we'll reduce our async calls by a lot and generally will move 'faster', but at the cost of storing more information in the chatmessage document.

akrigline commented 2 years ago

Done in 2.0.0. I opted to make this a breaking change but expect that the nature of this module is such that I will not need a migration.