See above “Steps to reproduce”
[Attachment Removed]
Steps to Reproduce
Hi,
Our team is using the new Property Access Node feature pretty heavily, but we’ve noticed some issues with it.
1. When used as a part of a condition in an animation FSM, the Property Node condition isn’t surfaced to the tooltip. We were able to make an engine mod to fix it with this change to add an override for `GetCompactNodeTitle` in `UK2Node_PropertyAccess`:
FText UK2Node_PropertyAccess::GetCompactNodeTitle() const
{
const FText SuperCompactNodeTitle = Super::GetCompactNodeTitle();
if (GetTextPath().IsEmptyOrWhitespace())
{
return SuperCompactNodeTitle;
}
return FText::Format(LOCTEXT("PropertyAccess_CompactNodeTitle", "{0} {1}"), SuperCompactNodeTitle, GetTextPath());
}
After this change, they show up properly: (before this, you would just see an unhelpful “()” as the condition)
[Image Removed]
It still doesn’t 100% work in cases where there are comparisons, so an actual fix from the engine team would certainly be better!
[Image Removed]
2. Asset validation seems to not work properly with property access nodes. If you have a property access node that’s part of the character’s base pose, and you right click -> Asset Actions -> Validate Assets, you get a `UKismetMathLibrary::ReportError_Divide_DoubleDouble` from `UKismetMathLibrary::Divide_DoubleDouble(double, double) KismetMathLibrary.inl:494`. I was able to track this down by setting `#define KISMET_MATH_INLINE_ENABLED false`, and then I was able to track it down to this property access node always returning 0 when the asset validates. Even hardcoding `GetGlobalTimeDilation` to always return “1.0” still repros the issue, so I’m pretty sure the root cause is the property access node not calling the function correctly in this context.
[Image Removed]
[Image Removed]
3. Property access nodes don’t show up in the search results for an ABP. If you do a search “By Name” they show up, but this is very confusing when refactoring.
[Image Removed]
4. Renaming is very brittle. The repro for this is pretty easy - if you rename the function, recompile, and then rename it back, the property access nodes break. This isn’t so bad when it creates an error, but we’ve had cases where it just silently points to the old name. Very scary and produces hard-to-find bugs.
[Image Removed]
[Attachment Removed]
Hi,
Thanks for taking the time to report all of these issues. I’ve spent a bit of time looking at each of them, I’ll go through them in turn.
1) This one is due to how the widget that is displayed by the tooltip is created. If you look in SKismetLinearExpression::MakeNodeWidget, you’ll see there’s a hard coded branch that checks for whether the node in question is a variable Get node (ie. UK2Node_VariableGet). That’s what’s responsible for rendering the pill image containing the name of the variable when a regular variable Get node is wired up to the transition condition. But we don’t have anything for UK2Node_PropertyAccess types.
The change that you have is fine; it leverages the fact that there’s another branch in MakeNodeWidget that uses the compact node title text. But I would want us to fix this fully in the engine by changing MakeNodeWidget so that it generates the correct widget for property access nodes. The problem is that the property access node lives in a plugin, so it’s not a trivial fix where we can just add another branch for that type, since the engine code can’t know about the plugin code. So this really needs a more invasive refactor where we change how MakeNodeWidget works to leave it up to the individual nodes (or possibly their factory classes) to generate the widget.
Since this will take a reasonable amount of work, I’ve added a JIRA for it so the dev team can take it on. You can track that here.
2) I haven’t been able to reproduce this one via the Validate Assets option, but I can reproduce it consistently upon opening the animation blueprint. I’ve seen issues like this previously, when either the copy record generated by property access hasn’t yet been generated, or property access hasn’t yet run to generate the value before something attempts to access it. I have a local change that you can try, although I’m not sure that it’ll affect the behaviour when running Validate Assets. In UAnimInstance::InitializeAnimation, you can add the following code to force an update of the GameThread PreUpdate callsites:
// before initialize, need to recalculate required bone list
RecalcRequiredBones();
GetProxyOnGameThread<FAnimInstanceProxy>().Initialize(this);
// CHANGES START
if (IAnimClassInterface* AnimBlueprintClass = IAnimClassInterface::GetFromClass(GetClass()))
{
AnimBlueprintClass->ForEachSubsystem(this, [this](const FAnimSubsystemInstanceContext& InContext)
{
if (InContext.SubsystemStruct == FAnimSubsystem_PropertyAccess::StaticStruct())
{
FAnimSubsystemUpdateContext UpdateContext(InContext, this, 0.f);
InContext.Subsystem.OnPreUpdate_GameThread(UpdateContext);
return EAnimSubsystemEnumeration::Stop;
}
return EAnimSubsystemEnumeration::Continue;
});
}
// CHANGES END
{
Some good news is that I can’t reproduce it in 5.8 at all, so it’s possible we’ve fixed it unintentionally.
3) I’ve created a JIRA on this for the dev team to take a look at as well. There are various issues with the blueprint find tool, so I’m not too surprised this fails with property access nodes, unfortunately.
4) I haven’t been able to reproduce this one yet. Can you show me what the property access node is connected to? And also what the GetGlobalTimeDilation function looks like?
Thanks,
Euan
[Attachment Removed]