forestia/bevy
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Switch ChildOf back to tuple struct (#18672)

Browse Source 
# Objective

In #17905 we swapped to a named field on `ChildOf` to help resolve
variable naming ambiguity of child vs parent (ex: `child_of.parent`
clearly reads as "I am accessing the parent of the child_of
relationship", whereas `child_of.0` is less clear).

Unfortunately this has the side effect of making initialization less
ideal. `ChildOf { parent }` reads just as well as `ChildOf(parent)`, but
`ChildOf { parent: root }` doesn't read nearly as well as
`ChildOf(root)`.

## Solution

Move back to `ChildOf(pub Entity)` but add a `child_of.parent()`
function and use it for all accesses. The downside here is that users
are no longer "forced" to access the parent field with `parent`
nomenclature, but I think this strikes the right balance.

Take a look at the diff. I think the results provide strong evidence for
this change. Initialization has the benefit of reading much better _and_
of taking up significantly less space, as many lines go from 3 to 1, and
we're cutting out a bunch of syntax in some cases.

Sadly I do think this should land in 0.16 as the cost of doing this
_after_ the relationships migration is high.
This commit is contained in:
Carter Anderson 2025-04-01 17:10:10 -07:00 committed by François Mockers
parent 5cbc20f787
commit 1553ee98ff
22 changed files with 90 additions and 118 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
benches/benches/bevy_ecs/entity_cloning.rs
View File

@ -155,7 +155,7 @@ fn bench_clone_hierarchy<B: Bundle + Default + GetTypeRegistration>(
for parent in current_hierarchy_level {
for _ in 0..children {
let child_id = world.spawn((B::default(), ChildOf { parent })).id();
let child_id = world.spawn((B::default(), ChildOf(parent))).id();
hierarchy_level.push(child_id);
}
}

6
crates/bevy_ecs/src/entity/clone_entities.rs
View File

@ -1337,9 +1337,9 @@ mod tests {
fn recursive_clone() {
let mut world = World::new();
let root = world.spawn_empty().id();
let child1 = world.spawn(ChildOf { parent: root }).id();
let grandchild = world.spawn(ChildOf { parent: child1 }).id();
let child2 = world.spawn(ChildOf { parent: root }).id();
let child1 = world.spawn(ChildOf(root)).id();
let grandchild = world.spawn(ChildOf(child1)).id();
let child2 = world.spawn(ChildOf(root)).id();
let clone_root = world.spawn_empty().id();
EntityCloner::build(&mut world)

97
crates/bevy_ecs/src/hierarchy.rs
View File

@ -54,9 +54,9 @@ use log::warn;
/// # use bevy_ecs::prelude::*;
/// # let mut world = World::new();
/// let root = world.spawn_empty().id();
/// let child1 = world.spawn(ChildOf { parent: root }).id();
/// let child2 = world.spawn(ChildOf { parent: root }).id();
/// let grandchild = world.spawn(ChildOf { parent: child1 }).id();
/// let child1 = world.spawn(ChildOf(root)).id();
/// let child2 = world.spawn(ChildOf(root)).id();
/// let grandchild = world.spawn(ChildOf(child1)).id();
///
/// assert_eq!(&**world.entity(root).get::<Children>().unwrap(), &[child1, child2]);
/// assert_eq!(&**world.entity(child1).get::<Children>().unwrap(), &[grandchild]);
@ -96,9 +96,21 @@ use log::warn;
)]
#[relationship(relationship_target = Children)]
#[doc(alias = "IsChild", alias = "Parent")]
pub struct ChildOf {
pub struct ChildOf(pub Entity);
impl ChildOf {
/// The parent entity of this child entity.
pub parent: Entity,
#[inline]
pub fn parent(&self) -> Entity {
self.0
}
/// The parent entity of this child entity.
#[deprecated(since = "0.16.0", note = "Use child_of.parent() instead")]
#[inline]
pub fn get(&self) -> Entity {
self.0
}
}
// TODO: We need to impl either FromWorld or Default so ChildOf can be registered as Reflect.
@ -108,9 +120,7 @@ pub struct ChildOf {
impl FromWorld for ChildOf {
#[inline(always)]
fn from_world(_world: &mut World) -> Self {
ChildOf {
parent: Entity::PLACEHOLDER,
}
ChildOf(Entity::PLACEHOLDER)
}
}
@ -316,7 +326,7 @@ impl<'w> EntityWorldMut<'w> {
pub fn with_child(&mut self, bundle: impl Bundle) -> &mut Self {
let parent = self.id();
self.world_scope(|world| {
world.spawn((bundle, ChildOf { parent }));
world.spawn((bundle, ChildOf(parent)));
});
self
}
@ -329,12 +339,9 @@ impl<'w> EntityWorldMut<'w> {
}
/// Inserts the [`ChildOf`] component with the given `parent` entity, if it exists.
#[deprecated(
since = "0.16.0",
note = "Use entity_mut.insert(ChildOf { parent: entity })"
)]
#[deprecated(since = "0.16.0", note = "Use entity_mut.insert(ChildOf(entity))")]
pub fn set_parent(&mut self, parent: Entity) -> &mut Self {
self.insert(ChildOf { parent });
self.insert(ChildOf(parent));
self
}
}
@ -394,7 +401,7 @@ impl<'a> EntityCommands<'a> {
/// [`with_children`]: EntityCommands::with_children
pub fn with_child(&mut self, bundle: impl Bundle) -> &mut Self {
let parent = self.id();
self.commands.spawn((bundle, ChildOf { parent }));
self.commands.spawn((bundle, ChildOf(parent)));
self
}
@ -406,12 +413,9 @@ impl<'a> EntityCommands<'a> {
}
/// Inserts the [`ChildOf`] component with the given `parent` entity, if it exists.
#[deprecated(
since = "0.16.0",
note = "Use entity_commands.insert(ChildOf { parent: entity })"
)]
#[deprecated(since = "0.16.0", note = "Use entity_commands.insert(ChildOf(entity))")]
pub fn set_parent(&mut self, parent: Entity) -> &mut Self {
self.insert(ChildOf { parent });
self.insert(ChildOf(parent));
self
}
}
@ -427,7 +431,7 @@ pub fn validate_parent_has_component<C: Component>(
return;
};
if !world
.get_entity(child_of.parent)
.get_entity(child_of.parent())
.is_ok_and(|e| e.contains::<C>())
{
// TODO: print name here once Name lives in bevy_ecs
@ -527,9 +531,9 @@ mod tests {
fn hierarchy() {
let mut world = World::new();
let root = world.spawn_empty().id();
let child1 = world.spawn(ChildOf { parent: root }).id();
let grandchild = world.spawn(ChildOf { parent: child1 }).id();
let child2 = world.spawn(ChildOf { parent: root }).id();
let child1 = world.spawn(ChildOf(root)).id();
let grandchild = world.spawn(ChildOf(child1)).id();
let child2 = world.spawn(ChildOf(root)).id();
// Spawn
let hierarchy = get_hierarchy(&world, root);
@ -550,7 +554,7 @@ mod tests {
assert_eq!(hierarchy, Node::new_with(root, vec![Node::new(child2)]));
// Insert
world.entity_mut(child1).insert(ChildOf { parent: root });
world.entity_mut(child1).insert(ChildOf(root));
let hierarchy = get_hierarchy(&world, root);
assert_eq!(
hierarchy,
@ -638,7 +642,7 @@ mod tests {
fn self_parenting_invalid() {
let mut world = World::new();
let id = world.spawn_empty().id();
world.entity_mut(id).insert(ChildOf { parent: id });
world.entity_mut(id).insert(ChildOf(id));
assert!(
world.entity(id).get::<ChildOf>().is_none(),
"invalid ChildOf relationships should self-remove"
@ -650,7 +654,7 @@ mod tests {
let mut world = World::new();
let parent = world.spawn_empty().id();
world.entity_mut(parent).despawn();
let id = world.spawn(ChildOf { parent }).id();
let id = world.spawn(ChildOf(parent)).id();
assert!(
world.entity(id).get::<ChildOf>().is_none(),
"invalid ChildOf relationships should self-remove"
@ -661,10 +665,10 @@ mod tests {
fn reinsert_same_parent() {
let mut world = World::new();
let parent = world.spawn_empty().id();
let id = world.spawn(ChildOf { parent }).id();
world.entity_mut(id).insert(ChildOf { parent });
let id = world.spawn(ChildOf(parent)).id();
world.entity_mut(id).insert(ChildOf(parent));
assert_eq!(
Some(&ChildOf { parent }),
Some(&ChildOf(parent)),
world.entity(id).get::<ChildOf>(),
"ChildOf should still be there"
);
@ -699,11 +703,11 @@ mod tests {
assert_eq!(
world.entity(child_a).get::<ChildOf>().unwrap(),
&ChildOf { parent }
&ChildOf(parent)
);
assert_eq!(
world.entity(child_c).get::<ChildOf>().unwrap(),
&ChildOf { parent }
&ChildOf(parent)
);
assert!(world.entity(child_b).get::<ChildOf>().is_none());
}
@ -739,7 +743,7 @@ mod tests {
assert_eq!(children.0, [child]);
assert_eq!(
world.entity(child).get::<ChildOf>().unwrap(),
&ChildOf { parent }
&ChildOf(parent)
);
}
@ -762,11 +766,11 @@ mod tests {
assert_eq!(
world.entity(child_a).get::<ChildOf>().unwrap(),
&ChildOf { parent }
&ChildOf(parent)
);
assert_eq!(
world.entity(child_b).get::<ChildOf>().unwrap(),
&ChildOf { parent }
&ChildOf(parent)
);
assert_eq!(
world.entity(parent).get::<Children>().unwrap().0,
@ -781,15 +785,15 @@ mod tests {
);
assert_eq!(
world.entity(child_a).get::<ChildOf>().unwrap(),
&ChildOf { parent }
&ChildOf(parent)
);
assert_eq!(
world.entity(child_c).get::<ChildOf>().unwrap(),
&ChildOf { parent }
&ChildOf(parent)
);
assert_eq!(
world.entity(child_d).get::<ChildOf>().unwrap(),
&ChildOf { parent }
&ChildOf(parent)
);
assert_eq!(
world.entity(parent).get::<Children>().unwrap().0,
@ -844,11 +848,11 @@ mod tests {
assert_eq!(
world.entity(child_a).get::<ChildOf>().unwrap(),
&ChildOf { parent }
&ChildOf(parent)
);
assert_eq!(
world.entity(child_b).get::<ChildOf>().unwrap(),
&ChildOf { parent }
&ChildOf(parent)
);
assert_eq!(
world.entity(parent).get::<Children>().unwrap().0,
@ -863,11 +867,11 @@ mod tests {
);
assert_eq!(
world.entity(child_c).get::<ChildOf>().unwrap(),
&ChildOf { parent }
&ChildOf(parent)
);
assert_eq!(
world.entity(child_d).get::<ChildOf>().unwrap(),
&ChildOf { parent }
&ChildOf(parent)
);
assert_eq!(
world.entity(parent).get::<Children>().unwrap().0,
@ -974,11 +978,10 @@ mod tests {
let mut world = World::new();
let parent = world.spawn_empty().id();
let other = world.spawn_empty().id();
let child = world.spawn(ChildOf { parent }).id();
world.entity_mut(child).insert_with_relationship_hook_mode(
ChildOf { parent: other },
RelationshipHookMode::Skip,
);
let child = world.spawn(ChildOf(parent)).id();
world
.entity_mut(child)
.insert_with_relationship_hook_mode(ChildOf(other), RelationshipHookMode::Skip);
assert_eq!(
&**world.entity(parent).get::<Children>().unwrap(),
&[child],

6
crates/bevy_ecs/src/relationship/related_methods.rs
View File

@ -521,9 +521,9 @@ mod tests {
let mut world = World::new();
let a = world.spawn_empty().id();
let b = world.spawn(ChildOf { parent: a }).id();
let c = world.spawn(ChildOf { parent: a }).id();
let d = world.spawn(ChildOf { parent: b }).id();
let b = world.spawn(ChildOf(a)).id();
let c = world.spawn(ChildOf(a)).id();
let d = world.spawn(ChildOf(b)).id();
world
.entity_mut(a)

4
crates/bevy_input_focus/src/lib.rs
View File

@ -165,7 +165,7 @@ impl<E: Event + Clone> Traversal<FocusedInput<E>> for WindowTraversal {
// Send event to parent, if it has one.
if let Some(child_of) = child_of {
return Some(child_of.parent);
return Some(child_of.parent());
};
// Otherwise, send it to the window entity (unless this is a window entity).
@ -338,7 +338,7 @@ impl IsFocused for World {
if e == entity {
return true;
}
if let Some(parent) = self.entity(e).get::<ChildOf>().map(|c| c.parent) {
if let Some(parent) = self.entity(e).get::<ChildOf>().map(ChildOf::parent) {
e = parent;
} else {
return false;

18
crates/bevy_input_focus/src/tab_navigation.rs
View File

@ -375,22 +375,8 @@ mod tests {
let world = app.world_mut();
let tab_group_entity = world.spawn(TabGroup::new(0)).id();
let tab_entity_1 = world
.spawn((
TabIndex(0),
ChildOf {
parent: tab_group_entity,
},
))
.id();
let tab_entity_2 = world
.spawn((
TabIndex(1),
ChildOf {
parent: tab_group_entity,
},
))
.id();
let tab_entity_1 = world.spawn((TabIndex(0), ChildOf(tab_group_entity))).id();
let tab_entity_2 = world.spawn((TabIndex(1), ChildOf(tab_group_entity))).id();
let mut system_state: SystemState<TabNavigation> = SystemState::new(world);
let tab_navigation = system_state.get(world);

2
crates/bevy_picking/src/events.rs
View File

@ -92,7 +92,7 @@ where
// Send event to parent, if it has one.
if let Some(child_of) = child_of {
return Some(child_of.parent);
return Some(child_of.parent());
};
// Otherwise, send it to the window entity (unless this is a window entity).

6
crates/bevy_render/src/view/visibility/mod.rs
View File

@ -411,7 +411,7 @@ fn visibility_propagate_system(
Visibility::Hidden => false,
// fall back to true if no parent is found or parent lacks components
Visibility::Inherited => child_of
.and_then(|c| visibility_query.get(c.parent).ok())
.and_then(|c| visibility_query.get(c.parent()).ok())
.is_none_or(|(_, x)| x.get()),
};
let (_, mut inherited_visibility) = visibility_query
@ -786,9 +786,7 @@ mod test {
.entity_mut(parent2)
.insert(Visibility::Visible);
// Simulate a change in the parent component
app.world_mut()
.entity_mut(child2)
.insert(ChildOf { parent: parent2 }); // example of changing parent
app.world_mut().entity_mut(child2).insert(ChildOf(parent2)); // example of changing parent
// Run the system again to propagate changes
app.update();

6
crates/bevy_scene/src/dynamic_scene.rs
View File

@ -325,7 +325,7 @@ mod tests {
.unwrap()
.get::<ChildOf>()
.unwrap()
.parent,
.parent(),
"something about reloading the scene is touching entities with the same scene Ids"
);
assert_eq!(
@ -335,7 +335,7 @@ mod tests {
.unwrap()
.get::<ChildOf>()
.unwrap()
.parent,
.parent(),
"something about reloading the scene is touching components not defined in the scene but on entities defined in the scene"
);
assert_eq!(
@ -345,7 +345,7 @@ mod tests {
.unwrap()
.get::<ChildOf>()
.expect("something is wrong with this test, and the scene components don't have a parent/child relationship")
.parent,
.parent(),
"something is wrong with this test or the code reloading scenes since the relationship between scene entities is broken"
);
}

21
crates/bevy_scene/src/scene_spawner.rs
View File

@ -892,30 +892,15 @@ mod tests {
// Spawn entities with different parent first before parenting them to the actual root, allowing us
// to decouple child order from archetype-creation-order
let child1 = scene_world
.spawn((
ChildOf {
parent: temporary_root,
},
ComponentA { x: 1.0, y: 1.0 },
))
.spawn((ChildOf(temporary_root), ComponentA { x: 1.0, y: 1.0 }))
.id();
let child2 = scene_world
.spawn((
ChildOf {
parent: temporary_root,
},
ComponentA { x: 2.0, y: 2.0 },
))
.spawn((ChildOf(temporary_root), ComponentA { x: 2.0, y: 2.0 }))
.id();
// the "first" child is intentionally spawned with a different component to force it into a "newer" archetype,
// meaning it will be iterated later in the spawn code.
let child0 = scene_world
.spawn((
ChildOf {
parent: temporary_root,
},
ComponentF,
))
.spawn((ChildOf(temporary_root), ComponentF))
.id();
scene_world

4
crates/bevy_text/src/text.rs
View File

@ -524,7 +524,7 @@ pub fn detect_text_needs_rerender<Root: Component>(
));
continue;
};
let mut parent: Entity = span_child_of.parent;
let mut parent: Entity = span_child_of.parent();
// Search for the nearest ancestor with ComputedTextBlock.
// Note: We assume the perf cost from duplicate visits in the case that multiple spans in a block are visited
@ -555,7 +555,7 @@ pub fn detect_text_needs_rerender<Root: Component>(
));
break;
};
parent = next_child_of.parent;
parent = next_child_of.parent();
}
}
}

2
crates/bevy_transform/src/helper.rs
View File

@ -124,7 +124,7 @@ mod tests {
let mut e = app.world_mut().spawn(transform);
if let Some(parent) = entity {
e.insert(ChildOf { parent });
e.insert(ChildOf(parent));
}
entity = Some(e.id());

14
crates/bevy_transform/src/systems.rs
View File

@ -53,14 +53,14 @@ pub fn mark_dirty_trees(
) {
for entity in changed_transforms.iter().chain(orphaned.read()) {
let mut next = entity;
while let Ok((parent, mut tree)) = transforms.get_mut(next) {
while let Ok((child_of, mut tree)) = transforms.get_mut(next) {
if tree.is_changed() && !tree.is_added() {
// If the component was changed, this part of the tree has already been processed.
// Ignore this if the change was caused by the component being added.
break;
}
tree.set_changed();
if let Some(parent) = parent.map(|p| p.parent) {
if let Some(parent) = child_of.map(ChildOf::parent) {
next = parent;
} else {
break;
@ -121,7 +121,7 @@ mod serial {
for (child, child_of) in child_query.iter_many(children) {
assert_eq!(
child_of.parent, entity,
child_of.parent(), entity,
"Malformed hierarchy. This probably means that your hierarchy has been improperly maintained, or contains a cycle"
);
// SAFETY:
@ -221,7 +221,7 @@ mod serial {
let Some(children) = children else { return };
for (child, child_of) in child_query.iter_many(children) {
assert_eq!(
child_of.parent, entity,
child_of.parent(), entity,
"Malformed hierarchy. This probably means that your hierarchy has been improperly maintained, or contains a cycle"
);
// SAFETY: The caller guarantees that `transform_query` will not be fetched for any
@ -452,7 +452,7 @@ mod parallel {
// Static scene optimization
return None;
}
assert_eq!(child_of.parent, parent);
assert_eq!(child_of.parent(), parent);
// Transform prop is expensive - this helps avoid updating entire subtrees if
// the GlobalTransform is unchanged, at the cost of an added equality check.
@ -586,8 +586,8 @@ mod test {
let root = commands.spawn(offset_transform(3.3)).id();
let parent = commands.spawn(offset_transform(4.4)).id();
let child = commands.spawn(offset_transform(5.5)).id();
commands.entity(parent).insert(ChildOf { parent: root });
commands.entity(child).insert(ChildOf { parent });
commands.entity(parent).insert(ChildOf(root));
commands.entity(child).insert(ChildOf(parent));
command_queue.apply(&mut world);
schedule.run(&mut world);

2
crates/bevy_ui/src/experimental/ghost_hierarchy.rs
View File

@ -150,7 +150,7 @@ impl<'w, 's> UiChildren<'w, 's> {
/// Returns the UI parent of the provided entity.
pub fn get_parent(&'s self, entity: Entity) -> Option<Entity> {
self.parents_query.get(entity).ok().map(|p| p.parent)
self.parents_query.get(entity).ok().map(ChildOf::parent)
}
/// Given an entity in the UI hierarchy, check if its set of children has changed, e.g if children has been added/removed or if the order has changed.

2
crates/bevy_ui/src/layout/mod.rs
View File

@ -132,7 +132,7 @@ pub fn ui_layout_system(
// Note: This does not cover the case where a parent's Node component was removed.
// Users are responsible for fixing hierarchies if they do that (it is not recommended).
// Detecting it here would be a permanent perf burden on the hot path.
if child_of.is_changed() && !ui_children.is_ui_node(child_of.parent) {
if child_of.is_changed() && !ui_children.is_ui_node(child_of.parent()) {
warn!(
"Node ({entity}) is in a non-UI entity hierarchy. You are using an entity \
with UI components as a child of an entity without UI components, your UI layout may be broken."

2
crates/bevy_ui/src/update.rs
View File

@ -174,7 +174,7 @@ pub fn update_ui_context_system(
}
for (entity, child_of) in reparented_nodes.iter() {
let Ok(computed_target) = computed_target_query.get(child_of.parent) else {
let Ok(computed_target) = computed_target_query.get(child_of.parent()) else {
continue;
};

2
crates/bevy_winit/src/accessibility.rs
View File

@ -260,7 +260,7 @@ fn queue_node_for_update(
window_children: &mut Vec<NodeId>,
) {
let should_push = if let Some(child_of) = child_of {
!node_entities.contains(child_of.parent)
!node_entities.contains(child_of.parent())
} else {
true
};

2
examples/3d/mixed_lighting.rs
View File

@ -460,7 +460,7 @@ fn move_sphere(
};
// Grab its transform.
let Ok(mut transform) = transforms.get_mut(child_of.parent) else {
let Ok(mut transform) = transforms.get_mut(child_of.parent()) else {
return;
};

2
examples/3d/visibility_range.rs
View File

@ -187,7 +187,7 @@ fn set_visibility_ranges(
break;
}
match child_of {
Some(child_of) => current = child_of.parent,
Some(child_of) => current = child_of.parent(),
None => break,
}
}

2
examples/animation/gltf_skinned_mesh.rs
View File

@ -51,7 +51,7 @@ fn joint_animation(
// Iter skinned mesh entity
for child_of in &children {
// Mesh node is the parent of the skinned mesh entity.
let mesh_node_entity = child_of.parent;
let mesh_node_entity = child_of.parent();
// Get `Children` in the mesh node.
let mesh_node_parent = parents.get(mesh_node_entity).unwrap();

2
examples/tools/scene_viewer/animation_plugin.rs
View File

@ -107,7 +107,7 @@ fn assign_clips(
}
// Go to the next parent.
current = children.get(entity).ok().map(|c| c.parent);
current = children.get(entity).ok().map(ChildOf::parent);
}
}

4
examples/ui/ghost_nodes.rs
View File

@ -110,14 +110,14 @@ fn button_system(
// Update parent counter on click
for (interaction, child_of) in &mut interaction_query {
if matches!(interaction, Interaction::Pressed) {
let mut counter = counter_query.get_mut(child_of.parent).unwrap();
let mut counter = counter_query.get_mut(child_of.parent()).unwrap();
counter.0 += 1;
}
}
// Update button labels to match their parent counter
for (children, child_of) in &labels_query {
let counter = counter_query.get(child_of.parent).unwrap();
let counter = counter_query.get(child_of.parent()).unwrap();
let mut text = text_query.get_mut(children[0]).unwrap();
**text = counter.0.to_string();