search

kakaroto / Beyond20

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

Elemental spell handling #885

Closed Aeristoka closed 2 years ago

Aeristoka commented 2 years ago

@kakaroto Also thinking maybe we should switch Spirit Guardians to this method. Your thoughts?

kakaroto commented 2 years ago

@kakaroto Also thinking maybe we should switch Spirit Guardians to this method. Your thoughts?

I thought you didn't like prompts? 🙄 The issue with moving Spirit Guardians is that the roll renderer is only used with digital dice or foundry, not when using Roll20 and rolling natively. On the other hand, we do use it for the other stuff already... I like having a single way of doing things, so nice work there. I do think if we can have the query happening on the DDB side, it would be beneficial as we'd not have to worry about a query on roll20/foundry, and it would work regardless of using roll renderer or not. Also, seeing how the 5 special cases are the exact same code right now, that little bit of code could be extracted in a function that takes name and types_list and does its thing on the damages/damage_types.

kakaroto commented 2 years ago

Any update on this btw? Are you changing it to do the prompt on the DDB side or do you want me to prior to merging?

Aeristoka commented 2 years ago

Any update on this btw? Are you changing it to do the prompt on the DDB side or do you want me to prior to merging?

Feel free to if you'd like, I don't know what my availability to work on this will look like coming up. Otherwise, I'll take a look at it when I'm able to.

Aeristoka commented 2 years ago

@kakaroto ready for review now I believe