[Request] Perform check for presence of .rolls data before attempting to enrich chat message

[daft-develop]

On the following lines, a chat message may get enriched (_enrichRollTooltip) if the "dice-tooltip" style class is part of the message content. As currently written, I believe it assumes all messages with the dice-tooltip class were generated by a Roll.toMessage() style call, which always includes a .rolls element. Any chat message containing the "dice-tooltip" without a .rolls element can result in an error during the _enrichRollTooltip call.

https://github.com/foundryvtt/dnd5e/blob/9508709e861aa5f9da48d2c4745a7f771fff2e47/module/documents/chat-message.mjs#L251-L255

I have a use case where it would be great to leverage the 5e and system formatting styles, but I don't generate the chat card via a .toMessage() call - I'm aggregating a bunch of rolls to a single chat message and presenting them to be visibly similar to a single attack. The rolls leading up to this message wouldn't make sense to represent like a single combined roll, so I'd rather craft the chat card a little more manually but retaining the style for users and ease of maintenance.

• If the 5e styles are meant to remain internal use only (e.g., not used by modules) please let me know and I'll create custom styles for the module to avoid the conflict.
• If you'd like, I can propose a PR for the change, checking for the presence of .rolls or the 5e roll flag either in the code segment before the _enrichRollTooltip call, or within the _enrichRollTooltip method. Please let me know which branch would be the preferred base if so.

[Handyfon]

I've stumbled on the same issue and can confirm multiple similar usecases.

[Fyorl]

I'm having some trouble understanding this issue. Is there an example code snippet or example chat message that makes it clearer?

[daft-develop]

I've written up a minimal reproducible macro/script below:

const messageText = `
Look at all my cool dice!
<div class="dice-tooltip">
  <section class="tooltip-part">
    <div class="dice">
      <ol class="dice-rolls">
        <li class="roll die d20 success">20</li>
        <li class="roll die d20 failure">1</li>
        <li class="roll die d20 discarded">5</li>
        <li class="roll die d20">10</li>
      </ol>
    </div>
  </section>
</div>
`;

const chatData = {
  user: game.user.id,
  speaker: {alias: "dice-tooltip class usage example"},
  content: messageText
};

await ChatMessage.create(chatData, {});

If you run this in D&D 5e system 2.x or earlier, it'll parse out and you get something like the following:

If you try the same macro in D&D 5e system 3.x or higher, this will cause an exception since the chat message a) has the dice-tooltip class in the content, so it passes line 253 in the OP snippet b) has an empty this.rolls element, since the message creation doesn't include one, which will throw an error inside the _enrichRollTooltip() call.

With a standard Roll.toMessage (i.e., await new Roll("1d20").toMessage();) then I believe the toMessage method incorprates that .rolls element in the chat before creating it. So this error only occurs when someone is deliberately building a custom message without a Roll, using the dice-tooltip class.

If I mock out a custom dice-tooltip class and all other downstream styles to bypass the check on line 253, we get the more expected output below for 3.x:

The above example is pretty superficial, but I encountered in maintaining the Mob Attack Tool module. In this module, I'll aggregate a bunch of attack rolls "behind the scene" and summarize it in a single card. I'd like to keep a style closer to the standard Roll cards, re-using style classes to avoid mirroring with custom identical classes. In this instance, passing a stack of rolls to the card wouldn't be helpful, since the _enrichRollTooltip() call isn't meaningful for this use case.

If @Handyfon has also encountered this, it's possible other people are trying to use the dice-tooltip class outside of the normal Roll.toMessage workflow, and it may be non-obvious to others that right now using dice-tooltip implies the chat message needs to have a populated rolls element.

Hope that helps illustrate the issue, let me know if you need any other details.

[Fyorl]

I think I understand now, thank you for clarifying.