Systems as "normal functions"

[cart]

I think it would be useful to be able to define systems as "normal functions":

Preferred Format

// borrows resource A and runs once for each entity with the X and Y components 
pub fn simple_system(a: Resource<A>, x: Ref<X>, y: RefMut<Y>) {
    println!("{} {}", x.value, y.value);
}

This sheds a large amount of boilerplate, increases clarity, and makes IDEs happy (i already tested Ref completions with rust-analyzer). Currently rust-analyzer is unable to give completions within systems defined using SystemBuilder.

I recently proposed a similar layout for Shipyard ECS and they actually just adopted it. Its really cool, but for a variety of reasons I want my current codebase to remain on legion.

Potential "Easy" Implementation

I also believe something similar is possible in legion without changing the current types / adding a bunch of new code.

pub struct X { value: usize }

pub struct Y { value: f32 }

pub fn simple_system((x, y): (Ref<X>, Ref<Y>)) {
    println!("{} {}", x.value, y.value);
}

pub fn main() {
    let mut world = World::new();
    let mut resources = Resources::default();
    let system = into_system("simple_system", simple_system);
    system.run(&mut world, &mut resources);
}

pub fn into_system<A, V, F>(name: &'static str, system: A) -> Box<dyn Schedulable>
where
    A: Fn(<<V as View>::Iter as Iterator>::Item) + Send + Sync + 'static,
    V: for<'a> View<'a> + DefaultFilter<Filter = F>,
    F: EntityFilter + Sync + 'static,
{
    SystemBuilder::new(name)
        .with_query(V::query())
        .build(move |_, world, _, query| {
            for x in query.iter_mut(world) {
                system(x);
            }
        })
}

This almost compiles, but I haven't been able to take it over the finish line. The example above fails with:

error[E0631]: type mismatch in function arguments
   --> legion_systems/src/system_fn.rs:98:36
    |
53  | pub fn into_system<A, V, F>(system: A) -> Box<dyn Schedulable>
    |        -----------
54  | where
55  |     A: Fn(<<V as View>::Iter as Iterator>::Item) + Send + Sync + 'static,
    |        ----------------------------------------- required by this bound in `system_fn::into_system`
...
98  |         let x = super::into_system(test);
    |                                    ^^^^ expected signature of `for<'r> fn(<<_ as legion_core::query::View<'r>>::Iter as std::iter::Iterator>::Item) -> _`
...
109 |     fn test<'a>(a: (Ref<'a, X>, Ref<'a, Y>)) {
    |     ---------------------------------------- found signature of `for<'a> fn((legion_core::borrow::Ref<'a, system_fn::tests::X>, legion_core::borrow::Ref<'a, system_fn::tests::Y>)) -> _`

I'm curious if someone familiar with the legion types could help me out?

However even if we can make that work there is the big caveat that it wouldn't support Resources. However maybe theres a way to work those in too 😄

[cart]

In general this is very similar to the macro proposal in #31, but I think this is preferable because its "pure rust" and almost as terse.

[cart]

For maximum flexibility, it would be useful to also support a style similar to the one outlined here.

fn update_positions(
    time: Resource<Time,>
    query: (Write<Position>, Read<Velocity>),
    world: &mut SubWorld)
{
    for (mut pos, vel) in query.iter(world) {
        *pos += *vel * time.delta_seconds();
    }
}

However i can see that being much harder to implement.

[cart]

I have a working proof of concept now!

struct X(usize);

fn simple_system(x: Ref<X>) {
    println!("{}", x.0);
}

#[test]
fn test_system() {
    let mut world = World::new();
    let mut resources = Resources::default();
    {
        world.insert((), vec![(X(1),), (X(2), )]);

    }
    let mut x = into_system::<Ref<_>, _, _>("simple_system", simple_system);
    x.run(&mut world, &mut resources);
}

The biggest trick was implementing View<'a> for Ref.

Sadly it isn't 100% useful yet because for some reason Rust can't elide the "Ref" type. This means that at registration time you need to re-specify the function signature, which kind of defeats the purpose. I'm hoping that if i can somehow be a bit more explicit with the types that we can omit it entirely.

[cart]

Added RefMut. And fortunately the existing View impl macro means we can already do this:

struct Y(usize);
struct X(usize);

fn simple_system((x, mut y): (Ref<X>, RefMut<Y>)) {
    y.0 += 1;
    println!("{} {}", x.0, y.0);
}

#[test]
fn test_system() {
    let mut world = World::new();
    let mut resources = Resources::default();
    {
        world.insert((), vec![(X(1), Y(1)), (X(2), Y(2))]);
    }
    let mut x = into_system::<(Ref<_>, RefMut<_>), _, _>("simple_system", simple_system);
    x.run(&mut world, &mut resources);
}

The last remaining problem before I would start using this in a project is type-elision on into_system.

 let mut x = into_system("simple_system", simple_system);

[cart]

Unfortunately I just discovered that 3ee0bb9 breaks my implementation for some reason. Hopefully fixable :)

[cart]

I've spent a bunch of time trying to resolve the lifetime and type inference problems. I sadly don't have more time to spend on this. If someone else wants to pick up the torch, here is my branch:
https://github.com/cart/legion/tree/system_fn

[cart]

Its based off of the commit right before 3ee0bb9. It works as-is if you're willing to deal with the redundant type definitions mentioned above.

[cart]

Thanks to some help from ThorrakVII on the Rust discord, we now have full type inference!

fn read_write_system((x, mut y): (Ref<X>, RefMut<Y>)) {
    y.0 += 1;
    println!("{} {}", x.0, y.0);
}

fn read_system(x: Ref<X>) {
    println!("{}", x.0);
}

fn main() {
    let mut world = World::new();
    let mut resources = Resources::default();
    world.insert((), vec![(X(1), Y(1)), (X(2), Y(2))]);

    let mut system = into_system("read_write", read_write_system);
    system.run(&mut world, &mut resources);

    let mut x = into_system("simple", read_system);
    x.run(&mut world, &mut resources);
}

The lifetime issue remains however. Ill try to sort it out so this works on the master branch. @TomGillen: if I get that working, do you have any interest in a pr?

[cart]

fixing the lifetimes is proving to be difficult. i believe the core of the issue is that this implementation relies on a "concrete" View lifetime, but legion uses for<'a> View<'a> almost everywhere.

I tried making the lifetimes concrete, but the changes are both massive and potentially incompatible with SystemBuilder

[chemicstry]

I'm interested in this implementation because it would potentially allow having systems as structs (same as in specs). This would greatly simplify system syntax for amethyst. Although, I don't see a way to support multiple queries with this.

Did you have any further progress on this? I tried your query-view-lifetimes branch, but it seems that it doesn't compile yet?

[cart]

i havent been able to fix the lifetime issue yet. it probably also makes sense to wait until the legion refactor lands before investing further.

[ezpuzz]

I'm interested in this implementation because it would potentially allow having systems as structs (same as in specs). This would greatly simplify system syntax for amethyst. Although, I don't see a way to support multiple queries with this.

Did you have any further progress on this? I tried your query-view-lifetimes branch, but it seems that it doesn't compile yet?

@chemicstry I've been able to create a proof of concept for systems as structs Is that what you meant?