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

Provide an API for fetching SystemParam from World #2089

Closed
concave-sphere opened this issue May 3, 2021 · 3 comments
Closed

Provide an API for fetching SystemParam from World #2089

concave-sphere opened this issue May 3, 2021 · 3 comments
Labels
A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible

Comments

@concave-sphere
Copy link

What problem does this solve or what need does it fill?

I want to write tests of a custom SystemParam.

For systems that act on resources and components, I can just use APIs on Worldto fetch the final state of the resources and components.

But a SystemParam doesn't have side effects -- instead, it has an API that I want to exercise.

What solution would you like?

I'd like to be able to fetch SystemParam instances out of World the same way I can fetch components and resources:

let app = new_test_app();
// set up app.world with state

let param: MySystemParam = app.world.get_system_param_mut();
assert_eq!(wanted_value, param.method1(args...));
// Call a method for side effects:
param.method2(args...);
assert_eq!(wanted_state, get_state(&app.world));

What alternative(s) have you considered?

I can write a test system, something like this:

struct TestMySystemParam(fn(MySystemParam) -> Result<(), anyhow::Error>);
// The test resource is optional so I can share the app setup between several
// system params in one module.
fn test_my_system_param(
    opt_test: Option<Res<TestMySystemParam>>,
    mut out: ResMut<Result<anyhow::Error>>,
    my_system_param: MySystemParam) {
  if let Some(test) = opt_test {
    *out = test(my_system_param);
  }
}

#[test] my_test() {
  let app = new_test_app();
  // Set up app.world.

  app.world.insert_resource(TestMySystemParam(|param| {
    let got = param.method1();
    ensure!(want == got, "method1: wanted {:?}, got {:?}", want, got);
  }));
  app.update();
  let result: Result<(), anyhow::Error> = app.world.get_resource();
  result.unwrap();
}

However, this is kind of fiddly, and I have to do it for every SystemParam I want to test. It would be much nicer if Bevy could handle the details for me.

@concave-sphere concave-sphere added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels May 3, 2021
@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events and removed S-Needs-Triage This issue needs to be labelled labels May 3, 2021
@concave-sphere
Copy link
Author

concave-sphere commented May 3, 2021

FTR, the workaround above doesn't work (I wrote it before compiling). What I ended up doing is even more involved:

struct TestMySystemParam(
    Bpx<dyn Send + Sync + Fn(MySystemParam) -> Result<(), anyhow::Error>)>
);
// The test resource is optional so I can share the app setup between several
// system params in one module.
fn test_my_system_param(
    opt_test: Option<Res<TestMySystemParam>>,
    mut out: ResMut<Result<anyhow::Error>>,
    my_system_param: MySystemParam) {
  if let Some(test) = opt_test {
    *out = test.0(my_system_param);
  }
}

#[test] my_test() {
  let app = new_test_app();  // Needs to include the test system and the Result resource now
  set_up_app_world(&mut app.world);

  app.world.insert_resource(TestMySystemParam(Box:new(
    // Rust needs the input+output of the lambda to be specified
    move |param: MySystemParam| -> Result<(), anyhow::Error> {
      let got = param.method1();
      ensure!(want == got, "method1: wanted {:?}, got {:?}", want, got);

      Ok(())
    }
  )));
  app.update();

  let result: &Result<(), anyhow::Error> = app.world.get_resource().unwrap();
  result.as_ref().unwrap();
}

@cart
Copy link
Member

cart commented May 3, 2021

I think its worth trying to generalize this into something like QueryState, but for SystemParams:

let mut params = world.system_params::<(Res<A>, ResMut<B>, Query<(&X, &mut Y)>)>();
let (a, b, query) = params.get_mut(&mut world);

Ideally this would be done in a way that is reused by FuncSystem.

This would also cut down the need for things like WorldCell.

@alice-i-cecile
Copy link
Member

Added in #2283 :D

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-Feature A new feature, making something new possible
Projects
None yet
Development

No branches or pull requests

3 participants