-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Self-referential relationships #22269
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR enables entities to have self-referential relationships by adding an optional allow_self attribute to the #[relationship] macro. By default, relationships pointing to their own entity are rejected with a warning, but this can now be explicitly allowed for semantically valid use cases like Likes(self), EmployedBy(self), or Healing(self).
Key changes:
- Added
ALLOW_SELF: bool = falseconstant to theRelationshiptrait with appropriate documentation warnings about infinite loops - Extended the macro parser to accept the
allow_selfattribute alongsiderelationship_target - Added comprehensive tests covering insertion and removal of self-referential relationships
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| release-content/release-notes/allow_self_relationships.md | Release notes documenting the new allow_self feature with usage examples |
| crates/bevy_ecs/src/relationship/mod.rs | Added ALLOW_SELF constant to trait, updated validation logic to check the flag, added documentation and tests |
| crates/bevy_ecs/macros/src/lib.rs | Added documentation examples for allow_self attribute and fixed existing typo in relationship_target example |
| crates/bevy_ecs/macros/src/component.rs | Extended parser to handle allow_self keyword and generate correct trait implementation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
apologies for the copilot noise, didnt realize this auto-review thing was on. |
|
I feel |
|
how about |
|
maybe allow_self_referential |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor notes, but nothing blocking :) I personally would prefer keeping the short and sweet allow_self, but allow_self_referential is also fine.
Co-authored-by: Jan Hohenheim <[email protected]>
| /// | ||
| /// When `ALLOW_SELF` is `true`, be careful when using recursive traversal methods | ||
| /// like `iter_ancestors` or `root_ancestor`, as they may loop infinitely if an entity | ||
| /// like `iter_ancestors` or `root_ancestor`, as they will loop infinitely if an entity |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking, could it be changed to be Relationship<const Allow_Self: bool> so that Traversal can check for Relationship<false>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, wait, you should still be able to create traversal on self referential
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a definitely a really useful feature, but the allow_self name feels potentially confusing to me. Even though allow_self has obviously nothing to do with self parameters, they are so ubiquitous in Rust, I think it would be a struggle to disassociate the two things.
Maybe reflexive would be better. It's a bit more mathy but the word precisely means something that is self referential.
Objective
Closes #18522
This pr allows entities to have, erm, self-relations..
By default pointing a relationship to its own entity will log a warning and remove the component. However, self-referential relationships are semantically valid in many cases:
Likes(self),EmployedBy(self),TalkingTo(self),Healing(self).Solution
const ALLOW_SELF:bool = falseto theRelationshiptrait,allow_selfflag to the macroTesting
Showcase
Relationships can now optionally point to their own entity by setting
#[relationship(allow_self)].By default pointing a relationship to its own entity will log a warning and remove the component. However, self-referential relationships are semantically valid in many cases:
Likes(self),EmployedBy(self),TalkingTo(self),Healing(self), and many more.Click to view usage
To allow a relationship to point to its own entity, add the
allow_selfattribute:Now entities can have relationships that point to themselves: