Can't store closest hit result in a pointer

Development Programming & Scripting C++
, , ,
1

I am still learning C++ so naturally I am having trouble with pointers.

For hours, I was trying to find the trace that is closest to another object out of multiple traces. I used a simple for loop with a simple if thisTrace is closer than closestTrace, closestTrace = thisTrace logic.

I tried to save the deepest HitResult in a pointer but the results were confusing. Sometimes I got an empty (ImpactPoint = (0, 0, 0)) hit result, sometimes got a random (probably not random) hit result.

When I tried to save just the impact point in a FVector, it worked perfectly. So I am thinking my method of assigning an address to a pointer is wrong.

Can you help me?
First picture is the working code, second is the broken one. (cleaned them up a little to post here, I may have missed some syntax or something while cleaning)
Working code:

float DistanceToImpactPoint = INT32_MAX;
FVector DepestImpactPoint = FVector::ZeroVector;
for (int i = 0; i < numberOfContacts; i++)
{
	FVector Start = Arrows[i]->GetComponentLocation();
	FVector Direction = Arrows[i]->GetComponentTransform().GetUnitAxis(EAxis::X);
	FHitResult HitResult(ForceInit);
	bool bIsHit = Trace(HitResult, Start, Direction, 20.f);
	if (bIsHit)
	{
		float Distance = FVector::DistSquared(Start, HitResult.ImpactPoint);
		if (Distance < DistanceToImpactPoint)
		{
			DepestImpactPoint = HitResult.ImpactPoint;
			DistanceToImpactPoint = Distance;
		}
	}
}
DrawDebugPoint(GetWorld(), DepestImpactPoint, 5.f, FColor::Red);
UE_LOG(LogTemp, Warning, TEXT("Dist: %f"), DistanceToImpactPoint);

Broken code:

FHitResult* DeepestHit = nullptr;
float DistanceToImpactPoint = INT32_MAX;
for (int i = 0; i < numberOfContacts; i++)
{
	FVector Start = Arrows[i]->GetComponentLocation();
	FVector Direction = Arrows[i]->GetComponentTransform().GetUnitAxis(EAxis::X);
	FHitResult HitResult(ForceInit);
	bool bIsHit = Trace(HitResult, Start, Direction, 20.f);
	if (bIsHit)
	{
		float Distance = FVector::DistSquared(Start, HitResult.ImpactPoint);
		if (DeepestHit == nullptr || Distance < DistanceToImpactPoint)
		{
			DeepestHit = &HitResult;
			DistanceToImpactPoint = Distance;
		}
	}
}
if (DeepestHit != nullptr)
{
	DrawDebugPoint(GetWorld(), DepestImpactPoint, 5.f, FColor::Red);
	UE_LOG(LogTemp, Warning, TEXT("Dist: %f"), DistanceToImpactPoint);
}
2

Sorry forgot to mention. The Trace function is just calling the LineTracingByChannel with some extra debug lines and points.

{
	FVector End = Start + (Direction * Length);
	FCollisionQueryParams CollisionParameters;
	CollisionParameters.AddIgnoredActor(this);
	bool bIsHit = GetWorld()->LineTraceSingleByChannel(HitResult, Start, End, ECC_WorldDynamic, CollisionParameters);
	DrawDebugLine(GetWorld(), Start, End, FColor::Green);
	if (bIsHit)
	{
		DrawDebugPoint(GetWorld(), HitResult.ImpactPoint, 5.f, FColor::Orange);
	}
	return bIsHit;
}
3

I think the problem is:

  1. HitResult, during UBT optimisation, moves out of for loop, so you pointer always point on same variable (if your traces succeed at least once). Otherwise, your pointer would always be invalid (if found result) because of for loop scope (general C++ syntaxsis).
  2. You overwrite HitResult every for loop iteration, even if no hit or new hit location is not suitable (LineTrace feature). And, because of 1), your pointer’s value changes every iteration too.

I think, the possible solution is to manually allocate memory for pointer if you find a solution and after what assign a value:

if (DeepestHit == nullptr){ // we found a hit, but, in case this is a first result (DeepestHit is nullptr), allocate memory
	DeepestHit = new FHitResult();
}
if (Distance < DistanceToImpactPoint) // always true on first found hit result
{
	*DeepestHit = HitResult; // copy data of found result to memory address in DeepestHit
	DistanceToImpactPoint = Distance;
}
... //just some your code
if (DeepestHit != nullptr)
{
	DrawDebugPoint(GetWorld(), DepestImpactPoint, 5.f, FColor::Red);
	UE_LOG(LogTemp, Warning, TEXT("Dist: %f"), DistanceToImpactPoint);
	//do whatever you want
	delete DeepestHit; // free memory to avoid memory leak (maybe not here, depends on your code, but manualy free required because it's not UPROPERTY or smart)
}

Working with raw pointers is hard (it’s a possible place of memory leaking), so try to use TSharedPtr<FHitResult> instead:

TSharedPtr<FHitResult> DeepestHit; //just make your DeepestHit as a smart pointer (nullptr is default).
... //other your code
if (Distance < DistanceToImpactPoint) // always true on first found hit result
{
	DeepestHit = MakeShared<FHitResult>(HitResult); // allocate memory for shared ptr with data of result, if there is previous, it automatically free if not saved somewhere else 
	DistanceToImpactPoint = Distance;
}
... //do whatever you want, smart pointer will be free automatically after become inaccessible.

You can read more about smart pointers here: Unreal Smart Pointer Library | Unreal Engine 4.27 Documentation

1 Like
4

In addition to what @mnelenpridumivat said, you’re assigning a pointer to the same variable address on each loop. Each iteration of the loop (probably) isn’t creating a unique variable that you can point to.

I would not recommend their strategy of smart pointers. While those have their place, in this case DeepestHit should probably just be an FHitResult and not a pointer. When you find a deeper one you just copy over the data you already have. Similar to what you’re doing for DistanceToImpactPoint.

Is there some other problem that you’re hoping to solve by using a pointer? is it an optimization? you think it will be faster somehow?

2 Likes
5

Cant add much more than already said by others.
You should take a read about Stack vs Heap and how each case works.
You are having Stack related issues and you need to move values to the Heap to preserve them in a new memory space (that should be later released for memory leak prevention)
Stack vs Heap Memory Allocation - GeeksforGeeks

Here is a lecture that can help you on understanding how this works and how to take advantage or fix issues like this.

2 Likes
6

Thank you for the detailed answer. I’ll read the documentation.

7

Why was I using a pointer?
I guess I thought this is the right way to do this. A pointer that has the address for the best FHitResult object.

I am not sure to be honest.

8

Definitely will check stack vs heap. Thanks.