[Merged by Bors] - Lift the 16-field limit from the SystemParam derive
#6867
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
james7132
added
A-ECS
| @@ -1663,4 +1663,33 @@ mod tests { | |||
|
|
|||
| #[derive(SystemParam)] | |||
| pub struct UnitParam {} | |||
|
|
|||
| #[derive(Resource)] | |||
| pub struct R<const I: usize>; | |||
There was a problem hiding this comment.
Any reason why you added this over re-using UnitParam?
There was a problem hiding this comment.
I want each field to be a distinct type, to make sure the derive macro doesn't mess up the order of the fields.
tim-blackbird
approved these changes
Dec 14, 2022
james7132
reviewed
Dec 17, 2022
| const LIMIT: usize = 16; | ||
| while tuple_types.len() > LIMIT { | ||
| let end = Vec::from_iter(tuple_types.drain(..LIMIT)); | ||
| tuple_types.push(parse_quote!( (#(#end,)*) )); |
There was a problem hiding this comment.
It's a bit difficult to follow this but does this make a tuple of tuples? Wouldn't that just raise the limit to 256 instead of removing it?
There was a problem hiding this comment.
Oh wait I think I see what this is doing. My god this is going to have a lot of codgen for anything more than 32 entries, but you probably already should be expecting that with a ParamSet that big.
james7132
approved these changes
Dec 17, 2022
| const LIMIT: usize = 16; | ||
| while tuple_types.len() > LIMIT { | ||
| let end = Vec::from_iter(tuple_types.drain(..LIMIT)); | ||
| tuple_types.push(parse_quote!( (#(#end,)*) )); |
There was a problem hiding this comment.
Oh wait I think I see what this is doing. My god this is going to have a lot of codgen for anything more than 32 entries, but you probably already should be expecting that with a ParamSet that big.
james7132
added
the
S-Ready-For-Final-Review
cart
approved these changes
Dec 20, 2022
|
Resolve conflicts and this is good to go! |
|
They are resolved :) |
|
bors r+ |
bors bot
pushed a commit
that referenced
this pull request
Dec 21, 2022
# Objective
* The `SystemParam` derive internally uses tuples, which means it is constrained by the 16-field limit on `all_tuples`.
* The error message if you exceed this limit is abysmal.
* Supercedes #5965 -- this does the same thing, but is simpler.
## Solution
If any tuples have more than 16 fields, they are folded into tuples of tuples until they are under the 16-field limit.
bors
bot
changed the title
SystemParam deriveSystemParam derive
JoJoJet
added a commit
to JoJoJet/bevy
that referenced
this pull request
Dec 21, 2022
* The `SystemParam` derive internally uses tuples, which means it is constrained by the 16-field limit on `all_tuples`.
* The error message if you exceed this limit is abysmal.
* Supercedes bevyengine#5965 -- this does the same thing, but is simpler.
If any tuples have more than 16 fields, they are folded into tuples of tuples until they are under the 16-field limit.
bors bot
pushed a commit
that referenced
this pull request
Dec 25, 2022
# Objective * Currently, the `SystemParam` derive does not support types with const generic parameters. * If you try to use const generics, the error message is cryptic and unhelpful. * Continuation of the work started in #6867 and #6957. ## Solution Allow const generic parameters to be used with `#[derive(SystemParam)]`.
alradish
pushed a commit
to alradish/bevy
that referenced
this pull request
Jan 22, 2023
# Objective
* The `SystemParam` derive internally uses tuples, which means it is constrained by the 16-field limit on `all_tuples`.
* The error message if you exceed this limit is abysmal.
* Supercedes bevyengine#5965 -- this does the same thing, but is simpler.
## Solution
If any tuples have more than 16 fields, they are folded into tuples of tuples until they are under the 16-field limit.
alradish
pushed a commit
to alradish/bevy
that referenced
this pull request
Jan 22, 2023
# Objective * Currently, the `SystemParam` derive does not support types with const generic parameters. * If you try to use const generics, the error message is cryptic and unhelpful. * Continuation of the work started in bevyengine#6867 and bevyengine#6957. ## Solution Allow const generic parameters to be used with `#[derive(SystemParam)]`.
ItsDoot
pushed a commit
to ItsDoot/bevy
that referenced
this pull request
Feb 1, 2023
# Objective
* The `SystemParam` derive internally uses tuples, which means it is constrained by the 16-field limit on `all_tuples`.
* The error message if you exceed this limit is abysmal.
* Supercedes bevyengine#5965 -- this does the same thing, but is simpler.
## Solution
If any tuples have more than 16 fields, they are folded into tuples of tuples until they are under the 16-field limit.
ItsDoot
pushed a commit
to ItsDoot/bevy
that referenced
this pull request
Feb 1, 2023
# Objective * Currently, the `SystemParam` derive does not support types with const generic parameters. * If you try to use const generics, the error message is cryptic and unhelpful. * Continuation of the work started in bevyengine#6867 and bevyengine#6957. ## Solution Allow const generic parameters to be used with `#[derive(SystemParam)]`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Objective
SystemParamderive internally uses tuples, which means it is constrained by the 16-field limit onall_tuples.Solution
If any tuples have more than 16 fields, they are folded into tuples of tuples until they are under the 16-field limit.