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.

Sign up for GitHub

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

Jump to bottom

added ability to get Res<T> from World with World::get_resource_ref #11561

Merged
merged 9 commits into from
Jan 28, 2024
Merged

added ability to get Res<T> from World with World::get_resource_ref #11561

merged 9 commits into from
Jan 28, 2024

Conversation

atornity
Copy link
Contributor

Objective

It's sometimes desirable to get a Res<T> rather than &T from World::get_resource.
Alternative to #9940, partly adresses #9926

Solution

added additional methods to World and UnsafeWorldCell to retrieve a resource wrapped in a Res.

  • UnsafeWorldCell::get_resource_ref
  • World::get_resource_ref
  • World::resource_ref

I can change it so World::resource_mut returns ResMut instead of Mut as well if that's desired, but that could also be added later in a seperate pr.

@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 labels Jan 27, 2024
@alice-i-cecile
Copy link
Member

I can change it so World::resource_mut returns ResMut instead of Mut as well if that's desired, but that could also be added later in a seperate pr.

Let's keep this split this out.

hymm
hymm approved these changes Jan 27, 2024
View reviewed changes
Copy link
Contributor

@hymm hymm left a comment

Choose a reason for hiding this comment

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

Looks good. Just some minor comments on wording.

crates/bevy_ecs/src/world/mod.rs Outdated Show resolved Hide resolved
crates/bevy_ecs/src/world/unsafe_world_cell.rs Outdated Show resolved Hide resolved
crates/bevy_ecs/src/world/unsafe_world_cell.rs Outdated Show resolved Hide resolved
@hymm hymm added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jan 27, 2024
@alice-i-cecile @hymm
Doc nits
5f60bb4 
Co-authored-by: Mike <mike.hsu@gmail.com>
@alice-i-cecile
Copy link
Member

alice-i-cecile commented Jan 27, 2024 •
edited
Loading

CI failure is real, but doesn't seem related to your PR.

     --> crates/bevy_ecs/src/world/mod.rs:1911:74
     |
1911 |     /// invalidating all [`Entity`] and resource fetches such as [`Res`](crate::system::Res), [`ResMut`](crate::system::ResMut)
     |                                                                   -----  ^^^^^^^^^^^^^^^^^^ explicit target is redundant
     |                                                                   |
     |                                                                   because label contains path that resolves to same destination

@mockersf
Copy link
Member

mockersf commented Jan 27, 2024 •
edited
Loading

CI failure is real, but doesn't seem related to your PR.

it is? Res is now imported by this PR, so CI is telling that the explicit path is not needed anymore

auto-merge was automatically disabled January 27, 2024 22:23

Head branch was pushed to by a user without write access

@atornity
Copy link
Contributor Author

CI failure is real, but doesn't seem related to your PR.

it is? Res is now imported by this PR, so CI is telling that the explicit path is not needed anymore

yup, should be fixed now

@Adamkob12 Adamkob12 mentioned this pull request Jan 27, 2024
Closed
@alice-i-cecile @MinerSebas
Doc fix
3c51ec3 
Co-authored-by: MinerSebas <66798382+MinerSebas@users.noreply.github.com>
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jan 28, 2024
Merged via the queue into bevyengine:main with commit 2391e44 Jan 28, 2024
22 checks passed
tjamaan pushed a commit to tjamaan/bevy that referenced this pull request Feb 6, 2024
@atornity @alice-i-cecile
@hymm @MinerSebas @tjamaan
added ability to get Res<T> from World with `World::get_resource_…
d7e3b1a 
…ref` (bevyengine#11561)

# Objective

It's sometimes desirable to get a `Res<T>` rather than `&T` from
`World::get_resource`.
Alternative to bevyengine#9940, partly adresses bevyengine#9926

## Solution

added additional methods to `World` and `UnsafeWorldCell` to retrieve a
resource wrapped in a `Res`.
- `UnsafeWorldCell::get_resource_ref`
- `World::get_resource_ref`
- `World::resource_ref`

I can change it so `World::resource_mut` returns `ResMut` instead of
`Mut` as well if that's desired, but that could also be added later in a
seperate pr.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Mike <mike.hsu@gmail.com>
Co-authored-by: MinerSebas <66798382+MinerSebas@users.noreply.github.com>
@Adamkob12 Adamkob12 mentioned this pull request Feb 7, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MinerSebas MinerSebas MinerSebas left review comments

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

@hymm hymm hymm approved these changes

Assignees
No one assigned
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@atornity @alice-i-cecile @mockersf @hymm @MinerSebas