Hello guys,
I was testing this function in blueprint and I realized that array of output actors is not cleaned. It is very easy to stuck whole engine from blueprint because everytime blueprint requests return from this function it just adds it to array.
I did some tests and created custom version of this function which works ok. It is not final fixed version because I set it as callable for testing:
.h
UFUNCTION(BlueprintCallable, Category = "HUD")
TArray<AActor*> GetActorsInSelectionRectNew(TSubclassOf<class AActor> ClassFilter, const FVector2D FirstPoint, const FVector2D SecondPoint, bool bActorMustBeFullyEnclosed = false);
.cpp
TArray<AActor*> ACOMHUD::GetActorsInSelectionRectNew(TSubclassOf<class AActor> ClassFilter, const FVector2D FirstPoint, const FVector2D SecondPoint, bool bActorMustBeFullyEnclosed)
{
TArray<AActor*> OutActors;
//Create Selection Rectangle from Points
FBox2D SelectionRectangle;
//This method ensures that an appropriate rectangle is generated,
// no matter what the coordinates of first and second point actually are.
SelectionRectangle.Min.X = FMath::Min(FirstPoint.X, SecondPoint.X);
SelectionRectangle.Min.Y = FMath::Min(FirstPoint.Y, SecondPoint.Y);
SelectionRectangle.Max.X = FMath::Max(FirstPoint.X, SecondPoint.X);
SelectionRectangle.Max.Y = FMath::Max(FirstPoint.Y, SecondPoint.Y);
//The Actor Bounds Point Mapping
const FVector BoundsPointMapping[8] =
{
FVector(1, 1, 1),
FVector(1, 1, -1),
FVector(1, -1, 1),
FVector(1, -1, -1),
FVector(-1, 1, 1),
FVector(-1, 1, -1),
FVector(-1, -1, 1),
FVector(-1, -1, -1)
};
//~~~
//For Each Actor of the Class Filter Type
for (TActorIterator<AActor> Itr(GetWorld(), ClassFilter); Itr; ++Itr)
{
AActor* EachActor = *Itr;
//Get Actor Bounds //casting to base class, checked by template in the .h
const FBox EachActorBounds = Cast<AActor>(EachActor)->GetComponentsBoundingBox(false); /* All Components */
//Center
const FVector BoxCenter = EachActorBounds.GetCenter();
//Extents
const FVector BoxExtents = EachActorBounds.GetExtent();
// Build 2D bounding box of actor in screen space
FBox2D ActorBox2D(0);
for (uint8 BoundsPointItr = 0; BoundsPointItr < 8; BoundsPointItr++)
{
// Project vert into screen space.
const FVector ProjectedWorldLocation = Project(BoxCenter + (BoundsPointMapping[BoundsPointItr] * BoxExtents));
// Add to 2D bounding box
ActorBox2D += FVector2D(ProjectedWorldLocation.X, ProjectedWorldLocation.Y);
}
//Selection Box must fully enclose the Projected Actor Bounds
if (bActorMustBeFullyEnclosed)
{
if (SelectionRectangle.IsInside(ActorBox2D))
{
OutActors.Add(Cast<AActor>(EachActor));
}
}
//Partial Intersection with Projected Actor Bounds
else
{
if (SelectionRectangle.Intersect(ActorBox2D))
{
OutActors.Add(Cast<AActor>(EachActor));
}
}
}
return OutActors;
}
#Analysis
Arrays are typically returned by reference for performance reasons, what you are really asking is whether I should make it so that every time the function is run, the entire previous contents of the array that is pass in by reference are cleared.
I could surely do this, and for something like a HUD function that is most likely running every tick, it does make sense as a protection mechanism.
#Global Array Variables
I did not think to do this in the C++ version of this function because I was not keeping global variables, however in blueprints it is very easy/intuitive to make a global variable of an array in your HUD BP and pass in this function, and this problem arises.
#Summary
This is an easy issue for people to encounter due to how intuitive/easy it is to make global variable arrays in Blueprints and then use them with this function, easily leading to a very large number of actors getting added to a single array because HUD functions tend to run every tick and my function is not emptying the array with each use.
Theoretically it is the responsability of you as the user to clear the array if that is the intended behavior you want, since you are the one using the global array,
In practice it is a mistake many many people are likely to make and I will work on submitting a fix to github.
#Thanks!
Thanks for bringing this issue to my attention!
#Solution 1
For now, you should clear your global array before each call to GetActorsInSelectionBox
#Solution 2
The alternative is to put the use of GetActorsInSelectionBox into a function and use a local variable Actor Array instead of a global variable.
Rama
#Submitted On Github
I’ve submitted the requested change on Github, making it so the array that is passed in is always cleared with each use
https://github.com/EpicGames/UnrealEngine/pull/370
It is awaiting Epic’s review.
Enjoy!
Rama
Thank you very much for your fast response :). I think it is responsibility of function to clean results because I think every other BP node does it too. If we would like to add it to array then we would use BP node for appending of arrays ^^.
Bug 2:
You do not initialize FBox2D:
FBox2D SelectionRectangle;
should be
FBox2D SelectionRectangle = FBox2D(0);
right now Min of FBox2D is always -1000000 so selection rectangle does not respect First Point.
You can see hotfix in my function above but using Min and Max is not needed. You just need to initialize FBox2D.
Also It wil be good to have some possibility specify component which bounds will be used because right now it is not very usable. For example default MyCharacter in Epic’s example game has huge bounds. It is not issue for me because I am able to fix this in code but I think it will help other BP users :).
Again, thank you for your interest, tutorials, features and other activity :))
#Fixed!
Thanks Nonder!
Somehow in my own code base I never had to do this initialization, but that is now done
https://github.com/EpicGames/UnrealEngine/pull/370/files
Great to hear from you!
#
Rama