Entity creation and add of components while iterating a query about arch HOT 14 CLOSED

First of all, thanks for reporting! :)

Im gonna check this immediately, looks like some "new" bug was introduced.
Could you test the 1.2.4.2-beta? Thats the latest preview which had some improvements and bug fixes meanwhile, i hope that's solved in the latest version :)

Iterating and creating entities should never be a problem. CommandBuffers only make sense for multithreading, that's their main purpose. Or to schedule structural changes ^^

from arch.

First of all, thanks for reporting! :)

Im gonna check this immediately, looks like some "new" bug was introduced. Could you test the 1.2.4.2-beta? Thats the latest preview which had some improvements and bug fixes meanwhile, i hope that's solved in the latest version :)

Iterating and creating entities should never be a problem. CommandBuffers only make sense for multithreading, that's their main purpose. Or to schedule structural changes ^^

I will get back to you on this with a repro if I can. Thank you !

from arch.

I will get back to you on this with a repro if I can. Thank you !

Perfect, but for now just test if the 1.2.4.2-beta produces the same error ^^ Would be great if you could tell me later :D

Yes this exists on 1.2.4.2-beta :(

It's really strange . That's what I have

public partial class SkySystem : BaseSystem<World, float>, IRenderSystem
{
    public SkySystem(World world) : base(world)
    {
    }

    public override void Initialize()
    {
        base.Initialize();
        InitializeSky();
    }

   private unsafe void InitializeSky()
    {
        Entity entity = World.Create(new Name { Value = "SkySystemData" });

        SkyData skyData = default;
        Sun sun = default;

        // some initialization code is skipped
        skyData.Vbh = World.CreateVertexBuffer(new nint(vertices), sizeof(ScreenPosVertex) * vertical_count * horizontal_count, screenPosVertex);
        skyData.Ibh = World.CreateIndexBuffer(new nint(indices), sizeof(ushort) * k);

        skyData.Time = 17.0f;
        skyData.TimeScale = 0.0f;

        skyData.USunLuminance = World.CreateUniform("u_sunLuminance", UniformType.Vec4);
        skyData.USkyLuminanceXYZ = World.CreateUniform("u_skyLuminanceXYZ", UniformType.Vec4);
        skyData.USkyLuminance = World.CreateUniform("u_skyLuminance", UniformType.Vec4);
        skyData.USunDirection = World.CreateUniform("u_sunDirection", UniformType.Vec4);
        skyData.UParameters = World.CreateUniform("u_parameters", UniformType.Vec4);
        skyData.UPerezCoeff = World.CreateUniform("u_perezCoeff", UniformType.Vec4, 5);

        skyData.Turbidity = 2.15f;
        skyData.SkyProgram = World.CreateProgram("vs_sky", "fs_sky", nameof(skyData.SkyProgram)).GetAwaiter().GetResult();

        entity.Add(skyData); // sky data is not getting assigned
        entity.Add(sun);
    }

But If I do

        var entity = World.Create(skyData);
        entity.Add(sun);

This would work and final entity will have both SkyData and Sun. I might go step debugger to see what's the problem with it. Can we close this ticket as it might be misleading and if I have more steps to reproduce the bug I will make a new one ?

from arch.

Hmmm dammit, than its an undiscovered bug. Is your project open source? I could help with debugging to speed up things.

Since each .Add operation triggers a structural change which results in the copying an entity from one to another archetype, i would assume that for some reason it just picks the wrong archetype for SkyData. And therefore the entity does not have it... still weird since it works when the action is combined.

Do you register components yourself somewhere with ComponentRegistry.Add() ?

Yes we can close this for now, In the meantime i will add some tests to check some stuff.

It's funny you ask, it will be a C# port of https://github.com/clibequilibrium/EquilibriumEngine I can open source it but it has a backend involved so you might need to sign up to http://login.playequilibrium.cloud/, unless I can create a no login branch for you. Your call 😁

Do you register components yourself somewhere with ComponentRegistry.Add() ?

No not really but my structs look like that

public struct Sun
{
    public Vector3 NorthDir;
    public Vector3 SunDir;
    public Vector3 UpDir;
    public float Latitude;
    public Month Month; // enum

    public float EclipticObliquity;
    public float Delta;
}

public struct SkyData
{
    // public VertexLayout ScreenPosVertex;

    public VertexBufferHandle Vbh;
    public IndexBufferHandle Ibh;
    public ProgramHandle SkyProgram;

    public UniformHandle USunLuminance;
    public UniformHandle USkyLuminanceXYZ;
    public UniformHandle USkyLuminance;
    public UniformHandle USunDirection;
    public UniformHandle UParameters;
    public UniformHandle UPerezCoeff;

    public float Time;
    public float TimeScale;
    public float Turbidity;
}

I commented vertex layout because it had some fixed byte buffers (maybe it would cause issues)
The handle structs are looking like this

	public struct ProgramHandle {
	    public ushort idx;
	    public bool Valid => idx != UInt16.MaxValue;
	}

So nothing out of the ordinary, no managed stuff . I don't want to take your time on this, also maybe it will help if you notice, first I create an entity with a managed component Name that has a string value. I will keep investigating and I find a root cause I will definetely submit a PR. thank you for your time ! @genaray

from arch.

It's funny you ask, it will be a C# port of https://github.com/clibequilibrium/EquilibriumEngine

That sounds pretty cool actually! :)

I commented vertex layout because it had some fixed byte buffers (maybe it would cause issues)
Hmm... however, it should not result into such a behaviour. The struct size should not conflict with the entity structure in general :/

Alright thanks! ^^ This should actually be "easy" to debug. Just pull the sources into your project, set a breakpoint in the .Add method and see what exactly happens in there. My guess is that its probably a iusse with the bitset calculation which results in the wrong archetype. That during .Add the same archetype is being picked as it is already in or something similar.

from arch.

@genaray I think there is a problem with the hashing function at CompileTimeStatics.cs

I used the FNV1A32 from Burst Unity Compiler and not having the issue again. I think hash collision occurs and it returns the same archetype.

   [MethodImpl(MethodImplOptions.AggressiveInlining)]
    public static int GetHashCode(Span<ComponentType> obj)
    {
        // From https://stackoverflow.com/a/52172541.
        unchecked
        {
            int hash = 0;
            foreach (var type in obj)
            {
                hash += GetHashCode32(type.Type);
            }

            return hash;
        }
    }
    

        /// <summary>
        /// Gets a 32-bits hashcode from a type computed for the <see cref="System.Type.AssemblyQualifiedName"/>
        /// This method cannot be used from a burst job.
        /// </summary>
        /// <param name="type">The type to compute the hash from</param>
        /// <returns>The 32-bit hashcode.</returns>
        public static int GetHashCode32(Type type)
        {
            return HashStringWithFNV1A32(type.AssemblyQualifiedName);
        }


        // method internal as it is used by the compiler directly
        internal static int HashStringWithFNV1A32(string text)
        {
            // Using http://www.isthe.com/chongo/tech/comp/fnv/index.html#FNV-1a
            // with basis and prime:
            const uint offsetBasis = 2166136261;
            const uint prime = 16777619;

            uint result = offsetBasis;
            foreach (var c in text)
            {
                result = prime * (result ^ (byte)(c & 255));
                result = prime * (result ^ (byte)(c >> 8));
            }
            return (int)result;
        }

Now that explains (another issue) why when I add a

   [Query]
   [All<SkyData, Sun>]
   private void DrawSky(ref SkyData skyData, ref Sun sun)

Some of my entities also got deleted maybe due to same hash collision? If I remove the callback the entities with total unrelated component ChatReceivedMessage are kept. Do you think it would be possible for you to review the hashing functions you have for components, queries , entities ? The ones from stackoverflow are fishy :D

In this scope it would be

    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    public static int GetHashCode(Span<uint> span)
    
    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    public static int GetHashCode(Span<ComponentType> obj) 

from arch.

Thanks a lot! :)

Im gonna review those hash functions and will pick something else instead. Couldnt we just use FNV? If it works in your project, it will probably also work in the main repo? ^^

However im not sure about its speed, probably a bit slower.

Ill do some research to see what else exists ^^

I think FNV is more for string hashing, so you could do something like

  public static int GetHashCode32(Span<ComponentType> types)
    {
        var typeString = string.Empty;

        foreach (var type in types)
        {
            typeString += type.Type.AssemblyQualifiedName;
        }

        return HashStringWithFNV1A32(typeString);
    }

The speed is okay of this hash function. MurmurHash will be better, but I also think you have Span of uints you need to hash ?

from arch.

I will get back to you on this with a repro if I can. Thank you !

Perfect, but for now just test if the 1.2.4.2-beta produces the same error ^^
Would be great if you could tell me later :D

from arch.

Hmmm dammit, than its an undiscovered bug. Is your project open source? I could help with debugging to speed up things.

Since each .Add operation triggers a structural change which results in the copying an entity from one to another archetype, i would assume that for some reason it just picks the wrong archetype for SkyData. And therefore the entity does not have it... still weird since it works when the action is combined.

Do you register components yourself somewhere with ComponentRegistry.Add() ?

Yes we can close this for now, In the meantime i will add some tests to check some stuff.

from arch.

Got this, I think I am onto something . I might be messing myself up because I do something from a Task...

  // A common mistake, happening in many cases.
        Debug.Assert(source != destination, "From-Archetype is the same as the To-Archetype. Entities cannot move within the same archetype using this function. Probably an attempt was made to attach already existing components to the entity or to remove non-existing ones.");

Do you think it's possible to include pdb symbols in nuget package? So asserts are triggered if user install the library not from sources?

EDIT: mistake #1:. having create, add , followed by set...

  entity = World.Create(authData);
                    entity.Add<AuthData>();
                    entity.Set(authData);

                    if (profile.Models.Count == 1)
                    {
                        entity.Add<ProfileComponent>();
                        entity.Set(new ProfileComponent { Value = profile.Models[0] });

                        entity.Add<Name>();
                        entity.Set<Name>(new Name { Value = profile.Models[0].UserName! });
                    }

from arch.

@genaray okay about my initial issue this is the callstack

from arch.

Thanks a lot! :)

Im gonna review those hash functions and will pick something else instead. Couldnt we just use FNV? If it works in your project, it will probably also work in the main repo? ^^

However im not sure about its speed, probably a bit slower.

Ill do some research to see what else exists ^^

from arch.

I actually think that this is kinda tricky. Researched a bit, Murmurhash is the "way to go" for BitSet hashing atleast.
However i have no clue yet if hashing the List<ComponentType> with it is possible. Since that one is not "sorted".

Possible it makes sense to convert that list to a stackalloc bitset and hash that one. That should still be performant ^^

from arch.

I actually think that this is kinda tricky. Researched a bit, Murmurhash is the "way to go" for BitSet hashing atleast. However i have no clue yet if hashing the List<ComponentType> with it is possible. Since that one is not "sorted".

Possible it makes sense to convert that list to a stackalloc bitset and hash that one. That should still be performant ^^

Hmm, can you ensure that the list of components will be sorted by their type name alphabetically or at least their ids ? So your hash function will always produce same results. Otherwise why the performance of the Add operation matters much ? it's something that happens infrequently 🤔

EDIT: I see get hash code is used if we want to count entities or query is retrieved from the query cache, so it matters after all but still debatable 😁

from arch.