foundryvtt / dnd5e

An implementation of the 5th Edition game system for Foundry Virtual Tabletop (http://foundryvtt.com).
MIT License
326 stars 219 forks source link

[Bug] Scale Values not being allowed in various fields #1942

Open sharkbruhaha opened 1 year ago

sharkbruhaha commented 1 year ago

Scale values are not being allowed in various fields, the ones I know of are:

Active Effects:

Item Sheet:

Actor Sheet:

Each of these just puts a e- when trying to paste in the scale identifier (ex: @scale.class.val), or doesn't honour it at all.

Scale values should be allowed in any field, to limit the amount of issues needing to be reported to keep fixing these. And in pretty much any field allowing for reference values (the @ values, in case I am not using the correct name there)

This should be a superseding of the original issue: #1504 as it is a pervasive issue throughout the actor sheet/item sheet, and each field should be evaluated and confirmed that it is allowing them.

sharkbruhaha commented 1 year ago

bump for at least getting tags on this? Coulda sworn it had them back on gitlab...

sharkbruhaha commented 1 year ago

bump, can we get this at least tagged with working? This is silly it has been ignored

MaxPat931 commented 1 year ago

So, some of these fields do accept Scale Values now; Active Effects - Both Attribute Key and Effect Value (for the keys that accept formulas at least) per #1605

Items - Duration and Limited Uses Max

Character - AC (as a/part of a custom formula), Initiative Bonus

I agree on an Item's Target and Range Normal/Max fields should accept them And I would add the Special Traits' Weapon Critical Hit Threshold, Spell Critical Hit Threshold, and Melee Critical Damage Dice fields

Other than that, I dont think that most of these would really benefit from accepting a Scale Value

sharkbruhaha commented 1 year ago

Still not able to make formulas in Numeric, so have to do so in "Anything" and Item Limited Uses Max won't accept an "Anything" scalar, even though it is a formula, even though you can put in that formula into the field and it is accepted. image when using: @scale.adept.exertion-pool image

So I guess it is either the Numeric Scalar needs to be fixed to allow for formulas, or stop filtering on type of scalar, and just filter on actual result for the field data validation, so that "Anything" can be used.

Also, seems like I failed to mention them in the original post, so adding here and updating the original, but the Primary/Secondary/Tertiary resource pools as well.

krbz999 commented 1 year ago

The problem here with the example is that your scale value does not evaluate to a number, but the string 2 * @prof + x, and it is not evaluated one more time to determine @prof.

You can instead use a scaling value for just the modifier added on top, and the result would be correct.

2 * @prof + @scale.adept.exertion-pool
sharkbruhaha commented 1 year ago

and it is not evaluated one more time to determine @prof.

Sorry, not coming through clearly. So you're saying that the issue is because @prof isn't actually expanded, and that is the issue?

krbz999 commented 1 year ago

Yes.

sharkbruhaha commented 1 year ago

Ok... Is that going to be the same for any data attributes? If so can we get that processing handled in scalars, because definitely have others I would want to do with attributes and things. Or just have scalar formulas expand inline to be inside parens Incase of other operands intending to be done?

ie: scalar: 2@prof+3 in a field would be: @scalar_name^2 would expand to: (2@prof+3)^2 before being evaluated in place.

(EDIT, apparently there is email censoring detection, of anything with "@", when replying via gmail...) On Mon, Apr 17, 2023, 11:49 Zhell @.***> wrote:

Yes.

— Reply to this email directly, view it on GitHub https://github.com/foundryvtt/dnd5e/issues/1942#issuecomment-1511631651, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKP54VHR2HAW767F4AC5KV3XBVRAXANCNFSM6AAAAAASK3JMOA . You are receiving this because you authored the thread.Message ID: @.***>

Fyorl commented 1 year ago

Recursively evaluating roll data is something that could be added by the system, but is probably best left to the core roll API. To address the original issue though: What it seems like is being asked for here, is to take almost every single number field that exists across Actor and Item models, and convert them to strings instead that accept roll expressions. That very much does not align with our goals around implementing data models in the first place. It might turn out to be the correct thing to do in order to accommodate all the various material out there, but there are lots of trade-offs involved, so it's not something that can be done with a single, sweeping change as the issue appears to be calling for. Most likely each field or group of related fields will need to be evaluated on a case-by-case basis, with some clear use-cases.