[Merged by Bors] - EntityMut: rename remove_intersection to remove and remove to take
#7810
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maniwani
changed the title
remove_intersection to remove and remove to takeEntityMut: rename remove_intersection to remove and remove to take
maniwani
added
A-ECS
alice-i-cecile
approved these changes
Feb 24, 2023
|
Just want to make sure I didn't choose the wrong method in |
MrGVSV
approved these changes
Feb 24, 2023
Comment on lines
-102
to
-105
| /// | ||
| /// # Panics | ||
| /// | ||
| /// Panics if there is no [`Component`] of the given type. |
There was a problem hiding this comment.
So entity.remove::<T>() won't panic if the component doesn't exist on the entity? (Did it even panic before?)
There was a problem hiding this comment.
Hmm, I don't think remove ever panicked (well, ig if you call unwrap when it returns None).
But there's a subtle difference when T is a tuple/bundle.
remove (take) does not remove anything unless the entity has all the components in the bundle. remove_intersection (remove) will remove the ones it does have.
It wasn't clear to me which one bevy_reflect should use.
There was a problem hiding this comment.
Gotcha. Well ReflectComponent only operates on a single component anyways, so either should work fine 😄
james7132
approved these changes
Feb 24, 2023
james7132
added
the
S-Ready-For-Final-Review
JoJoJet
suggested changes
Feb 24, 2023
JoJoJet
approved these changes
Feb 24, 2023
|
@maniwani mind fixing the CI failures? Will merge after that, though we may need to manually add the changes into the 0.10 migration guide. |
maniwani
force-pushed
the
remove_and_take
branch
from
c6776a5 to
1bae6b9
Compare
|
bors r+ |
bors bot
pushed a commit
that referenced
this pull request
Feb 25, 2023
… `take` (#7810) # Objective - A more intuitive distinction between the two. `remove_intersection` is verbose and unclear. - `EntityMut::remove` and `Commands::remove` should match. ## Solution - What the title says. --- ## Migration Guide Before ```rust fn clear_children(parent: Entity, world: &mut World) { if let Some(children) = world.entity_mut(parent).remove::<Children>() { for &child in &children.0 { world.entity_mut(child).remove_intersection::<Parent>(); } } } ``` After ```rust fn clear_children(parent: Entity, world: &mut World) { if let Some(children) = world.entity_mut(parent).take::<Children>() { for &child in &children.0 { world.entity_mut(child).remove::<Parent>(); } } } ```
|
Build failed:
|
|
bors r+ |
bors bot
pushed a commit
that referenced
this pull request
Feb 26, 2023
… `take` (#7810) # Objective - A more intuitive distinction between the two. `remove_intersection` is verbose and unclear. - `EntityMut::remove` and `Commands::remove` should match. ## Solution - What the title says. --- ## Migration Guide Before ```rust fn clear_children(parent: Entity, world: &mut World) { if let Some(children) = world.entity_mut(parent).remove::<Children>() { for &child in &children.0 { world.entity_mut(child).remove_intersection::<Parent>(); } } } ``` After ```rust fn clear_children(parent: Entity, world: &mut World) { if let Some(children) = world.entity_mut(parent).take::<Children>() { for &child in &children.0 { world.entity_mut(child).remove::<Parent>(); } } } ```
bors
bot
changed the title
EntityMut: rename remove_intersection to remove and remove to takeEntityMut: rename remove_intersection to remove and remove to take
Shfty
pushed a commit
to shfty-rust/bevy
that referenced
this pull request
Mar 19, 2023
… `take` (bevyengine#7810) # Objective - A more intuitive distinction between the two. `remove_intersection` is verbose and unclear. - `EntityMut::remove` and `Commands::remove` should match. ## Solution - What the title says. --- ## Migration Guide Before ```rust fn clear_children(parent: Entity, world: &mut World) { if let Some(children) = world.entity_mut(parent).remove::<Children>() { for &child in &children.0 { world.entity_mut(child).remove_intersection::<Parent>(); } } } ``` After ```rust fn clear_children(parent: Entity, world: &mut World) { if let Some(children) = world.entity_mut(parent).take::<Children>() { for &child in &children.0 { world.entity_mut(child).remove::<Parent>(); } } } ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Objective
remove_intersectionis verbose and unclear.EntityMut::removeandCommands::removeshould match.Solution
Migration Guide
Before
After