Duplicated results in query about arch HOT 16 CLOSED

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?

from arch.

        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)

from arch.

        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?

from arch.

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");

from arch.

@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?

from arch.

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

from arch.

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.

from arch.

Changing Base Component1 Component2 to structs produces same output.

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?

from arch.

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>()?

from arch.

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

from arch.

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? ^^

from arch.

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

from arch.

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?

from arch.

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

from arch.

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

from arch.

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

from arch.