RedReign / FoundryVTT-BetterRolls5e

A module for modifying certain sheet functions on Foundry VTT Character sheets for D&D 5th Edition.
GNU General Public License v3.0
36 stars 67 forks source link

3D dice roll don't roll on all devices with DsN enabled if canvas is off on the dice roller's settings #348

Open adacosta92 opened 2 years ago

adacosta92 commented 2 years ago

FVTT version: 0.8.9 Dice So Nice! version: 4.2.2 System: dnd5e 1.5.5 Better Rolls for 5E version: 1.6.16 midiQOL version: 0.8.85

Step to reproduce: Join on two devices. On device A, turn canvas off or turn 3d Dice from DsN off. Then roll dice from device A. no dice are rolled on Device B. Only when either BetterRolls for 5E is enabled, or when midiQOL is enabled, or both

these three modules, along with DAE & CUB, are some of the most popular 5e modules, and it would be great if low-power devices can be used just for character sheets while dice roll on the main battlemap on a tabletop TV (especially as we move back to in-person gaming)

adacosta92 commented 2 years ago

linked to midiQOL: https://gitlab.com/tposney/midi-qol/-/issues/613 linked to DsN: https://gitlab.com/riccisi/foundryvtt-dice-so-nice/-/issues/263

CarlosFdez commented 2 years ago

Are there any errors in the console when this is reproduced? There may be a canvas.ready check somewhere that I am missing.

adacosta92 commented 2 years ago

Yes, on the client side

TypeError: Cannot read properties of null (reading 'id') [Detected 3 packages: midi-qol, lib-wrapper, betterrolls5e] at processCreateBetterRollsMessage (chatMesssageHandling.js:67) at Hooks.js:204 at Function._call (eval at (listeners.js:91), :4:14) at Function.callAll (foundry.js:153) at ClientDatabaseBackend.callback (foundry.js:8777) at foundry.js:8716 at Array.map () at ClientDatabaseBackend._handleCreateDocuments (foundry.js:8716) at ClientDatabaseBackend._createDocuments (foundry.js:8612) at async Function.createDocuments (document.mjs:332) at async Function.create (document.mjs:431) at async CustomItemRoll.toMessage (custom-roll.js:850) at async Item5e.doItemRoll (itemhandling.js:400)

CarlosFdez commented 2 years ago

That stack trace isn't crashing at better rolls code, my guess is that its crashing when it moves to MidiQol.

I am not understanding this sentence: Only when either BetterRolls for 5E is enabled, or when midiQOL is enabled, or both. Do you mean that it only works in those situations, or it only reproduces in those situations?

EDIT: That said I managed to repro.

CarlosFdez commented 2 years ago

This is something Dice So Nice has to do unfortunately. Because of editable rolls, Better Rolls uses DSN's API, and their API doesn't propagate the roll if the dice are disabled on the current client.

The normal rolls work via setting the roll as part of the message's flags, which we can do for the initial roll, but it wouldn't handle the case of adding crit damage or clicking the roll damage button.

adacosta92 commented 2 years ago

Even getting it to work for the initial roll is good enough, TBH. I usually roll attacks and damage at once anyways. getting it to work for subsequent rolls would definitely require DsN to change their code, I think.

adacosta92 commented 2 years ago

When you say "setting the roll as part of the message's flags", is that something you've added or are thinking of adding to an update?