Struct Approach

Unreal Courses Talk
,
#1

Hello! This is my first post here.

During this refactoring lesson I decided to make a struct and return that from my function considering we were using the out parameters and LineTraceEnd.

My Code Follows.

// Copyright Justin Robertson 2020

#pragma once

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

struct FPlayerReachVector
{
	FVector PlayerPosition;
	FVector PlayerRotation;
	FVector PlayerArm;
};

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;

private:
	// My Variables
	
	UPhysicsHandleComponent* PhysicsHandle = nullptr;
	UInputComponent* InputComponent = nullptr;

	UPROPERTY(EditAnywhere)
	float Reach = 100.f;
	
	// My Functions

	void Grab();
	void Release();
	void FindPhysicsHandle();
	void SetupInputComponent();

	// Return the first Actor within reach with physics body.
	FHitResult GetFirstPhysicsBodyInReach() const;

	// Return the player's 'Arm' FVector by adding position and (rotation*Reach) vectors together
	// also returns players position and rotation FVectors
	FPlayerReachVector GetReachVector() const;
};

My Implementation

// Copyright Justin Robertson 2020


#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();
	SetupInputComponent();
}

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

	if (PhysicsHandle->GrabbedComponent)
	{
		PhysicsHandle->SetTargetLocation(GetReachVector().PlayerArm);
	}
}

// Grab & Release Physics Objects
void UGrabber::Grab()
{
	FHitResult HitResult = GetFirstPhysicsBodyInReach();

	UPrimitiveComponent* ComponentToGrab = HitResult.GetComponent();

	if (HitResult.GetActor())
	{	
		PhysicsHandle->GrabComponentAtLocation(ComponentToGrab, NAME_None, GetReachVector().PlayerArm);
	}
	
}

void UGrabber::Release()
{
	UE_LOG(LogTemp, Log, TEXT("Grabber released."));
	PhysicsHandle->ReleaseComponent();
}

// Setup Input Component
void UGrabber::SetupInputComponent()
{
	InputComponent = GetOwner()->FindComponentByClass<UInputComponent>();
	if (InputComponent)
	{
		InputComponent->BindAction("Grab", IE_Pressed, this, &UGrabber::Grab);
		InputComponent->BindAction("Grab", IE_Released, this, &UGrabber::Release);
	}
}

// Check for Physics Handle Component
void UGrabber::FindPhysicsHandle()
{
	PhysicsHandle = GetOwner()->FindComponentByClass<UPhysicsHandleComponent>();
	if (!PhysicsHandle)
	{
		UE_LOG(LogTemp, Error, TEXT("%s does not have Physics Handle Component!"), *GetOwner()->GetName());	
	}
}

FHitResult UGrabber::GetFirstPhysicsBodyInReach() const
{
	// Raycasts out to Reach checking for PhysicsObjects
	FHitResult Hit;
	FCollisionQueryParams TraceParams(FName(TEXT("")), false, GetOwner());
	FPlayerReachVector PawnArm = GetReachVector();

	GetWorld()->LineTraceSingleByObjectType(OUT Hit, PawnArm.PlayerPosition, PawnArm.PlayerArm, FCollisionObjectQueryParams(ECollisionChannel::ECC_PhysicsBody), TraceParams);

	// See what it hits
	AActor* ActorHit = Hit.GetActor();

	if (ActorHit)
	{
		UE_LOG(LogTemp, Warning, TEXT("Ray is colliding with %s."), *Hit.GetActor()->GetHumanReadableName());
	}

	return Hit;
}

FPlayerReachVector UGrabber::GetReachVector() const
{
	FVector PlayerViewPointLocation;
	FRotator PlayerViewPointRotation;

	GetWorld()->GetFirstPlayerController()->GetPlayerViewPoint(OUT PlayerViewPointLocation, OUT PlayerViewPointRotation);
	
	FPlayerReachVector ReachVector = {PlayerViewPointLocation, PlayerViewPointRotation.Vector(), PlayerViewPointLocation + PlayerViewPointRotation.Vector() * Reach};
	return ReachVector;
}

I asked my dad about doing it this way and he mentioned that when it comes to this method it is important to make a judgement call on when you can use the function in an argument or go ahead and load it into storage instead of calling it every time you need one variable.

Any thoughts on this approach?

#2

The first thought that comes to mind when reading through the code is “What is a FPlayerReachVector”. Without any context id look at it and and think would this be a single vector, well no when looking at the declaration it contains all of the values needed to calculate reach which player arm being the calculated reach.

Id ask whether or not you need to store the result in the same struct or if you can split it into its own function or just do the calculation at the location its needed. If you took it out you could easily rename the struct to FPlayerViewpoint.

struct FPlayerViewpoint
{
    FVector PlayerPosition;
    FVector PlayerRotation;
};

FVector UGrabber::GetPlayerArm(const FPlayerViewpoint& PlayerViewpoint) const
{
    return FPlayerViewpoint.PlayerPosition + FPlayerViewpoint.PlayerRotation;
}

The only changes im really mentioning is to try ot improve the readability of the code and not performance specifically.

#3

Okay awesome! I can see where you’re coming from in that the function name doesn’t quite make clear everything it’s giving you.

What actually led me to make the struct is that for GetFirstPhysicsBodyInReach() I’m using LineTraceSingleByObjectType() which requires the player position and the player’s arm, in the lesson was LineTraceEnd, as FVectors. When I first wrote the function to get LineTraceEnd and realized the PlayerPosition was still needed I thought it was silly to use a part of the code I just refactored just to get an OUT parameter from it.

I’ve probably overthought it or got distracted down an unnecessary path for this. Thank you for responding! I’ll look into improving my readability and really weigh the value in the structs and variables I create in the future.

#4

This is how i did the second refactoring

.h

#pragma once

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

#include "Grabber.generated.h"

struct FPlayerViewpoint
{
	FVector PlayerPosition;
	FRotator PlayerRotation;
};


UCLASS( ClassGroup=(Custom), meta=(BlueprintSpawnableComponent) )
class UNREALESCAPE_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;
	
private:

	UPROPERTY(EditAnywhere)
	float Reach = 100.f;

	UPhysicsHandleComponent* PhysicsHandle = nullptr;
	UInputComponent* InputComponent = nullptr;

	FPlayerViewpoint PlayerViewpoint;

	void Grab();
	void Release();
	void FindPhysicsHandle();
	void SetupInputComponent();
	void UpdatePlayerViewpoint();

	// Return the first actor within reach with physics body
	FHitResult GetFirstPhysicsBodyInReach() const;
	FVector GetLineTraceEnd() const;
	

};

.cpp


#include "Grabber.h"

#include "DrawDebugHelpers.h"
#include "Engine/World.h"
#include "GameFramework/PlayerController.h"
#include "Windows/LiveCoding/Private/External/LC_Logging.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();
	SetupInputComponent();
}


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

	// if the physics handle is attached
		// Move the object we are holding.

	if (PhysicsHandle->GrabbedComponent)
	{
		UpdatePlayerViewpoint();
		PhysicsHandle->SetTargetLocation(GetLineTraceEnd());
	}
}

// Try and reach any actors with a physics body collision channel set
// If we hit something then attach a physics handle.
void UGrabber::Grab()
{
	UE_LOG(LogTemp, Warning, TEXT("Grabber pressed"));

	UpdatePlayerViewpoint();
	FHitResult HitResult =  GetFirstPhysicsBodyInReach();
	UPrimitiveComponent* ComponentToGrab = HitResult.GetComponent();

	if (HitResult.GetActor())
	{
		PhysicsHandle->GrabComponentAtLocation
		(
			ComponentToGrab,
			NAME_None,
			GetLineTraceEnd()
		);
	}
}

//Let go of any object the player is holding
void UGrabber::Release()
{
	UE_LOG(LogTemp, Warning, TEXT("Grabber released"));
	if (PhysicsHandle->GrabbedComponent)
	{
		PhysicsHandle->ReleaseComponent();
	}
}

//Checks to see if a UPhysicsHandleComponent is present on the object
//Sets the PhysicsHandle member variable
void UGrabber::FindPhysicsHandle()
{
	PhysicsHandle = GetOwner()->FindComponentByClass<UPhysicsHandleComponent>();
	if (!PhysicsHandle)
	{
		UE_LOG(LogTemp, Error, TEXT("Object < %s > Does not contain a valid UPhysicsHandleComponent!"), *GetOwner()->GetName());
	}
}

//Initializes the UInputComponent and binds the needed functions
void UGrabber::SetupInputComponent()
{
	InputComponent = GetOwner()->FindComponentByClass<UInputComponent>();
	if (InputComponent)
	{
		InputComponent->BindAction("Grab", IE_Pressed, this, &UGrabber::Grab );
		InputComponent->BindAction("Grab", IE_Released, this, &UGrabber::Release );
	}
}

// Returns the first Actor hit by the ray-cast that has a PhysicsBody channel
FHitResult UGrabber::GetFirstPhysicsBodyInReach() const
{
	FHitResult Hit;
	
	FCollisionQueryParams TraceParams(FName(TEXT("")), false, GetOwner());
	GetWorld()->LineTraceSingleByObjectType(
        OUT Hit,
        PlayerViewpoint.PlayerPosition,
        GetLineTraceEnd(),
        FCollisionObjectQueryParams(ECollisionChannel::ECC_PhysicsBody),
        TraceParams
    );

	AActor* ActorHit = Hit.GetActor();

	if (ActorHit)
	{
		UE_LOG(LogTemp, Warning, TEXT("Hit object %s"), *ActorHit->GetName() );
	}
	return Hit;
}

//Returns the vector endpoint of the ray-cast
FVector UGrabber::GetLineTraceEnd() const
{
	return PlayerViewpoint.PlayerPosition + PlayerViewpoint.PlayerRotation.Vector() * Reach;
}

// Updates the PlayerViewpoint member with the current Viewpoint Information
void UGrabber::UpdatePlayerViewpoint()
{
	GetWorld()->GetFirstPlayerController()->GetPlayerViewPoint(
        OUT PlayerViewpoint.PlayerPosition,
        OUT PlayerViewpoint.PlayerRotation
    );
}

Placing the comments above each function allows me to see the comment as the description when i hover over the function elsewhere in the code.

I implemented a struct to Organize the Viewpoint information and added an instance of the struct as a member variable i could update. with the function UpdatePlayerViewport which i now think should be renamed to UpdateViewportInfo.

1 Like
#5

That is interesting. I did not create a struct but made them member variables. Doing this however means that member function that is updating these values is not a const function. Is this bad?

Privacy & Terms