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

Rename observe to observe_entity on EntityWorldMut #15616

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

kristoff3r
Copy link
Contributor

Objective

The current observers have some unfortunate footguns where you can end up confused about what is actually being observed. For apps you can chain observe like app.observe(..).observe(..) which works like you would expect, but if you try the same with world the first observe() will return the EntityWorldMut for the created observer, and the second observe() will only observe on the observer entity. It took several hours for multiple people on discord to figure this out, which is not a great experience.

Solution

Rename observe on entities to observe_entity. It's slightly more verbose when you know you have an entity, but it feels right to me that observers for specific things have more specific naming, and it prevents this issue completely.

Another possible solution would be to unify observe on App and World to have the same kind of return type, but I'm not sure exactly what that would look like.

Testing

Simple name change, so only concern is docs really.


Migration Guide

The observe() method on entities has been renamed to observe_entity() to prevent confusion about what is being observed in some cases.

@alice-i-cecile alice-i-cecile added C-Usability A targeted quality-of-life change that makes Bevy easier to use A-ECS Entities, components, systems, and events C-Breaking-Change A breaking change to Bevy's public API that needs to be noted in a migration guide labels Oct 3, 2024
@alice-i-cecile alice-i-cecile added this to the 0.15 milestone Oct 3, 2024
@alice-i-cecile alice-i-cecile added the S-Needs-Review Needs reviewer attention (from anyone!) to move forward label Oct 3, 2024
@alice-i-cecile alice-i-cecile added D-Trivial Nice and easy! A great choice to get started with Bevy 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 Oct 3, 2024
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-Breaking-Change A breaking change to Bevy's public API that needs to be noted in a migration guide C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Trivial Nice and easy! A great choice to get started with Bevy 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.

2 participants