UWorldPartitionRuntimeCellTransformerISM::Transform() breaks component attachments during cook because it doesn’t consider the possibility that merged UStaticMeshComponents might have attached children before destroying the original components.
In the attached repro, point light components are attached to the UStaticMeshComponents - when the UStaticMeshComponents are destroyed and marked for GC during UWorldPartitionRuntimeCellTransformerISM::Transform() processing (during cook), the AttachParent of the point light components are saved as nullptr (because they now point to components marked for GC).
When the cooked build is run, the affected point light components have lost their attachment and show up at the origin.
We’ve fixed this locally by skipping merges on components that have one or more attached children that won’t themselves be merged
Steps to Reproduce
1/ Open the default project provided and cook a build
2/ Run the resulting build
Expected behavior: two static meshes, one illuminated by a red point light, the other by green
[Image Removed]
Observed behavior: the two separate point lights in the scene have been moved to the origin, resulting what appears to be a single yellow light source in the wrong place
Thanks for sharing the code. It’s actually more complex than I thought. I didn’t consider the case where the hierarchy is deep and the transformed component is not the root. I’ll have to discuss this problem with the engine team to decide how this should be addressed.
I did find that the FastGeo transformer is properly dealing with the root component being transformed when there are children. I moved the code that reparents the child components to the new root and it resolves the bug in the test project you shared. I’ll submit this as a fix until we can come back with a more holistic solution.
I think that you are fine with the workaround code that you shared. I’m not sure what the engine team will decide to do. Rejecting the components that have childs is the easy solution but they might want to go with a solution that involves reparenting.
/*
== BEGIN FIX ==
Change: Don't merge static mesh components with attached children where one or more of the children won't be merged
Why? The children can wind up orphaned, and we get bugs where - for example - a collision component is missing in cooked
builds and the player can escape the game world. The natural and obvious way to implement this would be to iterate
over AttachedChildren, top-down, but unfortunately this routine can run before the AttachChildren have been set up on the
parent components, hence we have to do the whole thing bottom-up instead, relying only on AttachParent
*/
namespace
{
bool ShouldTransform(const UStaticMeshComponent& StaticMeshComponent)
{
return !StaticMeshComponent.IsEditorOnly() && StaticMeshComponent.IsVisible() && (StaticMeshComponent.Mobility == EComponentMobility::Static);
};
}
void UWorldPartitionRuntimeCellTransformerISM::Transform(ULevel* InLevel)
{
check(InLevel);
TSet<USceneComponent*> ComponentsToMerge;
{
TArray<USceneComponent*> DoNotMergeComponents;
for (TObjectPtr<AActor>& Actor : InLevel->Actors)
{
if (IsValid(Actor) && CanAutoInstanceActor(Actor))
{
// Gather components - some we *might* want to merge, some we *definitely* don't want to merge. At this stage, we
// segregate them naively into two such sets which we then further process to handle component parenting relationships
Actor->ForEachComponent<USceneComponent>(true, [&DoNotMergeComponents, &ComponentsToMerge, &Actor](USceneComponent* SceneComponent)
{
if (UStaticMeshComponent* StaticMeshComponent = Cast<UStaticMeshComponent>(SceneComponent))
{
if (ShouldTransform(*StaticMeshComponent))
{
ComponentsToMerge.Add(StaticMeshComponent);
}
else
{
DoNotMergeComponents.Add(StaticMeshComponent);
}
}
else
{
DoNotMergeComponents.Add(SceneComponent);
}
});
}
}
// Now handle the parenting implications - the parents of children that must-not/cannot be merged must also be spared from merging
// Note, we have to attack this bottom-up (by following children to their parents) because this processing can occur before the
// AttachChildren have been set up on the components
TArray<USceneComponent*> DoNotMergeComponentsScratch;
while (DoNotMergeComponents.GetNumElements() > 0)
{
for (USceneComponent* DoNotMergeComponent : DoNotMergeComponents)
{
if (USceneComponent* AttachParent = DoNotMergeComponent->GetAttachParent())
{
if (ComponentsToMerge.Remove(AttachParent))
{
// New information: this component shouldn't be merged, which means we now need to consider *its* parent
// (if any)
DoNotMergeComponentsScratch.Add(AttachParent);
}
}
}
Swap(DoNotMergeComponentsScratch, DoNotMergeComponents);
DoNotMergeComponentsScratch.Empty();
}
}
struct FActorComponentBatcherDescriptor
{
TMap<TObjectPtr<AActor>*, TArray<UStaticMeshComponent*>> ActorComponents;
FISMComponentBatcher ISMComponentBatcher;
};
TMap<FISMComponentDescriptor, FActorComponentBatcherDescriptor> ISMComponentBatchers;
for (TObjectPtr<AActor>& Actor : InLevel->Actors)
{
if (IsValid(Actor) && CanAutoInstanceActor(Actor))
{
// Gather potential components that can be merged
Actor->ForEachComponent<UStaticMeshComponent>(true, [&ComponentsToMerge, &ISMComponentBatchers, &Actor](UStaticMeshComponent* StaticMeshComponent)
{
if (ShouldTransform(*StaticMeshComponent))
{
if (ComponentsToMerge.Contains(StaticMeshComponent))
{
FISMComponentDescriptor ISMComponentDescriptor;
ISMComponentDescriptor.InitFrom(StaticMeshComponent);
ISMComponentBatchers.FindOrAdd(ISMComponentDescriptor).ActorComponents.FindOrAdd(&Actor).Add(StaticMeshComponent);
}
else
{
UE_LOG(LogWorldPartition, Warning, TEXT("WorldPartitionRuntimeCellTransformerISM skipped merging static mesh component %s because it had non-mergeable children"), *StaticMeshComponent->GetFullName());
}
}
});
}
}
/*
== END FIX ==
*/