Skip to content

Conversation

@alice-i-cecile
Copy link
Member

Context

Renaming Parent to ChildOf in #17247 has been contentious. While those users concerns are valid (especially around legibility of code IMO!), @cart has decided to stick with the new name.

In general this conversation is unsurprising to me, as it played out essentially the same way when I asked for opinions in my PR. There are strong opinions on both sides. Everyone is right in their own way.

I chose ChildOf for the following reasons:

  1. I think it derives naturally from the system we have built, the concepts we have chosen, and how we generally name the types that implement a trait in Rust. This is the name of the type implementing Relationship. We are adding that Relationship component to a given entity (whether it "is" the relationship or "has" the relationship is kind of immaterial ... we are naming the relationship that it "is" or "has"). What is the name of the relationship that a child has to its parent? It is a "child" of the parent of course!
  2. In general the non-parent/child relationships I've seen in the wild generally benefit from (or need to) use the naming convention in (1) (aka calling the Relationship the name of the relationship the entity has). Many relationships don't have an equivalent to the Parent/Child name concept.
  3. I do think we could get away with using (1) for pretty much everything else and special casing Parent/Children. But by embracing the naming convention, we help establish that this is in fact a pattern, and we help prime people to think about these things in a consistent way. Consistency and predictability is a generally desirable property. And for something as divisive and polarizing as relationship naming, I think drawing a hard line in the sand is to the benefit of the community as a whole.
  4. I believe the fact that we dont see as much of the XOf naming style elsewhere is to our benefit. When people see things in that style, they are primed to think of them as relationships (after some exposure to Bevy and the ecosystem). I consider this a useful hint.
  5. Most of the practical confusion from using ChildOf seems to be from calling the value of the target field we read from the relationship child_of. The name of the target field should be parent (we could even consider renaming child_of.0 to child_of.parent for clarity). I suspect that existing Bevy users renaming their existing code will feel the most friction here, as this requires a reframing. Imo it is natural and expected to receive pushback from these users hitting this case.

Objective

The new documentation doesn't do a particularly good job at quickly explaining the meaning of each component or how to work with them; making a tricky migration more painful and slowing down new users as they learn about some of the most fundamental types in Bevy.

Solution

  1. Clearly explain what each component does in the very first line, assuming no background knowledge. This is the first relationships that 99% of users will encounter, so explaining that they are relationships is unhelpful as an introduction.
  2. Add doc aliases for the rejected IsParent/IsChild/Parent names, to improve autocomplete and doc searching.
  3. Do some assorted docs cleanup while we're here.

@alice-i-cecile alice-i-cecile added C-Docs An addition or correction to our documentation A-ECS Entities, components, systems, and events X-Uncontroversial This work is generally agreed upon D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Feb 16, 2025
Copy link
Contributor

@ElliottjPierce ElliottjPierce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really great docs. As someone who hasn't messed with relationships that much, I think this is a brilliant introduction.

use log::warn;

/// A [`Relationship`](crate::relationship::Relationship) component that creates the canonical
/// Stores the parent entity of this child entity with this component.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Stores the parent entity of this child entity with this component.
/// This component stores the parent entity of this child entity.

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Feb 17, 2025
Copy link
Contributor

@Carter0 Carter0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

Merged via the queue into bevyengine:main with commit be3c6f7 Feb 17, 2025
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ECS Entities, components, systems, and events C-Docs An addition or correction to our documentation D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward X-Uncontroversial This work is generally agreed upon

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants