Hello. I have a
TArray<AActor*>* ControlledActorsInside
in my code.
In a function for the BP, I get an array from the BP
StartSelectionRealTime(UPARAM(ref)TArray<AActor*>>& ControlledActors)
ControlledActorsInside = &ControlledActors;
Then at any point in time, I access the ControlledActorsInside elements. Each time I do an IsValid(element) check.
How safe is this ? Can CG cause the editor to crash ?
Hi,
Hope you’re doing well.
Add UPROPERTY to your TArray<> and you are good to go. Don’t need worry about it.
And before check your array’s items, simple check if the current element is valid by if statement just for safety.
I work the same way as you mentioned, and I don’t get any crash at all.
Hope that helps - take care.
UPROPERTY cannot be added for TArray<AActor*>* ControlledActorsInside
Oh, sorry about that.
You’re right, it’s a pointer to an array of pointers.
You have raised a great issue.
Look, can’t you try TArray<AActor*> instead?
Since you are doing all the checks, I think you are good to go, but I probably would try to mix some const before touch any pointer. I think as long as you check your TArray before any operation (add, remove, etc.) you are good to go and won’t get any crash.
You kind catch me with that.
TArray<AActor*> would require copying the array passed from BP.
I want to avoid copying and just store a pointer to TArray<AActor*>*.
The data from the TArray is processed at different times depending on user actions.
I see.
How about TWeakObjectPtr<>?
I think using TWeakObjectPtr we can guarantee it’s going to work.
TWeakObjectPtr cannot be created in BP. User must pass TArray<AActor> created in BP.
What do you think about consider the code bellow:
////////////////////////////////////////////////////
// .h
UPROPERTY()
TArray<TWeakObjectPtr<AActor>> ControlledActorsInside;
UFUNCTION(BlueprintCallable)
void StartSelectionRealTime(UPARAM(ref) TArray<AActor*>& ControlledActors);
////////////////////////////////////////////////////
// .cpp
void UYourCoolComponent::StartSelectionRealTime(TArray<AActor*>& ControlledActors)
{
ControlledActorsInside.Empty();
for (AActor* Actor : ControlledActors)
{
if (IsValid(Actor))
{
ControlledActorsInside.Add(Actor);
}
}
// your remaing code goes here...
}
I think we need more heads on this 
How safe depends on how you manage the lifetime of those actors. The issue you will have is when actors in that array are destroyed and cleared by the GC you now have dangling pointers, once you try to access any of those actors you will crash.
If you really have to go with that array I would suggest binding to the event on destroyed for each actor added, this way you can remove any actors that are pending kill.
I tried gc.ForceCollectGarbageEveryFrame 1 after deleting actors and it didn’t cause the problem. Everything is working correctly.
I may have done everything right since I have a pointer to UPROPERTY TArray<AActor*> from BP.
My tests with deleting the actors did not show any problems.
IsValid only tells you if the pointer is pointing to a valid UObject, it won’t know if the memory it’s pointing to has been freed and then reallocated to another UObject (possibly of another type). In such a case IsValid will return true and then you’ll happily dereference it and poof! - that kinda crash is very difficult to track down, especially if you don’t expect it (trust me - been there). The test you describe with forcing GC won’t recreate that kinda situation, or not reliably anyway.
Weak pointers are the way to avoid what I’m describing since a weak pointer actually keeps track of the exact UObject it’s pointing to. So calling IsValid on it only returns true if that memory is still the exact same UObject (not just any valid UObject that just happens to now be there). See Ari’s notes on how weak pointers work
I think that everyone is overly focused on the wrong pointer in this question, the AActor*. The problem is the TArray<>*. Whatever actor has ControlledActorsInside is now relying on memory from the blueprint VM or another object that it has no control over. If that object goes away you’re going to have problems, not if one of the Actors referenced by the array (IsValid on elements should be fine for preventing this problem).
This is a recipe for disaster and you shouldn’t be storing this sort of thing.
This sounds like a premature optimization, and one that I don’t think you really understand. You’re not actually saving yourself anything and just setting yourself up for crashes.
I’m not sure what the owners of this data are actually involved in. From the naming it’s some sort of user selection set. Instead I would have some sort of subsystem responsible for tracking user selections. Other systems (including whatever has this StartSelectionRealTime function) would get the collection when it needs it, do work and then discard it. The subsystem would be the persistent storage for what makes up that collection.
Definitely agree there! I will also highlight - it’s an array of pointers. Why would one need to avoid copying pointers? It’s not like one would be copying the entire actors.
Though I’ll say it is a fun thought experiment of what exactly happens in all sorts of cases when you have a pointer to a TArray that may or may not be there anymore. Sure, crashes, but I don’t doubt they’re fun.
Welp, new fear unlocked! I’m going to have to make some refactoring.. Thanks for the reminder. It should be taken especially seriously that, such an error being quite low-probability to trigger, it may be invisible during development, and emerge only upon release with a large enough sample size.. The worst kind of error.
As for the original question, I’m with MagForceSeven here, you should avoid using a pointer to an array if you don’t have enough mastery over both generic C++ & UE-specific memory management. (And actually, master over the lifecycle of both the actual array owner & the array reference user)
I’m not going to say that it’s never useful (one never knows). But that seems to be a needlessly dangerous way of doing things.
Instead, I would recommend you to store a reference to the array’s owner, and then call the array directly through the owner, every time.
// .h
UMyArrayOwner* ArrayOwner;
// .cpp
if (/*ArrayOwner is not null & valid etc etc... Defensive programming boilerplate*/) {
DoStuff(ArrayOwner->ControlledActors);
}
In terms of actual performance, the difference between this method & your original method, OP, should be assumed to be overkill optimization, unless you have a (very rare) provable, measurable issue.
Of course, you can also use smart pointers if you know how to use them.
We are going a bit off topic here, but when I’ve seen this kinda crash it tended to happen consistently, but after a long runtime - last one was about 30 mins to an hour. But I suspect that you are correct in that you may never ever see it when there isn’t that much going on in the project so it could creep up on you much later on.