FGResearchTree: Do not crash/assert when reflecting #321
Conversation
|
Context: the original inspiration for this change was making the Asset Generator not crash when generating trees |
There was a problem hiding this comment.
Should probably return a warning if the validation couldn't be performed because of the invalid class/struct.
It also doesn't try to get the property again on a future attempt if it's still null, so if the class/property is fixed after the first validation, it would still require an editor restart.
In the interest of removing checks from the validation, you could also replace check(ResearchTreeNodeClass).
I can't seem to do that because I don't have a validation context in there, just an out parameter for errors. I believe this function signature is deprecated.
Let me see how to handle this. Are these static variables protected against garbage collection? I'm not too fond of having raw pointers.
Sounds good. |
With this old signature the way to do it is to Min/Max the
|
|
I have to use Min because the enum is defined as follows: enum class EDataValidationResult : uint8
{
Invalid,
Valid,
NotValidated
};Actually, just noticed that |
Quoting Mircea: > The whole point of validation is to safely validate the data, not > crash if it's invalid. Replace `CastFieldChecked` with `CastField` to avoid asserting when performing data validation. Also replace one `check` assertion with a regular NULL check. For the sake of readability, move the lazy initialisation variables to helper functions (placed in an anonymous namespace), which fixes dereferencing possibly NULL pointers. While at it, comment the code that gets the research tree node class, as it should not be used as an example on how to do things. Finally, skip reflection if the research tree has no nodes as there is nothing to validate. Signed-off-by: Angel Pons <[email protected]>
Th3Fanbus
force-pushed
the
datavalid-research-treeeeeeee
branch
from
a99b2a1 to
83b21ae
Compare
| } | ||
| } | ||
| if (!ResearchTreeNodeClass || !NodeDataStructProperty || !SchematicStructProperty) { | ||
| ValidationErrors.Add(NSLOCTEXT("ResearchTree", "Validation_ReflectionFailed", "Could not retrieve properties to validate using reflection, validation not performed")); |
There was a problem hiding this comment.
This is still an error. No idea if it will cause problems with the Asset Generator, though.
There was a problem hiding this comment.
I don't think it would cause any issues, since the asset generator still saves the changes, and nothing should depend on data in the CDO of a research tree.
But the asset generator should also provide the SkipDefaultObjectValidation flag (or something like that) to the blueprint compilation, so that the blueprint is still compiled after every change
There was a problem hiding this comment.
mircearoata/UEAssetToolkit#9 should do the skip default object validation, but I haven't tried it
Quoting Mircea:
Replace
CastFieldCheckedwithCastFieldto avoid asserting when performing data validation. Furthermore, add some NULL checks, some to handleNodeDataStructPropertybeing NULL, and another to avoid problems ifSchematicStructPropertyhappens to be NULL.