Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added hook to influence dice rolls #265

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

IrmatDen
Copy link

@IrmatDen IrmatDen commented Aug 3, 2022

Hello,

I wanted to implement the dnd5e Lucky feat for one of my players. I'm doing this using world scripts, and for midi-qol, it was relatively smooth sailing when I started to understand how both midi & foundry's APIs works.
For your module I had a harder time, since MTB provides no hook before commiting the roll to the request. Also, I need the hook to be async-based as I'm displaying a dialog box for the player to choose to use a lucky charge or not and another to choose the final roll.

This PR is the result of what I had to modify (tested only on the 1.0.68 version) to make it works. You'll see that I had to grab an helper method from midi-qol repository. I'm not sure what's the stance regarding this in the foundry community. It might be frown upon, but I don't know how to implement it any other way (or at all for that matter!). If you know any common method that's made to be used in every module let me know. Otherwise I'll open a ticket on midi-qol's repository asking for permission to use this snippet in this PR.

Also, please let me know if you want another name for the new hook.

@IrmatDen
Copy link
Author

IrmatDen commented Aug 7, 2022

I've noticed an issue with the async handler when awaiting too long (longer than 'Dice so nice' dice's take to roll): the result field remains set to "...". The final roll appears after clicking on it. I'll look into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant