kakaroto / Beyond20

D&D Beyond Character Sheet Integration in Roll20
GNU General Public License v3.0
497 stars 145 forks source link

Fix Beyond20 Compatibility with FoundryVTT v10 #1016

Closed Aeristoka closed 2 years ago

Aeristoka commented 2 years ago

image

image

SlothManJ commented 2 years ago

Having the same issue. Noting that the "onRenderChatMessage" errors only appears to occur for d20 rolls. However the broken hover display occurs for all beyond20 rolls.

Aeristoka commented 2 years ago

Seems to be made worse (especially the crazy CSS mess I posted above) with Dice So Nice! enabled

Conversation here: https://discord.com/channels/446863265545453588/646064614613975040/1021832206789906563

kakaroto commented 2 years ago

Not finding an issue with v10 compatibility itself for the display of the rolls, but it does look like it's a Dice-So-Nice issue which we have no ways of fixing, as they seem to add a style="display: block;" onto the HTML itself. Refreshing the page fixes it, which means it's nothing in the html itself we send, but rather it's DSN's method of hiding the results until the 3d dice have finished rolling. image I'll file an issue on DSN's repo as it seems to be something they need to fix from their side.

Still need to fix the onRenderChatMessage error though

martinlemahieu commented 2 years ago

Until DSN fixes it, I'm using this quick css fix that I've shared with other players in my current campaign. It just needs to be loaded (for exemple with an extension like Stylebot for chrome) for the url of your FoundryVTT instance :

.beyond20-tooltip-content {
  display: none !important;
}

*:hover > .beyond20-tooltip-content {
  display: block !important;
}

It's a simple & dirty fix but I hope it will help some of you until a clean fix comes from DSN dev, if you want to keep the result hidden until dice animation finishes.

Emi-T commented 2 years ago

It is a good solution @martinlemahieu. A simple way to fix it is adding your code css at the bottom of this file in your beyond20 module folder: /foundrydata/Data/modules/beyond20/beyond20.css

.beyond20-tooltip-content {
  display: none !important;
}

*:hover > .beyond20-tooltip-content {
  display: block !important;
}
Aeristoka commented 2 years ago

It is a good solution @martinlemahieu. A simple way to fix it is adding your code css at the bottom of this file in your beyond20 module folder: /foundrydata/Data/modules/beyond20/beyond20.css

.beyond20-tooltip-content {
  display: none !important;
}

*:hover > .beyond20-tooltip-content {
  display: block !important;
}

@kakaroto that actually raises an interesting question, do we want to include this for 2.8.0 so that we're not reliant on Dice So Nice releasing a fix, or having to tell people to change a DSN setting to have Beyond20 rolls look proper?

Aeristoka commented 2 years ago

And just as I suggest that, I see a Dice So Nice Update, that fixes the issue 😄

https://gitlab.com/riccisi/foundryvtt-dice-so-nice/-/commit/b9290567fbb7f75dfc2f0f259d4ff3a0d299a9e3

kakaroto commented 2 years ago

@kakaroto that actually raises an interesting question, do we want to include this for 2.8.0 so that we're not reliant on Dice So Nice releasing a fix, or having to tell people to change a DSN setting to have Beyond20 rolls look proper?

No, I didn't want to include it because it's a hack to workaround a temporary problem. If DSN had said they weren't going to fix it, I might have considered it though, but it would have been useless unless we made two releases of B20, one with the fix, and then one to remove it. (Also, I was told to never use !important 😱 ) Closing since DSN released the fix! 🥳

Aeristoka commented 2 years ago

Not finding an issue with v10 compatibility itself for the display of the rolls, but it does look like it's a Dice-So-Nice issue which we have no ways of fixing, as they seem to add a style="display: block;" onto the HTML itself. Refreshing the page fixes it, which means it's nothing in the html itself we send, but rather it's DSN's method of hiding the results until the 3d dice have finished rolling. image I'll file an issue on DSN's repo as it seems to be something they need to fix from their side.

Still need to fix the onRenderChatMessage error though

Did you fix the onRenderChatMessage part? 🤔 I can't recall

kakaroto commented 2 years ago

Did you fix the onRenderChatMessage part? 🤔 I can't recall

Yeah, I don't see it anymore, it got fixed with https://github.com/kakaroto/Beyond20/commit/74e6dcbb5ab6177949dcd94dc954771c67e390d8