Skip to content

Commit

Permalink
Lift the 16-field limit from the SystemParam derive (#6867)
Browse files Browse the repository at this point in the history
# 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.
  • Loading branch information
JoJoJet committed Dec 21, 2022
1 parent 0363e0b commit 025996b
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 5 deletions.
26 changes: 21 additions & 5 deletions crates/bevy_ecs/macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use proc_macro2::Span;
use quote::{format_ident, quote};
use syn::{
parse::{Parse, ParseStream},
parse_macro_input,
parse_macro_input, parse_quote,
punctuated::Punctuated,
spanned::Spanned,
token::Comma,
Expand Down Expand Up @@ -359,8 +359,8 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
)
})
.collect::<Vec<(&Field, SystemParamFieldAttributes)>>();
let mut field_locals = Vec::new();
let mut fields = Vec::new();
let mut field_indices = Vec::new();
let mut field_types = Vec::new();
let mut ignored_fields = Vec::new();
let mut ignored_field_types = Vec::new();
Expand All @@ -369,6 +369,7 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
ignored_fields.push(field.ident.as_ref().unwrap());
ignored_field_types.push(&field.ty);
} else {
field_locals.push(format_ident!("f{i}"));
let i = Index::from(i);
fields.push(
field
Expand All @@ -378,7 +379,6 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
.unwrap_or_else(|| quote! { #i }),
);
field_types.push(&field.ty);
field_indices.push(i);
}
}

Expand Down Expand Up @@ -424,6 +424,19 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
_ => unreachable!(),
}));

let mut tuple_types: Vec<_> = field_types.iter().map(|x| quote! { #x }).collect();
let mut tuple_patterns: Vec<_> = field_locals.iter().map(|x| quote! { #x }).collect();

// If the number of fields exceeds the 16-parameter limit,
// fold the fields into tuples of tuples until we are below the limit.
const LIMIT: usize = 16;
while tuple_types.len() > LIMIT {
let end = Vec::from_iter(tuple_types.drain(..LIMIT));
tuple_types.push(parse_quote!( (#(#end,)*) ));

let end = Vec::from_iter(tuple_patterns.drain(..LIMIT));
tuple_patterns.push(parse_quote!( (#(#end,)*) ));
}
// Create a where clause for the `ReadOnlySystemParam` impl.
// Ensure that each field implements `ReadOnlySystemParam`.
let mut read_only_generics = generics.clone();
Expand All @@ -448,7 +461,7 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {

#[doc(hidden)]
type State<'w, 's, #punctuated_generic_idents> = FetchState<
(#(<#field_types as #path::system::SystemParam>::State,)*),
(#(<#tuple_types as #path::system::SystemParam>::State,)*),
#punctuated_generic_idents
>;

Expand Down Expand Up @@ -484,8 +497,11 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
world: &'w #path::world::World,
change_tick: u32,
) -> Self::Item<'w, 's> {
let (#(#tuple_patterns,)*) = <
<(#(#tuple_types,)*) as #path::system::SystemParam>::State as #path::system::SystemParamState
>::get_param(&mut state.state, system_meta, world, change_tick);
#struct_name {
#(#fields: <<#field_types as #path::system::SystemParam>::State as #path::system::SystemParamState>::get_param(&mut state.state.#field_indices, system_meta, world, change_tick),)*
#(#fields: #field_locals,)*
#(#ignored_fields: <#ignored_field_types>::default(),)*
}
}
Expand Down
29 changes: 29 additions & 0 deletions crates/bevy_ecs/src/system/system_param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1661,6 +1661,35 @@ mod tests {
_local: Local<'s, T>,
}

#[derive(Resource)]
pub struct R<const I: usize>;

#[derive(SystemParam)]
pub struct LongParam<'w> {
_r0: Res<'w, R<0>>,
_r1: Res<'w, R<1>>,
_r2: Res<'w, R<2>>,
_r3: Res<'w, R<3>>,
_r4: Res<'w, R<4>>,
_r5: Res<'w, R<5>>,
_r6: Res<'w, R<6>>,
_r7: Res<'w, R<7>>,
_r8: Res<'w, R<8>>,
_r9: Res<'w, R<9>>,
_r10: Res<'w, R<10>>,
_r11: Res<'w, R<11>>,
_r12: Res<'w, R<12>>,
_r13: Res<'w, R<13>>,
_r14: Res<'w, R<14>>,
_r15: Res<'w, R<15>>,
_r16: Res<'w, R<16>>,
}

#[allow(dead_code)]
fn long_system(_param: LongParam) {
crate::system::assert_is_system(long_system);
}

#[derive(SystemParam)]
pub struct UnitParam;

Expand Down

0 comments on commit 025996b

Please sign in to comment.