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

Get ResMut of a resource from World #11765

Closed
Adamkob12 opened this issue Feb 7, 2024 · 5 comments
Closed

Get ResMut of a resource from World #11765

Adamkob12 opened this issue Feb 7, 2024 · 5 comments
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use X-Controversial There is active debate or serious implications around merging this PR

Comments

@Adamkob12
Copy link
Contributor

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.

Originally posted by @alice-i-cecile in #11561 (comment)

What problem does this solve or what need does it fill?

There should be a way to get change-detection-enabled, mutable access to a resource from the World. The type that's conventionally used for that is ResMut

What solution would you like?

Add a method to World that supports that / change the current implementation of resource_mut (and the like) to get ResMut instead of Mut and/or &mut T

What alternative(s) have you considered?

Keep as is

@Adamkob12 Adamkob12 added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Feb 7, 2024
@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 and removed C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Feb 7, 2024
@SpecificProtagonist
Copy link
Contributor

SpecificProtagonist commented Feb 7, 2024

What's the reason for needing both Mut & ResMut? Both are exclusive refs + change tracking, the difference between them is just that ResMut is a system param. There's also an From<ResMut> for Mut.

@lynn-lumen
Copy link
Contributor

lynn-lumen commented Feb 8, 2024

What's the reason for needing both Mut & ResMut?

ResMut<T> requires that T implements Resource whereas Mut<T> does not. Since the methods mentioned above are only concerned with Resources introducing this additional bound to them/their return types is sensible.

@SpecificProtagonist
Copy link
Contributor

The issue proposed two alternatives, the former being adding a new method, which I was curious about. I agree with the second one of changing the existing functions :)

@JoJoJet
Copy link
Member

JoJoJet commented Feb 12, 2024

We've always treated Mut<T> is bevy's primtive for change detection. The only reason that ResMut exists is because SystemParams don't have methods, so you need to use the type to specify that you are requesting a resource via the type. There's no other reason to use a ResMut instead of a Mut, so we should use them in as few places as possible. We already do this, see how ResMut::map_unchanged and ResMut::reborrow return Mut.

@JoJoJet JoJoJet added the X-Controversial There is active debate or serious implications around merging this PR label Feb 12, 2024
@JoJoJet
Copy link
Member

JoJoJet commented Feb 12, 2024

Thank you for the issue, but I'm going to close this since we're likely going to move in the direction described in #11825

@JoJoJet JoJoJet closed this as not planned Won't fix, can't repro, duplicate, stale Feb 12, 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-Usability A targeted quality-of-life change that makes Bevy easier to use X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants