How to handle the dangers of components "referencing" other entities?

So I just fixed a silly bug that took me way too long to grasp.
I have this Component:

Code (CSharp):

1. public struct AttackTarget : IComponentData{
2.     private TargetAcquisitionState TargetAcquisitionState;
3.     private Entity Target;
4.     public float3 Direction;
5. }

Then I delete Entities somewhere down the line, and a different Job tries this:

Spoiler: Job

Code (CSharp):

1. [BurstCompile]
2. struct AttackUpdateTargetDistanceJob : IJobProcessComponentDataWithEntity<AttackData, AttackTarget, Position>{
3.  
4.     [ReadOnly] public ComponentDataFromEntity<Position> PositionsFromEntity;
5.  
6.     public void Execute([ReadOnly] Entity entity, [ReadOnly] int index, ref AttackData attackData, ref AttackTarget attackTarget, [ReadOnly] ref Position position)
7.     {
8.         if(attackTarget.TargetAcquisitionState != TargetAcquisitionState.Searching){
9.             Entity target = attackTarget.Target;
10.  
11.             float targetRadius = 1.0f;
12.  
13.             //This check was missing, can this be done "more nicely?" rather than locally in this job
14.             if(PositionsFromEntity.Exists(attackTarget.Target)){
15.                 attackTarget.Direction = PositionsFromEntity[attackTarget.Target].Value - position.Value;
16.                 attackTarget.Distance = math.length(attackTarget.Direction);
17.                 attackTarget.DistanceToContact = attackTarget.Distance - targetRadius;
18.             }else{
19.                 attackTarget.Target = Entity.Null;
20.                 attackTarget.TargetAcquisitionState = TargetAcquisitionState.Searching;
21.             }
22.  
23.         }
24.     }
25. }

The fix is inside the job, see the comment.
I was not testing if the other entity still exists before trying to access it's component data.

Now this works, but it kind of bothers me for some reasons:
1st: Semantically, this suggests I only check if that component exists. Is there a test to see if an entity still exists at all? I could cache the ID and version and do tests but that seems overkill. Is there something built-in?

2nd: I would have this test every time I work with entity references and try to got their components.
But since this seems to potentially affect any component with an entity "reference", I am thinking about a system that just has a job for every component that got a entity "reference".
That system then makes sure to sanitize those. This would require a entity.IsDeleted() though.

3rd: Make every Entity "reference" private and have methods that set them. This is as to signal the "danger" of those entity references. In my particular case it even makes more sense since I can set my enum consistently too.

Code (CSharp):

1.     public struct AttackTarget : IComponentData{
2.         private TargetAcquisitionState TargetAcquisitionState;
3.         private Entity Target;
4.         public float3 Direction;
5.         public float Distance;
6.         public float DistanceToContact;
7.  
8.         public Entity getTargetUnsave(){
9.             return Target;
10.         }
11.  
12.         public void setTarget(Entity target){
13.             this.Target = target;
14.             this.TargetAcquisitionState = TargetAcquisitionState.Found;
15.         }
16.  
17.         public bool hasTarget(){
18.             //can I check if entity exists?
19.             return (TargetAcquisitionState != TargetAcquisitionState.Searching);
20.         }
21.  
22.         public void reset(){
23.             this.Target = Entity.Null;
24.             this.TargetAcquisitionState = TargetAcquisitionState.Searching;
25.         }
26.     }

How do you guys control the danger of such entity "references" being pulled away beneath you?

 

You could think of AttackTarget as an event. Some system detects a target should be attacked->That system adds the AttackTarget component with all the information->Your system picks up that event, processes it, and removes the component if it's no longer needed. You may need to rework/add some system/components a bit. For example,
if TargetAcquisitionState is searching, should there always be a target member? Or if you find a target, should a target be added instead? This way you will do your searching in one system, but the jobs/system for working with a target will only be run if there are any components to work with. If the target Entity doesn't exist, it's only because it was wasn't assigned or assigned incorrectly.

This seems to be the way to think of things, a component should either be:

1. A tagging component, no members, just used for filtering.

2. Components where you can always work with something and should always exist, like scale/position/rotation/etc.

3. Events, ideally empty but can have members, only exist when something should be done over a frame or more, then removed.

There are edge cases like a linked list/tree of entity components, but to use those/answer your original question on checking that an entity exists, you can compare the entity with default(Entity) or Entity.Null. Either of those return an entity with Version: 0 and Index: 0 which does not exist(correct me if I'm wrong). There is also EntityManager.Exists if you're not in a job.

 

Thanks guys!

Related follow up:
https://forum.unity.com/threads/entitymanager-exists-in-a-jobcomponentsystem.546625/

So, since I cant do that in jobs, I guess I stick to just querying the ComponentDataFromEntity.Exists(entity).
However I'll move it to the system that does the target acquisition in the first place, it fits semantically better in there and I can enter the acquisition branch if the target is gone.
However I may have to unnecessarily add ComponentDataFromEntity<Position> to the system just to have something to query for.

@RecursiveEclipse
This particular Component is not really a event, its a state that is used by multiple systems down the line.
Think State "CurrentActiveTarget" rather than Event "DoAttackTarget".
Currently the only events I have I gather in a NativeQueue in some system (I think the two-stick-shooter examples did it like that.)
The system holding the Queue is injected into other systems requiring the queue.

Thinking of it, this requires me to deal with manual dependency management for the Queue, so maybe emitting entities as events is actually simpler? I got to tinker with that.


Rule of thumb (for me anyways):
Before or in the job that populates/updates components with Entity "references", do the check if the entity is still valid.

 

Is there an impact on multi threading if you do this. If you reference other entities, then both entities cannot process in parallel right? Imagine trying to kill one entity while the other entity is trying to do something else.

 

That is difficult to have happen normally unless you're doing something very wrong. You're supposed to destroy entities in a job through a barrier/buffer which only get played back once the system(technically the barrier) is done. The entities that exist when the system runs, will always exist in the jobs you schedule in that system unless you intentionally delete them using EntityManager in OnUpdate(). Defer any entity deletions to PostUpdateCommands(in a ComponentSystem) or EntityCommandBufffer(JobSystem) and you should be fine.

I can see two main ways you can mess up in this way:

1. You manually/mistakenly place multiple copies of an entity in a nativearray, then in a buffer delete the Entities/entity references. When the barrier plays back, the barrier will complain that you're trying to delete an entity that doesn't exist(a copy of one you already deleted)

1.b You use IJobProcessComponentData, but schedule a job with ComponentA which contains a reference to Entity0, delete Entity0 in that job, then schedule a job with ComponentB which also references Entity0 and you use DestroyEntity/AddComponent/SetComponent on that entity.

Issues with using Set/Add can be avoided in these cases by placing destroy jobs last. However, if you have multiple components with the same entity reference, I might be questioning your design.

2. You have two or more jobs that write to or read/write the same component, but you don't pass the first job as a dependency to the second. If the component is marked [ReadOnly] in both jobs, this is fine.

 

Thanks for that excellent explanation!