Skip to content

Add EntityWorldMut::(try_)resource_scope #20162

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

Merged

Conversation

BigWingBeat
Copy link
Contributor

Objective

Fixes #20139

Solution

Implement the methods, and leverage them where applicable

Testing

Added unit tests


Showcase

let id = entity_world_mut.id();
let world = entity_world_mut.into_world_mut();
world.resource_scope::<_, _>(|world, res| {
    let entity_world_mut = world.entity_mut(id);
    /* ... */
});

becomes

entity_world_mut.resource_scope::<_, _>(|entity, res| {
    /* ... */
});

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use X-Uncontroversial This work is generally agreed upon D-Straightforward Simple bug fixes and API improvements, docs, test and examples labels Jul 16, 2025
@alice-i-cecile alice-i-cecile added the S-Needs-Review Needs reviewer attention (from anyone!) to move forward label Jul 16, 2025
if world.contains_resource::<EventRegistry>() {
world.resource_scope(|world, mut registry: Mut<EventRegistry>| {
registry.run_updates(world, *last_change_tick);
world.try_resource_scope(|world, mut registry: Mut<EventRegistry>| {
Copy link
Member

Choose a reason for hiding this comment

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

I don't quite see how this change was necessary for this PR? That said it's a good change, so I'm happy to keep it :)

@alice-i-cecile alice-i-cecile added the C-Code-Quality A section of code that is hard to understand or change label Jul 16, 2025
};
update_transform();
});
// FIXME: Replace this closure with a `try` block. See: https://github.com/rust-lang/rust/issues/31436.
Copy link
Member

Choose a reason for hiding this comment

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

The changes to this file similarly seem unrelated, but I also like the cleanup.

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.

Very solid implementation, tested, documented and something that we definitely want.

I don't love tackling cleanup in the same PR, but I won't look a gift horse in the mouth. The cleanup is vaguely related, and well done; might as well merge it.

My only blocking complaint is that I feel strongly that the TODOs in bevy_transform should not be done; we should remove them.

@alice-i-cecile alice-i-cecile added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jul 16, 2025
@BigWingBeat
Copy link
Contributor Author

I don't love tackling cleanup in the same PR, but I won't look a gift horse in the mouth. The cleanup is vaguely related, and well done; might as well merge it.

Yeah, it's just easy opportunistic stuff I noticed while looking at the _scope APIs. I suppose it's just somewhat of a habit from solo-dev projects, where PR hygiene isn't as important. I'll keep this in mind for the future.

@alice-i-cecile alice-i-cecile added S-Needs-Review Needs reviewer attention (from anyone!) to move forward and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Jul 16, 2025
@alice-i-cecile alice-i-cecile added this to the 0.17 milestone Jul 16, 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 Jul 16, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jul 16, 2025
Merged via the queue into bevyengine:main with commit e7c14bd Jul 16, 2025
36 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-Code-Quality A section of code that is hard to understand or change C-Usability A targeted quality-of-life change that makes Bevy easier to use 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 X-Uncontroversial This work is generally agreed upon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EntityWorldMut::(try_)resource_scope
3 participants