Weapon Rolling Refactor in v1.1.9 Improperly Implemented
Closed this issue · 3 comments
Describe the bug
As I understand it, weapon rolls were intended to have been significantly overhauled with the latest release. If the screenshots shown on PR69 are indeed the new expected behavior of weapon rolls, the implementation seems to have gone wrong.
The challenge dice icons in the chat are now excessively large. In previous versions as well as the PR69 screenshots, the dice are of a smaller size consistent with other dice rolls in Foundry.
The format of the screenshot shown in on PR69 is also not reflected in the current version. The character portrait and item image are both missing. The weapon qualities shown in the chat roll are also not present.
Finally, despite the item ID attribute being listed in included in the script, it is still not being included in the chat message. Instead of the item ID, the weapon roll contains "flavor text" with no usable information for Maestro or other such modules.
As far as I can tell, this is because what is being placed in the chat is not using the "generic item template" defined in roll.js, which is where data-item-id can be found, as well as the tags field which I believe should contain the weapon qualities.
To Reproduce
Steps to reproduce the behavior:
- Create actor with type: character
- Add an item under "weapons" category of character's belongings, with a damage of at least 1 challenge die
- Roll the weapon
- See result in chat messages. Inspect HTML element of the roll for confirmation of missing attributes.
Expected behavior
Weapon rolls should contain more information, including ID attributes and quality tags, as well as weapon and actor images, per the 1.1.9 release notes and screenshots shown in relevant pull requests. Presumably meant to use generic item roll rather than attribute roll.
Screenshots
Expected Weapon Roll:
Weapon Roll in 1.1.9:
Screenshot of the Weapon Roll HTML Element in 1.1.9:
At the end of the day I am acting under the assumption that the screenshots from PR69 are indeed indicative of the expected behavior in this version of the system. My programming expertise is also borderline nonexistent, so there is a good deal of supposition in this report, but hopefully it contains enough information for someone more capable than I to properly diagnose and solve the issues listed here.
Yeah that's not how it should look, this is a regression mistake pulled in from the upgrade changes. Let me see what I can do to get it back.
Yeah I've looked into this a bit more, there was a major refactor that I didn't notice had been done with the last upgrade that killed this functionality. It'll take me a bit to get it back (mostly cause I haven't looked at this code in a while) but I really want it back.
This should be fixed in the upcoming release for 1.1.10, currently is on the develop branch, the dice image is still a little large but the Id is being passed again, the properties are on the card again, and mostly appears to be returned to the older style. I'm on my laptop on a trip at the moment so I can't actually test against my normal setup but I think this should achieve the big asks here.
If there is anything else that I might have missed in your report, feel free to reply to this and/or re-open the issue and I'd be happy to take a look again! Thanks for the bug report and being so helpful with the info you gave!