derive(SystemParam) better lifetime param (#10331) #13794
Conversation
previously, only 'w and 's lifetime parameters were permitted. now, 'world and 'state are permitted aswell. this is achieved by replacing every occurance of 'world with 'w, and likewise with state
|
i feel so silly every time the tests fail |
alice-i-cecile
added
A-ECS
|
i made it so any lifetime names are possible. but i hate it, it looks gross haha |
| /// Implement `SystemParam` to use a struct as a parameter in a system | ||
| #[proc_macro_derive(SystemParam, attributes(system_param))] | ||
| pub fn derive_system_param(input: TokenStream) -> TokenStream { | ||
| let input = replace_lifetimes( | ||
| input, | ||
| Box::new(|s| match s { |
There was a problem hiding this comment.
I feel like we should just not special-case lifetime names at all 🤔 Are we able to do so?
There was a problem hiding this comment.
I respect your intuition, but i dont understand it. the whole point of that function is to abbreviate lifetimes, since 'world and 'state should be a special exception; replacing other things like variable names or punctuation is really not what were after.
Honestly id go back to the previous iteration where the values were hardcoded, and declare the function inside the only place where it is used (if rust allows that)
maybe someone with macro experience can actually find a more elegant solution, but idk if there is one
There was a problem hiding this comment.
Let's do that and solve the issue raised directly.
|
I wish I was better at macros so I could give you better help myself! |
|
@alice-i-cecile also i modified one Cargo.toml file (in bevy_macro_utils), and idk i am allowed to just do that PS: should i explicitly mention people in these discussions, or are they notified automatically? |
|
Yeah, if you need more features from |
previously, only 'w and 's lifetime parameters were permitted. now, 'world and 'state are permitted aswell.
this is achieved by replacing every occurrence of 'world with 'w, and 'state with 's.
I also updated the documentation to inform the user that they may use these new lifetimes now.
That is, in the compiler error message, and in the description of the trait (idk if there are more places that i should have updated).
it seems like a hack, and I am not confident if this is the perfect and best way to solve the issue. Especially since I modified the Cargo.toml in bevy_macro_utils
And it took me way longer than id like to admit bc I thought I was smarter than ChatGpt lol
Testing
I wrote some boilerplate code in the trait documentation in such a way that its hidden to the user. It seems like a hack aswell, but idk how else to unit test if code compiles
fixes #10331