Search Unity

  1. Good news ✨ We have more Unite Now videos available for you to watch on-demand! Come check them out and ask our experts any questions!
    Dismiss Notice

Deprecating IJobForEach, good replacement patterns

Discussion in 'Data Oriented Technology Stack' started by snacktime, Mar 13, 2020.

  1. cultureulterior

    cultureulterior

    Joined:
    Mar 15, 2015
    Posts:
    53
    I think you're deprecating these too early. For those of us who stick to Dots Physics, we're still on Entities 0.6, which hasn't even heard of SystemBase.

    Also, was the tiny per-frame memory leak when using local variables in ForEach fixed?
     
    Last edited: Mar 27, 2020
  2. desertGhost_

    desertGhost_

    Joined:
    Apr 12, 2018
    Posts:
    145
    I'm using Unity Physics with Entities 0.8 without any issues.
     
    cultureulterior likes this.
  3. Lucas-Meijer

    Lucas-Meijer

    Unity Technologies

    Joined:
    Nov 26, 2012
    Posts:
    166
    @cultureulterior what memory leak are you referring to? I don't think this is on our radar.
     
    MNNoxMortem likes this.
  4. Lucas-Meijer

    Lucas-Meijer

    Unity Technologies

    Joined:
    Nov 26, 2012
    Posts:
    166
    @GliderGuy I'm not sure where all the 20 is too much vibe comes from in this thread, but I can totally see legit usecases for them. Seeing this thread, I spent a few hours today working on a feature to make it possible (not beautiful) to allow any number of arguments as well as order of in/ref. From the future docs:

    "The limit of 8 arguments is chosen because under the hood we need to define a unique delegate type & ForEach overload for each combination of ref/i/value and number of arguments. Having too many has a negative effect on IDE performance. It is possible to use more than 8 arguments. To do that you have to provide your own delegate type & ForEach overload. The DOTS compiler will accept these as well, and it allows you to have as many arguments as you
    want with ref/in/value in any order you want."

    assuming nothing changes as I land this, the syntax will roughly be:

    Code (CSharp):
    1.    static class BringYourOwnDelegate
    2.     {
    3.         [Unity.Entities.CodeGeneratedJobForEach.WillNeverEscapeMethodBodyLifetime]
    4.         public delegate void CustomForEachDelegate<T0, T1, T2, T3, T4, T5, T6, T7, T8, T9, T10, T11>(T0 t0, in T1 t1, in T2 t2, in T3 t3, in T4 t4, in T5 t5, in T6 t6, in T7 t7, in T8 t8, in T9 t9, in T10 t10, in T11 t11);
    5.  
    6.         public static TDescription ForEach<TDescription, T0, T1, T2, T3, T4, T5, T6, T7, T8, T9, T10, T11>(this TDescription description, CustomForEachDelegate<T0, T1, T2, T3, T4, T5, T6, T7, T8, T9, T10, T11> codeToRun)
    7.             where TDescription : struct, Unity.Entities.CodeGeneratedJobForEach.ISupportForEachWithUniversalDelegate =>
    8.             LambdaForEachDescriptionConstructionMethods.ThrowCodeGenException<TDescription>();
    9.     }
    10.  
    If you'd prefer to not wait for this, what you can do is embed the unity.entities package, and inside UniversalDelegates.gen.cs, add one more delegate type exactly matching the order and amount of args you're looking for, and add a .ForEach overload in that same file. The dots compiler should deal with that just fine. You can then switch to the "officially supported way" when the first entities release with this feature comes out.
     
    GliderGuy, Timboc and MNNoxMortem like this.
  5. Lucas-Meijer

    Lucas-Meijer

    Unity Technologies

    Joined:
    Nov 26, 2012
    Posts:
    166
    Hi mf_andreich, could you share more info about this specific case? I'd love to figure out a way that you don't have to drop down to IJobChunk.

    if you wish to use the T as an lambda paremeter, what do you do with it? you don't know what the type is, so you cannot actually read any data from it.

    Would it help if we would allow T in .WithAll<T>, WithNone<T> ?

    I'd love to learn more about your usecase to see if we can do something to support it.
     
  6. GliderGuy

    GliderGuy

    Joined:
    Dec 14, 2018
    Posts:
    100
    :eek::eek::eek::eek::eek::eek::eek::eek::eek:
    You... You are incredible!
    I am greatly appreciative of your ability to see my use-cases and my concerns about this topic! For far too long have I had to slog through boilerplate writing... It brings tears to my eyes that this reign of tyranny will soon be over!
    Huzzah!
    All joking aside, you cannot believe how happy I am for this. Thank you so much for taking my thoughts into consideration and spending your precious time on adding support for this feature! <3
    #Lucas-MeijerForPresident #NewFavoriteUnityDev #GiveHimARaise

    You just made my day! :D

    Thank you for the instructions! (I'm a big fan of instructions).
    I'm going to have to try this out later sometime! Right now I just finished using IJobChunk on the code that I could use it for (it was painful, man. Horribly painful). But, I do plan to be writing some code soon that will likely use more than eight components, so this will be a game-changer!
    (Wooo! I can feel the adrenaline surge already! I could literally eat my hat (that I do not own) right now! Wooo!)
     
    Last edited: Mar 28, 2020
    deus0 likes this.
  7. cultureulterior

    cultureulterior

    Joined:
    Mar 15, 2015
    Posts:
    53
    @Lucas-Meijer sorry, not a memory leak, garbage being created every frame from the ecb + closure scenario- described here:

    https://forum.unity.com/threads/for...errupts-method-execution.646351/#post-4339621

    I've now upgraded up to 0.8.0. First of all, something (in physics?) now reliably crashes my editor on the second play. I can't deal with that, so I'll have to revert, but beyond that- I tried to use IJobChunk and I am still not entirely satisfied. I have big IJobForEach's with multiple functions in them, so they're not suitable for Entities.Foreach, and it feels like I'm having to add even more boilerplate.
     
  8. snacktime

    snacktime

    Joined:
    Apr 15, 2013
    Posts:
    3,009
    20 is too much is absolutely correct. For the same reason that methods taking 20 parameters would be wrong. It's a failure to abstract well.

    But big picture the failure is partly on ECS to provide better high level patterns and tools. Most of Unity's own solutions have taken the approach of simply move the data and logic outside of ECS proper, using ECS as more of an integration layer. I think a lot more could be surfaced into ECS and likely will be eventually. It would be unfortunate IMO if that didn't happen.

    An example off the top of my head would be component views. Be able to create multiple views, pass entire views to a job. In other cases it might just be other high level api's that are well integrated. Like the new data flow graph is early and a bit cumbersome still, but it's looking to be a very useful tool to solve a subset of problems that don't fit entities/components elegantly.
     
  9. GliderGuy

    GliderGuy

    Joined:
    Dec 14, 2018
    Posts:
    100
    Twenty components is too much? Why would twenty components be too much? I can agree that generally methods should not have an excessive number of parameters. But why would supporting ForEach iteration over eight components be bad, like you seem to be suggesting from being against large component numbers?
    I'm just curious. :oops:

    As for abstraction; are you referring to making systems use less data that is provided from outside systems and instead having the system get its required data for itself? If I may say... Wouldn't this cause inefficiencies? Why get data you already have if you can pass that data in for a system to use it?

    I'd quickly just like to say that I'm not against abstraction. Abstraction is important, and I believe in the rule too. I'm just a little confused as to how you can conclude that if someone were to be using twenty components worth of data in a system then they simply did not abstract their code enough.
    Could a lack of abstraction be the case? Sure.
    Will a lack of abstraction always be the case? I wouldn't bet on it! ;)
     
    Last edited: Mar 29, 2020
  10. Denoon

    Denoon

    Joined:
    Jan 6, 2018
    Posts:
    3
    For me this would help. In my case I was using generics as a tag to filter behavior in a system. I don't need to access the the data in the system and allowing T in .WithAll and WithNone<T> would fix it for me.
     
  11. snacktime

    snacktime

    Joined:
    Apr 15, 2013
    Posts:
    3,009
    20 input params is just too much to reason about well. It's a strong indicator what you are passing them to is doing too much or that you have poor abstractions at some other level. Like if you have a lot of systems that all use the same data and you make components very granular so you can feed them to any of those systems in whatever combination they need.

    ECS lacks strong encapsulation and doesn't force you to separate concerns like you should or create api's between various features and sub systems. The path of least resistance is to write code that leads directly to jobs that take many components. So at the least it's a red flag.

    That said I think ECS should allow for passing lots of components/buffers/whatever. You shouldn't put arbitrary limits on things like that without a good reason. That it's not good design isn't a good reason in this case. Good design is subjective in the sense that you might have good reasons for punting on a good design. Happens a lot in games.
     
    thelebaron and GliderGuy like this.
  12. Lucas-Meijer

    Lucas-Meijer

    Unity Technologies

    Joined:
    Nov 26, 2012
    Posts:
    166
    cultureulterior and phobos2077 like this.
  13. mf_andreich

    mf_andreich

    Joined:
    Jul 7, 2017
    Posts:
    37
    Thx for answer.

    First of all sorry for my bad english, its not my native language.

    About my problem. Using WithAll<T> and WithNone<T> not help me a lot, because I create new components with T type inside job. I do it with unsafe cast like this:

    Code (CSharp):
    1. var newComponent = UnsafeUtilityEx.As<Entity, TAttachedData>(ref entity);
    because I cant get information from it or set information to it directly, but its fine because I check on start that TAttachedData has only one field with Entity type.

    I can show code of one system. This system process new casted abilities and attach it to target units by adding additional component with specific type and link to ability entity. Its not full compiled sample but can give you some additional information.


    Code (CSharp):
    1. public interface IAbility: ISystemStateComponentData { }
    2.  
    3. public interface IAttachedAbility<TAbility> : ISystemStateComponentData where TAbility: struct, IAbility { }
    4.  
    5. public struct UnitData : ISystemStateComponentData { }
    6.  
    7. public abstract class AttachSimpleAbilitySystem<TAbilityData, TAttachedData> : JobComponentSystem
    8.         where TAbilityData: struct, IAbility
    9.         where TAttachedData: struct, IAttachedAbility<TAbilityData>
    10. {
    11.     //This my custom buffer system
    12.     private AfterAttachAbilitySyncSystem afterAttachAbilitySyncSystem;
    13.    
    14.     protected override void OnCreate()
    15.     {      
    16.         //In this point I check that TAttachedData has only one field with Entity type.
    17.         //I use it for cast TAttachedData to Entity and Entity to TAttachedData
    18.         AbilityExt.CheckAttachComponentType<TAttachedData>();
    19.         afterAttachAbilitySyncSystem = World.GetOrCreateSystem<AfterAttachAbilitySyncSystem>();
    20.         RequireForUpdate(GetEntityQuery(
    21.             ComponentType.ReadOnly<TAbilityData>(),
    22.             ComponentType.ReadOnly<AbilityTargetData>(),
    23.             ComponentType.Exclude<AttachedAbilityTag>(),
    24.             ComponentType.Exclude<RunningAbilityTag>()));
    25.     }
    26.    
    27.     protected override JobHandle OnUpdate(JobHandle inputDeps)
    28.     {
    29.         var handle = new AttachJob
    30.         {
    31.             ecb = afterAttachAbilitySyncSystem.CreateCommandBuffer().ToConcurrent(),
    32.             attachedSource = GetComponentDataFromEntity<TAttachedData>(),
    33.             unitSource = GetComponentDataFromEntity<UnitData>()
    34.         }.Schedule(this, inputDeps);
    35.  
    36.         afterAttachAbilitySyncSystem.AddJobHandleForProducer(handle);
    37.         return handle;
    38.     }
    39.    
    40.     [BurstCompile]
    41.     [ExcludeComponent(typeof(AttachedAbilityTag), typeof(RunningAbilityTag))]
    42.     public struct AttachJob : IJobForEachWithEntity_ECC<TAbilityData, AbilityTargetData>
    43.     {      
    44.         public EntityCommandBuffer.Concurrent ecb;  
    45.         [ReadOnly]
    46.         public ComponentDataFromEntity<TAttachedData> attachedSource;      
    47.         [ReadOnly]
    48.         public ComponentDataFromEntity<UnitData> unitSource;
    49.        
    50.         public void Execute(Entity entity, int index, [ReadOnly] ref TAbilityData abilityData, [ReadOnly] ref AbilityTargetData targetData)
    51.         {
    52.             var unitEntity = targetData.value;
    53.             var isUnit = unitSource.Exists(unitEntity);
    54.             var isAttach = attachedSource.Exists(unitEntity);
    55.             if (isUnit && !isAttach)
    56.             {
    57.                 ecb.AddComponent<TAttachedData>(index, unitEntity);
    58.                 var newComponent = UnsafeUtilityEx.As<Entity, TAttachedData>(ref entity);
    59.                 ecb.SetComponent(index, unitEntity, newComponent);
    60.                 ecb.AddComponent<RunningAbilityTag>(index, entity);
    61.             }
    62.             ecb.AddComponent<AttachedAbilityTag>(index, entity);
    63.         }
    64.     }
    65. }
    After this I can create some components and systems like:

    Code (CSharp):
    1. public struct TestSimpleAbilityData: IAbility { }
    2.  
    3. public struct TestSimpleAttachData : IAttachedAbility<TestSimpleAbilityData>
    4. {
    5.     public Entity abilityEntity;
    6. }
    7.  
    8. public class TestSimpleAbilityAttachSystem : AttachSimpleAbilitySystem<TestSimpleAbilityData, TestSimpleAttachData>{ }
    9.  
    And all works fine. In future I want make some codogen work for excluding manual implementation of AttachSimpleAbilitySystem for every components pair. I also have sistems for destroing, detacching, for stackable abilities with buffer elements attaching. And this approach using in other my "game modules" too.
     
  14. Lucas-Meijer

    Lucas-Meijer

    Unity Technologies

    Joined:
    Nov 26, 2012
    Posts:
    166
    Going through the feedback of IJobForEach deprecation, one theme that comes up a lot is that some people liked how IJobForEach made it easy to have your data transformation in a separate file, whereas Entities.ForEach nudges you more to have transormations be inline.

    Here are 3 techniques you can use with Entities.ForEach to have the code for the data transformation you're describing to be more isolated. I use a mix of them depending on the circumstances (and how I feel that day)

    Let's start with this example, and let's pretend the actual entities.foreach lambda body has tons of code in it:

    Code (CSharp):
    1. class MySystem: SystemBase
    2. {
    3.     protected override void OnUpdate()
    4.     {
    5.         var deltaTime = Time.DeltaTime;
    6.         Entities
    7.             .ForEach((ref Translation t, in Velocity v) =>
    8.             {
    9.                 t.Value += v.Value * deltaTime;
    10.                 //tons of inline code in here
    11.             })
    12.             .ScheduleParallel();
    13.     }
    14. }
    Technique1 is to just have the lambda body just call a static method:
    Code (CSharp):
    1.  
    2.  
    3. //In this example we avoid having tons of inline code by having the entities.foreach body call out to a static method.
    4. class MySystem2 : SystemBase
    5. {
    6.     protected override void OnUpdate()
    7.     {
    8.         var deltaTime = Time.DeltaTime;
    9.         Entities
    10.             .ForEach((ref Translation t, in Velocity v) => ApplyVelocity(t, v, deltaTime))
    11.             .ScheduleParallel();
    12.     }
    13.  
    14.     static void ApplyVelocity(Translation t, Velocity v, float deltaTime)
    15.     {
    16.         t.Value += v.Value * deltaTime;
    17.         //tons of inline code in here
    18.     }
    19. }
    20.  

    Technique2 is to use a struct. It feels a lot like how IJobForEach feels
    Code (CSharp):
    1. //In this example, we avoid tons of inline code by making a separate struct, much like how IJobForEach was before.
    2. //This struct can live in its own file, and you populate it, and then invoke into it, in a similar style to how
    3. //it worked with IJobForEach
    4. struct ApplyVelocity
    5. {
    6.     public float deltaTime;
    7.  
    8.     public void Execute(ref Translation t, in Velocity v)
    9.     {
    10.         t.Value += v.Value * deltaTime;
    11.         //tons of inline code in here
    12.     }
    13. }
    14.  
    15. class MySystem3: SystemBase
    16. {
    17.     protected override void OnUpdate()
    18.     {
    19.         var applyVelocity = new ApplyVelocity()
    20.         {
    21.             deltaTime = Time.DeltaTime
    22.         };
    23.      
    24.         Entities
    25.             .ForEach((ref Translation t, ref Velocity v) => applyVelocity.Execute(ref t,v))
    26.             .ScheduleParallel();
    27.     }
    28. }
    29.  
    Technique3 is to use a partial class. For big transformations I tend to like this option most.
    Code (CSharp):
    1.  
    2. //In this technique we also get the ability to get the transformation we want to do in a seperate file. We use a partial
    3. //class here, where from your main system you just invoke a method that's responsible for scheduling the transformation.
    4. //the method just happens to live in the same system, but in a different file that is dedciated to just this transformation.
    5. //It has a benefit over the previous technique in that you do not have to duplicate the list of components you want to operate
    6. //on (the entities.foreach lambda parameters) across two files that have to be kept in sync.
    7.  
    8. //Put this in a file like MySystem4_ApplyVelocity.cs
    9. partial class MySystem4 : SystemBase
    10. {
    11.     void ScheduleApplyVelocityJob()
    12.     {
    13.         var deltaTime = Time.DeltaTime;
    14.         Entities
    15.             .ForEach((ref Translation t, in Velocity v) =>
    16.             {
    17.                 t.Value += v.Value * deltaTime;
    18.                 //tons of inline code in here
    19.             })
    20.             .ScheduleParallel();
    21.     }
    22. }
    23.  
    24.  
    25. //This is your normal MySystem4.cs
    26. partial class MySystem4 : SystemBase
    27. {
    28.     protected override void OnUpdate()
    29.     {
    30.         ScheduleApplyVelocityJob();
    31.     }
    32. }
     
  15. optimise

    optimise

    Joined:
    Jan 22, 2014
    Posts:
    595
    @Lucas-Meijer Are these 3 techniques won't have any performance penalty especially Technique 2? Another question. Is that ForEach API the final way Unity wants to go?
     
    Last edited: Mar 29, 2020
  16. Joachim_Ante

    Joachim_Ante

    Unity Technologies

    Joined:
    Mar 16, 2005
    Posts:
    5,088
    It is best to check the burst compiler disassembly for the specific code in question. But burst is able to inline all of the code in the sample for technique 2. So all of these have equal performance.

    It's just a question of which style of code you like best.
     
  17. Zec_

    Zec_

    Joined:
    Feb 9, 2017
    Posts:
    98
    This is the way I've been wanting to convert my systems. It's a decent way to still have it feel like a job struct, where you can declare and group up all the input parameters. The additional invocation of function in the struct would be nice to be able to skip though, but I can live with that.

    My main problem with the approach is that I find that I want to use it when the jobs grow larger and more complex, but that's also when I run into issues with having both [ReadOnly] and writeable nativecollections in the job. I'm not sure how to tackle it. Perhaps you could shed some light?

    I've taken your previous example and expanded it a bit. I've also written down a couple of scheduling examples. How would you do this?
    Code (CSharp):
    1. public class TestSystem : SystemBase
    2. {
    3.     //In this example, we avoid tons of inline code by making a separate struct, much like how IJobForEach was before.
    4.     //This struct can live in its own file, and you populate it, and then invoke into it, in a similar style to how
    5.     //it worked with IJobForEach
    6.     struct ApplyVelocity
    7.     {
    8.         [ReadOnly] public NativeArray<Translation> ReadOnlyCollection;
    9.         public NativeArray<Translation> WriteableCollection;
    10.  
    11.         public float DeltaTime;
    12.  
    13.         public void Execute(ref Translation t, in Velocity v)
    14.         {
    15.             t.Value += v.Value * DeltaTime;
    16.             WriteableCollection[0] = ReadOnlyCollection[0];
    17.             //tons of inline code in here
    18.         }
    19.     }
    20.  
    21.     protected override void OnUpdate()
    22.     {
    23.         NativeArray<Translation> readOnlyCollection = new NativeArray<Translation>(1, Allocator.TempJob);
    24.         NativeArray<Translation> writeableCollection = new NativeArray<Translation>(1, Allocator.TempJob);
    25.         var applyVelocity = new ApplyVelocity()
    26.         {
    27.             ReadOnlyCollection = readOnlyCollection,
    28.             WriteableCollection = writeableCollection,
    29.             DeltaTime = Time.DeltaTime
    30.         };
    31.  
    32.         // Schedule Alternative 1 - The simplest, but potentially confusing way. I'm not even sure if it will work.
    33.         Entities
    34.             .WithReadOnly(applyVelocity) // Will it detect only the collections that are tagged with the readonly attribute, or will all collections be considered as readonly?
    35.             .ForEach((ref Translation t, ref Velocity v) => applyVelocity.Execute(ref t, v))
    36.             .ScheduleParallel();
    37.  
    38.         // Schedule Alternative 2 - The explicit way, but currently it won't compile.
    39.         Entities
    40.             .WithReadOnly(applyVelocity.ReadOnlyCollection) // error DC0038: Entities.WithReadOnly is called with an invalid argument ApplyVelocity.ReadOnlyCollection.
    41.             .ForEach((ref Translation t, ref Velocity v) => applyVelocity.Execute(ref t, v))
    42.             .ScheduleParallel();
    43.  
    44.         // Schedule Alternative 3 - Not too happy about it as it requires additional local variables.
    45.         Entities
    46.             .WithReadOnly(readOnlyCollection) // Will it detect that this is the same collection as the one in ApplyVelocity?
    47.             .ForEach((ref Translation t, ref Velocity v) => applyVelocity.Execute(ref t, v))
    48.             .ScheduleParallel();
    49.  
    50.         Dependency = readOnlyCollection.Dispose(Dependency);
    51.         Dependency = writeableCollection.Dispose(Dependency);
    52.     }
    53. }
     
    Last edited: Mar 29, 2020
  18. Joachim_Ante

    Joachim_Ante

    Unity Technologies

    Joined:
    Mar 16, 2005
    Posts:
    5,088
    My expectation is that you don't need to specify WithReadOnly in entities.Foreach because it is already on the captured struct. Are you getting any error messages that prompt you to do this?

    FYI. Something in your code sample went wrong all 3 examples are the same code.
     
  19. cultureulterior

    cultureulterior

    Joined:
    Mar 15, 2015
    Posts:
    53
    I'm also realizing I'm missing the index in IJobChunk (compared to IJobForEach) for when I want to submit things to a NativeArray, and when I want to do a ScheduleSingle and do a sorting of a NativeArray in the last/first iteration.
     
  20. Lucas-Meijer

    Lucas-Meijer

    Unity Technologies

    Joined:
    Nov 26, 2012
    Posts:
    166
    @optimise all three compile down to pretty much the same code, it shouldn't matter for performance.

    @Zec_ That's a good point. I think, but have not checked, that you should be able to put the [ReadOnly] on the actual field of the struct as you do on line 8, and then not declare it readonly in the Entities.ForEach(). For this specific example, I would personally go for technique3.
     
    optimise and Zec_ like this.
  21. Joachim_Ante

    Joachim_Ante

    Unity Technologies

    Joined:
    Mar 16, 2005
    Posts:
    5,088
    The following code works without any errors. The Temp ReaonlyCollection is correctly deallocated.

    Code (CSharp):
    1. public class ApplyVelocitySystem : SystemBase
    2. {
    3.     struct ApplyVelocity
    4.     {
    5.         [ReadOnly] [DeallocateOnJobCompletion]
    6.         public NativeArray<float3> ReadOnlyCollection;
    7.  
    8.         public float DeltaTime;
    9.  
    10.         public void Execute(ref Translation t, in Rotation v)
    11.         {
    12.             t.Value += ReadOnlyCollection[0] * DeltaTime;
    13.         }
    14.     }
    15.  
    16.     protected override void OnUpdate()
    17.     {
    18.         var applyVelocity = new ApplyVelocity
    19.         {
    20.             ReadOnlyCollection = new NativeArray<float3>( new[] { new float3(0, 1, 0)}, Allocator.TempJob),
    21.             DeltaTime = Time.DeltaTime
    22.         };
    23.         Entities
    24.             .ForEach((ref Translation t, in Rotation v) => applyVelocity.Execute(ref t, v))
    25.             .ScheduleParallel();
    26.     }
    27. }
     
    Last edited: Mar 29, 2020
  22. Zec_

    Zec_

    Joined:
    Feb 9, 2017
    Posts:
    98
    I have not been receiving any error messages, but it's always been a bit confusing to me which variables you would need to pass to WithReadOnly(). The documentation states that you should _always_ pass all readonly [NativeContainers] through it.
    Seeing that this is a nativecontainer that utilizes readonly access, I've assumed that I needed to do so. Is this not the case? Do we not need use WithReadonly() for nativecontainer fields tagged with the [ReadOnly] attribute?

    The three scheduling examples are scheduling using different approaches to WithReadOnly(), the middle one doesn't compile.

    Edit: Just read both of your responses that were posted while I was typing this! I now understand that I won't need it for [ReadOnly] fields. Thanks for the clarification!
     
    Last edited: Mar 29, 2020
  23. Joachim_Ante

    Joachim_Ante

    Unity Technologies

    Joined:
    Mar 16, 2005
    Posts:
    5,088
    WithReadOnly is required when you are directly capturing the container in the lambda. If you capture a struct that contains the container then it must be declared via [ReadOnly].
     
    phobos2077, eizenhorn, Jes28 and 2 others like this.
  24. 8bitgoose

    8bitgoose

    Joined:
    Dec 28, 2014
    Posts:
    252
    The lambdas are extremely messy just looking at them and writing separate functions just to call on lambdas which are already messy seems to try and reinvent the wheel when we already have simple structs.

    Using IJobForEachWithEntity is such a great way to access data in parallel from what I can see. I often use the index as well for writing to arrays that are created just in one round and then disposed of as I move data around. The more complex systems won't be handled by lambdas at all. As far as I can see IJobChunk makes your code 10 times messier. Sometimes I want to iterate over every item but not have the confusion of IJobChunk in there. Having HUGE long OnUpdate calls because of Lambdas seems like a big step back in my opinion. It gets away from functionized programming too.

    Sure leave the Lambdas in for new users. But leave IJobForEachWithEntity for people who want cleaner code.
     
    EwieElektro likes this.
  25. 8bitgoose

    8bitgoose

    Joined:
    Dec 28, 2014
    Posts:
    252
    I just reviewed the latest entity component stuff and yes you can provide the index, but LOOK how messy it looks. https://docs.unity3d.com/Packages/com.unity.entities@0.8/manual/ecs_entities_foreach.html You may just need a single line of code in your example, but some of my structs have 40 - 50 lines of code. Plus all the indenting due to the lambdas doesn't work very well on VS. And Lambdas almost never give you hints of what you are supposed to write next. Structs will allow you to auto complete.

    Don't get me wrong, the lambdas are great for simple stuff and I love ECS, but this is not good for usability in my opinion. This two page forum post about how it is a bad idea is definitely more feedback than other changes you've made that I've seen.
     
    EwieElektro and zachlindblad like this.
  26. desertGhost_

    desertGhost_

    Joined:
    Apr 12, 2018
    Posts:
    145
    What about defining the job code in a struct and just calling the execute method in your Foreach?

    This pattern has all the advantages of IJobForEach and none of the drawbacks. Plus there is no reason you couldn't use multiple structs in each ForEach (for increased code reuse).
     
  27. 8bitgoose

    8bitgoose

    Joined:
    Dec 28, 2014
    Posts:
    252
    Yup, I can understand that I can do that, but now you are creating a new struct and adding a bunch of unnecessary lamba expression. In my view, that code is way messier than how I can write it with IJobForEach. In the stuff I am doing, I may have 10 jobs all grabbing and manipulating at different stages. Plus it is frustrating using lambda expressions since auto complete isn't good with that. Which means more memorization.

    I mean, look the code is simpler with IJobForEach. You don't have to worry about giving the right inputs with a lambda expression that won't auto complete (as far as I've found).

    Code (CSharp):
    1.     [BurstCompile]
    2.     struct ApplyVelocity : IJobForEachWithEntity<Translation, Rotation>
    3.     {
    4.         [ReadOnly]
    5.         [DeallocateOnJobCompletion]
    6.         public NativeArray<float3> ReadOnlyCollection;
    7.  
    8.         public float DeltaTime;
    9.  
    10.         public void Execute(Entity entity, int index, ref Translation t, ref Rotation v)
    11.         {
    12.             t.Value += ReadOnlyCollection[0] * DeltaTime;
    13.         }
    14.     }
    15.  
    16.     protected JobHandle OnUpdate(JobHandle inputDeps)
    17.     {
    18.         var applyVelocity = new ApplyVelocity
    19.         {
    20.             ReadOnlyCollection = new NativeArray<float3>(new[] { new float3(0, 1, 0) }, Allocator.TempJob),
    21.             DeltaTime = Time.DeltaTime
    22.         };
    23.  
    24.         var handler = applyVelocity.Schedule(this, inputDeps);
    25.  
    26.         return handler;
    27.     }
    I usually have about 10 jobs in a row modifying data. Lambdas are gonna make this look like spaghetti code.

    The ECS team is removing (what I feel like) a vital intermediate place between simple stuff, Lambdas, and super complex IJobChunk. The best way to learn is have simple stuff. Oh wait, that isn't working very well or looking messy, why don't I use IJobForEachWithEntity. Oh okay I understand that now. I want more control, I'll jump to IJobChunk instead of going 0 - 100.

    And without a doubt there is some frustration that a core way to execute over data for each entity that was in all the stuff front and centre has been depreciated. It wasn't just side grabbing data in some way, it was how you did almost everything with the entity system and has been depreciate with a more verbose lambda system.

    I do understand the focus to make it simple. I think the lambdas are great for those who are doing simple stuff or introducing to ECS. All the examples are a single line of code. Looks great in the reading material. It's awesome and I don't want that to go away as an option. It just feels like this is a huge change and I am definitely not alone in puzzling why this is being depreciated.

    Realistically if this stays and IJobForEach is depreciated, I will probably wrap everything I've written in an IJobChunk which is going to increase the amount of messiness in my code without a doubt.
     
    EwieElektro and zachlindblad like this.
  28. snacktime

    snacktime

    Joined:
    Apr 15, 2013
    Posts:
    3,009
    You can abstract out ForEach yourself a lot really, including the lambda. Not ideal Unity should provide a method call variant as part of the api. But an extension method and custom struct with the right generic params is better then essentially using the wrong tool (IJobChunk).

    I'm holding off on finalizing our own setup for this until they remove IJobForEach completely, but builder/lambda is not going to be a thing in our game outside of where we actually want to use it. It's just one of those things that is well worth a day or two to fix given how irritating it is to deal with on a constant basis.
     
  29. 8bitgoose

    8bitgoose

    Joined:
    Dec 28, 2014
    Posts:
    252
    Yeah, it looks like I'll just have to strip the IJobForEach off all those structs I already made and use the Entities.ForEach pushing in the struct and executing the job. I also like being able to turn BurstCompile on and off for specific functions to get Debug Logs. I have a lot of code (my entire game is ECS) so it is gonna take a while to convert.
     
    zachlindblad likes this.
  30. zachlindblad

    zachlindblad

    Joined:
    Sep 29, 2016
    Posts:
    30
    Feeling the same pain everybody else is feeling here, been working on an ECS heavy project for about a year now and this is a total nightmare. Thought I was safe given that entities in every roadmap I've seen is supposed to be out of preview in 2020.1, past the point where, ya know, you might deprecate a primary method of processing entity queries up till now.

    Having your jobs be structs makes things explicit. I know what data is going into the job, I'm defining how it's accessed, and there's a little syntactic sugar so I don't have to actually deal with the chunks. Conversely when I scan over a lambda, I have no idea what's going into it, what data is being processed, unless I actually read every line of the function involved.

    Yes I can do the struct method that lucas suggested, but when it's run through a lambda system, that means the default is the kind of loosely defined closure that makes for garbage code. It means if I'm working with other coders, there's a higher chance they're gonna use that "good luck guessing what's involved with this lambda" default. It means if I'm looking for examples online, encapsulation, self containment, all of these good coding practices are not the default, you have to go out of your way to make it that way, So routing simple constants into an entity foreach, it's clear where they're coming from and what's being modified.

    A lambda ForEach was a good bonus, a nice option for one liner transformations, but the majority of my data transformations are not one liners. The majority of, at least my own, data transformations involve constants, a lot of them involve NativeCollections for writing to. Yes, I can and will be doing do the extra work to make my data transforms clearly encapsulated, but it makes me deeply worried when there isn't a base level system for this level of clarity.
     
    8bitgoose and PublicEnumE like this.
  31. 8bitgoose

    8bitgoose

    Joined:
    Dec 28, 2014
    Posts:
    252
    Here is another example where a simple Job that could have been entirely encapsulated inside a struct is now a bunch of code.

    Code (CSharp):
    1. NativeArray<int> intermediateSums = new NativeArray<int>(entitiesInQuery, Allocator.TempJob);
    2.         NativeArray<int> otherSumms = new NativeArray<int>(entitiesInQuery, Allocator.TempJob);
    3.         NativeArray<int> biggerSums = new NativeArray<int>(entitiesInQuery, Allocator.TempJob);
    4.         NativeArray<int> smallerSums = new NativeArray<int>(entitiesInQuery, Allocator.TempJob);
    5.         NativeArray<int> functionalSums = new NativeArray<int>(entitiesInQuery, Allocator.TempJob);
    6.         NativeArray<int> discountedSums = new NativeArray<int>(entitiesInQuery, Allocator.TempJob);
    7.  
    8.         //Schedule the second job, which depends on the first
    9.         Job
    10.             .WithCode(() =>
    11.             {
    12.                 int result = 0;
    13.  
    14.                 for(int i = 0; i < intermediateSums.Length; i++)
    15.                 {
    16.                     result += intermediateSums[i];
    17.                 }
    18.  
    19.                 for(int i = 0; i < otherSumms.Length; i++)
    20.                 {
    21.                     result += otherSumms[i];
    22.                 }
    23.  
    24.                 for(int i = 0; i < biggerSums.Length; i++)
    25.                 {
    26.                     result += biggerSums[i];
    27.                 }
    28.  
    29.                 for(int i = 0; i < smallerSums.Length; i++)
    30.                 {
    31.                     result += smallerSums[i];
    32.                 }
    33.  
    34.                 for(int i = 0; i < functionalSums.Length; i++)
    35.                 {
    36.                     result += functionalSums[i];
    37.                 }
    38.  
    39.                 for(int i = 0; i < discountedSums.Length; i++)
    40.                 {
    41.                     result += discountedSums[i];
    42.                 }
    43.  
    44.                 //Not burst compatible:
    45.                 Debug.Log("Final sum is " + result);
    46.             })
    47.             .WithDeallocateOnJobCompletion(intermediateSums)
    48.             .WithDeallocateOnJobCompletion(otherSumms)
    49.             .WithDeallocateOnJobCompletion(biggerSums)
    50.             .WithDeallocateOnJobCompletion(smallerSums)
    51.             .WithDeallocateOnJobCompletion(functionalSums)
    52.             .WithDeallocateOnJobCompletion(discountedSums)
    53.             .WithoutBurst()
    54.             .WithName("FinalSum")
    55.             .Schedule(); // Execute on a single, background thread
    This is a simple function. I have functions that around twice as long with just as many deallocated on complete job. The old system was great, you could just assign what was getting deallocated within the script with an attribute.

    I get that Unity is trying to put all the information of what you do with a job into one place so it is readable quickly. But this falls apart when you have anything more than a few lines of code in a function.
     
  32. davenirline

    davenirline

    Joined:
    Jul 7, 2010
    Posts:
    668
    One thing that I don't like about lambdas is that debugging the passed delegate is not great. Like line by line debugging is all over the place. Maybe this is because of the codegen. What we are debugging in the IDE is not the actual code. Not sure if this would be fixed by using an external struct.
     
    8bitgoose likes this.
  33. eizenhorn

    eizenhorn

    Joined:
    Oct 17, 2016
    Posts:
    2,062
    Also in addition to all I wrote above - code inconsistency. In real case we're ofen using not only Jobs from Entities package, but combine them with regular jobs like IJobParallelFor, IJob, IJobFor, now nice OnUpdate looks a bit mess :)
     
    zachlindblad likes this.
  34. Lucas-Meijer

    Lucas-Meijer

    Unity Technologies

    Joined:
    Nov 26, 2012
    Posts:
    166
    hey @8bitgoose. I'm going through your feedback, and want to make sure I understand everything you say.

    When you compare Joachim's example of what you can write with Entities.ForEach, to what you are writing today with IJobForEach, the only difference is you have the component types specified in the IJobForEach<Translation,Rotation>, whereas in the Entities.ForEach those are specified in the lambda expression.

    That is the gist of what you find unpleasing right?

    One of (many) reasons behind the deprecation is that operations on different components are hard to express in normal c# that IJobForEach tries. this results in the situation where we only support certain component types in certain order. support them with different numbers. that part of IJobForEach is a mess. Another big reason is that the implementation of IJobForEach tries to provide all the "magic" at runtime using reflection. It's slow and incompatible with smaller dot net runtimes like the ones we use for project tiny.

    Entities.ForEach departs a bit more from the c# you're used to, in that the signature of the lambda is freeform, and we will figure out what you meant from it (at compiletime instead of at runtime). It's not unimaginable we could implement something similar for the style you prefer. Not saying that we would (we'd have to balance it against the other 1000 things that we need to do on dots), if we'd make something like this, would you like/hate it?

    Code (CSharp):
    1.  
    2.     public class ApplyVelocitySystem : SystemBase
    3.     {
    4.         struct ApplyVelocity : INewJobForEach  //(INewJobForEach would be an interface with 0 methods)
    5.         {
    6.             [ReadOnly] [DeallocateOnJobCompletion]
    7.             public NativeArray<float3> ReadOnlyCollection;
    8.    
    9.             public float DeltaTime;
    10.    
    11.             public void Execute(ref Translation t, in Rotation v) //(this execute method is freeform, and wouldn't be autocompleted)
    12.             {
    13.                 t.Value += ReadOnlyCollection[0] * DeltaTime;
    14.             }
    15.         }
    16.    
    17.         protected override void OnUpdate()
    18.         {
    19.             var applyVelocity = new ApplyVelocity
    20.             {
    21.                 ReadOnlyCollection = new NativeArray<float3>( new[] { new float3(0, 1, 0)}, Allocator.TempJob),
    22.                 DeltaTime = Time.DeltaTime
    23.             };
    24.             Entities.ForEach(applyVelocity).ScheduleParallel();
    25.         }
    26.     }
    27.  
    here you wouldn't go through a lambda, but you'd provide a struct, and we'd go look for an Execute method on it, and from its parameters figure out which compenents you want to operate on, and then we'd codegen a IJobChunk for you in the background.
     
  35. 8bitgoose

    8bitgoose

    Joined:
    Dec 28, 2014
    Posts:
    252
    Hi @Lucas-Meijer, thanks for taking the time to answer me. Thanks for clarifying why you are removing the IJobForEach. I did not realize that it was taking a toll through reflection. As far as I could see from the responses, it was being depreciated to simply make it easier to program.

    When I first started programming in the ECS system that Unity had provided, I could see that there was one major problem. You had three different areas to program in.

    1. You had a struct at the start of your System class which you did all the data manipulation
    2. You had a declaration of all the structs in your OnUpdate function. For me this is about 5 - 10 structs usually in the order of execution to simplify it
    3. After all the struct declarations, you then schedule everything. In here, you have a lot of control on how each of the systems is executed.

    This definitely got some getting used to, but other than the annoyance of jumping around the code base, it works very well and is very clean.

    The Lamba expressions do a great job of putting everything in order which is great. But I don't see it working well for very big complex systems. You will need to break out ForEach and Jobs into their own methods and so you lose the initial benefit of not jumping around in your code base. I don't see the lambdas being good for complex systems.

    Now onto your example.

    So the IJobNewForEach is basically doing automatic component gathering. So almost the same as before, you just aren't declaring arguments? Can we use attributes on the struct or is that reflection that isn't supported?

    Code (CSharp):
    1.  
    2. [BurstCompile] // <--- ????
    3. [ExcludeType(typeof(Scale))] // <--- ????
    4. struct ApplyVelocity : INewJobForEach  //(INewJobForEach would be an interface with 0 methods)
    5.         {
    6.             [ReadOnly] [DeallocateOnJobCompletion]
    7.             public NativeArray<float3> ReadOnlyCollection;
    8.  
    9.             public float DeltaTime;
    10.  
    11.             public void Execute(Entity entity, int index, ref Translation t, in Rotation v) //(this execute method is freeform, and wouldn't be autocompleted)
    12.             {
    13.                 t.Value += ReadOnlyCollection[0] * DeltaTime;
    14.             }
    15.         }
    16.  
    17.             var applyVelocity = new ApplyVelocity
    18.             {
    19.                 ReadOnlyCollection = new NativeArray<float3>( new[] { new float3(0, 1, 0)}, Allocator.TempJob),
    20.                 DeltaTime = Time.DeltaTime
    21.             };
    22.  
    23.             var jobHandler = Entities.ForEach(applyVelocity).ScheduleParallel(inputDeps); // <--- Manually setting the handlers
    24.  
    25.  
    I'd like the struct to declare exactly what it needs in that code area. All you to do is feed in the struct and Unity does everything else. I want to include tags that are and aren't allowed on the struct too. The last line in my code chunk is all I'd ever need to do to schedule it.

    The primary purpose of this is when I need to modify a group of components. I write a struct, declare it and execute it. The struct / method is encapsulated and just needs to be called. It keeps the managing of what a struct does in just one spot.

    You mentioned that code generation for a IJobChunk can just be generated? Can I basically convert all my IJobForEachWithEntity to IJobChunk? Is the first Entity Index similar to index in IJobForEachWithEntity? It just gives you the first index of that chunk so it is always unique across whenever you are creating a junk. Sorry if that isn't clear.

    Thanks for the feedback. Sorry if I have come off as frustrated. I was pretty surprised at this change after using ECS for almost a year. I still love ECS and it has been a godsend coupled with the Burst Compiler :).
     
  36. snacktime

    snacktime

    Joined:
    Apr 15, 2013
    Posts:
    3,009
    The thing is a lot of us are going to work around this. Like we will be using extension methods and job struct not unlike IJobForEach to completely abstract away the lambda and provide a standard approach using generic structs. Most likely we will use codegen for that ourselves since it's fairly straight forward. I'm waiting on IJobForEach to be gone to see how you guys are generating that part, it might potentially change my specific implementation.

    I get it's more work to provide a method call alternative, probably would require some type of input param declaration. But whatever the most elegant approach is, it would be better for Unity to provide that then having developers reinventing the wheel on it all over, and that is exactly what will be happening. It's going to be a source of frustration until Unity solves it well.
     
  37. brunocoimbra

    brunocoimbra

    Joined:
    Sep 2, 2015
    Posts:
    418
    Personally, I would love it, as my only issue right now is having to type all parameters again in the lambda, thus making me maintain the parameters in two (or more) different places between logic changes.
     
  38. zachlindblad

    zachlindblad

    Joined:
    Sep 29, 2016
    Posts:
    30
    Thanks for the clarification Lucas. I'm very excited for project tiny to mature further, I've even done some small experiments with what's been put out there. I can see why it'd be difficult to maintain 2 interfaces for component data transformations, one of which wouldn't work in the tiny project.

    However another big pain point with this for me, is I feel uncomfortable when the only two systems involve either bare metal handling of chunks, or compiler code gen magic. The keyword arguments for entities/entity index can't be derived from the signature, my IDE isn't going to know about them, I'm not going to know about them unless I dig into the documentation.

    If DOTS really is the future of unity, and the only way to transform data is to use a system with magic variable names, I don't really feel like I'm programming in C#, I'm programming in some kind of UnityScript.
     
  39. 8bitgoose

    8bitgoose

    Joined:
    Dec 28, 2014
    Posts:
    252
    @Lucas-Meijer So I was looking at that code excerpt again. Is this possible??

    Code (CSharp):
    1.  
    2. public class ApplyVelocitySystem : SystemBase
    3. {
    4.   [BurstCompile] // <--- ????
    5.   [ExcludeType((Scale))] // <--- ????
    6.    struct ApplyVelocity  //(INewJobForEach would be an interface with 0 methods)
    7.    {
    8.        [ReadOnly]
    9.        [DeallocateOnJobCompletion]
    10.        public NativeArray<float3> ReadOnlyCollection;
    11.  
    12.        public float DeltaTime;
    13.  
    14.        public void Execute(ref Translation t, in Rotation v) //(this execute method is freeform, and wouldn't be autocompleted)
    15.        {
    16.            t.Value += ReadOnlyCollection[0] * DeltaTime;
    17.        }
    18.    }
    19.  
    20.    protected override void OnUpdate()
    21.    {
    22.        JobHandle handler = Entities.ForEach(new ApplyVelocity()
    23.            {
    24.                ReadOnlyCollection = new NativeArray<float3>(new[] { new float3(0, 1, 0) }, Allocator.TempJob),
    25.                DeltaTime = Time.DeltaTime
    26.            }).Schedule(inputDeps);
    27.    }
    28. }
    29.  
    Because with that format, you do simplify all the code gen into two areas. One where you create a struct with all the relevant information to execute an entity specific bunch of code. And then one area where you schedule that all inline so you know what is doing what in what order.

    I have a group of Systems that together have around 7000 lines of code. Even if the verbosity of it was paired down to 5000 lines, having 5000 lines in an OnUpdate function is not good for maintainability.

    I understand you are trying to collapse all this down into a simple OnUpdate() function where everything is laid out in order, but it is just going to lead to 1000s of lines of code in OnUpdate() with me. I need the OnUpdate to be an overview of the order of the system and then the structs before OnUpdate() what happens to the components in a more complex way.

    And everything needs to be executed this way. So IJobForParallel needs to be executable in the same way so you have consistency. Same with IJobChunk or else it won't sync up.
     
  40. wobes

    wobes

    Joined:
    Mar 9, 2013
    Posts:
    759
    I do not believe IJobForeach was strongly tied to reflection at first place. It does use some reflection initially to create a query yet it works on AOT platforms and should be supported by your "smaller dot net runtimes". If for a every new feature companies would deprecate already exciting features what would be left of their products? If you call IJobForEach a mess you better look into lambda mess. You want people to end up having insane amount of code in their OnUpdate methods without respecting any SOLID principles.
     
    Last edited: Apr 9, 2020
    Antypodish and Knightmore like this.
  41. Lucas-Meijer

    Lucas-Meijer

    Unity Technologies

    Joined:
    Nov 26, 2012
    Posts:
    166
    Yes, that's roughly what I meant. It would mean we have yet-another-way of creating entityquery's (now you want to control the entityquery through [ExcludeType] attrbiute), but that might be worth it. You wouldn't get the nice HasComponent/GetComponent/SetComponent magic. if you want that you'd have to manually do a GetComponentDataFromEntity(), pass that into the job. It also wouldn't support .Run(), just .Schedule() and .ScheduleParallel(). but that's probably a tradeoff you'll happily make.

    I'll think about this a bit more.
     
  42. Joachim_Ante

    Joachim_Ante

    Unity Technologies

    Joined:
    Mar 16, 2005
    Posts:
    5,088
    Code (CSharp):
    1. It also wouldn't support .Run(), just .Schedule() and .ScheduleParallel(). but that's probably a tradeoff you'll happily make.
    2.  
    We can easily support .Run() for this as well. IJobChunk itself has a Run method we can build the codegen on top of.
     
    thelebaron and brunocoimbra like this.
  43. Joachim_Ante

    Joachim_Ante

    Unity Technologies

    Joined:
    Mar 16, 2005
    Posts:
    5,088
    I like the INewJobForEach idea. It seems like the sticking point left is should we have [ExcludeType] or not.

    This is the sample with exclude type style API:

    Code (CSharp):
    1. public class ApplyVelocitySystem : SystemBase
    2. {
    3.    [BurstCompile]
    4.    [ExcludeType((Scale))]
    5.    struct ApplyVelocity : INewJobForEach //  would be an interface with 0 methods)
    6.    {
    7.        [ReadOnly]
    8.        [DeallocateOnJobCompletion]
    9.        public NativeArray<float3> ReadOnlyCollection;
    10.        public float DeltaTime;
    11.        public void Execute(ref Translation t, in Rotation v) //(this execute method is freeform, and wouldn't be autocompleted)
    12.        {
    13.            t.Value += ReadOnlyCollection[0] * DeltaTime;
    14.        }
    15.    }
    16.    protected override void OnUpdate()
    17.    {
    18.        JobHandle handler = Entities.ForEach(new ApplyVelocity()
    19.            {
    20.                ReadOnlyCollection = new NativeArray<float3>(new[] { new float3(0, 1, 0) }, Allocator.TempJob),
    21.                DeltaTime = Time.DeltaTime
    22.            }).Schedule(inputDeps);
    23.    }
    24. }
    25.  

    This is using Entities.WithNone()

    Code (CSharp):
    1.  
    2.  
    3. public class ApplyVelocitySystem : SystemBase
    4. {
    5.    [BurstCompile]
    6.    struct ApplyVelocity : INewJobForEach // (INewJobForEach would be an interface with 0 methods)
    7.    {
    8.        [ReadOnly]
    9.        [DeallocateOnJobCompletion]
    10.        public NativeArray<float3> ReadOnlyCollection;
    11.        public float DeltaTime;
    12.        public void Execute(ref Translation t, in Rotation v) //(this execute method is freeform, and wouldn't be autocompleted)
    13.        {
    14.            t.Value += ReadOnlyCollection[0] * DeltaTime;
    15.        }
    16.    }
    17.    protected override void OnUpdate()
    18.    {
    19.        JobHandle handler = Entities.WithNone<Scale>().ForEach(new ApplyVelocity()
    20.        {
    21.            ReadOnlyCollection = new NativeArray<float3>(new[] { new float3(0, 1, 0) }, Allocator.TempJob),
    22.            DeltaTime = Time.DeltaTime
    23.        }).Schedule(inputDeps);
    24.    }
    25. }
    26.  

    Not really convinced that having the [Exclude] attribute is really solving anything real.
    What problem specifically does it solve?
     
  44. snacktime

    snacktime

    Joined:
    Apr 15, 2013
    Posts:
    3,009
    This is basically what we came up with. INewJobForEach we have to define. Once it is it's then trivial to codegen the extension methods, create a menu item to codegen them all. INewJobForEach as an empty interface is better. I was thinking it's doable with reflection (at codegen time) but was waiting until the next iteration of Entities.ForEach to work through it. In any case even this is better then inlining all over. So we have to define 2-3 dozen interfaces worst case, I'm fine with that.

    Code (csharp):
    1.  
    2. public interface INewJobForEach<T0> where T0 : struct, IComponentData
    3.         {
    4.             void Execute(ref T0 co);
    5.         }
    6.  
    7.         public struct TestJob : INewJobForEach<AgentSteering>
    8.         {
    9.             public void Execute(ref AgentSteering steering)
    10.             {
    11.             }
    12.         }
    13.  
    14.         public static ForEachLambdaJobDescription IJobForEach(this ForEachLambdaJobDescription desc, INewJobForEach<AgentSteering> job)
    15.         {
    16.             return desc.ForEach((ref AgentSteering steering) =>
    17.             {
    18.                 job.Execute(ref steering);
    19.             });
    20.         }
    21.  
     
  45. 8bitgoose

    8bitgoose

    Joined:
    Dec 28, 2014
    Posts:
    252
    Hi @Joachim_Ante,

    What I am looking for is complete separation between the scheduling of jobs and what the job does.

    Code (CSharp):
    1.  
    2. JobHandle handler = Entities..ForEach(new ApplyVelocity() {...}).Schedule(inputDeps);
    3.  
    You would never have anything more than Entites.ForEach -> Job Struct -> Scheduling.

    If I write a struct job, I want to do everything related to what I components I need to access within that struct. The query should be be set up somehow within the struct. If you let .WithNone() creep in, may as well add tons of other lambda properties. I'd also like to see [IncludeComponentTag] in the struct as well?

    Does this make sense? I want to separate implementation and modification of data from the scheduling of the data. The Struct is where you figure out the query, do the data manipulation etc etc. The OnUpdate is where you create the struct and give it variables and schedule it for execution. Allows for much cleaner code. The OnUpdate is the table of contents and the structs are the chapters.

    By the way, I fully support the Lambda system. It will be amazing for doing quick systems that don't have a lot of complexity.
     
  46. Radu392

    Radu392

    Joined:
    Jan 6, 2016
    Posts:
    210
    For whatever my opinion is worth, I also dislike the lambda system for the same reasons posted by others on this thread, especially 8bitgoose and eizenhorn. It's good for newbies and prototyping, but otherwise gets in the way.

    We had a good thing going with IJobForEach. It's the best middle ground between boilerplate code and control over the code's organization. Please don't remove it!
     
  47. eizenhorn

    eizenhorn

    Joined:
    Oct 17, 2016
    Posts:
    2,062
    In summary of this thread:
    - ForEach pattern fine and we can live with that. It's nice for small granular things.
    - For things, bigger than just couple of lines, most of us can use extracted struct declaration in job style, but needs some butifying (@Lucas-Meijer INewJobForEach idea looks nice)
    - All this WithA.WithB.WithC.WithD ... .WithZ not what most of us want (reasons, messing code look and readability,), explicit EntityQuery declaration and using it as some argument for ForEach solves part of this WithA.WithB.WithC and give nicer and much readable code. For extracted ForEach body (see second bulletpoint) we have attributes as alternative for another part of WithA.WithB.WithC (read only, NativeDisable... etc.).
     
  48. mf_andreich

    mf_andreich

    Joined:
    Jul 7, 2017
    Posts:
    37
    Hm... nice summary, but generics supprot for ForEach its important too.
     
    Ylly likes this.
  49. flatterino

    flatterino

    Joined:
    Jan 22, 2018
    Posts:
    14
    For whatever it's worth: I understand the need to appeal to more novice coders (especially those coming from web backgrounds, JavaScript/Python flavors and whatnot), but I agree with everyone in this thread who supports a way of keeping data and actual logic as separate as possible without having to resort to chunk-based logic or lambda chains.

    Just want to also point out that, in my opinion, "over here is my data definition, and over here, separately, is my logic" is a nice, simple mental model that can be useful especially for novices to understand what's actually going on when writing ECS code in a more structured, explicit way than using "magic lambdas". Yes, I know they're not magic at all, but to new coders (or people used to OOPs), even if feels simpler to write that bit of code, it's still an opaque pattern that screams "you just gotta do it this way for it to work best, and don't ask why, because at your level of knowledge doing it explicitly to actually understand the process well from the start is more complicated than it's worth".

    Lambdas are supposed to offer a way to simplify an operation which, since you already know well, you don't need all the boilerplate for every single time, as opposed to telling the learner that using lambdas for everything you can is good and efficient for arcane reasons which (for a novice) can't be reasoned out by simply looking at the code, and which can't be easily replaced by a more explicit alternative without wondering about the negative implications of doing so.

    Since DOTS/ECS is all about making performance accessible to all, being able to easily tell if two ways of writing something are equivalent (or at least execute in a similar enough way) is essential, and as someone coming over from an extensive OOP background, that's the one hurdle I find myself having to work around most often when transitioning to ECS/DOTS.

    I'm sure lots of novices wouldn't care for it and just say "give me one right way to do it, 9/10 times I don't want to have to think this through", but there must be a viable, first-class alternative for those who do want it, instead of just googling for someone wondering the same thing and having to rely on possibly outdated dev posts saying "if you write it in this different and very specific way, the end result should be the same".

    Just the way I feel about it.

    P.S. It's so good the see the devs be in touch with the community and be open to alternatives and changes. Just a while ago I posted a thread with some basic questions and I was stunned to see Joachim reply directly. Good job!
     
  50. PhilSA

    PhilSA

    Joined:
    Jul 11, 2013
    Posts:
    1,472
    So far in my DOTS project, I really tend to just use IJobChunk all the time instead of Entities.ForEach, even for small/simple tasks. Entities.ForEach scares me, because to me it appears very "magical" and non-explicit about how everything works. The rules aren't always clear. I am often left asking myself if I have the right to do X or Y inside the ForEach, and then I switch to IJobChunk just in case my idea would've caused secret hard-to-detect problems.

    I guess what I'm trying to say is that I have a feeling that Entitie.ForEach isn't the thing that'll make DOTS accessible to the masses. When I show it to fellow programmers who have yet to start using DOTS in practice, their reaction is usually something like "hmm I don't know if I want to trust this". And my opinion on it is a bit similar as well. I know objectively that it works well, but there is just something off-putting about it. It seems to strand too far from what is standard and normal.

    But I also don't really know what I would do instead.

    I think, at the end of the day, the ideal solution is one where we end up with a simple file where we can implement these 3 core things:
    • the update loop of our job (where we can access & modify all parameters that are passed by 'ref' or 'in')
    • what entity query this job operates on
    • how the job is constructed (passing the deltaTime, the PhysicsWorld, etc... but maybe all the data arrays stuff could be handled automatically?)
    But the question would be; what's the best & most conventional way to get to that? There would be some form of codegen involved for sure. Sometimes I also wonder if DOTS Visual Scripting could become a tool to generate the skeleton of the system+IJobChunk that you want to make, and then the rest could be implemented in code? (far from conventional, but I think it might work well nonetheless)

    And one last thing; I think telling people to just manually write IJobChunks for more specialized cases is totally acceptable, if that could help make the easy-to-use approach more simple
     
    Last edited: May 2, 2020
unityunity