#WithNone and IEnableable Bug

1 messages · Page 1 of 1 (latest)

dusty magnet
#

Is there any solution to this bug on the horizon? There's been a bit of a discussion with cort on the dots channel but never any confirmed resolution coming soon~ (ideas were very much, 2.0 kind of stuff).
I understand the desire for a proper implementation and reluctance to patch a quick fix, but this is kind of a really nasty bug that has hung around for a year now. Dependency management breaking unexpectedly is extremely painful to track down.
It's caught a lot of users out and even crippled a team at the largest studio my country until I pointed out the problem to a friend who worked there.

(If anyone is unfamiliar what I'm talking about, WithNone does not add a dependency for enable components which breaks safety/dependency management. If anyone is running into this issue you can work around it by manually adding a dependency to the system with something like SystemAPI.GetComponentHandle<T>, use WithDiasbled instead of WithNone or I have a fix for this in a fork here)

prime crest
#

I don't think we have anything further to add beyond what we've already discussed elsewhere. To summarize those conclusions for anyone who finds this thread first: creating a query with WithNone<T> doesn't give us enough information to determine whether a a job running against the query should include a dependency on the T component or not. The suggested workarounds are to prefer the more explicit WithAbsent<T> (which does not insert a dependency on T) or WithDisabled<T> (which does), or to create a dummy ComponentLookkup<T> or ComponentTypeHandle<T> in the system to force a dependency on T for a specific WithNone<T>.

A real fix would probably involve the sort of substantial API-breaking change that we can't make in the 1.x release series.

The most conservative fix we could make in the 1.x timeframe would be for WithNone<T> to always insert a read dependency on T, which would guarantee correctness at the expense of syncing on more jobs than necessary. Would that be acceptable?

prime crest
#

(I see that the "conservative fix" is essentially what you've implemented in your fork, so I assume that's a sign that it's acceptable. On further reflection, not-maximally-efficient-but-correct is vastly preferable to quietly-broken, so that's what we should go with for now.