Initial code review to keep me honest

Hi all,
Thanks to this forum and a few tutorials around the place I have finally been able to make the beginning of a game in ECS. I am starting to be able to self correct my mistakes and trying a few ideas but what I think would be really helpful is pulling up the covers and getting some feedback on a couple of my systems and approach.

• I have x amount of locations (currently 50). They are just 'areas' 50 in the x and z.
• in those locations I spawn (currently) 100 entities
• These entities are basic cubes, no physics. They have 1 component (will split next code changes) called `HumanData`
• Will probably split Infection stuff to `InfectionData`
• Conceptually for the game, they can either be infected or uninfected. true or false. Keep it simple for now.

------------------------------------------------------------------------------------

Code (CSharp):

1. public struct HumanData : IComponentData {
2.     public int Location; // simple int will correspond to a location
3.     public float3 OffsetPosition; // offset for position
4.     public float3 StartPos;
5.     public float3 EndPos;
6.     public float3 TravelVector;
7.     public float Distance;
8.     public float Speed;
9.     public int MyIndex;
10.  
11.     // infection stuff
12.     public bool isInfected;
13.     public bool isAsymptomatic;
14. }

------------------------------------------------------------------------------------

• I randomise movement so they can move around to different points in there locations (HumanMoveSystem)
• They cannot currently move outside that initial location area. There new 'endPos' is offset by the locations position so they always stay in that area.

------------------------------------------------------------------------------------

Code (CSharp):

1.     [UpdateAfter(typeof(RandomSystem))]
2.     public class HumanMoveSystem : SystemBase
3.     {
4.  
5.         protected override void OnUpdate()
6.         {
7.             float deltaTime = Time.DeltaTime;
8.             var randomArray = World.GetExistingSystem<RandomSystem>().RandomArray;
9.             float locationSize = (GameDataManager.instance.locationSize - 1) / 2.0f;
10.  
11.             Entities
12.                 .WithNativeDisableParallelForRestriction(randomArray)
13.                 .WithName("HumanMoveSystem")
14.                 .WithBurst()
15.                 .ForEach((int nativeThreadIndex, ref Translation position, ref Rotation rotation, ref HumanData humanData) =>
16.                 {
17.                     float3 travelVector = humanData.EndPos - humanData.StartPos;
18.                     position.Value += math.normalize(travelVector) * deltaTime * humanData.Speed;
19.                     humanData.TravelVector = math.normalize(travelVector) * deltaTime * humanData.Speed;
20.                     float distance = math.distance(position.Value, humanData.EndPos);
21.                     humanData.Distance = distance;
22.                     if (distance < 1f)
23.                     {
24.                         humanData.StartPos = humanData.EndPos;
25.                         float distanceCheck;
26.  
27.                         do
28.                         {
29.                             var random = randomArray[nativeThreadIndex];
30.                             float x = random.NextFloat(-1 * locationSize, locationSize);
31.                             float z = random.NextFloat(-1 * locationSize, locationSize);
32.                             humanData.EndPos = new float3(x, 0, z) + humanData.OffsetPosition;
33.                             distanceCheck = math.distance(humanData.StartPos, humanData.EndPos);
34.                             randomArray[nativeThreadIndex] = random;
35.                         }
36.                         while (distanceCheck < locationSize/2.0f);
37.                     }
38.                 })
39.                 .Schedule();
40.         }
41.     }

------------------------------------------------------------------------------------

• Because of this, I have a patient zero in each location (for now, while I work on improvements)
• as an entity A distance to another entity B reaches close enough (distance < 0.1f) - if A or B are infected, then both become infected.(HumanInfectionDetectionSystem)

------------------------------------------------------------------------------------

Code (CSharp):

1.     [UpdateAfter(typeof(HumanMoveSystem))]
2.     public class HumanInfectionDetectionSystem : JobComponentSystem
3.     {
4.         EntityQuery m_Group;
5.         protected override void OnCreate()
6.         {
7.  
8.             var query = new EntityQueryDesc()
9.             {
10.                 All = new ComponentType[]
11.                 {
12.                     typeof(HumanData),
13.                     ComponentType.ReadOnly<Translation>()
14.                 }
15.             };
16.  
17.             m_Group = GetEntityQuery(query);
18.  
19.         }
20.  
21.         [BurstCompile]
22.         public struct DistanceCheckJob : IJobParallelFor
23.         {
24.             [NativeDisableParallelForRestriction] public NativeArray<HumanData> HumanData;
25.             [ReadOnly] public NativeArray<Translation> TranslationData;
26.  
27.             public void Execute(int index)
28.             {
29.                 var currentData = HumanData[index];
30.  
31.                 var currentPos = TranslationData[index];
32.                 for (var i = index + 1; i < HumanData.Length; i++)
33.                 {
34.                     var humanBPosition = TranslationData[i];
35.                     var humanBData = HumanData[i];
36.                     if (currentData.Location == humanBData.Location)
37.                     {
38.                         // if we are already infected, ignore
39.                         if ((!currentData.isInfected && humanBData.isInfected) ||
40.                             (currentData.isInfected && !humanBData.isInfected))
41.                         {
42.                             if (math.distance(currentPos.Value, humanBPosition.Value) < 0.5f)
43.                             {
44.                                 currentData.isInfected = true;
45.                                 humanBData.isInfected = true;
46.                                 HumanData[index] = currentData;
47.                                 HumanData[i] = humanBData;
48.                             }
49.                         }
50.                     }
51.  
52.                 }
53.             }
54.         }
55.  
56.         protected override JobHandle OnUpdate(JobHandle inputDependencies)
57.         {
58.             var humanData = m_Group.ToComponentDataArray<HumanData>(Allocator.TempJob);
59.             var translationData = m_Group.ToComponentDataArray<Translation>(Allocator.TempJob);
60.             var distanceCheckJob = new DistanceCheckJob
61.             {
62.                 HumanData = humanData,
63.                 TranslationData = translationData,
64.             };
65.             var collisionJobHandle = distanceCheckJob.Schedule(translationData.Length, 32);
66.             collisionJobHandle.Complete();
67.  
68.             // ToComponentDataArray copies the data, so we need to write it back again
69.             m_Group.CopyFromComponentDataArray(humanData);
70.  
71.             humanData.Dispose();
72.             translationData.Dispose();
73.             return collisionJobHandle;
74.  
75.         }
76.     }

------------------------------------------------------------------------------------

• If an entity is infected, I change the material on it. This is a bottleneck system due to needing to use EntityManager on a SharedComponent (ShowInfectionMaterialSystem)

------------------------------------------------------------------------------------

Code (CSharp):

1. [UpdateAfter(typeof(HumanMoveSystem))]
2.     public class ShowInfectionMaterialSystem : SystemBase
3.     {
4.         Material newMaterial;
5.        
6.         // this just checks to see if the object is infected, if so, change the material
7.         protected override void OnUpdate()
8.         {
9.             if (newMaterial == null)
10.                 newMaterial = GameDataManager.instance.infectedMaterial;
11.  
12.             Entities
13.                 .WithName("ShowInfectionMaterialSystem")
14.                 .WithoutBurst()
15.                 .WithStructuralChanges()
16.                 .ForEach((Entity entity, ref HumanData infectionData) =>
17.                 {
18.                     if (infectionData.isInfected && !infectionData.isAsymptomatic)
19.                     {
20.                         var render = EntityManager.GetSharedComponentData<RenderMesh>(entity);
21.                         EntityManager.SetSharedComponentData(entity, new RenderMesh() { mesh = render.mesh, material = newMaterial });
22.                         infectionData.isAsymptomatic = true;
23.                     }
24.                 })
25.                 .Run();
26.  
27.         }
28.     }

------------------------------------------------------------------------------------

• Finally just for visual purposes, over each location I have a text field I update with the '% infected' (InformationSystem)

------------------------------------------------------------------------------------

Code (CSharp):

1.     [UpdateAfter(typeof(HumanInfectionDetectionSystem))]
2.     public class InformationSystem : JobComponentSystem
3.     {
4.         EntityQuery m_Group;
5.  
6.         protected override void OnCreate()
7.         {
8.  
9.             var query = new EntityQueryDesc()
10.             {
11.                 All = new ComponentType[]
12.                 {
13.                     ComponentType.ReadOnly<HumanData>()
14.                 }
15.             };
16.  
17.             m_Group = GetEntityQuery(query);
18.         }
19.  
20.         [BurstCompile]
21.         public struct PercentageUpdateJob : IJobFor
22.         {
23.             [ReadOnly] public NativeArray<HumanData> HumanData;
24.             public NativeArray<int> Totals;
25.  
26.  
27.             public void Execute(int index)
28.             {
29.                 var currentData = HumanData[index];
30.                 if (currentData.isInfected && currentData.isAsymptomatic)
31.                     Totals[currentData.Location]++;
32.             }
33.         }
34.  
35.         protected override JobHandle OnUpdate(JobHandle inputDependencies)
36.         {
37.             var humanData = m_Group.ToComponentDataArray<HumanData>(Allocator.TempJob);
38.             NativeArray<int> totals = new NativeArray<int>(GameDataManager.instance.Locations.Length, Allocator.TempJob);
39.  
40.             var percentageUpdateJob = new PercentageUpdateJob
41.             {
42.                 HumanData = humanData,
43.                 Totals = totals
44.             };
45.             var collisionJobHandle = percentageUpdateJob.Schedule(humanData.Length, inputDependencies);
46.             collisionJobHandle.Complete();
47.             InfoManager.instance.PercentageTotals = totals.ToArray();
48.             totals.Dispose();
49.             humanData.Dispose();
50.             return collisionJobHandle;
51.  
52.         }
53.     }

------------------------------------------------------------------------------------

• I have messed around with chunks, but was unsure how I could chunk some of the ideas going on here.
• I think my distance check is actually expensive (have not measured). Would an AABB type check be better, that is just checking my position values?
• The bottleneck is the `ShowInfectionMaterialSystem`. How can I get an improvement from this idea?
• Since locations is set at the start, I was hoping to perhaps set my native int array 'totals', in the `InformationSystem` once, rather than every job, as it will never grow or shrink.
• Any other tips would be very helpful at this stage.

Cheers and Beers

 

Only looked through briefly, but instead of changing material, which is slow as you point out, you could change a material per instance property. E.g. if all you want to do is update the color of an infected entity, then you can do this very easily and it can be done in a job. HybridRenderer has a few in-built IComponentData types you can use for it that are picked up automatically (see link).

Not sure about the distance check. If it is slow, maybe using DOTS physics with simple AABB colliders triggering this would be faster.

 

@charleshendry thanks for the tip on the hybrid renderer. Previously I had tried to use this but failed hard. This time, I updated to 2020.1.2f1, followed the instructions, and now gained 25-30fps. I was previously sitting around 55-65 fps, now it's 85-90...very cool. Super easy change and a great performance boost!

Updated ShowInfectedMaterialSystem:

Code (CSharp):

1. [UpdateAfter(typeof(HumanMoveSystem))]
2. public class ShowInfectionMaterialSystem : SystemBase
3. {      
4.     // this just checks to see if the object is infected, if so, change the material
5.     protected override void OnUpdate()
6.     {
7.         Entities
8.             .WithName("ShowInfectionMaterialSystem")
9.             .WithBurst()
10.             .ForEach((ref HumanData infectionData, ref InfectedColourData colourData) =>
11.             {
12.                 if (infectionData.isInfected && !infectionData.isAsymptomatic)
13.                 {
14.                     colourData.Value = new float4(1, 0, 0, 1);
15.                     infectionData.isAsymptomatic = true;
16.                 }
17.             })
18.             .Schedule();
19.     }
20. }

I'll go back to the dots physics way I was doing it previously. It felt like a waste because the only thing I wanted was a trigger at the time, but I will give it another shot.

 

One tip that might be interesting to comment on is that the data-driven pattern has the use of simple components, instead of complex components, as it will prioritize code reuse as well as processing.

 

A couple of things:
1) Your Entities.ForEach is using Schedule instead of ScheduleParallel, so it is running single-threaded (the behavior is different from the old JobComponentSystem). I'm guessing from the rest of the code you did not intend for this to be single-threaded.
2) Typically, what you are doing in lines 39-47 of DistanceCheckJob is a race condition, as multiple threads could be writing to the same i-th value at the same time. However, you almost lucked out a little. Due to the rest of the logic, you are only ever writing one value. If you also never read that value within the same job, you'd actually be fine as the order that those threads write the identical value would not matter. (Fun fact, this is how ComponentDataFromEntity change filter updates get away with not modifying the version number atomically.) One solution would be to write the new infection values to a seperate boolean array, and then chain that into a job which copies those booleans back into the original humanData array.

 

@ScriptsEngineer Thanks for that, I will be breaking down that 'HumanData' component today, at least into 2 for now so one can carry infection information, the other can carry location data.

@DreamingImLatios Excellent reminder about the parrallel job stuff. Just from that tip a bunch of light bulbs are going off in regards to stuff I have read or experimented with. And see below on getting rid of my distance check.

@charleshendry Thanks so much for the reminder about the Dots Physics.
I had a trigger system before and dropped it at the time (experimenting, learning). Of course I should be using the DOTS physics. Another simple change to re-enable the physics collisions almost doubled my fps.

Code (CSharp):

1. [UpdateAfter(typeof(EndFramePhysicsSystem))]
2. public class InfectionTriggerDetectionSystem : JobComponentSystem
3. {
4.     // we need a couple of extra worlds that have finished simulating
5.     BuildPhysicsWorld physicsWorld;
6.     StepPhysicsWorld stepWorld;
7.  
8.     protected override void OnCreate()
9.     {
10.         physicsWorld = World.GetOrCreateSystem<BuildPhysicsWorld>();
11.         stepWorld = World.GetOrCreateSystem<StepPhysicsWorld>();
12.     }
13.  
14.  
15.     struct BoxTriggerEventJob : ITriggerEventsJob
16.     {
17.         public ComponentDataFromEntity<HumanData> HumanDataGroup;
18.  
19.         public void Execute(TriggerEvent triggerEvent)
20.         {
21.             Entity entityA = triggerEvent.Entities.EntityA;
22.             Entity entityB = triggerEvent.Entities.EntityB;
23.  
24.             var componentA = HumanDataGroup[entityA];
25.             var componentB = HumanDataGroup[entityB];
26.  
27.             bool infectedA = componentA.isInfected && componentA.isAsymptomatic;
28.             bool infectedB = componentB.isInfected && componentB.isAsymptomatic;
29.             if (infectedA || infectedB)
30.             {
31.                 componentA.isInfected = true;
32.                 componentB.isInfected = true;
33.                 HumanDataGroup[entityA] = componentA;
34.                 HumanDataGroup[entityB] = componentB;
35.             }
36.         }
37.     }
38.  
39.     protected override JobHandle OnUpdate(JobHandle inputDeps)
40.     {
41.         JobHandle jobHandle = new BoxTriggerEventJob
42.         {
43.             HumanDataGroup = GetComponentDataFromEntity<HumanData>()
44.         }.Schedule(stepWorld.Simulation, ref physicsWorld.PhysicsWorld, inputDeps);
45.  
46.         return jobHandle;
47.     }
48. }

I am not that familiar yet with the physics system, so I might be running this in a single thread again but based on the performance increase, I think I am getting the benefits. I'll do some more reading and checking.

After all the changes above (Hybrid Materials, ParallelJobs and DOTS physics) I am now hitting 155-165fp with 6000 entities.

As a test I bumped it to 25000 and was still hitting 50-60 fps. Now my bottleneck is probably displaying and updating 60 UI panels, even though they are all on their own canvas, they all get called at the same time.

 

Any reason you are using JobComponentSystem instead of SystemBase? As far as I'm aware, JobComponentSystem is deprecated and SystemBase should be used in all cases. You should be able to use the Dependency property from SystemBase instead of inputDeps.

Code (CSharp):

1. Dependency = new BoxTriggerEventJob
2. {
3.      HumanDataGroup = GetComponentDataFromEntity<HumanData>()
4.  
5. }.Schedule(stepWorld.Simulation, ref physicsWorld.PhysicsWorld, Dependency);

 

@Bivens32 legend, cheers. The only reason is that's from the tutorial I followed sprinkled with some misunderstanding about Execute v ForEach. I'll take that lead and refer to the documentation. My information system is also using the JobComponentSystem. I will update that and maybe try the idea of chaining this systems work as suggested above.

 

@ScriptsEngineer I have updated the components to be a little more particular about their concerns:

Code (CSharp):

1. [GenerateAuthoringComponent]
2. public struct HumanMovement : IComponentData
3. {
4.     public float3 StartPos;
5.     public float3 EndPos;
6.     public float3 TravelVector;
7.     public float Distance;
8.     public float Speed;
9. }
10.  
11. [GenerateAuthoringComponent]
12. public struct HumanLocation : IComponentData
13. {
14.     public int Location; // simple int will correspond to a location
15.     public float3 OffsetPosition; // offset for position
16. }
17.  
18. [GenerateAuthoringComponent]
19. public struct HumanInfection : IComponentData
20. {
21.     public bool isInfected;
22.     public bool isAsymptomatic;
23. }

I have now refactored the systems to use the right component data.
I might be scraping the sides of optimism but I would swear this has given me an increase in frames (5-10).
I am now hitting 190 FPS on my pc, and will test later on the mac.

 

Hi all, now I have added in all these changes, my final part consists of how I can get the concept of locations to work.

A location is somewhere a human can be. This can be linked to one or more locations.
In the above HumanLocation the 'int Location' should really be 'int index' as all it is is my location index when I initially spawn my entities.

Code (CSharp):

1.  void Start()
2.         {
3.             store = new BlobAssetStore();
4.             manager = World.DefaultGameObjectInjectionWorld.EntityManager;
5.             var settings = GameObjectConversionSettings.FromWorld(World.DefaultGameObjectInjectionWorld, store);
6.             var human = GameObjectConversionUtility.ConvertGameObjectHierarchy(HumanPrefab, settings);
7.             float locationSize = (GameDataManager.instance.locationSize - 1) / 2.0f;
8.             int totalLocations = GameDataManager.instance.Locations.Length;
9.             int patientZero = UnityEngine.Random.Range(0, totalLocations * EntitiesPerLocation);
10.  
11.             for (int x = 0; x < totalLocations; x++)
12.             {
13.                 for (int y = 0; y < EntitiesPerLocation; y++)
14.                 {
15.                     var instance = manager.Instantiate(human);
16.                     float startX = Random.Range(-1 * locationSize, locationSize);
17.                     float startZ = Random.Range(-1 * locationSize, locationSize);
18.                     float endX = Random.Range(-1 * locationSize, locationSize);
19.                     float endZ = Random.Range(-1 * locationSize, locationSize);
20.                     float speed = Random.Range(2.0f, 5.0f);
21.                     var startPos = new Vector3(startX, 0, startZ);
22.                     var endPos = new Vector3(endX, 0, endZ);
23.  
24.                     Vector3 offset = GameDataManager.instance.Locations[x].position;
25.                     manager.SetComponentData(instance, new HumanMovement
26.                     {
27.                         StartPos = startPos + offset,
28.                         EndPos = endPos + offset,
29.                         Speed = speed
30.                     });
31.  
32.                     manager.SetComponentData(instance, new HumanLocation
33.                     {
34.                         OffsetPosition = offset,
35.                         Location = x
36.                     });
37.  
38.                     manager.SetComponentData(instance, new HumanInfection
39.                     {
40.                         isAsymptomatic = false,
41.                         isInfected = ((x * 100) + y == patientZero)
42.                     });
43.  
44.                     manager.SetComponentData(instance, new Translation { Value = startPos + offset });
45.  
46.                 }
47.             }
48.         }

So spawning entities in the one location is not a problem, getting them to travel to different locations is where I am stuck.

Some initial ideas:
* I tried creating a normal 'location' game object that existed on the object and retained the location index and an array of indexes you could get to from this one.
* eg index 1, linked to indexes 2,3,5
* That meant I had to copy all that location data to a native container at some point so it could be used in a system somewhere. If I wanted to change locations, I had to know where I could go. NativeList of NativeArrays?
* Got stuck, crashed and burned.

Another Idea I have not tried yet:
* The move system could flag on a new component that the entity needs to exit the location
* Another system checks purely if this flag is set
* If location is an entity, then it could contain a new Location component that contains the indexes you can traverse to
* Here is where I am getting a little stuck in my thoughts - some system still needs to know locations and their exit nodes. They are always two different component groups that need information about the other and I am unsure which way to go.

Can anyone make any suggestions here?

Cheers

 

I'm not sure I fully understand what you are struggling with, but there's a few ideas I will throw at the wall for you.

First, you could have location entities which contain dynamic buffers of their connected locations in the form of an entity reference.

Second, you could instead build a blob asset containing all locations and all connections between them, and then give each human a BlobAssetReference pointing to that blob.

For getting the randomized movements, you will likely want both timer components and a random number generators (either Random as a component or as part of a system or singleton).

 

Hi @DreamingImLatios, the blob assets actually sound perfect. After some reading they really suit the idea of these locations as the location data wont ever change, but a reference to which location the human is currently in does change.

Cheers

 

Hi again, thanks to everyone for all the hints and tips so far. I feel like I am starting to make some progress in ECS.

In regards to this project, I am now trying to work with the concept of my humans being able to move from one location to another.

What I settled on was creating a new Location Entity.
The location entity has an index, an offset and a Dynamic buffer:

Code (CSharp):

1.     public struct LocationData : IComponentData
2.     {
3.         public int index;
4.         public float3 offset;
5.         public float scale;
6.     }
7.  
8.     [InternalBufferCapacity(8)]
9.     public struct DestinationBuffer : IBufferElementData
10.     {
11.         public int Value;
12.     }

The dynamic buffer retains an array of other location indexes.

When I create my entities now in my ECS manager, I loop over the location entities, then create an amount of humans per location. The humans retain the index of that location.

Code (CSharp):

1.     public struct HumanLocationData : IComponentData
2.     {
3.         public int Location; // simple int will correspond to a location
4.         public float3 OffsetPosition; // offset for position
5.         public bool changeDestination;
6.     }

At some point now in my HumanMoveSystem, I pretty much roll a dice and decide whether or not that human needs to move to a new location or stay in the same location and move to a new position:

Updated system:

Code (CSharp):

1.  
2. Entities
3.     .WithNativeDisableParallelForRestriction(randomArray)
4.     .WithName("HumanMoveSystem")
5.     .WithBurst()
6.     .ForEach((int nativeThreadIndex, ref Translation position, ref HumanMovement movementData, ref HumanLocationData locationData) =>
7.     {
8.         if (locationData.changeDestination)
9.             return;
10.  
11.         float3 travelVector = movementData.EndPos - movementData.StartPos;
12.         position.Value += math.normalize(travelVector) * deltaTime * movementData.Speed;
13.         movementData.TravelVector = math.normalize(travelVector) * deltaTime * movementData.Speed;
14.         float distance = math.distance(position.Value, movementData.EndPos);
15.         movementData.Distance = distance;
16.         var random = randomArray[nativeThreadIndex];
17.         if (distance < 1f)
18.         {
19.             movementData.StartPos = movementData.EndPos;
20.             float distanceCheck;
21.             float change = random.NextFloat(0, 100);
22.  
23.             if (change > 75.0f)
24.                 locationData.changeDestination = true;
25.             else
26.             {
27.                 do
28.                 {
29.                     float x = random.NextFloat(-1 * locationSize, locationSize);
30.                     float z = random.NextFloat(-1 * locationSize, locationSize);
31.                     movementData.EndPos = new float3(x, 0, z) + locationData.OffsetPosition;
32.                     distanceCheck = math.distance(movementData.StartPos, movementData.EndPos);
33.                     randomArray[nativeThreadIndex] = random;
34.                 }
35.                 while (distanceCheck < locationSize / 2.0f);
36.             }
37.         }
38.  
39.         randomArray[nativeThreadIndex] = random;
40.  
41.     })
42.     .ScheduleParallel();
43.  
44.  

This all works well and I get an indication the human wants to move locations.

My plan of attack at this point has been to create 2 new systems:

HumanGetNewLocationSystem

• Loop over the locations
• Foreach location, loop over all the humans
• If the human currently belongs to this location && they want to change location
• Go through the dynamic buffer of this location, pick a random new location index
• Write back the human data array

HumanSetNewLocationSystem

• Update after HumanGetLocationSystem
• Loop over the humans
• if the human wants to change
• loop over the locations
• when I find the right location, update the humans offset, to the same as the new location.

I have made the first system:

Code (CSharp):

1. using Unity.Entities;
2. using Unity.Jobs;
3. using OutbreakAgent.Component;
4. using Unity.Collections;
5.  
6. // https://docs.unity3d.com/Packages/com.unity.entities@0.13/manual/chunk_iteration_job.html
7. namespace OutbreakAgent.Systems
8. {
9.     [UpdateAfter(typeof(ShowInfectionMaterialSystem))]
10.     public class HumanGetNewLocationSystem : SystemBase
11.     {
12.         EntityQueryDesc infectedQuery;
13.         EntityQuery newDestinationGroup;
14.  
15.         protected override void OnCreate()
16.         {
17.  
18.             infectedQuery = new EntityQueryDesc()
19.             {
20.                 All = new ComponentType[]
21.                 {
22.                     ComponentType.ReadOnly<HumanLocationData>()
23.                 }
24.             };
25.  
26.             newDestinationGroup = GetEntityQuery(infectedQuery);
27.         }
28.        
29.         protected override void OnUpdate()
30.         {
31.             var randomArray = World.GetExistingSystem<RandomSystem>().RandomArray;
32.            
33.             NativeArray<HumanLocationData> humanLocationData = newDestinationGroup.ToComponentDataArray<HumanLocationData>(Allocator.TempJob);
34.            
35.             JobHandle pickDestination = Entities
36.                 .WithName("HumanGetNewLocationSystem")
37.                 .WithBurst()
38.                 .ForEach((int nativeThreadIndex, DynamicBuffer<DestinationBuffer> buffer, ref LocationData locationData) =>
39.             {
40.                 for (int x = 0; x < humanLocationData.Length; x++)
41.                 {
42.                     if (locationData.index == humanLocationData[x].Location)
43.                     {
44.                         if (humanLocationData[x].changeDestination)
45.                         {
46.                             var random = randomArray[nativeThreadIndex];
47.                             int newLocation = random.NextInt(0, buffer.Length - 1);
48.                             if (buffer[newLocation].Value > 0)
49.                             {
50.                                 HumanLocationData hld = humanLocationData[x];
51.                                 hld.Location = buffer[newLocation].Value;
52.                                 //hld.changeDestination = false;  // only set this for now so it returns back to the human move system, otherwise leave on for HumanSetLocationSystem
53.                                 humanLocationData[x] = hld;
54.                                 randomArray[nativeThreadIndex] = random;
55.                             }
56.                             else UnityEngine.Debug.Log("BAD BUFFER!");
57.                         }
58.                     }
59.                 }
60.             }).Schedule(this.Dependency);
61.  
62.             pickDestination.Complete();
63.             newDestinationGroup.CopyFromComponentDataArray(humanLocationData);
64.             humanLocationData.Dispose();
65.         }
66.     }
67. }
68.  

The issue I have is it just feels wrong in some way.
I am certain I am running the last system currently in a single Job, which is ok because I need to write to the human locations. But I am certain I gradually get a slow down in performance and I cant yet figure out why.

I feel like perhaps to speed this up I could:

• When rolling the dice to decide to move, attach a new tag component?
• SetNewLocation would then remove the component, and it could then become part of the move system archetype again.
• I would lose some speed in the move system initially because no entities would be tagged to change destination.
• I would need to use EntityManager to add a new component (which forces me back to main thread?)
• In both the move and Get/Set new location systems, I would gain a little speed from more defined archetypes - groups with or without the tag component.
• eg:
  
  Code (CSharp):

  1. infectedQuery = new EntityQueryDesc()
  2.             {
  3.                 All = new ComponentType[]
  4.                 {
  5.                     ComponentType.ReadOnly<Tag_ChangeDestination>()
  6.                 }
  7.             };
  8.  
  9.             newDestinationGroup = GetEntityQuery(infectedQuery);

Anyone got some help for me above? Is my approach good, bad, in between?

Cheers

 

Probably your biggest issue is the fact that you are looping through all humans for each location in a single-threaded job. You might want to consider building up a NativeHashMap<int, Entity> locationToLocationEntity and then iterating humans instead. That will also get rid of your need to use ToComponentDataArray which might help you avoid calling Complete in the system.

 

@DreamingImLatios

Thanks for the reply. That's right about the single thread, I felt I was forced to because of the loop over the humans and the write back their entries which I guessed would cause a race condition otherwise. That was where it felt wrong.

NativeHashMap<int, Entity> locationToLocationEntity

For each human that wants to change location, I then loop over the hash map of locations
If I have the Location entity in the loop and want the dynamic buffer on it, I have to use EntityManager.GetBuffer

https://docs.unity3d.com/Packages/c...tyManager_GetBuffer__1_Unity_Entities_Entity_

to get the buffer - I can probably assume this wont force main threadness (by using EntityManager) because it's a read operation and is not altering the structure of the location entity

So if I ForEach over the humans, I can see where I could get the win because I can now run in parrallel this way.

-------

For the HumanSetNewLocationSystem I could do the same thing. I don't need to write out at all to the location data, I just need to read it, update the HumanLocationData and I am fine.

Cheers and beers

 

That assumption was correct, because every thread is checking Location which you are also writing to. If you you had a second HumanLocationData array that you wrote to separate from the one you are reading from, then yes this would be thread-safe (barely).

SystemBase.GetBuffer or GetBufferFromEntity are actually what you want here. You also don't need to iterate over the hashmap. It is a hashmap, so you can do direct lookups.

 

@DreamingImLatios Thanks for the help, I have managed to get something going and it's been an excellent few days of ECS lessons. Of course, there is 1 final thing.
I cannot for the life of me seem to set the position of a given entity. I just cant see what I have missed.

Code (CSharp):

1.     [UpdateAfter(typeof(ShowInfectionMaterialSystem))]
2.     public class HumanGetNewLocationSystem : SystemBase
3.     {
4.         EntityQueryDesc locationQuery;
5.         EntityQuery locationGroup;
6.         protected override void OnCreate()
7.         {
8.  
9.             locationQuery = new EntityQueryDesc()
10.             {
11.                 All = new ComponentType[]
12.                 {
13.                     ComponentType.ReadOnly<LocationData>(),
14.                     ComponentType.ReadOnly<DestinationBuffer>()
15.                 }
16.             };
17.  
18.             locationGroup = GetEntityQuery(locationQuery);
19.  
20.         }
21.        
22.         protected override void OnUpdate()
23.         {
24.             var randomArray = World.GetExistingSystem<RandomSystem>().RandomArray;
25.  
26.             NativeArray<Entity> locationEntities = locationGroup.ToEntityArray(Allocator.TempJob);
27.             NativeArray<LocationData> locationDatas = locationGroup.ToComponentDataArray<LocationData>(Allocator.TempJob);
28.             NativeHashMap<int, Entity> locationsHashMap = new NativeHashMap<int, Entity>(locationEntities.Length, Allocator.TempJob);
29.  
30.             for (int i = 0; i < locationEntities.Length; i++)
31.             {
32.                 locationsHashMap.Add(locationDatas[i].index, locationEntities[i]);
33.             }
34.  
35.             locationEntities.Dispose();
36.             locationDatas.Dispose();
37.             BufferFromEntity<DestinationBuffer> bufferLookup = GetBufferFromEntity<DestinationBuffer>();
38.             ComponentDataFromEntity<LocationData> locationDataLookup = GetComponentDataFromEntity<LocationData>();
39.  
40.             for (int x = 0; x < locationEntities.Length; x++)
41.             {
42.                 UnityEngine.Debug.Log(locationsHashMap[x]);
43.             }
44.  
45.             Entities
46.                 .WithName("HumanGetNewLocationSystem")
47.                 .WithNativeDisableParallelForRestriction(randomArray)
48.                 .WithReadOnly(locationsHashMap)
49.                 .WithReadOnly(bufferLookup)
50.                 .WithReadOnly(locationDataLookup)
51.                 .WithDisposeOnCompletion(locationsHashMap)
52.                 .WithBurst()
53.                 .ForEach((int nativeThreadIndex, ref Translation position, ref HumanLocationData humanLocationData, ref HumanMovement humanMovement) =>
54.             {
55.                 if (humanLocationData.changeDestination)
56.                 {
57.                     // get a new location index
58.                     var destinations = bufferLookup[locationsHashMap[humanLocationData.Location]];
59.                     var random = randomArray[nativeThreadIndex];
60.                     int index = random.NextInt(0, destinations.Length - 1);
61.                     int newLocationIndex = destinations[index].Value;
62.  
63.                     //UnityEngine.Debug.Log($"{position.Value} {humanLocationData.Location} {humanMovement.EndPos}");
64.  
65.                     // set it on the human data
66.                     humanLocationData.Location = newLocationIndex;
67.  
68.                     // remove the previous offset first
69.                     humanMovement.StartPos -= humanLocationData.OffsetPosition;
70.                     humanMovement.EndPos -= humanLocationData.OffsetPosition;
71.  
72.                     // get the new locations data
73.                     var newLocationData = locationDataLookup[locationsHashMap[newLocationIndex]];
74.  
75.                     // reset our offsets back on this human
76.                     humanLocationData.OffsetPosition = newLocationData.offset;
77.                     humanMovement.StartPos += newLocationData.offset;
78.                     humanMovement.EndPos += newLocationData.offset;
79.  
80.                     // set our position
81.                     position.Value = humanMovement.StartPos;
82.  
83.                     // finish up so we can move back in the move system;
84.                     randomArray[nativeThreadIndex] = random;
85.                     humanLocationData.changeDestination = false;
86.  
87.                     //UnityEngine.Debug.Log($"{position.Value} {humanLocationData.Location} {humanMovement.EndPos}");
88.                 }
89.             }).ScheduleParallel();
90.         }
91.  
92.     }

I cannot figure out why position.Value = humanMovement.StartPos; simply does not work. It tells me if I Debug.Log that the position.Value is set to what I expect, but by the time my movement system comes about, it's as if it was ignored. From everything I am reading this should work? What have I missed?

Cheers

 

I see nothing wrong right now. This looks like it might be a system execution order issue or a job issue or something. Will need to see more of what is going on.

 

I have combined the HumanMoveSystem and HumanGetNewLocationSystem and finally got what I expected.
I noticed the archetype I was using in the for each was exactly the same and wondered if that was the reason I had the issue?

With that said, It's not that bad at all:

Code (CSharp):

1. [UpdateAfter(typeof(RandomSystem))]
2. public class HumanMoveSystem : SystemBase
3. {
4.     EntityQueryDesc locationQuery;
5.     EntityQuery locationGroup;
6.     protected override void OnCreate()
7.     {
8.  
9.         locationQuery = new EntityQueryDesc()
10.         {
11.             All = new ComponentType[]
12.             {
13.                 ComponentType.ReadOnly<LocationData>(),
14.                 ComponentType.ReadOnly<DestinationBuffer>()
15.             }
16.         };
17.  
18.         locationGroup = GetEntityQuery(locationQuery);
19.  
20.     }
21.  
22.     protected override void OnUpdate()
23.     {
24.         float deltaTime = Time.DeltaTime;
25.         var randomArray = World.GetExistingSystem<RandomSystem>().RandomArray;
26.         float locationSize = 98 / 2.0f;
27.  
28.         NativeArray<Entity> locationEntities = locationGroup.ToEntityArray(Allocator.TempJob);
29.         NativeArray<LocationData> locationDatas = locationGroup.ToComponentDataArray<LocationData>(Allocator.TempJob);
30.         NativeHashMap<int, Entity> locationsHashMap = new NativeHashMap<int, Entity>(locationEntities.Length, Allocator.TempJob);
31.  
32.         for (int i = 0; i < locationEntities.Length; i++)
33.         {
34.             locationsHashMap.Add(locationDatas[i].index, locationEntities[i]);
35.         }
36.  
37.         locationEntities.Dispose();
38.         locationDatas.Dispose();
39.         BufferFromEntity<DestinationBuffer> bufferLookup = GetBufferFromEntity<DestinationBuffer>();
40.         ComponentDataFromEntity<LocationData> locationDataLookup = GetComponentDataFromEntity<LocationData>();
41.  
42.         for (int x = 0; x < locationEntities.Length; x++)
43.         {
44.             UnityEngine.Debug.Log(locationsHashMap[x]);
45.         }
46.  
47.         Entities
48.             .WithName("HumanGetNewLocationSystem")
49.             .WithNativeDisableParallelForRestriction(randomArray)
50.             .WithReadOnly(locationsHashMap)
51.             .WithReadOnly(bufferLookup)
52.             .WithReadOnly(locationDataLookup)
53.             .WithDisposeOnCompletion(locationsHashMap)
54.             .WithBurst()
55.             .ForEach((int nativeThreadIndex, ref Translation position, ref HumanMovement humanMovementData, ref HumanLocationData humanLocationData) =>
56.             {
57.      
58.                 position.Value += humanMovementData.TravelVector * deltaTime;
59.                 float distance = math.distance(position.Value, humanMovementData.EndPos);
60.                 humanMovementData.Distance = distance;
61.                 var random = randomArray[nativeThreadIndex];
62.                 if (distance < 1f)
63.                 {
64.                     float change = random.NextFloat(0, 100);
65.                     if (change > 90.0f)
66.                     {
67.                            
68.                         // get a new location index
69.                         var destinations = bufferLookup[locationsHashMap[humanLocationData.Location]];
70.                         int index = random.NextInt(0, destinations.Length - 1);
71.                         int newLocationIndex = destinations[index].Value;
72.  
73.                         // set it on the human data
74.                         humanLocationData.Location = newLocationIndex;
75.  
76.                         // remove the previous offset first
77.                         humanMovementData.StartPos -= humanLocationData.OffsetPosition;
78.                         humanMovementData.EndPos -= humanLocationData.OffsetPosition;
79.  
80.                         // get the new locations data
81.                         var newLocationData = locationDataLookup[locationsHashMap[newLocationIndex]];
82.  
83.                         // reset our offsets back on this human
84.                         humanLocationData.OffsetPosition = newLocationData.offset;
85.                         humanMovementData.StartPos += newLocationData.offset;
86.                         humanMovementData.EndPos += newLocationData.offset;
87.  
88.                         // set our position
89.                         position.Value = humanMovementData.StartPos;
90.                     }
91.                     else
92.                     {
93.                         humanMovementData.StartPos = humanMovementData.EndPos;
94.                         float distanceCheck;
95.  
96.                         do
97.                         {
98.                             float x = random.NextFloat(-1 * locationSize, locationSize);
99.                             float z = random.NextFloat(-1 * locationSize, locationSize);
100.                             humanMovementData.EndPos = new float3(x, 0, z) + humanLocationData.OffsetPosition;
101.                             distanceCheck = math.distance(humanMovementData.StartPos, humanMovementData.EndPos);
102.                             float3 travelVector = humanMovementData.EndPos - humanMovementData.StartPos;
103.                             humanMovementData.TravelVector = math.normalize(travelVector) * humanMovementData.Speed;
104.                         }
105.                         while (distanceCheck < locationSize / 2.0f);
106.                     }
107.  
108.                 }
109.  
110.                 randomArray[nativeThreadIndex] = random;
111.  
112.             })
113.             .ScheduleParallel();
114.     }
115. }

Thanks for all your help on this. I have learnt a bunch about ECS getting this from what I had to now. There is some improvements to make as I am only getting 90-100 fps with 10k entities? which does not seem like close to what I should be getting.

Thanks again.