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

[Merged by Bors] - Conversion of ResMut and NonSendMut to Mut #5438

Closed
wants to merge 3 commits into from

Conversation

djeedai
Copy link
Contributor

@djeedai djeedai commented Jul 23, 2022

Objective

Enable treating components and resources equally, which can
simplify the implementation of some systems where only the change
detection feature is relevant and not the kind of object (resource or
component).

Solution

Implement From<ResMut<T>> and From<NonSendMut<T>> for
Mut. Since the 3 structs are similar, and only differ by their system
param role, the conversion is trivial.


Changelog

Added - From<ResMut> and From<NonSendMut> for Mut<T>.

Add a new `into_mut()` method allowing to convert a `ResMut` or
`NonSendMut` into a simple `Mut`.

This enables treating components and resources equally, which can
simplify the implementation of some systems where only the change
detection feature is relevant and not the kind of object (resource or
component).
@alice-i-cecile
Copy link
Member

I like the idea! IMO this should just be a From trait implementation though; that captures the semantics exactly and is more convenient and conventional.

@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 Jul 23, 2022
@alice-i-cecile alice-i-cecile 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 Jul 24, 2022
@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Jul 25, 2022
# Objective

Enable treating components and resources equally, which can
simplify the implementation of some systems where only the change
detection feature is relevant and not the kind of object (resource or
component).

## Solution

Implement `From<ResMut<T>>` and `From<NonSendMut<T>>` for
`Mut`. Since the 3 structs are similar, and only differ by their system
param role, the conversion is trivial.

---

## Changelog

Added - `From<ResMut>` and `From<NonSendMut>` for `Mut<T>`.
@bors
Copy link
Contributor

bors bot commented Jul 25, 2022

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Jul 25, 2022
# Objective

Enable treating components and resources equally, which can
simplify the implementation of some systems where only the change
detection feature is relevant and not the kind of object (resource or
component).

## Solution

Implement `From<ResMut<T>>` and `From<NonSendMut<T>>` for
`Mut`. Since the 3 structs are similar, and only differ by their system
param role, the conversion is trivial.

---

## Changelog

Added - `From<ResMut>` and `From<NonSendMut>` for `Mut<T>`.
@bors bors bot changed the title Conversion of ResMut and NonSendMut to Mut [Merged by Bors] - Conversion of ResMut and NonSendMut to Mut Jul 25, 2022
@bors bors bot closed this Jul 25, 2022
@djeedai djeedai deleted the into_mut branch July 25, 2022 21:56
@@ -206,6 +217,17 @@ change_detection_impl!(NonSendMut<'a, T>, T,);
impl_into_inner!(NonSendMut<'a, T>, T,);
impl_debug!(NonSendMut<'a, T>,);

impl<'a, T: Resource> From<NonSendMut<'a, T>> for Mut<'a, T> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is wrong, should have been:

Suggested change
impl<'a, T: Resource> From<NonSendMut<'a, T>> for Mut<'a, T> {
impl<'a, T: 'static> From<NonSendMut<'a, T>> for Mut<'a, T> {

FYI @alice-i-cecile , @maniwani

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Logged as #5456

inodentry pushed a commit to IyesGames/bevy that referenced this pull request Aug 8, 2022
# Objective

Enable treating components and resources equally, which can
simplify the implementation of some systems where only the change
detection feature is relevant and not the kind of object (resource or
component).

## Solution

Implement `From<ResMut<T>>` and `From<NonSendMut<T>>` for
`Mut`. Since the 3 structs are similar, and only differ by their system
param role, the conversion is trivial.

---

## Changelog

Added - `From<ResMut>` and `From<NonSendMut>` for `Mut<T>`.
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022
# Objective

Enable treating components and resources equally, which can
simplify the implementation of some systems where only the change
detection feature is relevant and not the kind of object (resource or
component).

## Solution

Implement `From<ResMut<T>>` and `From<NonSendMut<T>>` for
`Mut`. Since the 3 structs are similar, and only differ by their system
param role, the conversion is trivial.

---

## Changelog

Added - `From<ResMut>` and `From<NonSendMut>` for `Mut<T>`.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

Enable treating components and resources equally, which can
simplify the implementation of some systems where only the change
detection feature is relevant and not the kind of object (resource or
component).

## Solution

Implement `From<ResMut<T>>` and `From<NonSendMut<T>>` for
`Mut`. Since the 3 structs are similar, and only differ by their system
param role, the conversion is trivial.

---

## Changelog

Added - `From<ResMut>` and `From<NonSendMut>` for `Mut<T>`.
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 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