[Merged by Bors] - Fix unsound EntityMut::remove_children. Add EntityMut::world_scope
#6464
Conversation
alice-i-cecile
added
A-ECS
|
In #5845, I had added a method to update the world safely: Maybe you could add it? There were some discussion around it, you can find them there: https://github.com/bevyengine/bevy/pull/5845/files |
irate-devil
force-pushed
the
uh-oh-safety
branch
from
f1b2de4
to
201a3f9
Compare
| @@ -543,6 +543,12 @@ impl<'w> EntityMut<'w> { | |||
| self.world | |||
| } | |||
|
|
|||
| /// Gives mutable access to this `EntityMut`'s [`World`] in a temporary scope. | |||
There was a problem hiding this comment.
Might benefit from explaining why you'd want to use it.
There was a problem hiding this comment.
It's the only way of safely getting a &mut World from an EntityMut without consuming self. Which sort of speaks for itself?
I'm not sure what to add to the docs that could make that clearer (Aside from being very pedantic) :)
There was a problem hiding this comment.
You could discuss update_location here, but that's very gory-internals, and likely to go out of date in dangerous ways. If people care, they can read the code.
There was a problem hiding this comment.
I was thinking: someone reading the doc doesn't necessarily see the source code. If they encounter this method, they may ask themselves why its here. But I agree with alice, it's really a minor thing and really depends on internals.
alice-i-cecile
added
the
C-Usability
`EntityMut::remove_children` does not call `self.update_location()` which is unsound. Verified by adding the following assertion, which fails when running the tests. ```rust let before = self.location(); self.update_location(); assert_eq!(before, self.location()); ``` I also removed incorrect messages like "parent entity is not modified" and the unhelpful "Inserting a bundle in the children entities may change the parent entity's location if they were of the same archetype" which might lead people to think that's the *only* thing that can change the entity's location. Co-authored-by: devil-ira <justthecooldude@gmail.com>
|
Can you add a change log item for the new |
|
Build failed: |
irate-devil
changed the title
EntityMut::remove_childrenEntityMut::remove_children. Add EntityMut::world_scope
|
bors retry |
#6464) `EntityMut::remove_children` does not call `self.update_location()` which is unsound. Verified by adding the following assertion, which fails when running the tests. ```rust let before = self.location(); self.update_location(); assert_eq!(before, self.location()); ``` I also removed incorrect messages like "parent entity is not modified" and the unhelpful "Inserting a bundle in the children entities may change the parent entity's location if they were of the same archetype" which might lead people to think that's the *only* thing that can change the entity's location. # Changelog Added `EntityMut::world_scope`. Co-authored-by: devil-ira <justthecooldude@gmail.com>
bors
bot
changed the title
EntityMut::remove_children. Add EntityMut::world_scopeEntityMut::remove_children. Add EntityMut::world_scope
bevyengine#6464) `EntityMut::remove_children` does not call `self.update_location()` which is unsound. Verified by adding the following assertion, which fails when running the tests. ```rust let before = self.location(); self.update_location(); assert_eq!(before, self.location()); ``` I also removed incorrect messages like "parent entity is not modified" and the unhelpful "Inserting a bundle in the children entities may change the parent entity's location if they were of the same archetype" which might lead people to think that's the *only* thing that can change the entity's location. # Changelog Added `EntityMut::world_scope`. Co-authored-by: devil-ira <justthecooldude@gmail.com>
EntityMut::remove_childrendoes not callself.update_location()which is unsound.Verified by adding the following assertion, which fails when running the tests.
I also removed incorrect messages like "parent entity is not modified" and the unhelpful "Inserting a bundle in the children entities may change the parent entity's location if they were of the same archetype" which might lead people to think that's the only thing that can change the entity's location.
Changelog
Added
EntityMut::world_scope.