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

Possible to define a system as generic over SystemParam and provide concrete type? #3300

Closed
shelbyd opened this issue Dec 11, 2021 · 2 comments
Labels
A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Needs-Design This issue requires design work to think about how it would best be accomplished

Comments

@shelbyd
Copy link

shelbyd commented Dec 11, 2021

Here's a minimal example of what I'm trying to do:

#![allow(dead_code)]

use bevy::prelude::*;
use bevy::ecs::system::SystemParam;

fn main() {
    App::build()
        .add_system(generic_system::<NullImpl>.system())
        .add_system(bound_generic_system.system())
        .run()
}

fn generic_system<T>(t: T) {}
fn bound_generic_system(t: NullImpl) {}

#[derive(SystemParam)]
pub struct NullImpl<'a> {
    marker: Res<'a, ()>,
}

AFAIU, the problem is that generic_system::<NullImpl> is implicitly binding NullImpl<'_>, so that it doesn't satisfy for<'a>. I would think it should work if I were able to write something like: generic_system::<for<'a> NullImpl<'a>>.system(), but that doesn't parse.

Is there something I can do to make this compile? I've tried to make a non-bevy example to ask the broader Rust community, but I haven't been able to reproduce.

@DJMcNab
Copy link
Member

DJMcNab commented Dec 11, 2021

Your reasoning that it implicitly binds a concrete version, i.e. the 'static version is correct

I think one way to work around this would be would be for us to create a struct StaticSystemParam<'a, T>(<T as SystemParam<'a>>::Item) (or whatever the right syntax is)

The alternative is to create a struct and implement RunSystem for it instead, which should work as required.

I think that RunSystem exists is because of this use case, and I think it could be avoided if we added StaticSystemParam.

@mockersf mockersf added A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Dec 11, 2021
@shelbyd
Copy link
Author

shelbyd commented Dec 11, 2021

I'm looking into how to do this in 0.5.0. I've tried making a wrapping type with the goal of erasing the lifetime of the normal system param. It looks like even if I were to get it working, rust would still complain. I can't figure out a lifetime to add in the creation of the system that makes rust not complain about the lifetimes. Even adding 'static doesn't work, because it needs to be for<'all>.

It's possible that there is a doable lifetime change to the IntoSystem impls that will let 'static work.

Also, I don't understand the way the lifetimes work out to reproduce without bevy. If anyone can make a reproduction, I'll ask the broader rust community for help.

@alice-i-cecile alice-i-cecile added the S-Needs-Design This issue requires design work to think about how it would best be accomplished label Dec 12, 2021
@bors bors bot closed this as completed in 6e61fef Mar 15, 2022
aevyrie pushed a commit to aevyrie/bevy that referenced this issue Jun 7, 2022
# Objective

- Fixes bevyengine#3300
- `RunSystem` is messy

## Solution

- Adds the trick theorised in bevyengine#3300 (comment)

P.S. I also want this for an experimental refactoring of `Assets`, to remove the duplication of `Events<AssetEvent<T>>`


Co-authored-by: Carter Anderson <mcanders1@gmail.com>
ItsDoot pushed a commit to ItsDoot/bevy that referenced this issue Feb 1, 2023
# Objective

- Fixes bevyengine#3300
- `RunSystem` is messy

## Solution

- Adds the trick theorised in bevyengine#3300 (comment)

P.S. I also want this for an experimental refactoring of `Assets`, to remove the duplication of `Events<AssetEvent<T>>`


Co-authored-by: Carter Anderson <mcanders1@gmail.com>
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-Usability A targeted quality-of-life change that makes Bevy easier to use S-Needs-Design This issue requires design work to think about how it would best be accomplished
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants