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] - Use a default implementation for set_if_neq #7660

Closed
wants to merge 8 commits into from

Conversation

JoJoJet
Copy link
Member

@JoJoJet JoJoJet commented Feb 13, 2023

Objective

While porting my crate bevy_trait_query to bevy 0.10, I ran into an issue with the DetectChangesMut trait. Due to the way that the set_if_neq method (added in #6853) is implemented, you are forced to write a nonsense implementation of it for dynamically sized types. This edge case shows up when implementing trait queries, since DetectChangesMut is implemented for Mut<dyn Trait>.

Solution

Simplify the generics for set_if_neq and add the where Self::Target: Sized trait bound to it. Add a default implementation so implementers don't need to implement a method with nonsensical trait bounds.

@JoJoJet JoJoJet added C-Bug An unexpected or incorrect behavior A-ECS Entities, components, systems, and events C-Usability A simple quality-of-life change that makes Bevy easier to use labels Feb 13, 2023
@JoJoJet JoJoJet removed the C-Bug An unexpected or incorrect behavior label Feb 14, 2023
@JoJoJet JoJoJet changed the title Allow change detection for dynamically sized types. Use a default implementation for set_if_neq Feb 14, 2023
@alice-i-cecile alice-i-cecile added the C-Code-Quality A section of code that is hard to understand or change label Feb 15, 2023
@james7132 james7132 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 Feb 15, 2023
@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Feb 15, 2023
# Objective

While porting my crate `bevy_trait_query` to bevy 0.10, I ran into an issue with the `DetectChangesMut` trait. Due to the way that the `set_if_neq` method (added in #6853) is implemented, you are forced to write a nonsense implementation of it for dynamically sized types. This edge case shows up when implementing trait queries, since `DetectChangesMut` is implemented for `Mut<dyn Trait>`.

## Solution

Simplify the generics for `set_if_neq` and add the `where Self::Target: Sized` trait bound to it. Add a default implementation so implementers don't need to implement a method with nonsensical trait bounds.
@bors bors bot changed the title Use a default implementation for set_if_neq [Merged by Bors] - Use a default implementation for set_if_neq Feb 15, 2023
@bors bors bot closed this Feb 15, 2023
@JoJoJet JoJoJet deleted the unsized-change-detection branch February 16, 2023 02:46
myreprise1 pushed a commit to myreprise1/bevy that referenced this pull request Feb 16, 2023
# Objective

While porting my crate `bevy_trait_query` to bevy 0.10, I ran into an issue with the `DetectChangesMut` trait. Due to the way that the `set_if_neq` method (added in bevyengine#6853) is implemented, you are forced to write a nonsense implementation of it for dynamically sized types. This edge case shows up when implementing trait queries, since `DetectChangesMut` is implemented for `Mut<dyn Trait>`.

## Solution

Simplify the generics for `set_if_neq` and add the `where Self::Target: Sized` trait bound to it. Add a default implementation so implementers don't need to implement a method with nonsensical trait bounds.
myreprise1 pushed a commit to myreprise1/bevy that referenced this pull request Feb 16, 2023
# Objective

While porting my crate `bevy_trait_query` to bevy 0.10, I ran into an issue with the `DetectChangesMut` trait. Due to the way that the `set_if_neq` method (added in bevyengine#6853) is implemented, you are forced to write a nonsense implementation of it for dynamically sized types. This edge case shows up when implementing trait queries, since `DetectChangesMut` is implemented for `Mut<dyn Trait>`.

## Solution

Simplify the generics for `set_if_neq` and add the `where Self::Target: Sized` trait bound to it. Add a default implementation so implementers don't need to implement a method with nonsensical trait bounds.
JoJoJet added a commit to JoJoJet/bevy-trait-query that referenced this pull request Feb 16, 2023
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-Code-Quality A section of code that is hard to understand or change 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
Development

Successfully merging this pull request may close these issues.

None yet

3 participants