Something wrong with my Squad code. Garbage collection, maybe?

Development Programming & Scripting C++
1

I tried to implement simple squad system for the AI that allows AI of the same type to join the same squad and share information about enemies. Post includes a lot of code. Marked the important bits with bold.

The TLDR: Squad UObject has an array for the UEntruliaAIControllers that are in the Squad, but sometimes those pointers are invalid / point to invalid objects. Or the pointer to Squad UObject is pointing to a UObject that has already been destroyed.

My squad code:

The header file:



// Copyright Entrulia Team 2016

#pragma once

#include "EntrulianSquad.generated.h"

class APawn;

UCLASS(BlueprintType, Category = "AI")
class UEntrulianSquad : public UObject
{
	GENERATED_BODY()

public:
	UEntrulianSquad();

	void AddMember(class AEntruliaAIController *Controller);
	void RemoveMember(class AEntruliaAIController *Controller);
	void OnSeePawn(class APawn *Pawn);
	bool CanBeAddedToSquad(class AEntruliaCharacter *Character);
	static bool CanBeInSameSquad(class AEntruliaCharacter *Character1, class AEntruliaCharacter *Character2);

	UFUNCTION(BlueprintPure, Category = "Entrulian")
	bool IsInSquad(class AEntruliaCharacter *Character);

	UFUNCTION(BlueprintPure, Category = "Entrulian")
	int32 Num() const { return Members.Num(); }

private:
	UPROPERTY()
	TArray<AEntruliaAIController*>			Members;
};

And the .cpp file.



// Copyright Entrulia Team 2016

#include "Entrulia.h"
#include "EntruliaAIController.h"
#include "EntruliaCharacter.h"
#include "EntrulianSquad.h"

//=================================================================
// 
//=================================================================
UEntrulianSquad::UEntrulianSquad()
{
	
}

//=================================================================
// 
//=================================================================
void UEntrulianSquad::AddMember(class AEntruliaAIController *Controller)
{
	if (!Controller || Controller->IsPendingKill())
	{
		UE_LOG(LogTemp, Warning, TEXT("Tried to add invalid controller to squad"));
		return;
	}

	Members.AddUnique(Controller);

	//UE_LOG(LogTemp, Warning, TEXT("Squad %s now has %d members"), *GetName(), Members.Num());
}

//=================================================================
// 
//=================================================================
void UEntrulianSquad::RemoveMember(class AEntruliaAIController *Controller)
{
	Members.Remove(Controller);
}

//=================================================================
// 
//=================================================================
void UEntrulianSquad::OnSeePawn(class APawn *Pawn)
{
	for (int32 i = 0; i < Members.Num(); i++)
	{
		Members*->OnSeePawn(Pawn, false);
	}
}

//=================================================================
// 
//=================================================================
bool UEntrulianSquad::IsInSquad(class AEntruliaCharacter *Character)
{
	for (int32 i = Members.Num() - 1; i >= 0; i--)
	{
		class AEntruliaCharacter *SquadMember = Members*->GetEntrulian();
		if (SquadMember == Character)
			return true;
	}

	return false;
}

//=================================================================
// 
//=================================================================
bool UEntrulianSquad::CanBeAddedToSquad(class AEntruliaCharacter *Entrulian)
{
	//Check that everyone in the squad agrees they can be added
	for (int32 i = 0; i < Members.Num(); i++)
	{
		class AEntruliaCharacter *SquadMember = Members*->GetEntrulian();
		if (!SquadMember)
			continue;

		if (CanBeInSameSquad(SquadMember, Entrulian))
			continue;

		return false;
	}

	return true;
}

//=================================================================
// 
//=================================================================
bool UEntrulianSquad::CanBeInSameSquad(class AEntruliaCharacter *Character1, class AEntruliaCharacter *Character2)
{
	//Always allow to join people of same type
	if (Character1->GetCharacterType() == Character2->GetCharacterType())
		return true;

	//If we are a guard allow people of higher ranking to join
	if (Character1->GetCharacterType() >= ECharacterType::Guard && Character2->GetCharacterType() >= ECharacterType::Guard)
		return true;

	return false;
}

The part of the AI Controller that handles joining squad:



//=================================================================
// 
//=================================================================
void AEntruliaAIController::TryJoinSquad(class AEntruliaCharacter *MyPawn, class AEntruliaCharacter *OtherPawn)
{
	if (Squad && Squad->Num() > 1)
	{
		return;
	}

	class AEntruliaAIController *OtherController = OtherPawn->GetEntrulianAIController();
	if (!OtherController)
		return;

	if (OtherController->GetSquad())
	{
		if (!OtherController->GetSquad()->CanBeAddedToSquad(MyPawn))
			return;

		if (Squad)
			Squad->RemoveMember(this);

		Squad = OtherController->GetSquad();
		Squad->AddMember(this);
		return;
	}

	//Tell them to join our squad instead
	if (Squad)
	{
		OtherController->TryJoinSquad(OtherPawn, MyPawn);
		return;
	}

	Squad = NewObject<UEntrulianSquad>();
	Squad->AddMember(this);
	OtherController->TryJoinSquad(OtherPawn, MyPawn);
}

Also, for the sake of full explanation. The code that shares the information between the squad members.



//=================================================================
// 
//=================================================================
void AEntruliaAIController::OnSeePawn(class APawn *EnemyPawn, bool LinkToSquad)
{
	//Sanity check
	if (!EnemyPawn || EnemyPawn == GetPawn())
	{
		return;
	}

	//Link to all members of the squad if this is coming from the character OnSeePawn
	if (LinkToSquad && Squad)
	{
		Squad->OnSeePawn(EnemyPawn);
		return;
	}

	int32 iIndex = FindEnemyInfo(EnemyPawn);
	if (iIndex != -1)
	{
		Enemies[iIndex].TimeSinceLastSeen = 0;
	}
	else
	{
		FEnemyInfo info;
		info.Enemy = EnemyPawn;
		info.TimeSinceLastSeen = 0;
		Enemies.Add(info);
	}
}

But the code often crashes on UEntrulianSquad::IsInSquad(). It seems that Members* can often be NULL even though it shouldn’t be. I am not sure if I am using NewObject correctly. It seems that the Squad object:



private:
	UPROPERTY()
	UEntrulianSquad							*Squad;

sometimes gets destroyed even though there’s still squad members pointing to it? Or sometimes the Members Array are pointing to NULL in the squad code even though that shouldn’t be possible? Any hints what I am doing wrong. It is often when the first member of the squad dies that the game suddenly crashes. Even though other members of the squad should have a pointer to the squad and it should not get garbage collected.

EDIT: should mention I do this in my BeginDestroy of the controller:




//=================================================================
// 
//=================================================================
void AEntruliaAIController::BeginDestroy()
{
	PotentialItem = NULL;

	FollowTarget = NULL;
	PathTarget = NULL;
	OccludedActors.SetNum(0);
	Enemies.SetNum(0);

	if (Squad)
	{
		Squad->RemoveMember(this);
		Squad = NULL;
	}

	Super::BeginDestroy();
}
2 



//=================================================================
// 
//=================================================================
bool UEntrulianSquad::IsInSquad(class AEntruliaCharacter *Character)
{
	for (int32 i = Members.Num() - 1; i >= 0; i--)
	{
		class AEntruliaCharacter *SquadMember = Members*->GetEntrulian();
		if (SquadMember == Character)
			return true;
	}

	return false;
}

This is where you are seeing the nulls ?

what is Members*->GetEntrulian() ? I’m not seeing this method definition in your posted code.

3 


//=================================================================
// 
//=================================================================
AEntruliaCharacter *AEntruliaAIController::GetEntrulian() const
{
	return Cast<AEntruliaCharacter>(GetPawn());
}

It returns the pointer to the character. That’s one of the places it crashes. But even if it returned null for that pointer it shouldn’t crash unless Members* is NULL. My experience was that Cast<>() was safe to call even with NULL pointer as it checks if it’s NULL before trying to do anything.

4

Maybe you are running into this?

Paraphrasing answer

5

Definition for GetControlledPawn().



	DEPRECATED(4.4, "Use GetPawn() instead of GetControlledPawn()")
	APawn* GetControlledPawn() const { return GetPawn(); }

And even if GetPawn() was returning NULL it couldn’t be the cause for a crash because I don’t call it. I only check if it’s the Input Pawn.

I tried switching to SharedPtr but then realized those are only for structs. Even as class it doesn’t make sense to me that the squad members would become NULL or get destroyed. I am confused if there’s something with garbage collection I am doing wrong.

6

imo you should do more nullptr tests for e.g. Members* and in other cases to check function input parameters. if any of them nullptr you can put a message onscreen to help the debugging and avoiding crashes.

moreover, I had issues with BeginDestroy() in a similar case, and finally I used 3 steps for destroying my RTS units and groups:

  • create a DestroyUnit() and a DestroySquad() function that is called from the game, it should remove all unit-squad related connections first.
  • then call Destroy() at the end of this function to destroy the given actor.
  • use EndPlay() to clear other pointers and containers of the actor used only internally (maybe BeginDestroy() is also okay for it, I don’t know…).
7

Oooh, thanks for the hint! I noticed I had forgotten to add “override” at the end of BeginDestroy declaration (I had not used override before switching to Unreal so I assumed it was only needed if the parent function was not virtual). The crashing was somewhat random before, but I have not had it crash once after I added “override”.