WittleWolfie / WW-Blueprint-Core

A library to simplify modifying Pathfinder: Wrath of the Righteous.
MIT License
15 stars 5 forks source link

Automatically set safe defaults for Blueprint Fields #33

Closed pheonix99 closed 2 years ago

pheonix99 commented 2 years ago

It took me about six hours to figure out that AddAreaEffect was throwing undiagnosed NPEs in Validate because SetFx hadn't been called in the AbilityAreaEffectConfigurator.

This was pure brute force and ignorance problemsolving because the blueprints look like BlueprintAbilityAreaEffect.Fx should be allowed to be null because a lot of them have "Fx": "Resource::NULL", but that's a PrefabLink with no fields set.

Please add a warning in the validator so nobody else loses an afternoon.

In general, validations declaring which field had the NPE would super nice, if that's possible?

WittleWolfie commented 2 years ago

Thanks for the feedback!

There's no easy way to tell if a type will fail when null vs. empty.

Generally this is resolved w/ Elements & Components because the library automatically sets a value, but I never added that for blueprint fields. This was all based on my own experience getting weird errors because of null values (like LocalizedString) or from other devs in Discord indicating types which shouldn't be null.

Anything that has an empty constant in the Constants.Empty util likely needs to be set, but I'll definitely update handling for blueprint configurators as well so there's no faulty null fields.

I'm not sure if it'll hit the next release since I'm re-working the code generation to make changes like this easier to implement.

WittleWolfie commented 2 years ago

Updated description: I'll work on this for 2.1.

There's two paths here:

  1. Fix on a per-field basis. Requires knowing each field like from pheonix99's comment.
  2. Fix on a per-field type basis. Really convenient but I need to confirm whether or not this is actually safe--we shouldn't override defaults from the game library if they are sensible.
WittleWolfie commented 2 years ago

Rolling out with 2.1.