track_change_detection: Also track spawns/despawns #16047

SpecificProtagonist · 2024-10-21T21:49:03Z

Objective

Expand track_change_detection feature to also track entity spawns and despawns. Use this to create better error messages.

Solution

Adds Entities::entity_get_spawned_or_despawned_by as well as {all entity reference types}::spawned_by.

This also removes the deprecated get_many_entities_mut & co (and therefore can't land in 0.15) because we don't yet have no Polonius.

Testing

Added a test that checks that the locations get updated and these updates are ordered correctly vs hooks & observers.

Showcase

Access location:

let mut world = World::new();
let entity = world.spawn_empty().id();
println!("spawned by: {}", world.entity(entity).spawned_by());

spawned by: src/main.rs:5:24

Error message (with track_change_detection):

world.despawn(entity);
world.entity(entity);

thread 'main' panicked at src/main.rs:11:11:
Entity 0v1#4294967296 was despawned by src/main.rs:10:11

and without:

thread 'main' panicked at src/main.rs:11:11:
Entity 0v1#4294967296 does not exist (enable `track_change_detection` feature for more details)

Similar error messages now also exists for Query::get, World::entity_mut, EntityCommands creation and everything that causes B0003, e.g.

error[B0003]: Could not insert a bundle (of type `MaterialMeshBundle<StandardMaterial>`) for entity Entity { index: 7, generation: 1 }, which was despawned by src/main.rs:10:11. See: https://bevyengine.org/learn/errors/#b0003

crates/bevy_ecs/Cargo.toml

aevyrie · 2024-10-21T22:13:59Z

crates/bevy_ecs/src/query/error.rs

+            Self::NoSuchEntity(entity, world) => {
+                #[cfg(feature = "track_change_detection")]
+                {
+                    if let Some(location) = world.entities().get_entity_spawned_despawned_by(entity)


This is really cool.

bushrat011899

Really nice change, I think this philosophy should be extended to more areas of Bevy too (e.g., Components). However, there is a safety comment which I believe this change violates. Either the PR needs to be amended to satisfy the invariant, or a SAFETY comment should be added to the field to indicate why it is safe.

crates/bevy_ecs/src/entity/mod.rs

bushrat011899 · 2024-10-24T21:34:33Z

crates/bevy_ecs/src/query/error.rs

+                }
+                #[cfg(not(feature = "track_change_detection"))]
+                {
+                    write!(f, "The entity {entity} does not exist")


Might be nice to include a hint here to enable track_change_detection for a more detailed error message?

Great idea; implemented. Should I note in the feature description (in Cargo.toml) that this feature has a perf impact so people don't ship with it on?

Yeah that's a good idea. And like the tracing filter level features, we should include a note saying library authors should not enable this feature on their published crates, as it removes the ability for end-users to choose themselves.

crates/bevy_ecs/src/world/entity_ref.rs

crates/bevy_ecs/src/world/unsafe_world_cell.rs

crates/bevy_ecs/src/world/entity_ref.rs

# Objective `flush_and_reserve_invalid_assuming_no_entities` was made for the old rendering world (which was reset every frame) and is usused since the 0.15 retained rendering world, but wasn't removed yet. It is pub, but is undocumented apart from the safety comment. ## Solution Remove `flush_and_reserve_invalid_assuming_no_entities` and the safety invariants this method required for `EntityMeta`, `EntityLocation`, `TableId` and `TableRow`. This reduces the amount of unsafe code & safety invariants and makes #16047 easier. ## Alternatives - Document `flush_and_reserve_invalid_assuming_no_entities` and keep it unchanged - Document `flush_and_reserve_invalid_assuming_no_entities` and change it to be based on `EntityMeta::INVALID` ## Migration Guide - exchange `Entities::flush_and_reserve_invalid_assuming_no_entities` for `reserve` and `flush_as_invalid` and notify us if that's insufficient --------- Co-authored-by: Benjamin Brienen <[email protected]>

bushrat011899

Looks great!

NthTensor

Fantastic. I've marked this as ready for review.

alice-i-cecile · 2024-12-16T23:19:27Z

Really lovely. We definitely need to rename the feature, but that's for follow-up. @SpecificProtagonist once merge conflicts are resolved I'll press the button.

…16846) # Objective - Modify a comment in the shader file to describe what the shader actually does - Fixes bevyengine#16830 ## Solution - Changed the comment. ## Testing - Testing is not relevant to fixing comments (as long as the comment is accurate) --------- Co-authored-by: Freya Pines <[email protected]> Co-authored-by: Freya Pines <[email protected]>

This is the correct rotation type :)

alice-i-cecile · 2024-12-17T04:46:47Z

CI is happy; I'm happy. Let's go: this looks so sweet.

# Objective `flush_and_reserve_invalid_assuming_no_entities` was made for the old rendering world (which was reset every frame) and is usused since the 0.15 retained rendering world, but wasn't removed yet. It is pub, but is undocumented apart from the safety comment. ## Solution Remove `flush_and_reserve_invalid_assuming_no_entities` and the safety invariants this method required for `EntityMeta`, `EntityLocation`, `TableId` and `TableRow`. This reduces the amount of unsafe code & safety invariants and makes bevyengine#16047 easier. ## Alternatives - Document `flush_and_reserve_invalid_assuming_no_entities` and keep it unchanged - Document `flush_and_reserve_invalid_assuming_no_entities` and change it to be based on `EntityMeta::INVALID` ## Migration Guide - exchange `Entities::flush_and_reserve_invalid_assuming_no_entities` for `reserve` and `flush_as_invalid` and notify us if that's insufficient --------- Co-authored-by: Benjamin Brienen <[email protected]>

# Objective Expand `track_change_detection` feature to also track entity spawns and despawns. Use this to create better error messages. # Solution Adds `Entities::entity_get_spawned_or_despawned_by` as well as `{all entity reference types}::spawned_by`. This also removes the deprecated `get_many_entities_mut` & co (and therefore can't land in 0.15) because we don't yet have no Polonius. ## Testing Added a test that checks that the locations get updated and these updates are ordered correctly vs hooks & observers. --- ## Showcase Access location: ```rust let mut world = World::new(); let entity = world.spawn_empty().id(); println!("spawned by: {}", world.entity(entity).spawned_by()); ``` ``` spawned by: src/main.rs:5:24 ``` Error message (with `track_change_detection`): ```rust world.despawn(entity); world.entity(entity); ``` ``` thread 'main' panicked at src/main.rs:11:11: Entity 0v1#4294967296 was despawned by src/main.rs:10:11 ``` and without: ``` thread 'main' panicked at src/main.rs:11:11: Entity 0v1#4294967296 does not exist (enable `track_change_detection` feature for more details) ``` Similar error messages now also exists for `Query::get`, `World::entity_mut`, `EntityCommands` creation and everything that causes `B0003`, e.g. ``` error[B0003]: Could not insert a bundle (of type `MaterialMeshBundle<StandardMaterial>`) for entity Entity { index: 7, generation: 1 }, which was despawned by src/main.rs:10:11. See: https://bevyengine.org/learn/errors/#b0003 ``` --------- Co-authored-by: kurk070ff <[email protected]> Co-authored-by: Freya Pines <[email protected]> Co-authored-by: Freya Pines <[email protected]> Co-authored-by: Matty Weatherley <[email protected]>

alice-i-cecile requested a review from aevyrie October 21, 2024 21:50

alice-i-cecile added the M-Needs-Release-Note Work that should be called out in the blog due to impact label Oct 21, 2024

alice-i-cecile added this to the 0.16 milestone Oct 21, 2024

alice-i-cecile added A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Oct 21, 2024

aevyrie approved these changes Oct 21, 2024

View reviewed changes

aevyrie mentioned this pull request Oct 24, 2024

Add std::panic::Location field to Entities in debug builds #16070

Closed

bushrat011899 suggested changes Oct 24, 2024

View reviewed changes

BenjaminBrienen approved these changes Oct 31, 2024

View reviewed changes

crates/bevy_ecs/src/world/entity_ref.rs Show resolved Hide resolved

SpecificProtagonist mentioned this pull request Nov 21, 2024

Remove flush_and_reserve_invalid_assuming_no_entities #16460

Merged

SpecificProtagonist added 18 commits December 3, 2024 21:12

track despawns

d66bc02

fix

4769197

fix unused without feature

218b08e

*sigh*

2004f15

fix doctest; adjust feature description

4236af1

cargo run -p build-templated-pages -- update features

a5bee6e

doc links

bb26b83

QueryEntityError: Mention feature

f99b26f

fix outdated doc

02b2a79

function names

5768b81

error message: hint at feature

1b31a81

Improve B0003 error message

59fd3b8

wrap reference in option

fd3d506

core instead of std

e4283df

fix lint

96b391f

reorder despawn vs hooks/observers

a2c9c40

function naming

d32738f

remove safety invariant from EntityLocation

55931b1

typo

85dc8ca

SpecificProtagonist force-pushed the track-entity-spawn-despawn branch from 451ffc3 to 85dc8ca Compare December 3, 2024 20:15

bushrat011899 approved these changes Dec 3, 2024

View reviewed changes

SpecificProtagonist mentioned this pull request Dec 11, 2024

Comprehensive Source Tracking #16775

Open

6 tasks

NthTensor added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Dec 11, 2024

NthTensor approved these changes Dec 11, 2024

View reviewed changes

alice-i-cecile mentioned this pull request Dec 16, 2024

Rename track_change_detection feature of bevy_ecs #16852

Closed

SpecificProtagonist and others added 6 commits December 17, 2024 01:05

resolve conflicts

04fb505

Better error message for EntityWorldMut::panic_despawned

4a982d5

f32 -> Rot2 in bounding volume docs (bevyengine#16848)

ac1faf0

This is the correct rotation type :)

merge main

d016121

fix import in test

7278e94

SpecificProtagonist force-pushed the track-entity-spawn-despawn branch from ebb872b to 7278e94 Compare December 17, 2024 00:27

make ci happy

269f82d

alice-i-cecile added this pull request to the merge queue Dec 17, 2024

Merged via the queue into bevyengine:main with commit 21195a7 Dec 17, 2024
32 checks passed

SpecificProtagonist deleted the track-entity-spawn-despawn branch January 9, 2025 00:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

track_change_detection: Also track spawns/despawns #16047

track_change_detection: Also track spawns/despawns #16047

SpecificProtagonist commented Oct 21, 2024 •

edited

Loading

aevyrie Oct 21, 2024

bushrat011899 left a comment

bushrat011899 Oct 24, 2024

SpecificProtagonist Oct 25, 2024 •

edited

Loading

bushrat011899 Oct 25, 2024

bushrat011899 left a comment

NthTensor left a comment

alice-i-cecile commented Dec 16, 2024

alice-i-cecile commented Dec 17, 2024

track_change_detection: Also track spawns/despawns #16047

track_change_detection: Also track spawns/despawns #16047

Conversation

SpecificProtagonist commented Oct 21, 2024 • edited Loading

Objective

Solution

Testing

Showcase

aevyrie Oct 21, 2024

Choose a reason for hiding this comment

bushrat011899 left a comment

Choose a reason for hiding this comment

bushrat011899 Oct 24, 2024

Choose a reason for hiding this comment

SpecificProtagonist Oct 25, 2024 • edited Loading

Choose a reason for hiding this comment

bushrat011899 Oct 25, 2024

Choose a reason for hiding this comment

bushrat011899 left a comment

Choose a reason for hiding this comment

NthTensor left a comment

Choose a reason for hiding this comment

alice-i-cecile commented Dec 16, 2024

alice-i-cecile commented Dec 17, 2024

SpecificProtagonist commented Oct 21, 2024 •

edited

Loading

SpecificProtagonist Oct 25, 2024 •

edited

Loading