Refactored Code - Using pointers!

Hey, so I made my function have multiple outs using pointers. It works, but keen to hear if this is a good way to code this. I am a cpp noob, so I am sure it probably isn’t. I saw a post from @Maxidelius which used structs instead - I’m going to try it that way too.

PS. I’m still a bit stuck on const - hence why it is not in the code… need to go back over that.

Header FIle:

// Copyright Omar Sattaur 2021

#pragma once

#include "CoreMinimal.h"
#include "Components/ActorComponent.h"
#include "PhysicsEngine/PhysicsHandleComponent.h"
#include "Grabber.generated.h"

UCLASS( ClassGroup=(Custom), meta=(BlueprintSpawnableComponent) )
class BUILDINGESCAPE_API UGrabber : public UActorComponent
{
	GENERATED_BODY()

public:	
	// Sets default values for this component's properties
	UGrabber();

	// Called every frame
	virtual void TickComponent(float DeltaTime, ELevelTick TickType, FActorComponentTickFunction* ThisTickFunction) override;

protected:
	// Called when the game starts
	virtual void BeginPlay() override;

public:	
	UPROPERTY(EditAnywhere)
	float Reach = 300.f;
	
	UPhysicsHandleComponent* PhysicsHandle = nullptr;
	UInputComponent* InputPlayer = nullptr;

	void FindPhysicsHandle();
	void Grab();
	void GrabRelease();
	FHitResult PickUpItemInReach();
	void SetUpInput();
	void PlayerLocAndReach(FVector*PlayerViewpointVec, FVector*LineTraceEnd);
	
};

CPP file

// Copyright Omar Sattaur 2021

#include "Grabber.h"
#include "DrawDebugHelpers.h"
#include "Engine/World.h"
#include "GameFramework/PlayerController.h"

#define OUT

// Sets default values for this component's properties
UGrabber::UGrabber()
{
	// Set this component to be initialized when the game starts, and to be ticked every frame.  You can turn these features
	// off to improve performance if you don't need them.
	PrimaryComponentTick.bCanEverTick = true;
}

// Called when the game starts
void UGrabber::BeginPlay()
{
	Super::BeginPlay();
	FindPhysicsHandle();
	SetUpInput();
}

// Called every frame
void UGrabber::TickComponent(float DeltaTime, ELevelTick TickType, FActorComponentTickFunction* ThisTickFunction)
{
	Super::TickComponent(DeltaTime, TickType, ThisTickFunction);

	FVector PlayerViewpointVec;
	FVector LineTraceEnd;

	PlayerLocAndReach(&PlayerViewpointVec, &LineTraceEnd);

	if (PhysicsHandle->GrabbedComponent){PhysicsHandle->SetTargetLocation(LineTraceEnd);}
}

void UGrabber::FindPhysicsHandle()
{
	PhysicsHandle = GetOwner()->FindComponentByClass<UPhysicsHandleComponent>(); // Declare the PhysicsHandle

	if (PhysicsHandle == nullptr){UE_LOG(LogTemp, Error, TEXT("Missing 'PhysicsHandle' on %s"), *GetOwner()->GetName());}
}
void UGrabber::Grab()
{
	FVector PlayerViewpointVec;
	FVector LineTraceEnd;

	PlayerLocAndReach(&PlayerViewpointVec, &LineTraceEnd);

	// UE_LOG(LogTemp, Display, TEXT("Player pressed 'Grab' key"));

	FHitResult HitResult = PickUpItemInReach();
	UPrimitiveComponent* ComponentToGrab = HitResult.GetComponent();
	
	if (HitResult.GetActor())
	{
		PhysicsHandle->GrabComponentAtLocation(ComponentToGrab, NAME_None, LineTraceEnd);
	}
}
void UGrabber::GrabRelease()
{
	// UE_LOG(LogTemp, Display, TEXT("Player released 'Grab' key"));
	PhysicsHandle->ReleaseComponent();
}
void UGrabber::SetUpInput()
{

	InputPlayer = GetOwner()->FindComponentByClass<UInputComponent>();

	if (InputPlayer)
	{
		UE_LOG(LogTemp, Display, TEXT("'InputCompentent' found on %s"), *GetOwner()->GetName());
		InputPlayer->BindAction("Grab", IE_Pressed, this, &UGrabber::Grab);
		InputPlayer->BindAction("Grab", IE_Released, this, &UGrabber::GrabRelease);
	}
	// else{UE_LOG(LogTemp, Error, TEXT("Missing 'InputCompentent' on %s"), *GetOwner()->GetName());}
}
FHitResult UGrabber::PickUpItemInReach()
	{
		FVector PlayerViewpointVec;
		FVector LineTraceEnd;

		PlayerLocAndReach(&PlayerViewpointVec, &LineTraceEnd);
		
		FHitResult Hit;
		FCollisionQueryParams TraceParams(FName(TEXT("")), false, GetOwner());

		GetWorld()->LineTraceSingleByObjectType(
			OUT Hit,
			PlayerViewpointVec,
			LineTraceEnd,
			FCollisionObjectQueryParams(ECollisionChannel::ECC_PhysicsBody),
			TraceParams
		);

		if (Hit.IsValidBlockingHit()){UE_LOG(LogTemp, Warning, TEXT("Line trace hit %s"), *Hit.GetActor()->GetName());}

		DrawDebugLine(
			GetWorld(),
			PlayerViewpointVec,
			LineTraceEnd,
			FColor(0, 200, 0, 0),
			false, 0.f, 0, 5.f
		);

		// UE_LOG(LogTemp, Warning, TEXT("The PlayerViewpointVec is: %s and the PlayerViewpointRot is: %s"), *PlayerViewpointVec.ToString(), *PlayerViewpointRot.Vector().ToString());

		return Hit;
	}
void UGrabber::PlayerLocAndReach(FVector*PlayerViewpointVec, FVector*LineTraceEnd)
{
	FRotator PlayerViewpointRot;
	GetWorld()->GetFirstPlayerController()->GetPlayerViewPoint (OUT *PlayerViewpointVec, OUT PlayerViewpointRot);
	*LineTraceEnd = *PlayerViewpointVec + FVector(PlayerViewpointRot.Vector()*Reach);
}

It will work but why wouldn’t you use references instead? They are much nicer to work with and give cleaner code in my opinion.

The method in the header would then be:

void PlayerLocAndReach(FVector& PlayerViewpointVec, FVector& LineTraceEnd);

Calling would look like this:

PlayerLocAndReach(PlayerViewpointVec, LineTraceEnd);

And the method code would be

void UGrabber::PlayerLocAndReach(FVector& PlayerViewpointVec, FVector& LineTraceEnd)
{
	FRotator PlayerViewpointRot;
	GetWorld()->GetFirstPlayerController()->GetPlayerViewPoint (OUT PlayerViewpointVec, OUT PlayerViewpointRot);
	LineTraceEnd = PlayerViewpointVec + FVector(PlayerViewpointRot.Vector()*Reach);
}

As for const, it’s only for methods that do not modify the original class - this method could probably be a const method as it’s not actually changing the instance. The parameters can’t be const as they are changing.

1 Like

Thanks @beegeedee. This is really helpful.

So, rather than using memory addresses and then de-referencing them to get the value, instead, we can just look at the reference directly?

Also, when calling the function, would you have to declare the variables first, like I have done below?

	FVector PlayerViewpointVec, LineTraceEnd;
	PlayerLocAndReachByRef(PlayerViewpointVec, LineTraceEnd);

And finally, regarding using a struct instead, which I saw in another post. I suppose as the data which is being passed around is small, so there is no real positive/negative in going down that route in comparison to using references. Does it just come down to preference at that point?

1 Like

The best way to think about references is they are like pointers but hide the fact that they are. If using a structure, passing around as a reference is the way to go because instead of passing around the whole structure, you just pass a reference or the memory address to the structure.

As for using a structure: If the amount of data was more than just 2 vectors, I’d consider it. Right now, if it works, leave it alone. It results in clearer code but that’s my opinion. If you had a struct, all uses of the vectors are prefixed by the structure name.

1 Like

Privacy & Terms