Closed RachithP closed 11 months ago
Some clean-up and interface refinements to do but otherwise this PR is looking very good. Comment here when you think all items are addressed and it is ready to review again.
I've decided that the rules and conventions around CreateDefaultSubcomponent
vs NewComponent
, SetupAttachment
vs no SetupAttachment
, and RegisterComponent
vs no RegisterComponent
are too confusing. I propose to add the following functions to Unreal.h
, and to use them throughout the code where appropriate.
template <typename TComponent>
TComponent* Unreal::createComponentInsideOwnerConstructor(const std::string& name, AActor* parent)
{
// CreateDefaultSubobject is required when inside the constructor of the owner AActor (either directly or indirectly via
// the constructor of a child component). SetRootComponent is required to attach the newly created root component to its
// owner AActor.
SP_ASSERT(parent);
TComponent* component = CreateDefaultSubobject<TComponent>(Unreal::toFName(name));
SP_ASSERT(component);
parent->SetRootComponent(component);
}
template <typename TComponent>
TComponent* Unreal::createComponentInsideOwnerConstructor(const std::string& name, UActorComponent* parent)
{
// CreateDefaultSubobject is required when inside the constructor of the owner AActor (either directly or indirectly via
// the constructor of a child component). SetupAttachment is required to connect the newly created component to its
// parent component.
SP_ASSERT(parent);
TComponent* component = CreateDefaultSubobject<TComponent>(Unreal::toFName(name));
SP_ASSERT(component);
component->SetupAttachment(parent);
}
template <typename TComponent>
TComponent* Unreal::createComponentOutsideOwnerConstructor(const std::string& name, AActor* parent)
{
// NewObject and RegisterComponent are required when outside the constructor of the owner AActor (either directly or
// indirectly via the constructor of a child component). SetRootComponent is required to attach the newly created root
// component to its owner AActor. The Unreal codebase usually calls SetRootComponent before calling RegisterComponent
// so we follow the same convention convention here.
TComponent* component = NewObject<TComponent>(parent, Unreal::toFName(name));
SP_ASSERT(component);
parent->SetRootComponent(component);
component->RegisterComponent();
}
template <typename TComponent>
TComponent* Unreal::createComponentOutsideOwnerConstructor(const std::string& name, UActorComponent* parent)
{
// NewObject and RegisterComponent are required when outside the constructor of the owner AActor (either directly or
// indirectly via the constructor of a child component). SetupAttachment is required to attach the newly created
// component to its parent UActorComponent. The Unreal codebase usually calls SetupAttachment before calling
// RegisterComponent so we follow the same convention convention here.
SP_ASSERT(parent);
TComponent* component = NewObject<TComponent>(parent, Unreal::toFName(name));
SP_ASSERT(component);
component->SetupAttachment(parent);
component->RegisterComponent();
}
Does the behavior of each function look correct to you?
I've decided that the rules and conventions around
CreateDefaultSubcomponent
vsNewComponent
,SetupAttachment
vs noSetupAttachment
, andRegisterComponent
vs noRegisterComponent
are too confusing. I propose to add the following functions toUnreal.h
, and to use them throughout the code where appropriate.template <typename TComponent> TComponent* Unreal::createComponentInsideOwnerConstructor(const std::string& name, AActor* parent) { // CreateDefaultSubobject is required when inside the constructor of the owner AActor (either directly or indirectly via // the constructor of a child component). SetRootComponent is required to attach the newly created root component to its // owner AActor. SP_ASSERT(parent); TComponent* component = CreateDefaultSubobject<TComponent>(Unreal::toFName(name)); SP_ASSERT(component); parent->SetRootComponent(component); } template <typename TComponent> TComponent* Unreal::createComponentInsideOwnerConstructor(const std::string& name, UActorComponent* parent) { // CreateDefaultSubobject is required when inside the constructor of the owner AActor (either directly or indirectly via // the constructor of a child component). SetupAttachment is required to connect the newly created component to its // parent component. SP_ASSERT(parent); TComponent* component = CreateDefaultSubobject<TComponent>(Unreal::toFName(name)); SP_ASSERT(component); component->SetupAttachment(parent); } template <typename TComponent> TComponent* Unreal::createComponentOutsideOwnerConstructor(const std::string& name, AActor* parent) { // NewObject and RegisterComponent are required when outside the constructor of the owner AActor (either directly or // indirectly via the constructor of a child component). SetRootComponent is required to attach the newly created root // component to its owner AActor. The Unreal codebase usually calls SetRootComponent before calling RegisterComponent // so we follow the same convention convention here. TComponent* component = NewObject<TComponent>(parent, Unreal::toFName(name)); SP_ASSERT(component); parent->SetRootComponent(component); component->RegisterComponent(); } template <typename TComponent> TComponent* Unreal::createComponentOutsideOwnerConstructor(const std::string& name, UActorComponent* parent) { // NewObject and RegisterComponent are required when outside the constructor of the owner AActor (either directly or // indirectly via the constructor of a child component). SetupAttachment is required to attach the newly created // component to its parent UActorComponent. The Unreal codebase usually calls SetupAttachment before calling // RegisterComponent so we follow the same convention convention here. SP_ASSERT(parent); TComponent* component = NewObject<TComponent>(parent, Unreal::toFName(name)); SP_ASSERT(component); component->SetupAttachment(parent); component->RegisterComponent(); }
Does the behavior of each function look correct to you?
Yes, I think these behaviors are correct. I'll update the code with these changes.
As discussed in our meeting today, CreateDefaultSubject
is a public method on UObject
, not a free function like NewObject
, so the code snippet above needs to be modified slightly.
template <typename TComponent>
TComponent* Unreal::createComponentInsideOwnerConstructor(AActor* parent, const std::string& name)
{
return createComponentInsideOwnerConstructor(parent, parent, name); // assume parent is owner
}
template <typename TComponent>
TComponent* Unreal::createComponentInsideOwnerConstructor(UObject* owner, AActor* parent, const std::string& name)
{
// CreateDefaultSubobject is required when inside the constructor of the owner AActor (either directly or indirectly via
// the constructor of a child component). SetRootComponent is required to attach the newly created root component to its
// owner AActor.
SP_ASSERT(owner);
SP_ASSERT(parent);
TComponent* component = owner->CreateDefaultSubobject<TComponent>(Unreal::toFName(name));
SP_ASSERT(component);
parent->SetRootComponent(component);
return component;
}
As discussed in our meeting today,
CreateDefaultSubject
is a public method onUObject
, not a free function likeNewObject
, so the code snippet above needs to be modified slightly.template <typename TComponent> TComponent* Unreal::createComponentInsideOwnerConstructor(AActor* parent, const std::string& name) { return createComponentInsideOwnerConstructor(parent, parent, name); // assume parent is owner } template <typename TComponent> TComponent* Unreal::createComponentInsideOwnerConstructor(UObject* owner, AActor* parent, const std::string& name) { // CreateDefaultSubobject is required when inside the constructor of the owner AActor (either directly or indirectly via // the constructor of a child component). SetRootComponent is required to attach the newly created root component to its // owner AActor. SP_ASSERT(owner); SP_ASSERT(parent); TComponent* component = owner->CreateDefaultSubobject<TComponent>(Unreal::toFName(name)); SP_ASSERT(component); parent->SetRootComponent(component); return component; }
Thanks! For some reason I kept thinking that CreateDefaultSubobject
function was a static function! 🙈