bevy/crates/bevy_ecs/src/entity
Nuutti Kotivuori 76d610d465
Flush commands after every mutation in WorldEntityMut (#16219)
# Objective

- Currently adding observers spawns an entity which implicitly flushes
the command queue, which can cause undefined behaviour if the
`WorldEntityMut` is used after this
- The reason `WorldEntityMut` attempted to (unsuccessfully) avoid
flushing commands until finished was that such commands may move or
despawn the entity being referenced, invalidating the cached location.
- With the introduction of hooks and observers, this isn't sensible
anymore as running the commands generated by hooks immediately is
required to maintain correct ordering of operations and to not expose
the world in an inconsistent state
- Objective is to make command flushing deterministic and fix the
related issues
- Fixes #16212
- Fixes #14621 
- Fixes #16034

## Solution

- Allow `WorldEntityMut` to exist even when it refers to a despawned
entity by allowing `EntityLocation` to be marked invalid
- Add checks to all methods to panic if trying to access a despawned
entity
- Flush command queue after every operation that might trigger hooks or
observers
- Update entity location always after flushing command queue

## Testing

- Added test cases for currently broken behaviour
- Added test cases that flushes happen in all operations
- Added test cases to ensure hooks and commands are run exactly in
correct order when nested

---

Todo:

- [x] Write migration guide
- [x] Add tests that using `EntityWorldMut` on a despawned entity panics
- [x] Add tests that commands are flushed after every operation that is
supposed to flush them
- [x] Add tests that hooks, observers and their spawned commands are run
in the correct order when nested

---

## Migration Guide

Previously `EntityWorldMut` triggered command queue flushes in
unpredictable places, which could interfere with hooks and observers.
Now the command queue is flushed always immediately after any call in
`EntityWorldMut` that spawns or despawns an entity, or adds, removes or
replaces a component. This means hooks and observers will run their
commands in the correct order.

As a side effect, there is a possibility that a hook or observer could
despawn the entity that is being referred to by `EntityWorldMut`. This
could already currently happen if an observer was added while keeping an
`EntityWorldMut` referece and would cause unsound behaviour. If the
entity has been despawned, calling any methods which require the entity
location will panic. This matches the behaviour that `Commands` will
panic if called on an already despawned entity. In the extremely rare
case where taking a new `EntityWorldMut` reference or otherwise
restructuring the code so that this case does not happen is not
possible, there's a new `is_despawned` method that can be used to check
if the referred entity has been despawned.
2024-12-05 20:30:12 +00:00
..
clone_entities.rs Add Immutable Component Support (#16372) 2024-12-05 14:27:48 +00:00
hash.rs Add core and alloc over std Lints (#15281) 2024-09-27 00:59:59 +00:00
map_entities.rs Change ReflectMapEntities to operate on components before insertion (#15422) 2024-10-01 18:34:09 +00:00
mod.rs Flush commands after every mutation in WorldEntityMut (#16219) 2024-12-05 20:30:12 +00:00
visit_entities.rs Add VisitEntities for generic and reflectable Entity iteration (#15425) 2024-09-30 17:32:03 +00:00