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

as_deref_mut() method for Mut-like types #9912

Merged
merged 5 commits into from
Oct 2, 2023
Merged

as_deref_mut() method for Mut-like types #9912

merged 5 commits into from
Oct 2, 2023

Conversation

Testare
Copy link
Contributor

@Testare Testare commented Sep 23, 2023 •
edited
Loading

Objective

Add a new method so you can do set_if_neq with dereferencing components: as_deref_mut()!

Solution

Added an as_deref_mut method so that we can use set_if_neq() without having to wrap up types for derefencable components

@JoJoJet
Copy link
Member

JoJoJet commented Sep 23, 2023

I feel like as_deref would be more generally useful. That would allow you to do .as_deref_mut().set_if_neq(...). Even though it's longer, I think that chaining two methods is less of a mouthful than set_deref_if_neq

@Testare
Copy link
Contributor Author

Testare commented Sep 26, 2023

I agree tbh, but I didn't think of it until the PR ready haha. Updated the PR. I didn't add an as_deref method because there isn't really a point of a Mut that isn't mutable, but if I didn't keep the _mut I think people might be surprised that it requires DerefMut.

@Testare Testare changed the title set_deref_if_neq method as_deref_mut() method for Mut-like types Sep 26, 2023
JoJoJet
JoJoJet approved these changes Sep 26, 2023
View reviewed changes
Copy link
Member

@JoJoJet JoJoJet left a comment

Choose a reason for hiding this comment

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

 I didn't add an as_deref method because there isn't really a point of a Mut that isn't mutable

I agree, a Ref variant is unnecessary. Similarly, we have map_unchanged for Mut but not for Ref. Generally there's just less of a need to map Ref since it doesn't trigger anything when dereferenced.

crates/bevy_ecs/src/change_detection.rs Outdated Show resolved Hide resolved
@alice-i-cecile @JoJoJet
Formatting nit
e0e0434 
Co-authored-by: Joseph <21144246+JoJoJet@users.noreply.github.com>
@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Usability A simple 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 labels Sep 29, 2023
@Testare
Copy link
Contributor Author

Testare commented Sep 30, 2023

Thank you both! 🙏

@james7132 james7132 added this pull request to the merge queue Oct 2, 2023
Merged via the queue into bevyengine:main with commit dfdc9f8 Oct 2, 2023
24 checks passed
ameknite pushed a commit to ameknite/bevy that referenced this pull request Oct 3, 2023
@Testare @alice-i-cecile
@JoJoJet @ameknite
as_deref_mut() method for Mut-like types (bevyengine#9912)
4c9d9cd 
# Objective

Add a new method so you can do `set_if_neq` with dereferencing
components: `as_deref_mut()`!

## Solution

Added an as_deref_mut method so that we can use `set_if_neq()` without
having to wrap up types for derefencable components

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Joseph <21144246+JoJoJet@users.noreply.github.com>
ameknite pushed a commit to ameknite/bevy that referenced this pull request Oct 3, 2023
@Testare @alice-i-cecile
@JoJoJet @ameknite
as_deref_mut() method for Mut-like types (bevyengine#9912)
620bc50 
# Objective

Add a new method so you can do `set_if_neq` with dereferencing
components: `as_deref_mut()`!

## Solution

Added an as_deref_mut method so that we can use `set_if_neq()` without
having to wrap up types for derefencable components

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Joseph <21144246+JoJoJet@users.noreply.github.com>
regnarock pushed a commit to regnarock/bevy that referenced this pull request Oct 13, 2023
@Testare @alice-i-cecile @JoJoJet
as_deref_mut() method for Mut-like types (bevyengine#9912)
b65985e 
# Objective

Add a new method so you can do `set_if_neq` with dereferencing
components: `as_deref_mut()`!

## Solution

Added an as_deref_mut method so that we can use `set_if_neq()` without
having to wrap up types for derefencable components

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Joseph <21144246+JoJoJet@users.noreply.github.com>
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this pull request Jan 9, 2024
@Testare @alice-i-cecile @JoJoJet
as_deref_mut() method for Mut-like types (bevyengine#9912)
338fe38 
# Objective

Add a new method so you can do `set_if_neq` with dereferencing
components: `as_deref_mut()`!

## Solution

Added an as_deref_mut method so that we can use `set_if_neq()` without
having to wrap up types for derefencable components

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Joseph <21144246+JoJoJet@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JoJoJet JoJoJet JoJoJet approved these changes

@james7132 james7132 james7132 approved these changes

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

Assignees
No one assigned
Labels
A-ECS Entities, components, systems, and events C-Usability A simple 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.
4 participants
@Testare @JoJoJet @james7132 @alice-i-cecile