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

[Merged by Bors] - Add a method for mapping Mut<T> -> Mut<U> #6199

Closed
wants to merge 10 commits into from
Closed

[Merged by Bors] - Add a method for mapping Mut<T> -> Mut<U> #6199

wants to merge 10 commits into from

Conversation

JoJoJet
Copy link
Member

@JoJoJet JoJoJet commented Oct 8, 2022

Objective

When designing an API, you may wish to provide access only to a specific field of a component or resource. The current options for doing this in safe code are

  • *Mut::into_inner, which flags a change no matter what.
  • *Mut::bypass_change_detection, which misses all changes.

Solution

Add the method map_unchanged.

Example

// When run, zeroes the translation of every entity.
fn reset_all(mut transforms: Query<&mut Transform>) {
    for transform in &mut transforms {
        // We pinky promise not to modify `t` within the closure.
        let translation = transform.map_unchanged(|t| &mut t.translation);
        // Only reset the translation if it isn't already zero.
        translation.set_if_not_equal(Vec2::ZERO);
    }
}

Changelog

  • Added the method map_unchanged to types Mut<T>, ResMut<T>, and NonSendMut<T>.

@inodentry
Copy link
Contributor

I don't quite understand why this is necessary? What are you planning to use it for?

@maniwani
Copy link
Contributor

maniwani commented Oct 8, 2022

I think #4413 is related.

@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 labels Oct 8, 2022
@alice-i-cecile
Copy link
Member

Clever. If we can improve the docs I'm on board.

MrGVSV
MrGVSV approved these changes Oct 8, 2022
View reviewed changes
crates/bevy_ecs/src/change_detection.rs Outdated Show resolved Hide resolved
crates/bevy_ecs/src/change_detection.rs Show resolved Hide resolved
@maniwani
Copy link
Contributor

maniwani commented Oct 8, 2022 •
edited

Should this method be unsafe? There's no memory safety issue, but the "caller pinky promises not to mutate" part seems like it shares the sentiment. (Edit: Like we're asking the user to uphold our "change detection is reliable" invariant, but I guess bypass_change_detection isn't marked as unsafe either.)

@JoJoJet
Copy link
Member Author

JoJoJet commented Oct 8, 2022

Should this method be unsafe?

I don't think there's any need for it to be unsafe, but I'm willing to change it if the consensus disagrees with me.

@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 Oct 8, 2022
@mockersf
Copy link
Member

mockersf commented Oct 9, 2022

This is not unsafe, it's "if you do dumb things, dumb things will happen". Until we can declare our own function colours in Rust, we can't really mark those 🤷

@mockersf
Copy link
Member

mockersf commented Oct 9, 2022

@TheRawMeatball should this close #4413 if it's merged?

@alice-i-cecile
Copy link
Member

bors r+

@alice-i-cecile alice-i-cecile mentioned this pull request Oct 10, 2022
Closed
bors bot pushed a commit that referenced this pull request Oct 10, 2022
@JoJoJet
Add a method for mapping Mut<T> -> Mut<U> (#6199)
2cff227 
# Objective

When designing an API, you may wish to provide access only to a specific field of a component or resource. The current options for doing this in safe code are

* `*Mut::into_inner`, which flags a change no matter what.
* `*Mut::bypass_change_detection`, which misses all changes.

## Solution

Add the method `map_unchanged`.

### Example

```rust
// When run, zeroes the translation of every entity.
fn reset_all(mut transforms: Query<&mut Transform>) {
    for transform in &mut transforms {
        // We pinky promise not to modify `t` within the closure.
        let translation = transform.map_unchanged(|t| &mut t.translation);
        // Only reset the translation if it isn't already zero.
        translation.set_if_not_equal(Vec2::ZERO);
    }
}
```

---

## Changelog

+ Added the method `map_unchanged` to types `Mut<T>`, `ResMut<T>`, and `NonSendMut<T>`.
@bors
Copy link
Contributor

bors bot commented Oct 10, 2022

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title Add a method for mapping Mut<T> -> Mut<U> [Merged by Bors] - Add a method for mapping Mut<T> -> Mut<U> Oct 10, 2022
@bors bors bot closed this Oct 10, 2022
@JoJoJet JoJoJet deleted the map-mut branch October 10, 2022 17:33
@mockersf mockersf added the hacktoberfest-accepted A PR that was accepted for Hacktoberfest, an annual open source event label Oct 10, 2022
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 19, 2022
@JoJoJet @james7132
Add a method for mapping Mut<T> -> Mut<U> (bevyengine#6199)
d103067 
# Objective

When designing an API, you may wish to provide access only to a specific field of a component or resource. The current options for doing this in safe code are

* `*Mut::into_inner`, which flags a change no matter what.
* `*Mut::bypass_change_detection`, which misses all changes.

## Solution

Add the method `map_unchanged`.

### Example

```rust
// When run, zeroes the translation of every entity.
fn reset_all(mut transforms: Query<&mut Transform>) {
    for transform in &mut transforms {
        // We pinky promise not to modify `t` within the closure.
        let translation = transform.map_unchanged(|t| &mut t.translation);
        // Only reset the translation if it isn't already zero.
        translation.set_if_not_equal(Vec2::ZERO);
    }
}
```

---

## Changelog

+ Added the method `map_unchanged` to types `Mut<T>`, `ResMut<T>`, and `NonSendMut<T>`.
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022
@JoJoJet @james7132
Add a method for mapping Mut<T> -> Mut<U> (bevyengine#6199)
b774e16 
# Objective

When designing an API, you may wish to provide access only to a specific field of a component or resource. The current options for doing this in safe code are

* `*Mut::into_inner`, which flags a change no matter what.
* `*Mut::bypass_change_detection`, which misses all changes.

## Solution

Add the method `map_unchanged`.

### Example

```rust
// When run, zeroes the translation of every entity.
fn reset_all(mut transforms: Query<&mut Transform>) {
    for transform in &mut transforms {
        // We pinky promise not to modify `t` within the closure.
        let translation = transform.map_unchanged(|t| &mut t.translation);
        // Only reset the translation if it isn't already zero.
        translation.set_if_not_equal(Vec2::ZERO);
    }
}
```

---

## Changelog

+ Added the method `map_unchanged` to types `Mut<T>`, `ResMut<T>`, and `NonSendMut<T>`.
Pietrek14 pushed a commit to Pietrek14/bevy that referenced this pull request Dec 17, 2022
@JoJoJet @Pietrek14
Add a method for mapping Mut<T> -> Mut<U> (bevyengine#6199)
86b4ede 
# Objective

When designing an API, you may wish to provide access only to a specific field of a component or resource. The current options for doing this in safe code are

* `*Mut::into_inner`, which flags a change no matter what.
* `*Mut::bypass_change_detection`, which misses all changes.

## Solution

Add the method `map_unchanged`.

### Example

```rust
// When run, zeroes the translation of every entity.
fn reset_all(mut transforms: Query<&mut Transform>) {
    for transform in &mut transforms {
        // We pinky promise not to modify `t` within the closure.
        let translation = transform.map_unchanged(|t| &mut t.translation);
        // Only reset the translation if it isn't already zero.
        translation.set_if_not_equal(Vec2::ZERO);
    }
}
```

---

## Changelog

+ Added the method `map_unchanged` to types `Mut<T>`, `ResMut<T>`, and `NonSendMut<T>`.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
@JoJoJet @ItsDoot
Add a method for mapping Mut<T> -> Mut<U> (bevyengine#6199)
9a1651b 
# Objective

When designing an API, you may wish to provide access only to a specific field of a component or resource. The current options for doing this in safe code are

* `*Mut::into_inner`, which flags a change no matter what.
* `*Mut::bypass_change_detection`, which misses all changes.

## Solution

Add the method `map_unchanged`.

### Example

```rust
// When run, zeroes the translation of every entity.
fn reset_all(mut transforms: Query<&mut Transform>) {
    for transform in &mut transforms {
        // We pinky promise not to modify `t` within the closure.
        let translation = transform.map_unchanged(|t| &mut t.translation);
        // Only reset the translation if it isn't already zero.
        translation.set_if_not_equal(Vec2::ZERO);
    }
}
```

---

## Changelog

+ Added the method `map_unchanged` to types `Mut<T>`, `ResMut<T>`, and `NonSendMut<T>`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@afonsolage afonsolage afonsolage approved these changes

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

@TheRawMeatball TheRawMeatball TheRawMeatball approved these changes

@mockersf mockersf mockersf approved these changes

@MrGVSV MrGVSV MrGVSV 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 hacktoberfest-accepted A PR that was accepted for Hacktoberfest, an annual open source event 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.

None yet
8 participants
@JoJoJet @inodentry @maniwani @alice-i-cecile @mockersf @afonsolage @TheRawMeatball @MrGVSV