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

Immediate Execution of Commands on &mut World #6184

Closed
Zeenobit opened this issue Oct 6, 2022 · 2 comments · Fixed by #9366
Closed

Immediate Execution of Commands on &mut World #6184

Zeenobit opened this issue Oct 6, 2022 · 2 comments · Fixed by #9366
Labels
A-ECS Entities, components, systems, and events C-Testing A change that impacts how we test Bevy or how users test their apps C-Usability A targeted quality-of-life change that makes Bevy easier to use

Comments

@Zeenobit
Copy link
Contributor

Zeenobit commented Oct 6, 2022

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

Currently, to write a test for a command, we have 2 options:

  1. Either create a minimal app with a system that uses the command, or
  2. Use CommandQueue with a World to execute commands "in place"

I've noticed we already use option 2 in Bevy code:

fn remove_resources() {
    let mut world = World::default();
    let mut queue = CommandQueue::default();
    {
        let mut commands = Commands::new(&mut queue, &world);
        commands.insert_resource(W(123i32));
        commands.insert_resource(W(456.0f64));
    }

    queue.apply(&mut world);
    assert!(world.contains_resource::<W<i32>>());
    assert!(world.contains_resource::<W<f64>>());

    {
        let mut commands = Commands::new(&mut queue, &world);
        // test resource removal
        commands.remove_resource::<W<i32>>();
    }
    queue.apply(&mut world);
    assert!(!world.contains_resource::<W<i32>>());
    assert!(world.contains_resource::<W<f64>>());
}

My proposal is that we simplify this by adding a function to execute commands directly on a world.

What solution would you like?

I have a minimal working implementation of this in my own project:

use bevy::prelude::*;

use bevy::ecs::system::CommandQueue;

///
/// Trait to execute [`Commands`] on a [`World`].
///
pub trait Execute {
    fn execute<F: FnOnce(&World, Commands) -> R, R>(self, f: F) -> R;
}

///
/// Any mutable reference to a [`World`] may be used to execute [`Commands`] on it.
///
impl Execute for &mut World {
    fn execute<F: FnOnce(&World, Commands) -> R, R>(self, f: F) -> R {
        let mut queue = CommandQueue::default();
        let commands = Commands::new(&mut queue, self);
        let result = f(self, commands);
        queue.apply(self);
        result
    }
}

///
/// Any mutable reference to an [`App`] may be used to execute [`Commands`] on its [`World`].
///
impl Execute for &mut App {
    fn execute<F: FnOnce(&World, Commands) -> R, R>(self, f: F) -> R {
        self.world.execute(f)
    }
}

This allows you to just do this anywhere you have &mut World access:

world.execute(|_world, mut commands| {
    /* ... */
});

What alternative(s) have you considered?

See above for current options.

Additional context

I tried making this a direct pull request. But as I was implementing it, I wasn't sure where this should go. I was thinking of putting Execute in bevy_ecs/src/system/commands/mod.rs, but I wasn't sure if it's smart to import Commands in bevy_app/src/lib.rs

If we're ok with the implementation I've proposed above, and importing Commands in Bevy app, then I can just turn this into a PR.
Also I was internally debating guarding this behind a feature flag, so that it's only available for tests/examples. Still not sure if that's wise.

@Zeenobit Zeenobit added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Oct 6, 2022
@alice-i-cecile alice-i-cecile added C-Usability A targeted quality-of-life change that makes Bevy easier to use C-Testing A change that impacts how we test Bevy or how users test their apps A-ECS Entities, components, systems, and events and removed C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Oct 6, 2022
@alice-i-cecile
Copy link
Member

Duplicate of #3096, but this issue is better. See also #5940.

@alice-i-cecile
Copy link
Member

I'd be very happy with that impl, please open a PR <3

@Zeenobit Zeenobit mentioned this issue Aug 5, 2023
github-merge-queue bot pushed a commit that referenced this issue Aug 11, 2023
Add a `RunSystem` extension trait to allow for immediate execution of
systems on a `World` for debugging and/or testing purposes.

# Objective

Fixes #6184

Initially, I made this CL as `ApplyCommands`. After a discussion with
@cart , we decided a more generic implementation would be better to
support all systems. This is the new revised CL. Sorry for the long
delay! 😅

This CL allows users to do this:
```rust
use bevy::prelude::*;
use bevy::ecs::system::RunSystem;

struct T(usize);

impl Resource for T {}

fn system(In(n): In<usize>, mut commands: Commands) -> usize {
    commands.insert_resource(T(n));
    n + 1
}

let mut world = World::default();
let n = world.run_system_with(1, system);
assert_eq!(n, 2);
assert_eq!(world.resource::<T>().0, 1);
```

## Solution

This is implemented as a trait extension and not included in any
preludes to ensure it's being used consciously.
Internally, it just initializes and runs a systems, and applies any
deferred parameters all "in place".
The trait has 2 functions (one of which calls the other by default):
- `run_system_with` is the general implementation, which allows user to
pass system input parameters
- `run_system` is the ergonomic wrapper for systems with no input
parameter (to avoid having the user pass `()` as input).

~~Additionally, this trait is also implemented for `&mut App`. I added
this mainly for ergonomics (`app.run_system` vs.
`app.world.run_system`).~~ (Removed based on feedback)

---------

Co-authored-by: Pascal Hertleif <[email protected]>
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-Testing A change that impacts how we test Bevy or how users test their apps C-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
None yet
2 participants