StateTree 5.6 Incomplete handling of StructReferenceType

Development Programming & Scripting
, ,
1

I am integrating some of our studio changes into the 5.6 engine upgrade, including various incomplete coverages of handling of struct references. I noticed that a lot of the property binding stuff was moved into a new module. In 5.5.3, I had fixed some issues related to struct reference binding and context variables, as well as added some code for FInstancedStruct based binding.

Looking at 5.6 with fresh eyes against state tree bindings I have from prior, I wanted to give a fresh look at the holes in functionality that remain.

There is some incomplete handling of FStateTreeStructRef

  1. The if (SourceProperty == nullptr) block has no handling of StructReferenceType, so struct reference types are unsupported in the circumstance where SourceProperty is null. This is true for situations where you are binding to context variables via a struct reference.
  2. UStateTreeComponentSchema::CollectExternalData doesn’t find components that live on the Pawn, only the controller. Related, I think it would be preferable to use UE_CVLOG_UELOG, rather than just UE_CVLOG. You see Failed to collect external data … in the normal log, but you have no idea what didn’t resolve without looking in the visual logger.

In the handling of the StructReferenceType block, the comment and conditional suggest that StructReferenceType->StructReferenceType will fall through and be resolved with some normal proerty copy? Do I understand that right? Should that not be resolved to a EPropertyCopyType::CopyStruct if the structs are equal? I don’t see handling further down that would do that.

Fix to #1

[Image Removed]

Bug #2 - Unrelated to property binding, I am trying to understand the changes to the state tree schema. I appreciate that the SetContextData function is now virtual on the schema, to allow you to subclass and feed in additional context data. I did notice that my state tree was failing to resolve all external data, and noticed that UStateTreeComponentSchema::CollectExternalData is a static function, and is not extendable by the schema.

In this case, the issue is that the actor component it is trying to resolve is only resolved via FindComponentByClass on the Owner, which is the AI controller. If the component is placed on the pawn, it will not find it through this function. I would expect external data dependencies to be resolvable to components on the controller and the pawn.

I will update this post as I find other issues.

Edit1:

With the struct ref fix above, you also need the following fix to FPropertyBindingBindingCollection::PerformCopy

[Image Removed]

This requires the delegate to change though, since the property is not guaranteed

[Image Removed] Edit2:

Related to context data resolution, I don’t know if this is new to 5.6, but I noticed while testing in 5.6 that I am getting state machine errors during

In AAIController::OnUnPossess(), there is a block to bStopAILogicOnUnposses, which stops the brain from ticking. Problem is, that Super::OnUnPossess(); has already been called, and so the Pawn has already been cleared. This means, that if the StateTree has a context property for the Pawn, or external data references to components that would normally live on the pawn, UStateTreeComponent::StopLogic will fail to stop the state tree, because the SetContextRequirements(Context) will fail, due to the Pawn already having been cleared. It seems reasonable to move the if (bStopAILogicOnUnposses) block to run above the Super::OnUnPossess(); Does that seem reasonable?

2

Hi Jeremy,

Looking at 5.5, I do not believe we had any handling for FStateTreeStructRef in that portion of the function either. I will relay this back to the team. I know that we recently bumped into an issue with FStateTreeStructRef where it was pointing to garbage memory after a transition. So some work is needing to be done to ensure we update the bindings in that event as well. And thank you for the code diffs. It helps speeding things along.

For your second point about allowing binding to components on the Pawn and not just the controller, I believe there is an open task in our JIRA for looking into this. I remember a conversation about this, and it may have even started from a post of yours.

For the issue in unpossess, I know there was an issue with StateTree leading into 5.6 release where runtime validation could fail in EndPlay because the AI controller could get GC’d before the tree was stopped. This is the first report I have heard of it happening when unpossessing the pawn as well. Is there an ensure or crash? Or is it just the StateTree continuing to run in the background (which is still not desired)?

-James

3

I fully concur with every point in your post. It has almost certainly gone unnoticed until now. I believe moving the call for CleanupBrainComponent ahead in OnUnPossess should be fine, but since it is fairly old code, I would like to check with the rest of the team in case FN or other code paths. Is it something that you have tried locally? Have you noticed any fallout or odd behaviors with the change?

4

Spoke with the team and our primary dev for StateTree. Your fix for StopLogic seems correct, and we like the idea of stopping logic prior to releasing the Pawn as a premise. We currently have that change in review.

As for FStateTreeStructRef, our current assessment is that it is dangerous and risky for the benefit it has. It functions as a hard memory reference and if the bindings are not updated properly, it can be invalid. Our suggestion is to use FStateTreePropertyRef as a replacement. The risk can be reproduced with a tree like this:

Root StateA (with struct) StateB (with StateTreeStructRef, bound in the editor with StateA struct) StateC StateD* Default is StateC.

  • It enter StateA/StateB/StateC. The binding are executed. The StateTreeStructRef is valid.
  • Transition to StateD
    • The runtime doesn’t need to use the StateC memory. It removes it. It needs the StateD memory. It adds it.
    • Since the memory managed similarly to a TArray, the memory might reallocate if the buffer is not big enough.
    • StateB StateTreeStructRef points to invalid data.

We are still looking at some things with the struct ref, but I believe the sentiment is that PropertyRef will be our way forward in StateTree.

5

Yes! It is mutable. There is a BP type for it as well which I believe also allows for writes in 5.6. In 5.5, I do not remember if all the changes got implemented to allow for it. You can use either TStateTreePropertyRef or FStateTreePropertyRef. It is more on how you prefer to format the C++. I believe we use mostly TStateTreePropertyRef internally, but that could vary based on project or team.

6

The idea was for the binding system to be used by other parts of the engine. I have not worked too much with setting it up myself, and our dev who was doing this has left for the Epic summer break. When we return, I can ask him about the setup needed to use it. I think it should not need loads of editor code as it should be contained inside of its Editor module in the plugin.

7

Unpossess: It’s not a crash/check/or ensure failure. It’s just an order of operations issue that causes AI Unpossess to fail to stop the state tree if there are any dependencies on the pawn. It happens silently, where UStateTreeComponent::StopLogic will just fail to call stop on the Context, because SetContextRequirements fails to gather all the required context data.

Since UStateTreeAIComponentSchema defaults to a pawn context actor class, it’s probably the case that all UStateTreeAIComponentSchema users are silently failing to stop the context in StopLogic.

In many cases it might be harmless, but it should be expected to be able to run successfully, even during unpossession. There’s no telling what resource locks a state tree might perform on objects it runs on that could be stuck in limbo even after a call to StopLogic, where you expect the state tree to release them.

8

I have been running with the suggested change for a while now and have not seen any other fallout, and the state tree stop execution works reliably.

9

Ah interesting. Good to know. I will make note to convert all my use cases to FStateTreePropertyRef. I did not even know that existed until now, but looking at the comment above the declaration, I’m excited that it supports varying data types.

It looks like it allows mutable access? Meaning it could be used to “write” into a state higher up the tree?

10

Very cool.

My favorite part of engine upgrades is when I can abandon some of my own fixes or improvements for better alternatives the engine now provides.

One other question tangentially related to the state tree. I noticed that the property binding system was pulled out into its own PropertyBindingUtils module, and is becoming more widely utilized in many contexts other than the state tree.

Is it difficult to take advantage of this approach in custom project level assets?

Looks like I would need to implement the IPropertyBindingBindingCollectionOwner interface, like USmartObjectDefinition, and a binding collection, ala FSmartObjectBindingCollection, but is there also a bunch of custom editor code needed to facilitate a data asset with property binding?

Thanks