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

Deriving SystemParam fails for non public types #1869

Closed
concave-sphere opened this issue Apr 10, 2021 · 0 comments
Closed

Deriving SystemParam fails for non public types #1869

concave-sphere opened this issue Apr 10, 2021 · 0 comments
Labels
A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior

Comments

@concave-sphere
Copy link

Bevy version

0.5

Operating system & version

Debian GNU/Linux "buster"

What you did

Attached #[derive(SystemParam)] to a type that is not fully public (private, crate-private, or otherwise restricted). For instance:

mod private {
    use bevy::ecs::system::SystemParam;
    use bevy::prelude::*;
    #[derive(SystemParam)]
    struct Foo<'a> {
        some_resource: Res<'a, i32>,
    }
}

What you expected to happen

I would get an implementation of SystemParam for my private type.

What actually happened

error[E0446]: private type Foo<'a> in public interface
--> src/main.rs:6:14
|
6 | #[derive(SystemParam)]
| ^^^^^^^^^^^ can't leak private type
7 | struct Foo<'a> {
| -------------- Foo<'a> declared as private
|
= note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

Here's the macroexpansion (click to expand):

``` mod private { use bevy::ecs::system::SystemParam; use bevy::prelude::*; struct Foo<'a> { some_resource: Res<'a, i32>, } impl <'a> bevy::ecs::system::SystemParam for Foo<'a> { type Fetch = FooState<( as SystemParam>::Fetch,)>; } pub struct FooState { state: TSystemParamState, marker: std::marker::PhantomData<()>, } unsafe impl bevy::ecs::system::SystemParamState for FooState { type Config = TSystemParamState::Config; fn init(world: &mut bevy::ecs::world::World, system_state: &mut bevy::ecs::system::SystemState, config: Self::Config) -> Self { Self{state: TSystemParamState::init(world, system_state, config), marker: std::marker::PhantomData,} } fn new_archetype(&mut self, archetype: &bevy::ecs::archetype::Archetype, system_state: &mut bevy::ecs::system::SystemState) { self.state.new_archetype(archetype, system_state) } fn default_config() -> TSystemParamState::Config { TSystemParamState::default_config() } } impl <'a> bevy::ecs::system::SystemParamFetch<'a> for FooState<( as SystemParam>::Fetch,)> { type Item = Foo<'a>; unsafe fn get_param(state: &'a mut Self, system_state: &'a bevy::ecs::system::SystemState, world: &'a bevy::ecs::world::World, change_tick: u32) -> Self::Item { Foo{some_resource: < as SystemParam>::Fetch as bevy::ecs::system::SystemParamFetch>::get_param(&mut state.state.0, system_state, world, change_tick),} } } } ```

The macro is defining pub struct FooState<TSystemParamState>, then implementing SystemParamFetch on it. The SystemParamFetch declares type Item = Foo, which is what "leaks" Foo.

I think that this would be resolved if the derive macro copied the visibility attribute of the input struct onto the State struct. Each of the structs references the other in a trait impl, so they need to be equally visible.

@TheRawMeatball TheRawMeatball added C-Bug An unexpected or incorrect behavior A-ECS Entities, components, systems, and events labels Apr 10, 2021
@bors bors bot closed this as completed in d508923 Apr 16, 2021
ostwilkens pushed a commit to ostwilkens/bevy that referenced this issue Jul 27, 2021
Examples creating a public type to derive `SystemParam` on were updated
to create a private type where a public one is no longer needed.

Resolves bevyengine#1869
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-Bug An unexpected or incorrect behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants