Hello Epic!
I found a bug in UE5.5.4 that is giving me some headache and I need advice.
As described in the Repro Steps section, it seems that State Trees are not cleaning their Shared Instance Data upon stopping the logic. The main issue I’m facing with this is that Conditions store their Instanced Data in the Shared Data and, if you have conditions that use their instanced data to keep track of values, said values never reset in between PIE sessions. This causes the ST’s to behave badly when playing a second time in Editor.
I figured that, maybe, StopLogic was not running properly. I noticed that it’s called in EndPlay from the State Tree Component, but that doesn’t work since SetContextRequirements (within StopLogic function) will fail: there’s no pawn own by the AI Controller in End Play. So I tried to force-call it during UnPossess while the pawn is present. The logic run this time but the problem persisted. StopLogic doesn’t seem to reset the runtime data of the Shared Instanced Data.
I tried a second approach that is clearing the Shared Instanced Data myself by calling “StateTreeRef.GetStateTree()->GetSharedInstanceData()->GetMutableStorage().Reset();”, but that ends up crashing the game. Tried to track it down but it’s quite difficult to follow how things work for Default Instanced Data and Shared Instanced Data. Unless I’m mistaken (correct me if I’m wrong), parts of both are created during compilation of the tree and parts are created/set at runtime and, by calling Reset, I’m just obliterating everything.
I need a fix for this but I’m unable to figure one out.
If it’s fixed in 5.6, that’s nice, but doesn’t help me right now since it will take a good while until we upgrade our engine.
I’m adding the condition example that will fail as described here for the ones reading this that want to test (the provided project is only available to Epic). Observe how LastEvaluationTimeStamp keeps its value in between PIE sessions (Play until condition fails -> Stop -> Play again and it will instantly fail because it’s not zero)
Any help will be greatly appreciated
Cheers
Steps to Reproduce
Using the provided c++ project:
- Set a breakpoint in […]\Source\StateTreeTestsStateTreeCondition_TimeStamp.cpp line #9 (if (instanceData.LastEvaluationTimeStamp <= 0.0))
- Play In Editor and observe the value of LastEvaluationTimeStamp when the breakpoint breaks the first time. Value will be 0 (expected). Upon the condition execution several times, the value will be > 0 and the condition will fail (expected)
- Stop and Play In Editor again. Observe the value of LastEvaluationTimeStamp when the breakpoint breaks the first time in this new PIE session. The value of LastEvaluationTimeStamp is NOT zero but the last value stored in the last PIE session when the conditions failed (> 0) (Unexpected, this is the bug). This causes the condition to fail when it shouldn’t, since in the new PIE session we haven’t recorded anything yet. The expected behaviour is to have a clean Data Instance for conditions upon new play (like what happens with Tasks).
Hi Bruno,
Thank you for the report, and sorry about the delay. I have reproduced the behavior you described in both UE 5.5 and UE 5.6, but I am not sure it is a bug, since testing a condition is probably not expected to have side-effects (no built-in conditions do). I am looking into the engine code and doing some experiments to see if I can find a viable workaround or an engine source code change that can result in the behavior you expect, so I should get back to you soon. Meanwhile, would you say the timestamp in your test project represents the needs of your actual use-case, or is it just a very simplified way to repro the unexpected engine behavior?
Best regards,
Vitor
hi!
No worries with the delayed answer, it’s understandable.
" but I am not sure it is a bug, since testing a condition is probably not expected to have side-effects"
Not sure what you mean by “side effects”.
To me, this is like if a Decorator in a BT would serialize a value (save it in the asset) and have the BT behaving differently every time the player plays the game and that decorator reaches the first time. Makes no sense from a design point of view (it would make sense if you actually want that, but there are other mechanisms for it I guess).
“would you say the timestamp in your test project represents the needs of your actual use-case”
It’s a real example. We have a condition that only runs the condition test every 0.5s. To explain:
- the first time it runs time stamp is 0, we check against the current time stamp and the delta is > 0.5s. We will run the condition test logic
- let’s say it runs the next frame. The delta will be (at 60fps) 0.016s. We will just return false from the condition
- Gameplay happens and it runs again 2s later. delta will be > 0.5s and we will check the condition logic
Aside this being the time stamp, I can think few other examples where this would be a problem.
For now I have a workaround where I do this
// Conditions Instanced Data is not being reset in between PIE sessions if (currentTimeStamp < instanceData.LastEvaluationTimeStamp) { instanceData.LastEvaluationTimeStamp = 0.0; }but this will take a while for people to realise since Tasks Instanced Data is reset without problems. There’s some unexpected inconsistency in that.
Thanks for looking into this, it’s much appreciated.
Cheers
Hi Bruno,
To clarify, when I said that evaluating a condition was probably not expected to have side-effects, I meant that conditions could have been designed assuming that evaluating them would not cause externally observable changes. This is similar to how blueprint impure nodes can have side-effects, while pure nodes must not. If a condition evaluation does not have side-effects, then the perceived state of the entire application (including the internal state of the condition object itself) does not change when that evaluation is performed, and performing it multiple times with the same inputs should always produce the same outputs.
In any case, after investigating this further, I now believe that there is indeed a bug with how the InstanceData for Conditions is handled. I noticed that every time PIE is entered or exited (and probably every time StartLogic and StopLogic is called on a State Tree), a new InstanceData object is created or destroyed for every Task, but not for conditions. Furthermore, if multiple actors in the level use the same State Tree, each one gets its own InstanceData object for every Task, but not for conditions. This means that the same condition InstanceData ends up being shared by multiple actors, which does not seem correct and will probably also affect your project.
I am now doing some additional checks and preparing an internal bug report with all this information to pass to the engine devs. I will get back to you with a public bug tracker number once I have one.
Regarding workarounds, I believe it would be safest to avoid changing the internal state of a condition’s InstanceData while evaluating the condition. Maybe you can model your State Tree with helper states that track those timestamps inside tasks, and use conditions on transitions from those helper states?
Hi Victor.
Thanks for the explanation and for looking into this. I’m glad you found the bug. I hope the fix is easy.
About the workaround, knowing this issue, we will think about it. For now we are just resetting the time stamp but, if other cases happen, we might need to re-think it.
Looking forward for your bug case.
Thanks!
Hi Bruno,
Sorry about the extended delay. Here’s the bug tracker number: UE-302300. It should become accessible once the engine devs mark it as public.
One of the reasons it took me a while is that I did some more research on this issue, and I ended up finding the following commit/CL:
`SHA-1: 97d82b0cb01891d92eb5c4a526e54d5618e8272a
- StateTree: shared instance data
- Separated condition instance data into a shared instance data (mutable but no persistent state)
- Made BP TestCondition() const, conditions should not hold state
- Added macro to define runtime data POD for faster init
- Added custom serialization version for UStateTree, older assets need recompile
- Updated memory reporting to be a bit more accurate, separated shared and unique data
#jira UE-147508
#rb Mieszko.Zielinski
#preflight 627b657d2d608c533b5cde15
[CL 20134929 by mikko mononen in ue5-main branch]`So, it now seems clear to me that this commit consciously made State Tree Conditions share a single “InstanceData” between all actors and PIE sessions. The assumption expressed on the commit message is that “conditions should not hold state”.
I also found this on the StateTreeConditionBase.h header:
Note: The condition instance data is shared between all the uses a State Tree asset. You should not modify the instance data in this callback.This is a comment above methods EnterState(), ExitState() and StateCompleted(). It is not repeated above TestCondition(), but I believe it applies there too.
So, based on my findings above, I believe there is some chance that the bug report will be flagged as “by design” or “won’t fix”. I did mention your use case on the report, though, so we’ll have to wait and see.
Kind regards,
Vitor
Hi Vitor,
Thanks a lot for the deep search.
I’m still a bit surprised by conditions sharing memory when nothing else in the state tree does it. It’s weird that we can rely on the instanced data to be unique for any other node in the tree but not for conditions. I still can’t think of a good reason for said limitation.
But knowing this, we will be very careful in the future with how we handle data
Very much appreciated.
Cheers!