Native component parenting error

anonymous_user_3518c7d0 · December 30, 2015, 9:31pm

When adding multiple instances of a native component to an actor, the other created components change which component they are parented to.

Here is a MCVE:

TempComponent.h

#pragma once

#include "Components/SceneComponent.h"
#include "TempComponent.generated.h"


UCLASS( ClassGroup=(Custom), meta=(BlueprintSpawnableComponent) )
class LASTHOPE_API UTempComponent : public USceneComponent {
	GENERATED_BODY()

public:	
	UPROPERTY( Transient )
	UBoxComponent*	Box;
			
					UTempComponent();

	void			OnRegister( void ) override;
	void			OnUnregister( void ) override;
};

TempComponent.cpp

#include "LastHope.h"
#include "TempComponent.h"

UTempComponent::UTempComponent() :
	Box( nullptr )
{}

void UTempComponent::OnRegister( void ) {
	Super::OnRegister();

	Box = NewObject<UBoxComponent>( this, TEXT( "BoxCollider" ) );
	Box->AttachTo( this );
}

void UTempComponent::OnUnregister( void ) {
	Super::OnUnregister();

	if ( Box != nullptr && Box->IsValidLowLevel() ) {
		Box->DestroyComponent( false );
	}
}

Steps:

In the editor you add an empty actor, and then the custom component. Img
Repeat, so that there are now 2 attached. Img
Now select another actor, and then re-select the one with the custom components.

Outcome:

Both are now shown as children of the second component. Img

I was digging around in the source code, and it seems to be caused by this code doing a component lookup by name ( BoxCollider being the same on both children ) in SSCSEditor.ccp ( line 958 ):

UActorComponent* FSCSEditorTreeNodeInstanceAddedComponent::GetComponentTemplate() const
{
	if (InstancedComponentOwnerPtr.IsValid())
	{
		TInlineComponentArray<UActorComponent*> Components;
		InstancedComponentOwnerPtr.Get()->GetComponents(Components);

		for (auto It = Components.CreateConstIterator(); It; ++It)
		{
			UActorComponent* ComponentInstance = *It;
			if (ComponentInstance->GetFName() == InstancedComponentName)
			{
				return ComponentInstance;
			}
		}
	}

	return nullptr;
}

In conjunction with this code in SSCSEditor.cpp ( line 524 ) moving the node to a new parent based on that name look up:

else if (IsInstanced())
	{
		USceneComponent* ChildInstance = Cast<USceneComponent>(InChildNodePtr->GetComponentTemplate());
		if (ensure(ChildInstance != nullptr))
		{
			USceneComponent* ParentInstance = Cast<USceneComponent>(GetComponentTemplate());
			if (ensure(ParentInstance != nullptr))
			{
				// Handle attachment at the instance level
				if (ChildInstance->AttachParent != ParentInstance)
				{
					AActor* Owner = ParentInstance->GetOwner();
					if (Owner->GetRootComponent() == ChildInstance)
					{
						Owner->SetRootComponent(ParentInstance);
					}
					ChildInstance->AttachTo(ParentInstance, NAME_None, EAttachLocation::KeepWorldPosition);
				}
			}
		}
	}

Hope these steps help to reproduce it.

Sean_L · December 31, 2015, 2:58pm

Hello,

After reproducing your issue, it looks like the reason the Box Colliders are being parented to the latest added Scene Component is because OnRegistered is being called again, which is causing them to be added to the latest component.

Instead of adding that Box Collider OnRegister, try adding this code to your constructor and removing the Box(nullptr) line:

Box = CreateDefaultSubobject<UBoxComponent>(TEXT("BoxCollider"));
	Box->AttachTo(this);

When I did it this way, I did not see the same behavior.

After giving this a try, let me know if you see any different results.

Have a great day

anonymous_user_3518c7d0 · December 31, 2015, 10:44pm

Hi Sean,

I have made this change and am still seeing the error. Just to be sure the version of UE I’m using is: 4.10.1-2791327+++depot+UE4-Releases+4.10

In my actual project I’ve made it so that the name for the child component is generated using the parent component’s name. This ensures that the name is unique, and causes this bug not to occur.

Sean_L · January 4, 2016, 2:27pm

Just to clarify, could you provide the code that you used after making the changes I suggested? I’d like to see exactly how you set it up so I can make sure it is the same as the code I used when I wasn’t seeing the error. Thank you.

anonymous_user_3518c7d0 · January 4, 2016, 4:00pm

Of course, Sean.

This is the header file:

UCLASS( ClassGroup=(Custom), meta=(BlueprintSpawnableComponent) )
class LASTHOPE_API UTempComponent : public USceneComponent {
	GENERATED_BODY()
public:
	UPROPERTY( Transient )
	UBoxComponent*		Box;
						UTempComponent();
	void					OnComponentDestroyed( void ) override;	
};

And the .cpp file:

UTempComponent::UTempComponent() {
	Box = CreateDefaultSubobject<UBoxComponent>( TEXT( "BoxCollider" ) );
	Box->AttachTo( this );
}

void UTempComponent::OnComponentDestroyed( void ) {
	Box->DestroyComponent( false );
}

Then I just did the same switching of selected actors and the component has re-parented itself.

Sean_L · January 4, 2016, 8:35pm

In this setup, what I am seeing is that Box Component won’t be saved based on the Transient keyword. Could you explain the reasoning behind your use of that keyword?

Based on the documentation linked below, this keyword prevents that object from being saved, which could be a cause of your issue: https://docs.unrealengine.com/latest/INT/Programming/UnrealArchitecture/Reference/Properties/Specifiers/Transient/index.html

anonymous_user_3518c7d0 · January 4, 2016, 8:46pm

From what I understood of the documentation this is used for when you want to expose a reference that will be filled by an external source. Since I’m creating the object in the constructor at run-time in native code I assumed I don’t need the object stored on the disk because it’s being built in there.

However, I have tried it without the Transient keyword, and this error still occurs.

Sean_L · January 4, 2016, 8:53pm

The only thing different that I’ve noticed in your code was the override of the OnComponentDestroyed function. Try removing this and seeing if you can get the same results as me.

So the steps would be:

Create a new SceneComponent-based c++ class.
Add this code to the .h

UPROPERTY(EditAnywhere, BlueprintReadWrite)
UBoxComponent* Box;
Add this code to the .cpp

Box = CreateDefaultSubobject(TEXT(“BoxCollider”));
Box->AttachTo(this);

Compile the project
Create a new actor bp
Add two instances of the scene component to the actor
Close the blueprint and reopen it

anonymous_user_3518c7d0 · January 4, 2016, 9:03pm

This bug doesn’t show up in the Blueprint editor, as far as I can see. It’s hard to tell though, because it hides the children that are created in native code.

If you follow the directions you’ve given me, and then drag an instance of that blueprint into the scene. Deselect that instance, and re-select it you will also see that the two child boxes are both attached to one instance.

Sean_L · January 4, 2016, 9:08pm

Okay, I was under the impression that you were seeing this issue in the Blueprint Editor. Thank you for the clarification.

I added an instance of my blueprint to the level, deselected it and then reselected it, and my details panel looked like this:

What results are you seeing when you follow the same process? Would you mind attempting a quick repro in a clean project and seeing if you get the same results? Thank you

anonymous_user_3518c7d0 · January 4, 2016, 9:33pm

I’ve made an entirely clean project now and sill see this happening.

When I first add the actor I see this.
Then upon re-selecting I see it like this.

I notice in your image the child components are called Box, and Box1. As I mentioned in the original post, this only occurs when both of the child components have the same name. In mine they are both called BoxCollision, with no number incrementing.

Sean_L · January 5, 2016, 5:53pm

Hello,

I have been able to reproduce your issue, and have entered a bug report (UE-24950). Thank you for your report.

Have a great day

anonymous_user_3518c7d0 · January 5, 2016, 6:06pm

Thanks, a lot. The same to you.