Code Review - Dynamic C++ Blueprint Nodes with Casting

Development Programming & Scripting
, ,
1

Hey, looking for some feedback on some C++ code. I am getting braver trying more advanced C++ helper functions but don’t want to create ‘unsafe’ functions.

Objective/Requirements: I would like a node that can scan the level for all actors of a class, filter by a tag, and return the first match:

  • I would like all error handling and ‘casting’ to be done within the C++ code of the node itself to keep my BP clean
  • I would like the output pin to dynamically change to the object class itself so I can directly access it
  • I would like it to output ‘pins’ for failure routes
  • I would like the WorldContext to be set automatically (no dragging ‘self’ nodes)
  • I would like to utilise the ‘GetAllActorsOfClass’ node which is confirmed to be ‘efficient’ (not the ‘GetAllActorsOfClassWithTag’ which is not efficient)

Concerns:

  • Is this ‘safe’?
  • Any concerns on reliability / maintainability?
  • Anything I have missed?

My Result:

image
image855×334 40.9 KB

My Code (header)

UFUNCTION(BlueprintCallable, meta = (
    WorldContext = "WorldContextObject",
    DeterminesOutputType = "ActorClass",
    ExpandEnumAsExecs = "Branches",
    DynamicOutputParam = "OutActor",
    DisplayName = "Find Actor by Class and Tag (Typed)"
), Category = "Helpers|Actor")
static void FindTypedActorByClassAndTag_DynamicOutput(
    UObject* WorldContextObject,
    TSubclassOf<AActor> ActorClass,
    FName Tag,
    AActor*& OutActor,
    EHelperFunctionResult& Branches
);

My Code (cpp)

#include "YourHelperLibrary.h"
#include "Kismet/GameplayStatics.h"
#include "Engine/World.h"
#include "Engine/Engine.h"

void UYourHelperLibrary::FindTypedActorByClassAndTag_DynamicOutput(
	UObject* WorldContextObject,
	TSubclassOf<AActor> ActorClass,
	FName Tag,
	AActor*& OutActor,
	EHelperFunctionResult& Branches
)
{
	OutActor = nullptr;
	Branches = EHelperFunctionResult::Failure;

	if (!WorldContextObject || !*ActorClass)
	{
		UE_LOG(LogTemp, Warning, TEXT("FindTypedActorByClassAndTag: Invalid input."));
		return;
	}

	UWorld* World = GEngine->GetWorldFromContextObjectChecked(WorldContextObject);
	if (!World)
	{
		UE_LOG(LogTemp, Warning, TEXT("FindTypedActorByClassAndTag: Could not retrieve world."));
		return;
	}

	TArray<AActor*> FoundActors;
	UGameplayStatics::GetAllActorsOfClass(World, ActorClass, FoundActors);

	for (AActor* Actor : FoundActors)
	{
		if (Actor && Actor->Tags.Contains(Tag))
		{
			// Ensure the found actor is valid and of the desired type
			if (Actor->IsA(ActorClass))
			{
				OutActor = Actor;
				Branches = EHelperFunctionResult::Success;
				return;
			}
		}
	}

	UE_LOG(LogTemp, Warning, TEXT("FindTypedActorByClassAndTag: No actor of class '%s' with tag '%s' found."),
		*ActorClass->GetName(), *Tag.ToString());
}

*Edit - Formatting

2

  • The IsA check seems redundant, GetAllActorsOfClass already filters by class, and you already checked validity in the previous if.

  • You might want to check that Tag is not NAME_None, either return failure or skip the tag check in that case. For consistency reasons I’d suggest handling it the same way you do with other parameters : if (!WorldContextObjec || !*ActorClass || Tag == NAME_None) { ...return; }

  • Using Actor->ActorHasTag(Tag) might be “more proper” than accessing Tags array directly. Tags property might become private in the future.

  • Your implementation is suboptimal. You retrieve all actors of class using GetAllActorsOfClass, and then iterate them to return the first one with given Tag. You could save iterations by doing both at once. Something like this :

for (TActorIterator<AActor> It(World, ActorClass); It; ++It)
{
    AActor* Actor = *It;
    if (IsValid(Actor) && Actor->ActorHasTag(Tag))
    {
        OutActor = Actor;
        Branches = EHelperFunctionResult::Success;
        return;
    }
}
1 Like