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

Deprecate type aliases for WorldQuery::Fetch #8843

Merged
merged 1 commit into from
Jun 19, 2023

Conversation

JoJoJet
Copy link
Member

@JoJoJet JoJoJet commented Jun 14, 2023

Objective

WorldQuery::Fetch is a type used to optimize the implementation of queries. These types are hidden and not intended to be outside of the engine, so there is no need to provide type aliases to make it easier to refer to them. If a user absolutely needs to refer to one of these types, they can always just refer to the associated type directly.

Solution

Deprecate these type aliases.


Changelog

  • Deprecated the type aliases QueryFetch and ROQueryFetch.

Migration Guide

The type aliases bevy_ecs::query::QueryFetch and ROQueryFetch have been deprecated. If you need to refer to a WorldQuery struct's fetch type, refer to the associated type defined on WorldQuery directly:

// Before:
type MyFetch<'w> = QueryFetch<'w, MyQuery>;
type MyFetchReadOnly<'w> = ROQueryFetch<'w, MyQuery>;

// After:
type MyFetch<'w> = <MyQuery as WorldQuery>::Fetch;
type MyFetchReadOnly<'w> = <<MyQuery as WorldQuery>::ReadOnly as WorldQuery>::Fetch;

@JoJoJet JoJoJet added A-ECS Entities, components, systems, and events C-Breaking-Change A breaking change to Bevy's public API that needs to be noted in a migration guide labels Jun 14, 2023
Copy link
Member

@james7132 james7132 left a comment

Choose a reason for hiding this comment

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

Have you tried removing these from the codebase? I remember it cropping up a lot in generated code from WorldQuery derive macros. Removing these may actually end up with much less readable code, and that's a feat given the readability status of that macro right now.

@JoJoJet
Copy link
Member Author

JoJoJet commented Jun 16, 2023

Everything still compiles when I remove them. Neither of these type aliases are currently used anywhere in the engine.

Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

I'm glad to see we no longer need this :)

Copy link
Member

@james7132 james7132 left a comment

Choose a reason for hiding this comment

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

If we can remove these without too much fuss, then I'm all for removing them.

@james7132 james7132 added this pull request to the merge queue Jun 19, 2023
Merged via the queue into bevyengine:main with commit f636560 Jun 19, 2023
@JoJoJet JoJoJet deleted the no-fetch-alias branch September 20, 2023 05:24
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-Breaking-Change A breaking change to Bevy's public API that needs to be noted in a migration guide
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants