What is the correct way to initialize a UObject derived class?

I have a UObject derived class that I am using as weapon state data. It’s very simple right now, but is currently causing the replication code in UE4 to crash with an access violation.

What it boils down to is that the object has an uninitialized InternalIndex, and when it goes to replicate, there isn’t adequate checks in place and UE4 crashes. That’s fine, and I’ll patch that.

However - the remaining issue is how to get it an internal index. It’s a private member in UObjectBase, and is initialized to the value I want it to be (INDEX_NONE) in most of the constructors. But not the one that gets called when you call NewObject. That one is this:

UObject::UObject(const class FPostConstructInitializeProperties& PCIP)
{
	check(!PCIP.Obj || PCIP.Obj == this);
	const_cast<FPostConstructInitializeProperties&>(PCIP).Obj = this;
}

As you can see, no call to any of its ancestors constructors, so no initialization of InternalIndex.

InternalIndex needs to be initialized to INDEX_NONE or the function that gives it a real index in FUObjectArray asserts.

void FUObjectArray::AllocateUObjectIndex(UObjectBase* Object)
{
	int32 Index;
	check(Object->InternalIndex == INDEX_NONE);

So, how do I give it an internal index the right way?

The classes header:

UCLASS()
class UWeaponState : public UObject
{
	GENERATED_UCLASS_BODY()

	int32 RepObjId;

	int32 RepKey;

	UPROPERTY(replicated)
	FName weaponName;

	UPROPERTY(replicated)
	FName mountName;
};

The init of one of these objects:

void ARobotPlayerState::initWeaponStates()
{
	if (WeaponMounts.Num() == 0)
	{
		for (int i = 0; i < NumberOfWeaponMounts; i++)
		{
			FName key = FName(*FString::Printf(TEXT("mount_%02d"), i));

			UWeaponState  *state = NewObject<UWeaponState>();
			state->mountName = key;
			state->weaponName = NAME_None;
			state->RepObjId = i + 1;
			state->SetFlags(RF_RootSet | RF_WasLoaded);

			WeaponMounts.Add(key, state);
		}
	}
}

EDIT: And the function it now asserts in

bool ARobotPlayerState::ReplicateSubobjects(UActorChannel *Channel, FOutBunch *Bunch, FReplicationFlags *RepFlags)
{
	check(Channel);
	check(Bunch);
	check(RepFlags);

	bool WroteSomething = false;

	if (Channel->KeyNeedsToReplicate(0, WeaponMountsRepKey))	// Does the array need to replicate?
	{
		TArray<FName> keys;

		WeaponMounts.GetKeys(keys);

		for (auto wmIt = keys.CreateIterator(); wmIt; wmIt++)
		{
			FName key = *wmIt;

			UWeaponState * weaponState = WeaponMounts[key];
			if (Channel->KeyNeedsToReplicate(weaponState->RepObjId, weaponState->RepKey))
			{
				WroteSomething |= Channel->ReplicateSubobject(weaponState, *Bunch, *RepFlags);
			}
		}
	}

	return WroteSomething;
}

Can you show us how you are trying to initialize your new UObject derived class? And possibly some of the header of the class itself?

There is quite a bit of magic in how UObjects are initialized and I would expect that code to be pretty stable and well traversed. The destructor of the FPostConstructInitializeProperties object is where most of the magic takes place. It’s actually pretty cool, I’d never thought to use a destructor in that way.

No one said the question is resolved. This is the just the system Epic has setup to answer your questions. I guess I could have started as a comment on your question.

I’ve added the info.

There is only one ConstructObject call, called by the only two NewObject functions. Where are you calling your function? Is it inside the construction of another UObject? UObjectBase is called explicitly inside StaticConstructObject with a placement new(). There is also only one StaticConstructObject function.

I took a capture of a normally constructed object’s callstack, down to the place where the UObject should be calling AllocateUObjectIndex for you. NewObject is new to UE4, I’m used to just calling ConstructObject directly, but as a helper it really shouldn’t matter. You could try calling ConstructObject directly, the only difference is you add UWeaponState::GetClass() as the first argument.

UE4Editor-CoreUObject.dll!UObjectBase::AddObject(FName InName) Line 221
	
UE4Editor-CoreUObject.dll!UObjectBase::UObjectBase(UClass * InClass, EObjectFlags InFlags, UObject * InOuter, FName InName) Line 130

UE4Editor-CoreUObject.dll!StaticAllocateObject(UClass * InClass, UObject * InOuter, FName InName, EObjectFlags InFlags, bool bCanRecycleSubobjects, bool * bOutRecycledSubobject) Line 1801

UE4Editor-CoreUObject.dll!StaticConstructObject(UClass * InClass, UObject * InOuter, FName InName, EObjectFlags InFlags, UObject * InTemplate, bool bCopyTransientsFromClassDefaults, FObjectInstancingGraph * InInstanceGraph) Line 2158

UE4Editor-MyGame-Win64-DebugGame.dll!ConstructObject(UClass * Class, UObject * Outer, FName Name, EObjectFlags SetFlags, UObject * Template, bool bCopyTransientsFromClassDefaults, FObjectInstancingGraph * InstanceGraph)

Yes, the call happens in the context of the player state constructor. And yes, I see that new call. It’s wrapped in (!bSubObject) - would this be counted as a subobject, since it’s being created in an actors constructor? Have to read that code.

Thanks for the help Josh, I’ve tinkered with this some more. Still not quite sure what is happening. As the question states, I am trying to eliminate a cause of this access violation. If the way I am initializing the object is correct, then I still don’t know my problem.

On my last 2 runs,

I got an assertion in PackageMapClient saying that my WeaponState was not supported for networking (don’t have full message, but it failed the FNetGUIDCache::SupportsObject() test.

The second I had an access violation somewhere under ReplicateSubobject() (as my usual).

Is the code saying that your object is a Sub Object? Probably because it is inside the constructor of the player state actor.

Can you move your creation of this object into PostInitializeComponents? It’s an easily overridable function on any AActor. This call is effectively still inside the SpawnActor call you make for PlayerState. It shouldn’t interfere with order of operations of your future calls.

No, it’s not. Walked through the code and it’s not being tagged as a subobject.

What was throwing me off is what happens after init. I’m storing the weapon mounts/weapon state data in a TMap(FName, UWeaponState *). When I look at the data after it leaves the init method, the slots are no longer full of UWeaponStates. There’s a UPolys object in there, an IntProperty. It seems like random crap really.

Have my UWeaponState objects been GCed?

My previous comment got deleted, but I think my objects are getting GCed.

I guess storing them in a TMap isn’t enough of a reference?

Thanks for the help so far Josh, but still another issue. I was going to post a different question but it really is still the same question.

Now, when ReplicateSubObject() gets called, it no longer gives me access violation. Instead it gives me:

[2014.07.10-08.40.21:803][462]LogNetPackageMap:Warning: FNetGUIDCache::SupportsObject: WeaponState /Game/Maps/UEDPIE_1_Minimal_Default.Minimal_Default:PersistentLevel.RobotPlayerState_0.mount_01 NOT Supported.
C:\Projects\UnrealEngine4.3\Engine\Source\Runtime\Engine\Private\PackageMapClient.cpp(1434): Assertion failed: Object->IsSupportedForNetworking()

It’s no longer a raw pointer in this attempt, I used CreateDefaultSubobject and am storing it in the TMap as a TSubobjectPtr. With the raw pointer I get a similar message.

It’s my understanding that the sub object is supposed to be supported as part of the replication process:

bool UActorChannel::ReplicateSubobject(UObject *Obj, FOutBunch &Bunch, const FReplicationFlags &RepFlags)
{
	// Hack for now: subobjects are SupportsObject==false until they are replicated via ::ReplicateSUbobject, and then we make them supported
	// here, by forcing the packagemap to give them a NetGUID.
	//
	// Once we can lazily handle unmapped references on the client side, this can be simplified.
	if ( !Connection->Driver->GuidCache->SupportsObject( Obj ) )
	{
		FNetworkGUID NetGUID = Connection->Driver->GuidCache->AssignNewNetGUID_Server( Obj );	//Make sure he gets a NetGUID so that he is now 'supported'
	}

The assertion happens in AssignNewNetGUID_Server()

I guess it would be good if you could give me a high level overview of how this is supposed to work and what I’ve done wrong? Thanks again.

At a high level, garbage collection will pass over any object that is reachable from the root to your object. Your object is reachable if an object has a member variable that is a UPROPERTY(). All pointers to UObjects should be UPROPERTY(). TMaps cannot be UPROPERTY(). TArrays can.

Only UObjects can be marked up with UPROPERTY, so any “native” class that wants to hold a pointer or reference to a UObject has to add some special code to allow the garbage collector a chance to query it. If you only reference your objects through a TMap within any class “UObject or native”, you can use this too.

You need your native class to derive from FGCObject and then implement

virtual void AddReferencedObjects(FReferenceCollector& Collector) override;

Inside this function you need to add the UObjects that you want to preserve.

Collector.AddReferencedObject(OneOfMyUObjects);

I think UObjects can just implement

AddReferencedObjects(UObject* InThis, FReferenceCollector& Collector)

and do the same thing, the InThis is the UObject that has the TMap.

All of this being said, I wouldn’t expect your call to immediately mess up your TMap. GC happens at a very specific spot in the game tick, so if you are in the game thread stepping through your own code, it shouldn’t even call GC code until the tick makes a full loop.

They were being GCed. Adding the flag RF_RootSet caused them to no longer be GCed.

The issue wasn’t happening immediately after creation, it was happening in response to a value changing and replication trying to happen.

That said, see the comments below - still not initialized correctly. :slight_smile:

Please use the functions above to do this properly. Or at least use AddToRoot and RemoveFromRoot to do the RF_RootSet.

As for the initialization, did you move it to PostInitializeComponents?

Can you also post above the entire function you are creating your weapon state object in? Remove calls inside that are irrelevant, is this the constructor of your derived PlayerState?

At this moment the only thing I can see is that you are calling NewObject in a bad spot. I’ve never had an object initialization problem myself and creating objects doesn’t have too many moving parts. I think we just need to move where you’re adding the code.

I’ve added the full function in the question. It’s called at the bottom of the player state constructor.

With the GC issue removed (I will do it properly next) - the issue is solely that it will assert when I try to call ReplicateSubObject on it.

I’ve just moved the initialization to PostInitializeComponents() and now no assertion. The servers pawn disappears on the client, but that’s some other issue. Thanks for all your help Josh, it’s very much appreciated.

Glad to hear that helped. I’ll point some people here to this thread to see if they can provide a more detailed answer as to why “new object inside another object creation” is bad. It sounds bad :slight_smile: but I think its the assumption that UObject creation inside a constructor is for sub objects and default properties.

Thanks!

I have a follow up question if you have time.

Getting non-Actor subobject replication to work

I sent a notification to someone who should be able to help you with this additional question.