[BUG] EntityManager.SetComponentEnabled(EntityQuery query) doesn't trigger change filter | Official Unity Discord | Page 1

tacit coral Feb 17, 2024, 2:58 AM

#

Noticed this while writing some editor world tooling but, the regular EntityManager.SetComponentEnabled(Entity) (and all other variations of SetComponentEnabled via lookup etc) correctly triggers a change filter because it goes through

ChunkDataUtility.GetEnabledRefRW(ChunkIndex chunk, Archetype* archetype, int indexInTypeArray, uint globalSystemVersion, out int* ptrChunkDisabledCount)```
But as far as I can tell the path that `EntityManager.SetComponentEnabled(EntityQuery)` takes via 
```cs
ChunkIterationUtility.SetEnabledBitsOnAllChunks(ref EntityQueryImpl queryImpl, TypeIndex typeIndex, bool value)```
never calls SetChangeVersion on the chunks with the component
@spring drift

tacit coral Feb 28, 2024, 1:45 AM

#

Reminding myself to make a bug report on the weekend

spring drift Mar 5, 2024, 5:58 PM

#

I somehow didn't see the Discord notification until now. Thanks, I'll file a bug and try to get this fixed for the next 1.2 release

spring drift Mar 12, 2024, 7:01 PM

#

We discussed this internally, and my colleagues were surprised to learn that enabling/disabling a component would bump the component's change version in any code path. Change versions are generally used to track changes in actual values, not entities being added/removed/reordered, so having enable/disable increment the change version seems like it would trigger false positives on change-tracking systems; if anything this sounds more like a use case for the per-component Order Version than the Change Version.

We agree that we should be consistent between the single-entity and the query variants either way though, but I'm curious what your thoughts are long-term about which version should be updated when a component is enabled/disabled?

#

We're also curious, is the current inconsistency causing an active problem in your project, or are you just helpfully flagging a likely-unintended inconsistency in the API?

tacit coral Mar 12, 2024, 10:05 PM

#

We're also curious, is the current inconsistency causing an active problem in your project, or are you just helpfully flagging a likely-unintended inconsistency in the API?
Well I noticed this because I ran into this bug when writing an Editor system that was just doing a mass reset for me and my change filters weren't triggering - so kind of yes because I ran into it while solving a problem. However I ended up doing this another (better) way anyway so it's not actively causing me problems

#

We agree that we should be consistent between the single-entity and the query variants either way though, but I'm curious what your thoughts are long-term about which version should be updated when a component is enabled/disabled?
well i guess let me give you an example

#

        [BurstCompile]
        [WithChangeFilter(typeof(TestEnableable))]
        private partial struct TestJob : IJobEntity
        {
            public PhysicsWorld World;

            private void Execute(in TestEnableable test, in LocalTransform lt, ref TestResult result)
            {
                var ray = new RaycastInput { Start = lt.Position, End = lt.Position + test.Displacement, Filter = CollisionFilter.Default };
                result.Value = World.CastRay(ray, out _);
            }
        }

        public struct TestEnableable : IComponentData, IEnableableComponent
        {
            public float3 Displacement;
        }

        public struct TestResult : IComponentData
        {
            public bool Value;
        }```

#

expectation is is that anytime TestEnableable changes (within rules of changefilter of course), it should do a raycast

#

as a user if I toggle TestEnableable on without changing the value, would I expect it to trigger this job once?

#

I kind of think yes?

#

Now I personally i don't use IEnableableComponent on anything but tag components so I'm never going to run into this case, it was more just a thought process of what another user would expect.
i'd be quite happy to switch over to order filtering (assuming you actually provided support for IJobEntity and order filtering without a custom query as it's currently missing)

kind minnow Mar 13, 2024, 1:48 PM

#

spring drift We discussed this internally, and my colleagues were surprised to learn that ena...

I'd rather have it trigger change version, because otherwise there's no way to run the same logic for enabled bits

#

and mostly enabled components are split into 0 fields anyway

#

(but that's due to other bugs)

spring drift Mar 13, 2024, 10:50 PM

#

kind minnow and mostly enabled components are split into 0 fields anyway

By "split into zero fields", do you mean that they're typically zero-size tag components?

#

And which other bugs, out of curiosity?

kind minnow Mar 14, 2024, 5:19 AM

#

spring drift By "split into zero fields", do you mean that they're typically zero-size tag co...

Yes

kind minnow Mar 14, 2024, 5:20 AM

#

spring drift And which other bugs, out of curiosity?

Ije not supporting both ref and enabledref

tacit coral Mar 14, 2024, 5:39 AM

#

kind minnow Ije not supporting both ref and enabledref

for the most part it works fine, they fixed it a while ago

kind minnow Mar 14, 2024, 5:40 AM

#

There are still some cases

tacit coral Mar 14, 2024, 5:40 AM

#

yeah withdisable breaks it and there are other bugs

#

#1212178806769459211 message

kind minnow Mar 14, 2024, 5:40 AM

#

But I keep doing split just out of personal convenience at this point

tacit coral Mar 14, 2024, 5:41 AM

#

but yeah i separate them as well, i conceptually like enable components without data more

#

apart from bugs, avoids change filter mishaps

kind minnow Mar 14, 2024, 5:41 AM

#

Actually would like them to fully split enablable into their own component type that may not have fields

#

So it'll be glorified boolean practically

#

Easier to manage

tacit coral Mar 14, 2024, 5:42 AM

#

you can do that already? why place restrictions on others

kind minnow Mar 14, 2024, 5:42 AM

#

Because it's just makes things much more clear

#

There are far too many "little details" about dots already

#

And most such details about enablable state can be solved by just simplifying architecture

#

But I know it's too late at this point

#

(or maybe 2.0 😅)

tacit coral Mar 14, 2024, 5:45 AM

#

kind minnow (or maybe 2.0 😅)

might more likely force every component to be enabled by default 😄

kind minnow Mar 14, 2024, 5:45 AM

#

notlikethis

#

How often you mix data with enabled state anyway these days?

tacit coral Mar 14, 2024, 5:47 AM

#

I stopped doing it long ago because of the bugs

#

but i treat enable components a bit weird these days

#

so i'm not sure i'm a good use case here

kind minnow Mar 14, 2024, 5:48 AM

#

Don't you find it just more convenient to split?

tacit coral Mar 14, 2024, 5:54 AM

#

i mostly use them as triggers

#

most of the time have pairs of them

#

Active + ActivePrevious

#

    public struct CopyEnableable<TTo, TFrom>
        where TTo : unmanaged, IComponentData, IEnableableComponent
        where TFrom : unmanaged, IComponentData, IEnableableComponent
    {
        private EntityQuery query;
        private ComponentTypeHandle<TTo> toHandle;
        private ComponentTypeHandle<TFrom> fromHandle;

        public void OnCreate(ref SystemState state)
        {
            this.query = new EntityQueryBuilder(Allocator.Temp)
                .WithAllRW<TTo>()
                .WithAll<TFrom>()
                .WithOptions(EntityQueryOptions.IgnoreComponentEnabledState)
                .Build(ref state);

            this.query.AddChangedVersionFilter(ComponentType.ReadOnly<TFrom>());

            this.toHandle = state.GetComponentTypeHandle<TTo>();
            this.fromHandle = state.GetComponentTypeHandle<TFrom>(true);
        }

        public void OnUpdate(ref SystemState state, SetPreviousJob job = default)
        {
            this.toHandle.Update(ref state);
            this.fromHandle.Update(ref state);

            job.ToHandle = this.toHandle;
            job.FromHandle = this.fromHandle;

            state.Dependency = job.ScheduleParallel(this.query, state.Dependency);
        }

        [BurstCompile]
        public struct SetPreviousJob : IJobChunk
        {
            public ComponentTypeHandle<TTo> ToHandle;

            [ReadOnly]
            public ComponentTypeHandle<TFrom> FromHandle;

            public void Execute(in ArchetypeChunk chunk, int unfilteredChunkIndex, bool useEnabledMask, in v128 chunkEnabledMask)
            {
                chunk.CopyEnableMaskFrom(ref this.ToHandle, ref this.FromHandle);
            }
        }
    }```

#

I even have util to copy the enable state from 1 component type to another

#

actually Cort this ^ is one of the things I use ChangeFilters on enable components for if you were interested

#

i have a bunch of extensions for enable components, this just being 1 of many

📎 message.txt

tacit coral Mar 14, 2024, 6:00 AM

#

tacit coral but i treat enable components a bit weird these days

^ but yeah this is why i don't think i should necessarily be the base case for this as i doubt anyone else has a job like this

minor pagoda Mar 14, 2024, 3:02 PM

#

tacit coral as a user if I toggle TestEnableable on without changing the value, would I expe...

you would also expect it to run if you added the component instead of enabling it, I imagine?

tacit coral Mar 14, 2024, 4:53 PM

#

minor pagoda you would also expect it to run if you added the component instead of enabling i...

I would and actually that's a good point, that does trigger a change filter

minor pagoda Mar 14, 2024, 7:12 PM

#

tacit coral I would and actually that's a good point, that does trigger a change filter

even if you only add and not write explitly, I imagine?

tacit coral Mar 14, 2024, 7:14 PM

#

Yes

vernal gust Mar 19, 2024, 1:02 PM

#

just ran into this problem. so yeah, i don't think it's a niche behaviour. my use case here is a destruction pipeline that manages an enabled comp DestroyEntity. usually this is handled individually at runtime and using a change filter. (though it could be argued the change filter is not really needed) when unloading a subscene i mass flag every entity which doesn't trigger a change so the cleanup job doesn't run.

tacit coral Mar 19, 2024, 6:08 PM

#

Oh yeah forgot to warn you when I posted that code, glad you had probably already seen this

vernal gust Mar 19, 2024, 8:12 PM

#

i ran into it the bug, saw the change filter and it clicked that i read about it some time ago 😄

#[BUG] EntityManager.SetComponentEnabled(EntityQuery query) doesn't trigger change filter