Bug ICollisionEventsJob raises events many times at once

The ICollisionEventsJob calls Execute on 8 threads in parallel for a single collision, causing my projectiles to deal damage 8x the amount they should. Except on the first collision, it works properly there. Even though I remove the damage component after the first collision, it doesn't care.

ICollisionEventsJob also does not allow me to get the JobIndex, so I can't use a concurrent ECB to enforce it to only happen once.

Code (CSharp):

1. using System;
2. using Unity.Burst;
3. using Unity.Collections;
4. using Unity.Entities;
5. using Unity.Jobs;
6. using Unity.Physics;
7. using Unity.Physics.Systems;
8.  
9. [UpdateAfter(typeof(ExcludeCollisionPairJobSystem))]
10. [UpdateAfter(typeof(EndFramePhysicsSystem))]
11. // [UpdateAfter(typeof(StepPhysicsWorld))]
12. [UpdateBefore(typeof(DestroyOnCollisionJobSystem))]
13. [UpdateBefore(typeof(DealDamageJobSystem))]
14. public class DealsDamageOnCollisionJobSystem : JobComponentSystem
15. {
16.     private bool systemRunning = true;
17.  
18.     private EndSimulationEntityCommandBufferSystem endSimECBSystem;
19.     private BuildPhysicsWorld buildPhysicsWorld;
20.     private StepPhysicsWorld stepPhysicsWorld;
21.  
22.     // [BurstCompile]
23.     private struct DealsDamageOnCollision : ICollisionEventsJob
24.     {
25.         private EntityCommandBuffer ecb;
26.         [ReadOnly] private ComponentDataFromEntity<Tag_DealsDamageOnCollision> dealsDamageOnCollisionData;
27.         [ReadOnly] private ComponentDataFromEntity<HealthComponent> healthData;
28.         [ReadOnly] private ComponentDataFromEntity<DealsDamageComponent> damageData;
29.  
30.         private BufferFromEntity<DamageBufferData> damageBuffer;
31.  
32.         #region Constructor
33.         public DealsDamageOnCollision(EntityCommandBuffer ecb,
34.             [ReadOnly] ComponentDataFromEntity<Tag_DealsDamageOnCollision> destroyCollisionData,
35.             [ReadOnly] ComponentDataFromEntity<HealthComponent> healthData,
36.             [ReadOnly] ComponentDataFromEntity<DealsDamageComponent> damageData,
37.             BufferFromEntity<DamageBufferData> damageBuffer)
38.         {
39.             this.ecb = ecb;
40.             this.dealsDamageOnCollisionData = destroyCollisionData;
41.             this.healthData = healthData;
42.             this.damageData = damageData;
43.             this.damageBuffer = damageBuffer;
44.         }
45.         #endregion
46.  
47.         public void Execute(CollisionEvent collisionEvent)
48.         {
49.             // UnityEngine.Debug.Log($"{collisionEvent.Entities.EntityA}, {collisionEvent.Entities.EntityB}");
50.             CheckAndDealDamage(collisionEvent.Entities.EntityA, collisionEvent.Entities.EntityB);
51.             CheckAndDealDamage(collisionEvent.Entities.EntityB, collisionEvent.Entities.EntityA);
52.                      
53.         }
54.  
55.         private void CheckAndDealDamage(Entity entity1, Entity entity2)
56.         {
57.             if(dealsDamageOnCollisionData.Exists(entity1) && damageData.Exists(entity1))
58.             {
59.                 if(healthData.Exists(entity2))
60.                 {
61.                     DynamicBuffer<DamageBufferData> buffer = damageBuffer.Exists(entity2) ?
62.                         damageBuffer[entity2] :
63.                         ecb.AddBuffer<DamageBufferData>(entity2);
64.  
65.                     buffer.Add(new DamageBufferData{
66.                         damage = damageData[entity1].damage,
67.                         dealtBy = damageData[entity1].dealtBy
68.                     });
69.                     ecb.RemoveComponent<DealsDamageComponent>(entity1);
70.                 }
71.             }
72.         }
73.     }
74.  
75.     protected override void OnCreate()
76.     {
77.         endSimECBSystem = World.GetOrCreateSystem<EndSimulationEntityCommandBufferSystem>();
78.         buildPhysicsWorld = World.GetOrCreateSystem<BuildPhysicsWorld>();
79.         stepPhysicsWorld = World.GetOrCreateSystem<StepPhysicsWorld>();
80.     }
81.  
82.     protected override JobHandle OnUpdate(JobHandle inputDeps)
83.     {
84.         if(!systemRunning)
85.             return default;
86.      
87.         // If physics are off, get out of here
88.         if(stepPhysicsWorld.Simulation.Type == SimulationType.NoPhysics)
89.             return default;
90.      
91.         // inputDeps.Complete();
92.  
93.         JobHandle jobHandle = new DealsDamageOnCollision(
94.             endSimECBSystem.CreateCommandBuffer(),
95.             GetComponentDataFromEntity<Tag_DealsDamageOnCollision>(true),
96.             GetComponentDataFromEntity<HealthComponent>(true),
97.             GetComponentDataFromEntity<DealsDamageComponent>(true),
98.             GetBufferFromEntity<DamageBufferData>(false)
99.         ).Schedule(stepPhysicsWorld.Simulation, ref buildPhysicsWorld.PhysicsWorld, inputDeps);
100.      
101.         endSimECBSystem.AddJobHandleForProducer(jobHandle);
102.  
103.         return jobHandle;
104.     }
105. }

Edit: The problem still exists (and I've tried many different approaches at this point) My work around was to have my job that deals the damage from the damage buffer to check if any of the damage buffer elements were duplicate.

 

Same issue here (

 

I'm surprised to hear that. ICollisionEventsJob is a single threaded job and should only show up as many times as you scheduled it in one step. Looking at your code it seems like you really only schedule one instance of it, so no sure what could be wrong here. The DisplayCollisionEventsSystem implements an ICollisionEventsJob, and it only runs once. Can you confirm that one runs once for you?

 

I just confirmed that this bug definitely still exists. I wrote code to check if it applied duplicate instances of damage, which is still confirming that for me. When disabled, my unit goes from being killed in 10 shots to being killed in 1.
As far as I can tell, when a collision is detected it passes the same collision off to multiple threads, each do their own work, each result in the same conclusion, an instance of damage added to the dynamic buffer.
Though out of curiosity, is there a reason ICollisionEventsJob doesn't give entityInQueryIndex/JobIndex for the use of concurrent command buffers? This might be because I'm passing in a non-concurrent ECB but doing multithreaded work?

The oddest part in all of this, is I rewrote this code entirely and get the same result.
(Yes the rewrite is super slow. I wish we had some .ForEach() syntax that we could do over these collision events)

Code (CSharp):

1. public static bool TryCompleteAndGetPhysicsSimulation(this ISimulation iSimulation, out Simulation simulation)
2.         {
3.             if(iSimulation.Type != SimulationType.NoPhysics && iSimulation is Simulation)
4.             {
5.                 simulation = (Simulation)iSimulation;
6.                 simulation.FinalJobHandle.Complete();
7.                 return true;
8.             }
9.  
10.             simulation = default;
11.             return false;
12.         }

Code (CSharp):

1. using AveaUtils;
2. using Damage;
3. using Unity.Entities;
4. using Unity.Physics;
5. using Unity.Physics.Systems;
6.  
7. [UpdateAfter(typeof(ExcludeCollisionPairJobSystem))]
8. [UpdateAfter(typeof(EndFramePhysicsSystem))]
9. [UpdateAfter(typeof(StepPhysicsWorld))]
10. [UpdateBefore(typeof(DestroyOnCollisionJobSystem))]
11. [UpdateBefore(typeof(DealDamageJobSystem))]
12. public class DealsDamageOnCollisionJobSystem : SystemBase
13. {
14.     private StepPhysicsWorld stepPhysicsWorld;
15.     private BuildPhysicsWorld buildPhysicsWorld;
16.  
17.     protected override void OnCreate()
18.     {
19.         buildPhysicsWorld = World.GetOrCreateSystem<BuildPhysicsWorld>();
20.         stepPhysicsWorld = World.GetOrCreateSystem<StepPhysicsWorld>();
21.     }
22.  
23.     protected override void OnUpdate()
24.     {
25.         if(!stepPhysicsWorld.Enabled)
26.             return;
27.  
28.         // If physics are off, get out of here
29.         Simulation simulation;
30.         if(!stepPhysicsWorld.Simulation.TryCompleteAndGetPhysicsSimulation(out simulation))
31.             return;
32.  
33.         BufferFromEntity<DamageBufferData> damageBuffer = GetBufferFromEntity<DamageBufferData>(false);
34.         ComponentDataFromEntity<DealsDamageComponent> damageData = GetComponentDataFromEntity<DealsDamageComponent>(true);
35.         ComponentDataFromEntity<TeamIDComponent> teamData = GetComponentDataFromEntity<TeamIDComponent>(true);
36.  
37.         foreach(CollisionEvent collisionEvent in simulation.CollisionEvents)
38.             collisionEvent.Entities.DealDamageIfApplicable(
39.                 damageBuffer, in damageData, in teamData, UnityEngine.Time.time
40.             );
41.  
42.         foreach(TriggerEvent triggerEvent in simulation.TriggerEvents)
43.             triggerEvent.Entities.DealDamageIfApplicable(
44.                 damageBuffer, in damageData, in teamData, UnityEngine.Time.time
45.             );
46.     }
47. }

Code (CSharp):

1.  
2. using Unity.Entities;
3. using Unity.Burst;
4. using Unity.Physics;
5. using Teams;
6.  
7. namespace Damage
8. {
9.     public static class DamageUtils
10.     {
11.         [BurstCompile] public static void DealDamageIfApplicable(
12.             this EntityPair pair,
13.             BufferFromEntity<DamageBufferData> damageBuffer,
14.             in ComponentDataFromEntity<DealsDamageComponent> damageData,
15.             in ComponentDataFromEntity<TeamIDComponent> teamData,
16.             float time
17.         )
18.         {
19.             if(damageData.Exists(pair.EntityA))
20.             {
21.                 DealDamageIfApplicable(
22.                     dealingDamage: damageData[pair.EntityA].dealtBy,
23.                     receivingDamage: pair.EntityB,
24.                     deliveredBy: pair.EntityA,
25.                     damageBuffer, damageData[pair.EntityA].damage, in teamData, time
26.                 );
27.             }
28.             if(damageData.Exists(pair.EntityB))
29.             {
30.                 DealDamageIfApplicable(
31.                     dealingDamage: damageData[pair.EntityB].dealtBy,
32.                     receivingDamage: pair.EntityA,
33.                     deliveredBy: pair.EntityB,
34.                     damageBuffer, damageData[pair.EntityB].damage, in teamData, time
35.                 );
36.             }
37.         }
38.  
39.         [BurstCompile] public static void DealDamageIfApplicable(Entity dealingDamage, Entity receivingDamage, Entity deliveredBy,
40.             BufferFromEntity<DamageBufferData> damageBuffer,
41.             int damage, in ComponentDataFromEntity<TeamIDComponent> teamData, float time)
42.         {
43.             // If the receiving entity can't take damage, leave
44.             if(!damageBuffer.Exists(receivingDamage))
45.                 return;
46.          
47.             // No friendly fire
48.             if(TeamUtils.IsOnSameTeam(dealingDamage, receivingDamage, in teamData))
49.                 return;
50.  
51.             // Add a new instance of damage to the damage buffer
52.             damageBuffer[receivingDamage].Add(new DamageBufferData{
53.                 damage = damage,
54.                 dealtBy = dealingDamage,
55.                 deliveredBy = deliveredBy,
56.                 time = time
57.             });
58.         }
59.     }
60. }
61.  

 

You do have a foreach syntax for iterating through collision events - foreach(var event in Simulation.CollisionEvents)...

Now back to ICollisionEventsJob. I would expect it does do work multiple times if scheduled on multiple threads. But it's an IJob, so it shouldn't occupy multiple threads unless scheduled in that way. Can you please verify that you are only scheduling 1 instance of this job, not more than that?

 

Ok, I found the cause.

The doc here states that "A single GameObject with a Physics Body component can contain children with Physics Shapes. These are going to be merged into one compound PhysicsCollider"

I assumed that under the hood it's treated as a single collider, which apparently is not the case. ICollisionEventsJob fires Execute for each PhysicsShape on each entity, thus exploding the number of events with the same EntityA/B, which looked to me like it's being executed multiple times.

Now, it's still looking very inconsistent. The code is simple:

Code (CSharp):

1.  
2. public class TestSystem : JobComponentSystem
3. {
4.     private EndSimulationEntityCommandBufferSystem endSimECBSystem;
5.     private BuildPhysicsWorld buildPhysicsWorld;
6.     private StepPhysicsWorld stepPhysicsWorld;
7.  
8.     private struct TestJob : ICollisionEventsJob
9.     {
10.         private EntityCommandBuffer ecb;
11.  
12.         public TestJob(EntityCommandBuffer ecb)
13.         {
14.             this.ecb = ecb;
15.         }
16.  
17.         public void Execute(CollisionEvent collisionEvent)
18.         {
19.             Debug.Log(World.DefaultGameObjectInjectionWorld.Time.ElapsedTime);
20.             Debug.Log(collisionEvent.EntityA.Index);
21.             Debug.Log(collisionEvent.ColliderKeyA.Value);
22.             Debug.Log(collisionEvent.EntityB.Index);
23.             Debug.Log(collisionEvent.ColliderKeyB.Value);
24.         }
25.     }
26.  
27.     protected override void OnCreate()
28.     {
29.         endSimECBSystem =   World.GetOrCreateSystem<EndSimulationEntityCommandBufferSystem>();
30.         buildPhysicsWorld = World.GetOrCreateSystem<BuildPhysicsWorld>();
31.         stepPhysicsWorld =  World.GetOrCreateSystem<StepPhysicsWorld>();
32.     }
33.  
34.     protected override JobHandle OnUpdate(JobHandle inputDeps)
35.     {
36.         JobHandle jobHandle = new TestJob(endSimECBSystem.CreateCommandBuffer())
37.             .Schedule(stepPhysicsWorld.Simulation, ref buildPhysicsWorld.PhysicsWorld, inputDeps);
38.  
39.         endSimECBSystem.AddJobHandleForProducer(jobHandle);
40.  
41.         return jobHandle;
42.     }
43. }
44.  

Setup is simple - a cube with two PhysicsShape falls down on a plane with two PhysicsShape as well, all of them set to collide with events:



And the output is the following:

0.613342821598053
1
1073741823
0
1073741823

0.613342821598053
1
2147483647
0
1073741823

0.613342821598053
1
2147483647
0
2147483647

Timestamp ensures we're in the same step.

So entities 1 and 0 collide - that's correct. It raises only one event per pair of objects, which seems to be different from PhysX, but maybe it's by design, just noting. There are 3 events in total, which is weird - cube is set to overlap all colliders, so I'd expect 4 pairs of colliders and 4 events. Collider keys don't make any sense to me, if that's colliders pair-wise collision between entities A and B.

 

In my case, both colliders are non-compound, that is, they each have 1 physics body, and 1 physics shape. No children other than the one with only the RenderMesh.

And I meant foreach that we could filter like the Entities.ForEach(). Collisions.Involving<T>.ForEach() or something. Some way to filter which collisions are being iterated over. If a unit collides with the floor, I shouldn't need to check if either entity has the Tag_CanExplode component, I should be able to just filter for all collisions involving a specific component, or component set. I really don't want to loop over 30k unit to ground collisions every frame searching for if any might be a projectile dealing damage, or something.

 

@WildMaN Yes, events are raised per collider instance in the compound, and RigidBodyIndex+ColliderKey give you a unique collider, and multiple of those could be belonging to the same entity. It's just how this works by design. In your example, I'm assuming that you are hitting an edge case where one of the diagonal pairs collides, while the other one doesn't. Try rotating the cube (upper one) by 90 degrees around vertical axis so that they all overlap for sure, you should get 4 of them.

Regarding collider keys, you can use the rigid body index to access the body and its compound collider, and then use GetLeaf() using the collider key to access the child collider.

 

@aveakrause Yes, I understand. Unfortunately we don't have that at the moment. However, in latest release you have the option of using DynamicBufferCollisionEventAuthoring.cs (in the samples) where you can store events in dynamic buffers of entities that participated in them. However, it comes at a small cost (since conversion from stream to dynamic buffers needs to happen), and also introduces state since events have Enter/Stay/Exit information, so be careful if you are relying on deterministic rollback.

 

Totally understandable. Just a suggestion on a something that would be helpful

Either way, I still encounter it raising collisions between 2 bodies far more than expected in a single frame. My duplicate check does check a timestamp as well to filter out all the duplicates.

 

Thanks for clarifying that!

And speaking of 3 vs 4 events - seems to be a bug in Unity Physics, first colliders pair isn't recognized for some reason, but switching to Havok correctly returns all 4 events. Filing a bug report.

 

Yes, it could be a bug, but it also could be due to slightly different collision algorithms - if you are not seeing a penetration or tunneling, it's probably just that. However, if it's penetrating with that collider, please file a bug.

 

I'd still like to get to the bottom of this. If you have physics samples project, can you try the DisplayCollisionEventsSystem and let me know if it runs one or more jobs for you?

 

I am not sure this is a bug.

I see this behaviour with Unity Physics and single colliders. One of the colliders, a long capsule, moves relatively fast (magnitude 3.5) and strikes a convex hull generated from a mesh.

The average contact position is roughly in the same place, but there is some jitter, also with the normals.

Between 1 and 4 collision events are generated. I spawn an impact object for each, which is how I found the behaviour.

The 2nd and 3rd step in this debug video shows a duplicated impact (literally all entities involved are the same).



The contacts / events displayed look like a plethora of collisions. In my case, it seems like the collision event is triggered for both the inner (mesh) as well as the outer collider (hull), which makes sense in a way... but also not?

The interesting part is, the lead-up to this was already showing contacts with the outer collider, but it didn't trigger an event. As the image leads one to believe, this is indeed a quadruple impact.As far as I understand, a mesh collider generates an event for every triangle hit? There are three colliders in this image, but the tight inner hull is in a different category and does not play a role.



This is how this plays out step by step:

 

I indeed think it's the Mesh Collider reporting multiple events (which I believe is intended behaviour). Here it is without the Mesh Collider.

The only thing that baffles me is that when the inner collider is gone, the

collisionEvent.CalculateDetails(ref PhysicsWorld).AverageContactPointPosition;

returns a value that's pretty far from the contacts shown by the debug view.

Odd. This really shouldn't be the contact point here.

 

I'll just recap what one should expect from collision events, although you covered most of it.

1. Capsule vs. mesh will give a collision event for each capsule vs. triangle impact - which means you will get same entity pair (capsule and mesh entities), but different ColliderKey that identifies a triangle inside a mesh. Please make sure there are no duplicates when involving the ColliderKey in the equation, there should be none. So, if a capsule touches 3 triangles of the mesh (which is relatively likely), there will be 3 collision events.
2. Green contacts don't mean there will be a collision for sure, it's just a potential contact point that might get solved by the solver. And solver works in iterations, so it might decide at some point (due to some other contact) that impulse shouldn't be applied at a particular point, so you end up not getting collision events (they are raised only for non zero impulse contacts).
3. Physics engine is predictive, which means it can't really know the exact order of contacts. Your hull inside of a mesh doesn't have any guarantees that it won't get an applied impulse. If your capsule is fast enough (and it appears to be), it will generate a contact pair for capsule vs. outer mesh and capsule vs. inner mesh. If you are lucky, solver will solve outer mesh first, apply impulse and then realize that inner mesh won't get hit and discard it as a collision event. But if you are not lucky, inner mesh might get solved first. Now, since this is an iterative solver, subsequent iterations might decide to reduce the impact on inner mesh (because outer one also gets impulse applied), but it will always end up with some small impulse. And the order solver accesses contacts is based on body index, not proximity (it would be much slower to calculate order of reaching contacts). So keep that in mind when seeing contacts with bodies that are hidden by other bodies. (same may happen with bodies hidden by a wall and a car approaching it with high velocity) Not sure what your exact goal with the inner mesh is, but you might want to look into collision filters for that one, just to avoid this sort of behavior.

I hope this makes it a bit clearer. Let me know if there are more uncertainties here. I'd be glad to help.

 

Hey all, I made simple system that collects all collisions raised by unity system and discards all but first for given Entity pair:
https://github.com/Sroka/unity_physics_filtered_collisions
It's usage is the same as for "ICollisionEventsJob" so it's just a matter of changing interface and imported systems.
I believe it's also worth checking out as an Example on how to implement custom job types. This blog post was super helpful as a reference: https://www.jacksondunstan.com/articles/5406
If anyone knows how to enumerate on values in NativeHashMap values in Job without allocation I could also use some help here:

Code (CSharp):

1.                 var collisionEvents = jobData.CollisionEvents.GetValueArray(Allocator.Temp);
2.                 for (var index = 0; index < collisionEvents.Length; index++)
3.                 {
4.                     jobData.UserJobData.Execute(collisionEvents[index]);
5.                 }
6.  
7.                 collisionEvents.Dispose();
8.