Skip to content
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

Add missing return in default Relationship::on_insert impl #17675

Merged
merged 2 commits into from
Feb 5, 2025

Conversation

grind086
Copy link
Contributor

@grind086 grind086 commented Feb 4, 2025

Objective

There was a bug in the default Relationship::on_insert implementation that caused it to not properly handle entities targeting themselves in relationships. The relationship component was properly removed, but it would go on to add itself to its own target component.

Solution

Added a missing return and a couple of tests (self_relationship_fails failed on its second assert prior to this PR).

Testing

See above.

@alice-i-cecile alice-i-cecile added this to the 0.16 milestone Feb 5, 2025
@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-ECS Entities, components, systems, and events labels Feb 5, 2025
@alice-i-cecile alice-i-cecile requested a review from cart February 5, 2025 18:38
@alice-i-cecile alice-i-cecile added the S-Needs-Review Needs reviewer attention (from anyone!) to move forward label Feb 5, 2025
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Good tests, and a solid explanation. Thanks :)

@alice-i-cecile alice-i-cecile added the D-Straightforward Simple bug fixes and API improvements, docs, test and examples label Feb 5, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Feb 5, 2025
@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Feb 5, 2025
Merged via the queue into bevyengine:main with commit 0335f34 Feb 5, 2025
33 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-Bug An unexpected or incorrect behavior D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants