bevyengine / bevy

A refreshingly simple data-driven game engine built in Rust
https://bevyengine.org
Apache License 2.0
36.75k stars 3.62k forks source link

Separate Changed<T> and Added<T>, readding `Mutated` query filter #15070

Open mdeering24 opened 2 months ago

mdeering24 commented 2 months ago

What problem does this solve or what need does it fill?

The confusion and added complexity of having a Query filter performing multiple filters.
Currently the Changed<T> filter is set up to "only retains results the first time after they have been added OR mutably dereferenced." If I am querying for Changed components I want to only know when they are changed not added.

What solution would you like?

I propose removing the "first time after they have been added" condition on the Changed filter. If users wanted first time added then they can use the Added<T> filter, or if they want to keep the keep both conditions they can use Or<(Changed<T>, Added<T>)>

What alternative(s) have you considered?

The only other work around to this problem is to use Ref<T> to then filter out components that were just changed. But this in an unnecessary burden to have to put on users. It seems silly to have to do the below amount of work to get only changed entities.

pub fn simplified_example(q_maybe_changed: Query<(Entity, Ref<MyComponent)>) {
    let changed: Vec<Entity> = q_entities
        .iter()
        .filter_map(|(e, r)| if r.is_changed() { Some(e) } else { None })
        .collect();
   if changed.is_empty() {
     return;
  }

  // now do what I want with changed only entities
}
alice-i-cecile commented 2 months ago

This used to exist, under the name Mutated. It was generally an unhelpful footgun, and re-implementing it when we refactored change detection was nontrivial.

mdeering24 commented 2 months ago

I'm confused how this would be considered unhelpful. Something being Added and something changing/mutated are two very different events. Assuming every Bevy user is going to want Added and mutated events to be in one filter isn't right.

ItsDoot commented 2 months ago

This would be helpful for when "on added" logic is handled in a hook/observer and "on mutated" logic is handled in a system.

alice-i-cecile commented 2 months ago

Yep, with the presence of observers and hooks, this makes more sense again. I would be in support of a PR adding Mutated again, as long as the documentation is very clear that Changed is often going to be preferred.

mdeering24 commented 2 months ago

While adding 'Mutated' is sufficient. I am curious for the reason why Changed means a component mutated or was added?

Y'all seem determined to keep it that way despite it going against the definition of "changed" and the fact that 'Added' is its own filter that excludes mutation means you realize mutated and added are not the same. So maybe I'm missing some context.

alice-i-cecile commented 2 months ago

In most use cases, Changed filters are used for keeping state in sync. When A is changed, update B to match. But simply looking at mutation events misses an important edge case: when A is spawned in, B should also match it. Historically (back in Bevy 0.3), Bevy users would reach for Mutated by default for these patterns resulting in surprising bugs, and the overwhelming majority of Bevy's internal uses wanted "added or changed" behavior.