Search Unity

  1. Megacity Metro Demo now available. Download now.
    Dismiss Notice
  2. Unity support for visionOS is now available. Learn more in our blog post.
    Dismiss Notice

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

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

  1. TheSmokingGnu

    TheSmokingGnu

    Joined:
    May 1, 2017
    Posts:
    22
    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?
    Edit:
    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):
    1.  
    2. public ref struct ComponentReference<T> where T : unmanaged, IComponentData
    3. {
    4.     T _component;
    5.     Entity _entity;
    6.  
    7.     public ComponentReference(Entity entity) { // World.DefaultGameObjectInjectionWorld.EntityManager.GetComponentData }
    8.  
    9.     public ref T Value();
    10.  
    11. public void Dispose()
    12. {
    13.     var eManager = World.DefaultGameObjectInjectionWorld.EntityManager;
    14.     eManager.SetComponentData(_entity, _component);
    15.  
    16. }
    17.  
    18.  
     
    Last edited: Dec 3, 2022
    JesOb likes this.
  2. Enzi

    Enzi

    Joined:
    Jan 28, 2013
    Posts:
    954
    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

    TheSmokingGnu

    Joined:
    May 1, 2017
    Posts:
    22
    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.
     
  4. Pieter_Weterings

    Pieter_Weterings

    Joined:
    Dec 28, 2023
    Posts:
    1
    Not sure if anyone is still looking for this, but it turns out you can extend this into your project:

    However, (1) you're going to have to unsafe and (2) please be aware RefRW<T> lose their reference after *any* structural change (and you don't generally know if one has occured given the async nature of jobs) - particularly the latter issue is probably going to prevent you from actually getting any use out of this; hope this helps anyways:

    1| Option 1 : As extension to the EntityManager (to be called on mainthread)

    1.1 | Create a new folder in your project (pref. call it Internal/Entities)
    1.2 | Add a new Assembly Definition Reference Asset to the folder, and target the assembly definition to Unity.Entities
    1.3 | Add an EntityManagerExtension.cs script with the following code


    Code (CSharp):
    1. using System;
    2. using Unity.Collections;
    3. namespace Unity.Entities
    4. {
    5.     public unsafe partial struct EntityManager : IEquatable<EntityManager>
    6.     {
    7.         /// <summary>
    8.         /// Gets the value of a component for an entity associated with a system.
    9.         /// </summary>
    10.         /// <param name="system">The system handle.</param>
    11.         /// <typeparam name="T">The type of component to retrieve.</typeparam>
    12.         /// <returns>A <see cref="RefRW{T}"/> struct of type T containing access to the component value.</returns>
    13.         /// <exception cref="ArgumentException">Thrown if the component type has no fields.</exception>
    14.         /// <exception cref="InvalidOperationException">Thrown if the system isn't from thie world.</exception>
    15.         [GenerateTestsForBurstCompatibility(GenericTypeArguments = new[] { typeof(BurstCompatibleComponentData) })]
    16.         public RefRW<T> GetComponentDataRW<T>(Entity _entity) where T : unmanaged, IComponentData
    17.         {
    18.             var access = GetUncheckedEntityDataAccess();
    19.  
    20.             var typeIndex = TypeManager.GetTypeIndex<T>();
    21.             var data = access->GetComponentDataRW_AsBytePointer(_entity, typeIndex);
    22. #if ENABLE_UNITY_COLLECTIONS_CHECKS
    23.             return new RefRW<T>(data, access->DependencyManager->Safety.GetSafetyHandle(typeIndex, false));
    24. #else
    25.             return new RefRW<T>(data);
    26. #endif
    27.         }
    28.  
    29.     }
    30. }
    2 | Option 2 : Get RefRW directly in job without component lookup (IJobChunk)
    Code (CSharp):
    1. /// <summary>
    2. /// Job that stores a reference to the local transform of entities
    3. /// </summary>
    4. [BurstCompile]
    5. public unsafe struct GetLocalTransformJobRW : IJobChunk
    6. {
    7.     public ComponentTypeHandle<LocalTransform> LocalTransformtypeHandle;
    8.  
    9.     public EntityTypeHandle participantEntityHandle;
    10.  
    11.     public UnsafeParallelHashMap<Entity, RefRW<LocalTransform>>.ParallelWriter participantTransformWriter;
    12.  
    13.     [BurstCompile]
    14.     public void Execute(in ArchetypeChunk chunk, int unfilteredChunkIndex, bool useEnabledMask, in v128 chunkEnabledMask)
    15.     {
    16.         //Get a pointer to the start ptr of the local transforms
    17.         IntPtr _startOfLocalTransforms = (IntPtr) chunk.GetComponentDataPtrRW<LocalTransform>(ref LocalTransformtypeHandle);
    18.  
    19.         //Get the entities
    20.         NativeArraySafetyWrapper<Entity> entities = new NativeArraySafetyWrapper<Entity>(chunk.GetNativeArray(participantEntityHandle), false);
    21.  
    22.         //Get the count of entities
    23.         int count = chunk.Count;
    24.  
    25.         //Iterate over the entities
    26.         for (int i = 0; i < count; i++)
    27.         {
    28.             //Use undocumented InternalCompilerInterface API to get the ref to the local transform
    29.             RefRW<LocalTransform> participantTransform = InternalCompilerInterface.GetRefRW<LocalTransform>(_startOfLocalTransforms, i, ref LocalTransformtypeHandle);
    30.  
    31.             //Add the local transform to the participant transform writer
    32.             participantTransformWriter.TryAdd(entities[i], participantTransform);
    33.         }
    34.     }
    35. }