Search Unity

  1. Welcome to the Unity Forums! Please take the time to read our Code of Conduct to familiarize yourself with the forum rules and how to post constructively.
  2. Voting for the Unity Awards are OPEN! We’re looking to celebrate creators across games, industry, film, and many more categories. Cast your vote now for all categories
    Dismiss Notice
  3. Dismiss Notice

Any reason why public fields are used in Job structs?

Discussion in 'Entity Component System' started by Nadall, Nov 1, 2018.

  1. Nadall

    Nadall

    Joined:
    Dec 13, 2016
    Posts:
    4
    Every Jobs demo I've seen uses public fields initialized through object initializer when constructing Jobs structs. Is there a particular reason for it? It makes more sense to me to use private fields initialized though constructor, this would make refactoring easier, since when you're changing job dependencies compiler will force you to update all usages of the job. To clarify I'm specifically talking about Job structs here, that implement IJob or IJobParallelFor, not ECS components.
     
  2. eizenhorn

    eizenhorn

    Joined:
    Oct 17, 2016
    Posts:
    2,653
    Nobody forbids to use the constructor.
     
  3. alexzzzz

    alexzzzz

    Joined:
    Nov 20, 2010
    Posts:
    1,447
    I was wondering about it too and came up with two possible reasons:
    1. Less code to write ― no need for a constructor.
    2. Less code to run ― no need to call the constructor whose only job is to copy data from one place to another. Hopefully it will be inlined, but may be not.

    But frankly, both reasons seem insignificant. Calling a constructor is a more reliable and idiomatic way to initialize an object, and creating a new job is not something you do millions times a frame.
     
  4. Lurking-Ninja

    Lurking-Ninja

    Joined:
    Jan 20, 2015
    Posts:
    9,900
    Use whatever you like. I think Unity is trying to be consistent here. Since this entire Jobs-ECS-Burst is in the sake of performance, it's understandable that they don't want to paint a picture where anyone thinks that the constructor call is faster than the direct property access.
    Also keep in mind that Unity (Joachim and others stated multiple times) is trying to distance themselves from the dogmatic OOP/OOD.
    So does not matter. You won't/can't reach these properties outside of the place where you put them anyway, because wouldn't make any sense, so it really does not matter other than the l'art pour l'art OOP-ist elitism.

    You can write your own code however you like. From Unity, we expect the fastest code examples (best practices) possible. It's a good thing to build habits which results higher performance.
     
  5. davenirline

    davenirline

    Joined:
    Jul 7, 2010
    Posts:
    942
    There's very little reason to encapsulate a job struct. Their lifetime is too short. They're not meant to stick around and have some kind of state to protect. This is actually good practice so that you won't treat job structs as something that holds state that persist throughout the game. Struct initialization using public fields also looks neat.

    If you want to be pedantic about encapsulation, no one is stopping you. I used to be this way (OOP baggage). I bit the bullet and started using public initializers. That's when I realized that this is probably for the best so that I won't ever make job structs that holds state.
     
  6. 5argon

    5argon

    Joined:
    Jun 10, 2013
    Posts:
    1,554
    I think it is related to how the code looks, not necessary the safest. In a way using public field feels like when writing JavaScript, the freedom is dangerous but once you get a control of what could went wrong the resulting code looks quite nice. By using constructor it is safer, but there are lots of variable movement "visually".

    Writing C# I also often unknowingly conditioned to lock myself in all possible way so that nothing could go wrong, because there are a lot of language tools to do so and it is tempting to stay neat. (like trying to encapsulate to property on everything) But I do realize merit in writing a little more loose code, so that it is easier to read and faster to write. The possibility of error might be increased, but then I have to train enough to avoid going into those errors in order to have this code style's advantage.

    *Also there is a specific runtime error saying that "all public field of the job must be fully assigned before starting" or something, that also helps coping with errors that could occur with this style.
     
    SugoiDev likes this.
  7. Nadall

    Nadall

    Joined:
    Dec 13, 2016
    Posts:
    4
    Except when you add a field to a Job struct and forget to update it's usages, your code compiles just fine even though your field remains uninitialized. In cases where the added field is a NativeArray, you'll get an error at runtime and probably discover the bug early (a bug that would still have been prevented following standard OOP practices), but if your field is a float, int or a custom struct made from these types then it will be initialized with default values of 0, and now you have a potentially hard to discover bug that will corrupt your data.

    EDIT: Apparently Unity itself will throw a runtime error on unitialized public fields.

    In most software projects code correctness and maintainability are far more important than performance. It doesn't matter if your code runs slow or fast, when your app crashes every few minutes or is full of bugs. There's a reason why telling beginner programmers to always optimize is considered a bad advice.
     
    Last edited: Nov 4, 2018
  8. Nadall

    Nadall

    Joined:
    Dec 13, 2016
    Posts:
    4
    That's good to hear. It seams that ECS guys at Unity did a good job at making procedural programming (which is what ECS is) as bearable as possible. It would be great if someone in the future would make a Visual Studio extension that throws compiler errors in these sort of cases instead.
     
  9. Lurking-Ninja

    Lurking-Ninja

    Joined:
    Jan 20, 2015
    Posts:
    9,900
    Well... Then you forgot something. Which means you messed up and Unity will tell you that.

    Go for it. Don't wait for others, you need it, not us. I'm coming from a very different path, where if you messed up, you froze your computer (assembly) a lot. So forgetting an initialization is a piece of cake. I don't need that tool and most people who actually pay attention to the code don't need it. You know, we don't change code just like that, we think through the change, what happens, what implications the change carries, etc. After that I assure you, you won't forget in the 99% of the changes. The 1% is always up, but that's the reason we have the error messages.

    What? Default values are 'hard to discover'? Well. Okay.

    Except for games. All the business applications made by this principle, I know, that's my day job. But games are made on different principle: don't waste the frame time.

    If your app crashes, you did something wrong. It's simple.
     
    xVergilx and e199 like this.
  10. Nadall

    Nadall

    Joined:
    Dec 13, 2016
    Posts:
    4
    Following your logic, all advancements in computer science made over the past 50 years are unnecessary. I mean, who needs a type system, a programmer can just remember what var is a string and what is an int. Forget about naming as well, just name everything var1, var2 etc., I mean you can always just remember which is which, amiright?
    Anyways, I have no wish to start a flame war over a thing as trivial as this. I asked a question and people answered, now have a nice day sir.
     
  11. tertle

    tertle

    Joined:
    Jan 25, 2011
    Posts:
    3,624
    Difference is unity collections will actually throw an error if unallocated going into a job, you can't really miss them. But other fields will skip by (though your IDE might warn you.)

    Anyway, I agree with you somewhat and actually quite like constructors for the same reason but sometimes they become super unwieldy when you have a lot of fields - take my pathfinding job for instance.

    Code (CSharp):
    1.             public PathRequest Request;
    2.             public NavigationCapabilities Capabilities;
    3.             public int DimX;
    4.             public int DimY;
    5.             public int Iterations;
    6.  
    7.             [ReadOnly]
    8.             public NativeArray<Neighbour> Neighbours;
    9.  
    10.             [ReadOnly]
    11.             public NativeArray<ArchetypeChunk> GridChunks;
    12.  
    13.             [ReadOnly]
    14.             public ArchetypeChunkBufferType<Cell> CellTypeRO;
    15.  
    16.             public DynamicBuffer<float3> Waypoints;
    17.             public NativeArray<float> CostSoFar;
    18.             public NativeArray<int2> CameFrom;
    19.             public NativeMinHeap OpenSet;
    20.  
    21.             public float3 Position;
    That'd be a giant constructor.
     
    Last edited: Nov 6, 2018