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

Round out Mut<T> api using lenses + auto-unwrapping #4413

Closed
wants to merge 9 commits into from

Conversation

TheRawMeatball
Copy link
Member

Objective

  • Allow going from Mut<LargeStruct> to Mut<Field>
  • Simplify working with components made with the newtype pattern

Solution

Add lenses, which allow for getting a Mut<U> from a Mut<T> where T contains a U, and add a "default lens" option to the Component trait, allowing for automatic unwrapping of newtypes

Changelog

  • Added Lens trait
  • Added associated methods to Mut for lensing
  • Added DefaultLens associated type to Component for automatic lensing
  • Added Raw and RawMut query params which bypass automatic lensing

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Apr 4, 2022
@TheRawMeatball TheRawMeatball added C-Feature A new feature, making something new possible A-ECS Entities, components, systems, and events and removed S-Needs-Triage This issue needs to be labelled labels Apr 4, 2022
@alice-i-cecile alice-i-cecile self-requested a review April 4, 2022 17:02
@@ -124,7 +124,7 @@ pub fn calculate_bounds(
}

pub fn update_frusta<T: Component + CameraProjection + Send + Sync + 'static>(
mut views: Query<(&GlobalTransform, &T, &mut Frustum)>,
mut views: Query<(&GlobalTransform, Raw<T>, &mut Frustum)>,
Copy link
Member

Choose a reason for hiding this comment

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

The need for this sort of change makes me worried: I don't love that users will have to reason about this when trying to write generic systems.

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe, but it's a fairly mindless fix - just use the Raw and RawMut wrappers for directly generic components. I think ergonomic wins for the more common cases outweigh this slight loss here.

@alice-i-cecile
Copy link
Member

Interesting. My first impressions are:

@TheRawMeatball
Copy link
Member Author

This is similar to #4328, but distinct in an important way: This method has benefits even when working with newtypes that wrap primitives. As for #4328, it can also be used for resources which could be its main selling point.

TheRawMeatball and others added 2 commits April 4, 2022 22:39
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
@alice-i-cecile
Copy link
Member

As for #4328, it can also be used for resources which could be its main selling point.

This only holds until we have #[derive(Resource)] though 🧪

@alice-i-cecile alice-i-cecile added the X-Controversial There is active debate or serious implications around merging this PR label Jul 2, 2022
@alice-i-cecile
Copy link
Member

Closed in favor of #6199.

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-Feature A new feature, making something new possible X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants