Duplicated results in query

[k-u-s]

I found an issue with query results. I get duplicated results during single query

To get some context I have 2 classes where BaseEntity class has property with EntityReference to capture whats going on I first iterated over query and stored all results in list then check content.

        class BaseEntity { Arch.Core.EntityReference Id { get; init; } }

        _queryDesc = new Arch.Core.QueryDescription()
            .WithAll<BaseEntity, TComponent>();   // TComponent stores few int

        var queryRes = new List<(Arch.Core.EntityReference, BaseEntity, TComponent)>();
        var query = World.Query(in _queryDesc);
        foreach (ref var chunk in query)
        foreach (var index in chunk)
        {
            ref readonly var e = ref chunk.Entity(index);
            if(!e.IsAlive())
                continue;
            
            if (!e.Has<BaseEntity, TComponent>())
                continue;

            var refs = e.Get<BaseEntity, TComponent>();
            queryRes.Add((e.Reference(), refs.t0.Value, refs.t1.Value));
        }

        foreach (var (entityReference, entity, component) in queryRes)
        {
            if(entityReference.Entity.Id != entity.Entity.Id)
                continue;
            
            var c = component;
            // Do smth...
        }

Sometimes query returns same BaseEntity 2 times. For example given i had BaseEntity with id 4 it was returned once by query for entity with id 4 as expected and second time by entity with id 41. Both entity reference has IsAlive flag set to true

I am aware that attaching/detaching components from entity can cause reference issues as described in #74 but in this situation during query no operation are performed its only used to store them in local variable.

I would expect that condition in 2nd foreach should never be triggered but it is.

I'll try to create reproducible example but if you have any tip what is source of such behavior it would be much appreciated

[genaray]

A reproducible example would be great, otherwise this sounds like a reference issue. I see you are using a class "BaseEntity", correct? What happens if you make it a struct? And how do you create your entities? With classes it can quickly happen that you accidently set another reference to it instead of creating a new one e.g. ^^

And what happens if you use : World.Query(in desc, (in Entit en, ref BaseEntity be, ref TComponent c) => {} instead?

[k-u-s]

        var lambdaRes = new List<(EntityReference, BaseEntity, TComponent)>();
        World.Query(in _queryDesc, (in Entity e, ref BaseEntity b, ref TComponent c) =>
        {
            lambdaRes.Add((e.Reference(), b, c));
        });
        

added above code before foreach and it seams that both methods produces same results (same items in respective lists)

[genaray]

        var lambdaRes = new List<(EntityReference, BaseEntity, TComponent)>();
        World.Query(in _queryDesc, (in Entity e, ref BaseEntity b, ref TComponent c) =>
        {
            lambdaRes.Add((e.Reference(), b, c));
        });
        

added above code before foreach and it seams that both methods produces same results (same items in respective lists)

Alright, have you tried to use a struct instead of a class?
And can you show how you create those entities?

[k-u-s]

using Arch.Core.CommandBuffer;
using Arch.Core.Utils;
using Arch.Core;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using Arch.Core.Extensions;

namespace ConsoleTestApp;

internal class ArchDuplicatedResults
{
    private static int _index;
    private static World _world;
    private static QueryDescription _queryDesc;
    private static QueryDescription _queryDesc1;
    private static QueryDescription _queryDesc2;
    private static List<All> _list;

    public static void Execute()
    {
        _world = World.Create();
        _queryDesc = new QueryDescription().WithAll<Base, Component1>();
        _queryDesc1 = new QueryDescription().WithAll<Component2>();
        _queryDesc2 = new QueryDescription().WithAll<Component3>();

        _list = new List<All>();

        for (var i = 0; i < 1_000; i++)
            RunIteration();

    }

    private static void RunIteration()
    {
        for (var i = 0; i < 10; i++)
            _list.Add(SpawnEntity());

        var res1 = CheckQuery();
        if (res1 != _list.Count)
            Console.WriteLine("hmm1");

        for (var i = 0; i < _list.Count; i += 15)
            _list[i].Base.Ref.Entity.Remove<Component2>();

        var res2 = CheckQuery();
        if (res2 != _list.Count)
            Console.WriteLine("hmm2");

        for (var i = _list.Count - 1; i >= 0; i -= 10)
        {
            _world.Destroy(_list[i].Base.Ref.Entity);
            _list.RemoveAt(i);
        }

        var res3 = CheckQuery();
        if (res3 != _list.Count)
            Console.WriteLine("hmm3");

        for (var i = 1; i < _list.Count; i += 7)
        {
            var cur = _list[i];
            var c3 = new Component3() { Val = cur.Component1.Val };
            cur.Component3 = c3;
            _world.Add(cur.Base.Ref.Entity, in c3);
        }

        var res4 = CheckQuery();
        if (res4 != _list.Count)
            Console.WriteLine("hmm4");

        for (var i = 0; i < 10; i++)
            _list.Add(SpawnEntity());

        var res5 = CheckQuery();
        if (res5 != _list.Count)
            Console.WriteLine("hmm5");

    }

    private static int CheckQuery()
    {
        var queryRes = new List<(Arch.Core.EntityReference, Base, Component1)>();
        var query = _world.Query(in _queryDesc);
        foreach (ref var chunk in query)
        foreach (var index in chunk)
        {
            ref readonly var e = ref chunk.Entity(index);
            if (!e.IsAlive())
                continue;

            if (!e.Has<Base, Component1>())
                continue;

            var refs = e.Get<Base, Component1>();
            queryRes.Add((e.Reference(), refs.t0, refs.t1));
        }

        foreach (var (entityReference, entity, component) in queryRes)
        {
            if (entityReference.Entity.Id != entity.Ref.Entity.Id)
            {
                Console.WriteLine("Hmm - i do not think it should be possible");
                continue;
            }

            
        }

        return queryRes.Count;
    }

    private static All SpawnEntity()
    {
        var e = _world.Create();
        var entityRef = e.Reference();
        var index = ++_index;
        var b = new Base() { Ref = entityRef };
        var c1 = new Component1() { Val = index };
        var c2 = new Component2() { Val = index.ToString() };
        _world.Add(e, in b);
        _world.Add(e, in c1);
        _world.Add(e, in c2);

        return new All
        {
            Base = b,
            Component1 = c1,
            Component2 = c2
        };
    }

    class All
    {
        public Base Base { get; init; }
        public Component1 Component1 { get; set; }
        public Component2 Component2 { get; set; }
        public Component3 Component3 { get; set; }
    }

    class Base
    {
        public EntityReference Ref { get; init; }
    }

    class Component1
    {
        public int Val { get; set; }
    }

    class Component2
    {
        public string Val { get; set; }
    }

    class Component3
    {
        public double Val { get; set; }
    }
}

Above example in Execute() during 2nd invocation of RunIteration() method prints Console.WriteLine("Hmm - i do not think it should be possible");

[genaray]

@k-u-s Thanks, i'll look at that :) Have you tried to use structs instead? Or to create the entity as one piece instead : world.Create(first, second, thirdComponent); ? Is it possible to make the sample a bit more smaller and more understandable?

[k-u-s]

Not yet, I wanted first to reproduce it. I prefer to find cause of it and fix rather then create workaround since I do not have any strict schedule to follow.

But now when i am able to reproduce it in quickly i can check it

[genaray]

Not yet, I wanted first to reproduce it. I prefer to find cause of it and fix rather then create workaround since I do not have any strict schedule to follow.

But now when i am able to reproduce it in quickly i can check it

Its just to narrow the cause, e.g. when it works with structs i can safely assume that its a class related problem ^^
Would be great if you could narrow it down a bit more and reduce the sample to the minimal.

[k-u-s]

Changing Base Component1 Component2 to structs produces same output.

Current example

using Arch.Core.CommandBuffer;
using Arch.Core.Utils;
using Arch.Core;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using Arch.Core.Extensions;

namespace ConsoleTestApp;

internal class ArchDuplicatedResults
{
    private static int _index;
    private static World _world;
    private static QueryDescription _queryDesc;
    private static QueryDescription _queryDesc1;
    private static List<All> _list;

    public static void Execute()
    {
        _world = World.Create();
        _queryDesc = new QueryDescription().WithAll<Base, Component1>();
        _queryDesc1 = new QueryDescription().WithAll<Base, Component2>();

        _list = new List<All>();

        for (var i = 0; i < 1_000; i++)
            RunIteration();

    }

    private static void RunIteration()
    {
        for (var i = 0; i < 10; i++)
            _list.Add(SpawnEntity());

        var res1 = CheckQuery();
        if (res1 != _list.Count)
            Console.WriteLine("hmm1");

        for (var i = 0; i < _list.Count; i += 15)
            _list[i].Base.Ref.Entity.Remove<Component2>();

        var res2 = CheckQuery();
        if (res2 != _list.Count)
            Console.WriteLine("hmm2");

        for (var i = _list.Count - 1; i >= 0; i -= 10)
        {
            _world.Destroy(_list[i].Base.Ref.Entity);
            _list.RemoveAt(i);
        }

        var res3 = CheckQuery();
        if (res3 != _list.Count)
            Console.WriteLine("hmm3");

        for (var i = 0; i < 10; i++)
            _list.Add(SpawnEntity());

        var res4 = CheckQuery();
        if (res4 != _list.Count)
            Console.WriteLine("hmm5");

    }

    private static int CheckQuery()
    {
        var queryRes = new List<(Arch.Core.EntityReference, Base, Component1)>();
        var query = _world.Query(in _queryDesc);
        foreach (ref var chunk in query)
        foreach (var index in chunk)
        {
            ref readonly var e = ref chunk.Entity(index);
            if (!e.IsAlive())
                continue;

            if (!e.Has<Base, Component1>())
                continue;

            var refs = e.Get<Base, Component1>();
            queryRes.Add((e.Reference(), refs.t0, refs.t1));
        }

        foreach (var (entityReference, entity, component) in queryRes)
        {
            if (entityReference.Entity.Id != entity.Ref.Entity.Id)
            {
                Console.WriteLine("Hmm - i do not think it should be possible");
                continue;
            }

            
        }

        return queryRes.Count;
    }

    private static All SpawnEntity()
    {
        var e = _world.Create();
        var entityRef = e.Reference();
        var index = ++_index;
        var b = new Base() { Ref = entityRef };
        var c1 = new Component1() { Val = index };
        var c2 = new Component2() { Val = index.ToString() };
        _world.Add(e, in b, in c1, in c2);

        return new All
        {
            Base = b,
            Component1 = c1,
            Component2 = c2
        };
    }

    class All
    {
        public Base Base { get; init; }
        public Component1 Component1 { get; set; }
        public Component2 Component2 { get; set; }
    }

    struct Base
    {
        public EntityReference Ref { get; init; }
    }

    struct Component1
    {
        public int Val { get; set; }
    }

    struct Component2
    {
        public string Val { get; set; }
    }
}

I think its related _list[i].Base.Ref.Entity.Remove<Component2>(); since when it is removed problem in example seams to not happen

Is it save to call Remove() with type that is not attached?

[genaray]

Is it save to call Remove() with type that is not attached?

Hmmm good question. Actually not, I cant tell you rn how it exactly behaves tho.
Normally each operation should be guarded by a has statement if you are not sure if the entity has the component you operate on. Will it work when you do : if(.... .Has<Component2>()) .... .Remove<Component2>()?

[k-u-s]

In current example it seams to resolve problem. Shouldnt it be covered be either throwing ex or doing nothing?

But it does not seam to be the case that im encountering. Ill investigate further

[genaray]

In current example it seams to resolve problem. Shouldnt it be covered be either throwing ex or doing nothing?

But it does not seam to be the case that im encountering. Ill investigate further

So a .Has<T> before the .Remove<> solves the problem?
But at the same time that's not the issue you are encountering? Could you clarify a bit? ^^

[k-u-s]

It resolve problem in sample that I provided here not in app that made me create this issue

[genaray]

It resolve problem in sample that I provided here not in app that made me create this issue

Ah i see. And if you add .Has<> to your app aswell?

[k-u-s]

Yeah in fact in my app Has<>() never returned false but I added it anyway just in case

[k-u-s]

OK it looks like it in my case it was done cause of 2nd call .... Add<>() when entity already had such component.

I assume that no check for Remove and Add is done cause of perf, I think it would be good to have some kind of safe guard mechanism but not sure if that fits current model.

Any way adding comment like "Removing non existing component may cause undefined behavior if not sure use Has<> method before ..." / "Adding component that is already added may cause undefined behavior, To replace component use Set<> method" to method summary would be good

[genaray]

OK it looks like it in my case it was done cause of 2nd call .... Add<>() when entity already had such component.

I assume that no check for Remove and Add is done cause of perf, I think it would be good to have some kind of safe guard mechanism but not sure if that fits current model.

Any way adding comment like "Removing non existing component may cause undefined behavior if not sure use Has<> method before ..." / "Adding component that is already added may cause undefined behavior, To replace component use Set<> method" to method summary would be good

Alright glad you found the issue ^^
We could include this in #110