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.

Feedback Really hoped for RefRW/RefRO GetComponentRW/RO(Entity)

Discussion in 'Entity Component System' started by TheSmokingGnu, Dec 3, 2022.

  1. TheSmokingGnu


    May 1, 2017
    I got excited seeing GetComponentRW method, but turns out it's GetComponentRW(SystemHandle) that "returns entity associated with this system".
    Edit: There's also GetSingletonRW... which is only one step away...
    Why is there still no way to get a ref to component for an arbitrary entity?I understand that unmanaged references do not guarantee the lifetime, and are not safe, yada-yada.
    But for more than a year of working with ECS I have needed to use them exclusively in the local scope of a function, a million times.

    Whenever you work with more than one level of entities (e.g. you query for entity A, which has a reference to entities of type B), you can't get a reference to components of entity B. Which makes you use GetComponent, and now you have to make sure that you pair every single GetComponent with a SetComponent, at the end of the method, which can be 2 screens away. This was a constant source of bugs, and hurts readability, and debugging, since you're never really sure if the 5 SetComponents here are enough, or if there is a 6th one missing... Imagine now that the function has > 1 returns, you will now need to add necessary sets to each return...
    Correct if I'm wrong, but I think even the new idiomatic foreach won't allow to get references to a list of entities? It just allowes you to have a query inside a query? which I didn't find a use of.

    If then you need to extract some function in the middle, you have to pass that component by ref, and now a lot of your functions can't just take entity, they need to take all the components as refs together with it. If you need to make a overload that takes entity, you need to write all the get-sets again.

    1. Am I the only one going crazy because of this? Please share your thoughts on this! Imho, that's the part of the boilerplate that everyone criticizes the ECS with, except this time it's actually damaging maintainability, instead of just being a technical difficulty of writing more verbose code.

    2. RefRW/RefRO GetComponentRW/RO(Entity) would solve this. What's the rationale of not having it?... I can guess it's that "returing unmanaged refs is dangerous...". I think the expectation is that a lot of people will confuse those with managed references, and save them to the member scope, and than recieve ObjectDisposed errors... Which may be a valid concern, but I fill like if we've already taken whole managed C# from people it would be really hard for them to use ecs without understanding this most core concept of unsafe code. They already have to live with the fact that Entity or native collection reference can be invalidated, for example, just as even before the GameObject could...
    I was explaining the need to do a get+set pair to my junior coworkers coming from OOP, and they were as buffled by this as I think they would if they encountered a crash after using a reference after the method end.
    Is this worth having such an dangerous inconvenience for people who know how references work? Edit: also, looks like we already have GetSingletonRW<> and GetAspectRW<> which have all the same lifetime issues.

    3. Maybe someone found a workaround for this?
    Looks like now we can get the references via Aspects. But, it feels like it makes you create a lot of 1:1 aspects just to get the refs.

    Previously, I did this, which is ugly:
    Mine is using RAII via IDisposable to automatically do the set:

    Code (CSharp):
    2. public ref struct ComponentReference<T> where T : unmanaged, IComponentData
    3. {
    4.     T _component;
    5.     Entity _entity;
    7.     public ComponentReference(Entity entity) { // World.DefaultGameObjectInjectionWorld.EntityManager.GetComponentData }
    9.     public ref T Value();
    11. public void Dispose()
    12. {
    13.     var eManager = World.DefaultGameObjectInjectionWorld.EntityManager;
    14.     eManager.SetComponentData(_entity, _component);
    16. }
    Last edited: Dec 3, 2022
    Jes28 likes this.
  2. Enzi


    Jan 28, 2013
    ComponentLookup has GetRef now. Is this not sufficient or do I miss some context?
    My Entities.Exposed library had refs for a while and I'm still exposing some data that you can't get otherwise. (Mainly direct pointers)
    MadeFromPolygons and Kmsxkuse like this.
  3. TheSmokingGnu


    May 1, 2017
    Missed that it's there in lookups at the time of writing, thanks. Which is good, as the Aspect workaround apparently is blocked by current bug with compilation error when using GetAspect in jobs...

    This changes things:
    1. For jobs, it's solved, since it's all on lookups anyway
    2. For System code, it's technically working, but now you need to create and pass/store all the lookups, like in jobs, which is mildly tedious.
    3. Outside of systems though, the problem remains. E.g. if you have a shared utils code between systems, or some generic extensions...
    Seems like Entities.Exposed does what we now have in ComponentLookups for my use case, but man, it was just what I wanted in earlier versions :)

    I guess this means that it's not a design desicision, we are just missing a part of api for some reason. So I hope we will get the same thing for GetComponents in EntityManager.