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
37 stars 67 forks source link

Compatibility issue with Magic Items module #326

Open Nedrapter opened 3 years ago

Nedrapter commented 3 years ago

Magic Items module allows items to have their own spells as magic items, which when equipped by the actor, appear in their spellbook and use charges on the item to be casted. This also has the option to upcast a spell (spending more charges to cast it on a higher level, similar way to spell slots). Better Rolls does not scale the damage when the spell is upcasted (probably doesn't recognize that is up-casted at all).

CarlosFdez commented 3 years ago

I think this might be an issue with Magic Items. I remember disabling Better Rolls to test and it still didn't upcast. If it does work core now though I'll fix it.

Nedrapter commented 3 years ago

It did work for me when disabled Better Rolls.

CarlosFdez commented 3 years ago

Seems it got fixed at some point. I'll take a look into it for better rolls.

CarlosFdez commented 3 years ago

I've dug into it. Its not possible for Better Rolls to support Magic Items, the way it works even without better rolls is quite the hack (in short, Magic Items modifies the item to trick 5e and take advantage of the flags used by consumables to get the lower leveled item when the damage button is clicked), and that hack can't work for any module that pre-rolls the damage. Like Better Rolls. Even if the damage button is enabled, the damage is pre-rolled, its just hidden.

You'll have to ask Magic Items to add something so that we can support them. Here are the approaches that come to mind, pass it on I suppose. 1) They can Include base level in item data. I can use that to check for upcasting. 2) Instead of creating a new item, MagicItems can set this.item.data.data.level, and then Better Rolls can check the item source data and check for upcasting. 3) They can ask 5e core to support a spell level parameter for item.roll(). They can then pass the level there rather than doing all this hackery.