We previously [asked this in a [Content removed] but failed to give repro steps. But we still have an issue where when an actor binds to a delegate of another (via the serialized actor bound event feature) there are duplicated/extra bindings - causing the logic off the event binding to happen multiple times.
We are using a slightly modified version of the engine, but we don’t think we have any changes that should affect this. We are wondering if we can get some help understanding what should be preventing th8is issue, or how/where in the engine the serialized bindings on Actor A should be managed properly so we can see if we caused an issue.
Local Fix:
We have a fix that we are looking to apply. We are calling UBlueprintGeneratedClass::UnbindDynamicDelegates just before UBlueprintGeneratedClass::BindDynamicDelegates from within AActor::ExecuteConstruction to clear out the bindings. And are marking any affected actors as dirty for any of our OFPA maps.
This issue is present in both non-OFPA maps and OFPA maps. In the case of non-OFPA maps we have a simple fix, but in OFPA maps because you need to save both the instance of Actor A and Actor B for the fix to apply it’s possible to still get these invalid bindings to occur when reloading into the map.
Steps to Reproduce
1. Setup two Actor Blueprints
- Actor A which has a delegate on the actor class level
- Actor B has a instance settable variable of type “Actor A”
2. Have Actor B use the ‘Component Bound Event’ to bind to the Actor A variable’s delegate (see Screen Shot)
3. Place down an instance of Actor A and Actor B and assign the variable to the instance
The following issues can happen:
- Whenever you compile Actor B it will create a new delegate binding to Actor A, and will not clear the old binding - resulting in multiple calls.
- Whenever you unassign the member on Actor B, the delegate binding still remains on Actor A - resulting in unintended calls.
Hey William, apologies for the delay - thanks for providing the 5.4 and 5.5 repro projects that illustrate what appears to be a regression. I confirm I’m seeing the same behavior in the 5.5 repro project where delegates keep piling up after recompiling actor B and saving actor A. Gonna ask around internally if anyone has an idea of what caused this regression yet and otherwise I’ll investigate throughout the week.
Hey William, a quick update on my end. After comparing behavior between 5.4.4 and UE5-Main, I see that TMulticastScriptDelegate::CompactInvocationList() which is called when adding is having a different outcome between the two engine versions where in 5.4.4 it would cleanup the old binding due to a stale pointer whereas currently the pointer isn’t stale.
I haven’t landed on a full explanation yet, but I’ll update you when I land on a good fix for this. In the meantime, your workaround of calling UnbindDynamicDelegates sounds sensible.
We found the cause for this regression: between 5.4 and 5.5 we fixed that weak object pointers get fixed up when actors are reinstanced (recreated after recompiling). This causes these bindings that previously automatically became stale and removed to stay alive and pile up. Weak object pointer fixup was working in the past, broken for a bit, then fixed again. You can get the 5.4 behavior again with this CVar or running this console command:
UObject.UseSerializeToFindWeakReferencers 0As a local workaround you can run that. Weak object pointers to an actor won’t survive if an actor gets recompiled, so other editor-time issues may appear.
I’ll work towards cleaning up these bindings explicitly, so that we can have the best of both worlds.
I’m working on a fix that I expect to merge into UE5-main early next week. I’ll provide a CL when it’s ready!
After discussing with colleagues we’re leaning towards cleanup in RerunConstructionScripts, right next to where constructed components get cleaned up:
`// Destroy existing components
DestroyConstructedComponents();
// Reset random streams
ResetPropertiesForConstruction();
// Cleanup any bindings from this actor to other objects, like bindings to referenced level actors.
// This is intended to only cleanup bindings by engine code that correspond to user nodes, like
// Event nodes in blueprint graphs for another actor’s events dispatchers. If those nodes still exist
// the bindings will be rebound on ExecuteConstruction.
#if WITH_EDITOR
UBlueprintGeneratedClass::UnbindDynamicDelegates(GetClass(), this);
#endif`I’m trying to fix some additional issues such as when assets have already been resaved with duplicate bindings, to clean those up as well. Since UnbindDynamicDelegates by default will only remove one binding. The repro project that you provided that already had 10 duplicates was very helpful to help me realize we also need to fix up assets that already have been saved in a bad state, so much appreciated.
Hey Timothy and William. I’m currently still honing the fix. The fix is ready but needs some further internal discussion with regards to how to fix up assets resaved in 5.5 and 5.6.0 who already have duplicate bindings. If you want a solution now I’m happy to share a code snippet. Otherwise, I’m working to get this addressed in a minor engine version update (5.6.1 or 5.6.2).
For sure, I’ll paste the snippet here.
In PropertyMulticastDelegate.cpp:
`void FMulticastInlineDelegateProperty::RemoveDelegate(const FScriptDelegate& ScriptDelegate, UObject* Parent, void* PropertyValue) const
{
ResolveDelegateReference(this, Parent, PropertyValue);
FMulticastScriptDelegate& MulticastDelegate = ((FMulticastScriptDelegate)PropertyValue);
// UE-273286: A bug in UE 5.5 and 5.6.0 allowed duplicates of a delegate to accumulate increasingly in this multicast delegate’s
// invocation list during actor reinstancing, due to reinst and non-reinst actors being both added to the invocation list bypassing
// AddUnique and the reinst reference then being fixed up to the non-reinst actor. These duplicate entries in assets saved in engine
// versions 5.5-5.6.0 would remain unless we consider removing more than a single entry here.
do {
// Remove this delegate from our multicast delegate’s invocation list
MulticastDelegate.Remove(ScriptDelegate);
} while (MulticastDelegate.Contains(ScriptDelegate));
}`In ActorConstruction.cpp in RerunConstructionScripts:
`// Destroy existing components
DestroyConstructedComponents();
// Reset random streams
ResetPropertiesForConstruction();
// Cleanup any bindings from this actor to other objects, like bindings to referenced level actors.
// This is intended to only cleanup bindings by engine code that correspond to user nodes, like
// Event nodes in blueprint graphs for another actor’s events dispatchers. If those nodes still exist
// the bindings will be rebound on ExecuteConstruction.
#if WITH_EDITOR
UBlueprintGeneratedClass::UnbindDynamicDelegates(GetClass(), this);
#endif`These should clean up your duplicate bindings and prevent more from accumulating. Likely the do-while I’ll refactor into something less computationally complex for worst case numbers.
[mention removed], thanks for the snippet - what’s the current status checking in the final version of this fix? We would prefer to integrate the final version of this. Could you provide the CL when you have it checked in?
Hey folks, I will provide the CL once I’ve checked it in - I’m a bit behind my schedule on this one since I was attending UE Fest Bali. Hopefully the snippet suffices in the meantime. My target fix for this will be UE 5.7 but the final CL will be cherrypickable into 5.5 / 5.6 versions.
We were able to reproduce the issue in 5.5.4
We also tested these same steps in 5.4.4 there was no issue.
- At the step 9 there will be still a single message (as expected)
- At the step 14 there will be no message (as expected)
Thank you very much! We will keep an eye out if you have questions!
This is great, thank you! What should we expect as a follow up on that cleanup. Would you be able to post a CL we might be able to cherry pick when available?
Hi,
What’s the current status of the fix with the duplicate cleanup?
We started seeing this issue as well after upgrading to 5.5 and it’s currently blocking several people on our team on maps that already have saved the duplicates.
We are currently on 5.5.X and aren’t currently planning to do the major update to 5.6 for a while.
Could we get the snippet or CL so we can try and cherry pick the fix into our existing version.
We are happy to wait until the asset fixup is thought through as well. Its just we won’t be able to get 5.6.1/2, we will have to integrate this 5.5.X