Change ChildOf to Childof { parent: Entity} and support deriving Relationship and RelationshipTarget with named structs (#17905)

# Objective

fixes #17896 

## Solution

Change ChildOf ( Entity ) to ChildOf { parent: Entity }

by doing this we also allow users to use named structs for relationship
derives, When you have more than 1 field in a struct with named fields
the macro will look for a field with the attribute #[relationship] and
all of the other fields should implement the Default trait. Unnamed
fields are still supported.

When u have a unnamed struct with more than one field the macro will
fail.
Do we want to support something like this ? 

```rust
 #[derive(Component)]
 #[relationship_target(relationship = ChildOf)]
 pub struct Children (#[relationship] Entity, u8);
```
I could add this, it but doesn't seem nice.
## Testing

crates/bevy_ecs - cargo test


## Showcase


```rust

use bevy_ecs::component::Component;
use bevy_ecs::entity::Entity;

 #[derive(Component)]
 #[relationship(relationship_target = Children)]
 pub struct ChildOf {
     #[relationship]
     pub parent: Entity,
     internal: u8,
 };

 #[derive(Component)]
 #[relationship_target(relationship = ChildOf)]
 pub struct Children {
     children: Vec<Entity>
 };

```

---------

Co-authored-by: Tim Overbeek <oorbecktim@Tims-MacBook-Pro.local>
Co-authored-by: Tim Overbeek <oorbecktim@c-001-001-042.client.nl.eduvpn.org>
Co-authored-by: Tim Overbeek <oorbecktim@c-001-001-059.client.nl.eduvpn.org>
Co-authored-by: Tim Overbeek <oorbecktim@c-001-001-054.client.nl.eduvpn.org>
Co-authored-by: Tim Overbeek <oorbecktim@c-001-001-027.client.nl.eduvpn.org>
This commit is contained in:
Tim Overbeek 2025-02-27 20:22:17 +01:00 committed by GitHub
parent 67146bdef7
commit ccb7069e7f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
17 changed files with 170 additions and 89 deletions

View File

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

View File

@ -9,8 +9,8 @@ use syn::{
punctuated::Punctuated,
spanned::Spanned,
token::{Comma, Paren},
Data, DataStruct, DeriveInput, ExprClosure, ExprPath, Fields, Ident, Index, LitStr, Member,
Path, Result, Token, Visibility,
Data, DataStruct, DeriveInput, ExprClosure, ExprPath, Field, Fields, Ident, Index, LitStr,
Member, Path, Result, Token, Visibility,
};
pub fn derive_event(input: TokenStream) -> TokenStream {
@ -260,6 +260,18 @@ fn visit_entities(data: &Data, bevy_ecs_path: &Path, is_relationship: bool) -> T
Data::Struct(DataStruct { fields, .. }) => {
let mut visited_fields = Vec::new();
let mut visited_indices = Vec::new();
if is_relationship {
let field = match relationship_field(fields, "VisitEntities", fields.span()) {
Ok(f) => f,
Err(e) => return e.to_compile_error(),
};
match field.ident {
Some(ref ident) => visited_fields.push(ident.clone()),
None => visited_indices.push(Index::from(0)),
}
}
match fields {
Fields::Named(fields) => {
for field in &fields.named {
@ -276,9 +288,7 @@ fn visit_entities(data: &Data, bevy_ecs_path: &Path, is_relationship: bool) -> T
}
Fields::Unnamed(fields) => {
for (index, field) in fields.unnamed.iter().enumerate() {
if index == 0 && is_relationship {
visited_indices.push(Index::from(0));
} else if field
if field
.attrs
.iter()
.any(|a| a.meta.path().is_ident(ENTITIES_ATTR))
@ -289,7 +299,6 @@ fn visit_entities(data: &Data, bevy_ecs_path: &Path, is_relationship: bool) -> T
}
Fields::Unit => {}
}
if visited_fields.is_empty() && visited_indices.is_empty() {
TokenStream2::new()
} else {
@ -651,25 +660,24 @@ fn derive_relationship(
let Some(relationship) = &attrs.relationship else {
return Ok(None);
};
const RELATIONSHIP_FORMAT_MESSAGE: &str = "Relationship derives must be a tuple struct with the only element being an EntityTargets type (ex: ChildOf(Entity))";
if let Data::Struct(DataStruct {
fields: Fields::Unnamed(unnamed_fields),
let Data::Struct(DataStruct {
fields,
struct_token,
..
}) = &ast.data
{
if unnamed_fields.unnamed.len() != 1 {
return Err(syn::Error::new(ast.span(), RELATIONSHIP_FORMAT_MESSAGE));
}
if unnamed_fields.unnamed.first().is_none() {
return Err(syn::Error::new(
struct_token.span(),
RELATIONSHIP_FORMAT_MESSAGE,
));
}
} else {
return Err(syn::Error::new(ast.span(), RELATIONSHIP_FORMAT_MESSAGE));
else {
return Err(syn::Error::new(
ast.span(),
"Relationship can only be derived for structs.",
));
};
let field = relationship_field(fields, "Relationship", struct_token.span())?;
let relationship_member: Member = field.ident.clone().map_or(Member::from(0), Member::Named);
let members = fields
.members()
.filter(|member| member != &relationship_member);
let struct_name = &ast.ident;
let (impl_generics, type_generics, where_clause) = &ast.generics.split_for_impl();
@ -682,12 +690,15 @@ fn derive_relationship(
#[inline(always)]
fn get(&self) -> #bevy_ecs_path::entity::Entity {
self.0
self.#relationship_member
}
#[inline]
fn from(entity: #bevy_ecs_path::entity::Entity) -> Self {
Self(entity)
Self {
#(#members: core::default::Default::default(),),*
#relationship_member: entity
}
}
}
}))
@ -702,30 +713,29 @@ fn derive_relationship_target(
return Ok(None);
};
const RELATIONSHIP_TARGET_FORMAT_MESSAGE: &str = "RelationshipTarget derives must be a tuple struct with the first element being a private RelationshipSourceCollection (ex: Children(Vec<Entity>))";
let collection = if let Data::Struct(DataStruct {
fields: Fields::Unnamed(unnamed_fields),
let Data::Struct(DataStruct {
fields,
struct_token,
..
}) = &ast.data
{
if let Some(first) = unnamed_fields.unnamed.first() {
if first.vis != Visibility::Inherited {
return Err(syn::Error::new(first.span(), "The collection in RelationshipTarget must be private to prevent users from directly mutating it, which could invalidate the correctness of relationships."));
}
first.ty.clone()
} else {
return Err(syn::Error::new(
struct_token.span(),
RELATIONSHIP_TARGET_FORMAT_MESSAGE,
));
}
} else {
else {
return Err(syn::Error::new(
ast.span(),
RELATIONSHIP_TARGET_FORMAT_MESSAGE,
"RelationshipTarget can only be derived for structs.",
));
};
let field = relationship_field(fields, "RelationshipTarget", struct_token.span())?;
if field.vis != Visibility::Inherited {
return Err(syn::Error::new(field.span(), "The collection in RelationshipTarget must be private to prevent users from directly mutating it, which could invalidate the correctness of relationships."));
}
let collection = &field.ty;
let relationship_member = field.ident.clone().map_or(Member::from(0), Member::Named);
let members = fields
.members()
.filter(|member| member != &relationship_member);
let relationship = &relationship_target.relationship;
let struct_name = &ast.ident;
@ -739,18 +749,56 @@ fn derive_relationship_target(
#[inline]
fn collection(&self) -> &Self::Collection {
&self.0
&self.#relationship_member
}
#[inline]
fn collection_mut_risky(&mut self) -> &mut Self::Collection {
&mut self.0
&mut self.#relationship_member
}
#[inline]
fn from_collection_risky(collection: Self::Collection) -> Self {
Self(collection)
Self {
#(#members: core::default::Default::default(),),*
#relationship_member: collection
}
}
}
}))
}
/// Returns the field with the `#[relationship]` attribute, the only field if unnamed,
/// or the only field in a [`Fields::Named`] with one field, otherwise `Err`.
fn relationship_field<'a>(
fields: &'a Fields,
derive: &'static str,
span: Span,
) -> Result<&'a Field> {
match fields {
Fields::Named(fields) if fields.named.len() == 1 => Ok(fields.named.first().unwrap()),
Fields::Named(fields) => fields.named.iter().find(|field| {
field
.attrs
.iter()
.any(|attr| attr.path().is_ident("relationship"))
}).ok_or(syn::Error::new(
span,
format!("{derive} derive expected named structs with a single field or with a field annotated with #[relationship].")
)),
Fields::Unnamed(fields) => fields
.unnamed
.len()
.eq(&1)
.then(|| fields.unnamed.first())
.flatten()
.ok_or(syn::Error::new(
span,
format!("{derive} derive expected unnamed structs with one field."),
)),
Fields::Unit => Err(syn::Error::new(
span,
format!("{derive} derive expected named or unnamed struct, found unit struct."),
)),
}
}

View File

@ -1291,9 +1291,9 @@ mod tests {
fn recursive_clone() {
let mut world = World::new();
let root = world.spawn_empty().id();
let child1 = world.spawn(ChildOf(root)).id();
let grandchild = world.spawn(ChildOf(child1)).id();
let child2 = world.spawn(ChildOf(root)).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 clone_root = world.spawn_empty().id();
EntityCloner::build(&mut world)

View File

@ -52,9 +52,9 @@ use log::warn;
/// # use bevy_ecs::prelude::*;
/// # let mut world = World::new();
/// let root = world.spawn_empty().id();
/// let child1 = world.spawn(ChildOf(root)).id();
/// let child2 = world.spawn(ChildOf(root)).id();
/// let grandchild = world.spawn(ChildOf(child1)).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();
///
/// assert_eq!(&**world.entity(root).get::<Children>().unwrap(), &[child1, child2]);
/// assert_eq!(&**world.entity(child1).get::<Children>().unwrap(), &[grandchild]);
@ -94,12 +94,15 @@ use log::warn;
)]
#[relationship(relationship_target = Children)]
#[doc(alias = "IsChild", alias = "Parent")]
pub struct ChildOf(pub Entity);
pub struct ChildOf {
/// The parent entity of this child entity.
pub parent: Entity,
}
impl ChildOf {
/// Returns the parent entity, which is the "target" of this relationship.
pub fn get(&self) -> Entity {
self.0
self.parent
}
}
@ -108,7 +111,7 @@ impl Deref for ChildOf {
#[inline]
fn deref(&self) -> &Self::Target {
&self.0
&self.parent
}
}
@ -119,7 +122,9 @@ impl Deref for ChildOf {
impl FromWorld for ChildOf {
#[inline(always)]
fn from_world(_world: &mut World) -> Self {
ChildOf(Entity::PLACEHOLDER)
ChildOf {
parent: Entity::PLACEHOLDER,
}
}
}
@ -196,9 +201,9 @@ impl<'w> EntityWorldMut<'w> {
///
/// [`with_children`]: EntityWorldMut::with_children
pub fn with_child(&mut self, bundle: impl Bundle) -> &mut Self {
let id = self.id();
let parent = self.id();
self.world_scope(|world| {
world.spawn((bundle, ChildOf(id)));
world.spawn((bundle, ChildOf { parent }));
});
self
}
@ -213,7 +218,7 @@ 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(entity))")]
pub fn set_parent(&mut self, parent: Entity) -> &mut Self {
self.insert(ChildOf(parent));
self.insert(ChildOf { parent });
self
}
}
@ -244,8 +249,8 @@ impl<'a> EntityCommands<'a> {
///
/// [`with_children`]: EntityCommands::with_children
pub fn with_child(&mut self, bundle: impl Bundle) -> &mut Self {
let id = self.id();
self.commands.spawn((bundle, ChildOf(id)));
let parent = self.id();
self.commands.spawn((bundle, ChildOf { parent }));
self
}
@ -259,7 +264,7 @@ 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(entity))")]
pub fn set_parent(&mut self, parent: Entity) -> &mut Self {
self.insert(ChildOf(parent));
self.insert(ChildOf { parent });
self
}
}
@ -375,9 +380,9 @@ mod tests {
fn hierarchy() {
let mut world = World::new();
let root = world.spawn_empty().id();
let child1 = world.spawn(ChildOf(root)).id();
let grandchild = world.spawn(ChildOf(child1)).id();
let child2 = world.spawn(ChildOf(root)).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();
// Spawn
let hierarchy = get_hierarchy(&world, root);
@ -398,7 +403,7 @@ mod tests {
assert_eq!(hierarchy, Node::new_with(root, vec![Node::new(child2)]));
// Insert
world.entity_mut(child1).insert(ChildOf(root));
world.entity_mut(child1).insert(ChildOf { parent: root });
let hierarchy = get_hierarchy(&world, root);
assert_eq!(
hierarchy,
@ -457,7 +462,7 @@ mod tests {
fn self_parenting_invalid() {
let mut world = World::new();
let id = world.spawn_empty().id();
world.entity_mut(id).insert(ChildOf(id));
world.entity_mut(id).insert(ChildOf { parent: id });
assert!(
world.entity(id).get::<ChildOf>().is_none(),
"invalid ChildOf relationships should self-remove"
@ -469,7 +474,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"
@ -480,10 +485,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"
);

View File

@ -35,12 +35,24 @@ use log::warn;
///
/// [`Relationship`] and [`RelationshipTarget`] should always be derived via the [`Component`] trait to ensure the hooks are set up properly.
///
/// ## Derive
///
/// [`Relationship`] and [`RelationshipTarget`] can only be derived for structs with a single unnamed field, single named field
/// or for named structs where one field is annotated with `#[relationship]`.
/// If there are additional fields, they must all implement [`Default`].
///
/// [`RelationshipTarget`] also requires that the relationship field is private to prevent direct mutation,
/// ensuring the correctness of relationships.
/// ```
/// # use bevy_ecs::component::Component;
/// # use bevy_ecs::entity::Entity;
/// #[derive(Component)]
/// #[relationship(relationship_target = Children)]
/// pub struct ChildOf(pub Entity);
/// pub struct ChildOf {
/// #[relationship]
/// pub child: Entity,
/// internal: u8,
/// };
///
/// #[derive(Component)]
/// #[relationship_target(relationship = ChildOf)]

View File

@ -277,9 +277,9 @@ mod tests {
let mut world = World::new();
let a = world.spawn_empty().id();
let b = world.spawn(ChildOf(a)).id();
let c = world.spawn(ChildOf(a)).id();
let d = world.spawn(ChildOf(b)).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();
world
.entity_mut(a)

View File

@ -375,8 +375,22 @@ 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(tab_group_entity))).id();
let tab_entity_2 = world.spawn((TabIndex(1), ChildOf(tab_group_entity))).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 mut system_state: SystemState<TabNavigation> = SystemState::new(world);
let tab_navigation = system_state.get(world);

View File

@ -786,7 +786,9 @@ mod test {
.entity_mut(parent2)
.insert(Visibility::Visible);
// Simulate a change in the parent component
app.world_mut().entity_mut(child2).insert(ChildOf(parent2)); // example of changing parent
app.world_mut()
.entity_mut(child2)
.insert(ChildOf { parent: parent2 }); // example of changing parent
// Run the system again to propagate changes
app.update();

View File

@ -524,7 +524,7 @@ pub fn detect_text_needs_rerender<Root: Component>(
));
continue;
};
let mut parent: Entity = span_parent.0;
let mut parent: Entity = span_parent.get();
// 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_parent.0;
parent = next_parent.get();
}
}
}

View File

@ -123,8 +123,8 @@ mod tests {
for transform in transforms {
let mut e = app.world_mut().spawn(transform);
if let Some(entity) = entity {
e.insert(ChildOf(entity));
if let Some(parent) = entity {
e.insert(ChildOf { parent });
}
entity = Some(e.id());

View File

@ -591,8 +591,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(root));
commands.entity(child).insert(ChildOf(parent));
commands.entity(parent).insert(ChildOf { parent: root });
commands.entity(child).insert(ChildOf { parent });
command_queue.apply(&mut world);
schedule.run(&mut world);

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(|parent| parent.0)
self.parents_query.get(entity).ok().map(|p| p.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.

View File

@ -177,7 +177,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.0) else {
let Ok(computed_target) = computed_target_query.get(child_of.parent) else {
continue;
};

View File

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

View File

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

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.get();
let mesh_node_entity = child_of.parent;
// Get `Children` in the mesh node.
let mesh_node_parent = parents.get(mesh_node_entity).unwrap();

View File

@ -117,7 +117,7 @@ fn button_system(
// Update button labels to match their parent counter
for (children, child_of) in &labels_query {
let counter = counter_query.get(child_of.get()).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();