System Definition Alternatives

[TomGillen]

We currently provide the SystemBuilder for constructing systems. This API is needed because it is very difficult to manually express the type of a legion system, as it statically encodes an arbitrary number of queries and resource accesses, and queries statically encode into their type all of their data accesses (tag/component types and mutable/immutable) and filter expressions (which can be arbitrary boolean expressions).

However, in many cases the full power of SystemBuilder is not needed; most systems only use one query, and do not request any complex filter logic on that query. There may be scope for providing a higher level simplified API for system construction which would still be sufficient to cover the most common use cases.

System Traits

The first option may be to provide system traits which can be implemented on a unique struct for each system, similar to how specs/amethyst defines its systems:

// basic wrapper for `SystemBuilder`
struct MySystem;
impl IntoSystem for MySystem {
    fn into_system(self) -> Box<dyn Schedulable> {
        SystemBuilder::new("MySystem")
            .with_query(<(Read<Velocity>, Write<Position>)>::query())
            .read_resource::<Time>()
            .build(|_, world, query, time| {
                for (vel, mut pos) in query.iter(&mut world) {
                    *pos = *pos + *vel * time.delta_seconds();
                }
            })
    }
}

// simplified wrapper
struct MySystem;
impl ForEachSystem for MySystem {
    type Resources = Read<Time>;
    type Query = (Read<Velocity>, Write<Position>);

    fn for_each(entity: Entity, (vel: &Velocity, pos: &mut Position), time: &Time) {
        *pos = *pos + *vel + time.delta_seconds();
    }
}

There are a few limitations on ForEachSystem:

• There can be only one query.
• The query can't have any additional filters.
• There is no world access. It could be possible, but only the unsafe APIs would be usable.
• You can't run code outside of the loop body.

A ParForEachSystem alternative could be provided which uses par_for_each internally, but it would require the additional restriction that Resources must be ReadOnly.

These system structs can be converted into a System via .into_system().

Macro

Alternatively, we could use a procedural macro to define systems. I do not have experience writing procedural macros, so this is mostly speculative:

#[system(UpdatePositions)]
fn for_each(pos: &mut Position, vel: &Velocity, #[resource] time: &Time) {
    *pos = *pos + *vel * time.delta_seconds();
}

This would create an UpdatePositions system struct and implement IntoSystem. It has many of the restrictions of the ForEachSystem trait, except:

• We can use argument position attributes to define #[tag], #[resource] and some filters such as #[on_changed].
• The implementation would use either for_each or par_for_each, depending on whether or not all resources are accessed immutably, although the syntax does not make the performance impact obvious.
• Requesting the Entity and CommandBuffer would be optional.
• Access to PreparedWorld could be allowed, provided we check safety.

The above macro would generate something like:

struct UpdatePositions;

impl UpdatePositions {
    fn for_each(pos: &mut Position, vel: &Velocity, time: &Time) {
        *pos = *pos + *vel * time.delta_seconds();
    }
}

impl IntoSystem for UpdatePositions {
    fn into_system(self) -> Box<dyn Schedulable> {
        SystemBuilder::new(std::any::type_name::<Self>())
            .read_resource::<Time>()
            .with_query(<(Write<Position>, Read<Velocity>)>::query())
            .build(|_, world, query, time| {
                query.for_each(world, |(mut pos, vel)| {
                    Self::for_each(pos, vel, time);
                });
            })
    }
}

[TomGillen]

With regards to the safety of random access in the for-each trait/macro:

• It is never safe to provide access to &mut PreparedWorld, as the query is already borrowing it.
• It is safe to provide &PreparedWorld access if (and only if) the query's view is ReadOnly.
  • The safe APIs are fully usable
  • The unsafe _unchecked APIs are safe to use provided the entity accessed is not the current loop iteration entity.
• If the view is not ReadOnly, then only the _unchecked APIs can be used, with the same caveats as above. There is currently no way to provide access to only the _unchecked functions.

[caelunshun]

I hacked together a quick system macro here a while back. It only supports resource access and not queries, but you're welcome to look it over for reference.

[caelunshun]

Using the new system macro in tonks, it's possible to write systems with queries and any resource access:

#[system]
fn system(
    resource1: &Resource1, 
    resource2: &mut Resource2, 
    mut query: PreparedQuery<(Write<Position>, Read<Velocity>)>, 
    world: &mut PreparedWorld) {
     for (mut pos, vel) in query.iter(world) { /* ... */
}

It should be possible to implement something like this in Legion as well.

(The only caveat is it doesn't support query filters yet, since those are handled at runtime. I need to figure out a way to encode filters at the type level.)

[TomGillen]

Iterating a little on the macro design:

#[system]
fn update_positions(
    #[res] time: &Time,
    #[query] entities: (Write<Position>, Read<Velocity>),
    world: &mut PreparedWorld)
{
    for (mut pos, vel) in entities.iter(world) {
        *pos += *vel * time.delta_seconds();
    }
}

#[for_each]
fn update_positions(#[res] time: &Time, #[com] pos: &mut Position, #[com] vel: &Velocity) {
    *pos += *vel * time.delta_seconds();
}

Two macros, system and for_each. system wraps a function into a system, and can inject resources and queries, and a world and command buffer. It can accept multiple queries, but they cannot have any additional filters attached beyond the default ones that come with the view. for_each wraps the inner body of a query into a system.

Both compile into a function which builds a system via the SystemBuilder. The macros have a few options:

• fn (not actually sure if you can use a keyword here): names the generated function. Defaults to the ident of the function marked with the macro.
• struct: If present, generates a zero-sized struct with the given name, implements the generator function on that struct, and implements Into<Box<dyn Schedulable>>.
• name: name of the system. Defaults to the ident of the function.

So the default behaviour is to create a generator function with the same name as the input function.

e.g:

#[for_each]
fn update_positions(
    #[res] time: &Time,
    #[com] pos: &mut Position,
    #[com] vel: &Velocity)
{
    *pos += *vel * time.delta_seconds();
}

#[for_each(fn = "build_update_positions")]
fn update_positions(
    #[res] time: &Time,
    #[com] pos: &mut Position,
    #[com] vel: &Velocity)
{
    *pos += *vel * time.delta_seconds();
}

#[for_each(struct = UpdatePositions)]
fn update_positions(
    #[res] time: &Time, 
    #[com] pos: &mut Position, 
    #[com] vel: &Velocity)
{
    *pos += *vel * time.delta_seconds();
}

let mut scheduler = SystemScheduler::new();
scheduler.add_system(Stages::Update, update_positions()); // create system via function call
scheduler.add_system(Stages::Update, build_update_positions()); // create system via function call
scheduler.add_system(Stages::Update, UpdatePositions); // create system via .into() on struct

#[system] uses helper attributes to annotate the function parameters:

• #[res] marks a reference as a resource.
• #[query] marks an argument as generating a query.

neither of these are strictly needed. We could make resources implicit if the type isnt a query, world or command buffer. Queries are always tuples and not references, but unlike all the other parameters, their type is re-written by the macro into the proper query type, so there is some magic there.

#[for_each] also uses helper attributes:

• #[res] marks a reference as a resource.
• #[tag] marks a reference as a tag.
• #[com] marks a reference as a component.
• #[on_change] adds a changed filter to a component, requires the parameter be marked as #[com].
• #[maybe] marks an Option<T> component as a Try[Read/Write]<T>, rather than just a component which happens to be an Option.

We could make one of these implicit, but I am not sure whether or not that is desirable or, if so, which one. Preferably there would be some consistency between #[system] and #[for_each].

[chemicstry]

I've experimented with alternative system definitions and I think I came up with a pretty good solution, but it obviously still needs refining. I shared code on https://github.com/chemicstry/legion_test but keep in mind it's a test bed mess. It is based on experimental legion and you need to implement Default for Query (similar to IntoQuery) to compile.

Anyway, skipping the implementation details, system syntax looks very similar to specs. Here is an example:

impl System for TestSystem {
    type QueryData = (
        Query<Write<Position>, <Write<Position> as DefaultFilter>::Filter>,
        Query<Read<Velocity>, EntityFilterTuple<ComponentFilter<Position>, Passthrough>>,
    );

    type ResourceData = (
        Read<TestResourceA>,
        Write<TestResourceB>,
    );

    fn run(
        &mut self,
        (res_a, res_b): &mut <Self::ResourceData as ResourceSet>::Result,
        (pos, _vel): &mut Self::QueryData,
        _command_buffer: &mut CommandBuffer,
        world: &mut SubWorld,
    ) {
        println!("TestResourceA: {}", res_a.a);
        println!("TestResourceB: {}", res_b.b);

        for position in pos.iter_mut(world) {
            position.x += 1.0;
            println!("Position x: {}", position.x);
        }
    }
}

Most of the syntax looks okay, but I couldn't figure out how to express filters only with a type system or rather the expression gets really long. Legion filters are constructed at run time and this does not work with QueryData syntax. Looking for comments and ideas.

[AnneKitsune]

Do you think something along the lines of the following is possible?
I'm using a similar syntax with specs and it works so much better than the regular syntax. Making system creation easy encourages users to make smaller system, so its worth taking the time to get to a syntax we are happy with. :)

make_system!(build_my_system, |(res_a: Read<TestResourceA>,), 
(query_a: Query<Write<...>,...>::filter(...),),
 _command_buffer: &mut CommandBuffer, 
_world: &mut SubWorld| {
do the things;
});

[chemicstry]

I guess it would be possible, but hard to tell. I'm not that experienced with macros yet. Although it still has the same problem of filters needing to be defined at type level.

Having a low-boilerplate system syntax is nice, but at the same time macros make it really hard to understand what is happening under the hood. I would prefer to have a pure rust way to define them and have such a simplified macro as an alternative. Also, the reason I'm trying to implement systems as structs is because some systems (rendering) in amethyst have to do resource initialization (solved by double wrapping in closures) and deallocation (unsolved). Having a trait with build and dispose functions as in specs solves this really nicely.

[TomGillen]

I had a little play in the rust playground here a few days ago looking into whether I could work around the lifetime issues that were blocking #134, and while what I had there is very much just hashing stuff out, it looks like it should be entirely possible to have something like:

let system = System::for_each(|a: &A, b: &mut B, c: Option<&C>, d: Option<&mut D>| {
    ...
});

For a basic single-query loop for the query <(Read<A>, Write<B>, TryRead<C>, TryWrite<D>)>::query(). The struct returned could also have a .filter fn which appends extra filters to its query much the same as queries themselves do.

The syntax is still very limited though, as (most notably) it does not allow access to resources. That might perhaps look something like:

let system = System::for_each_with_resources(
    |cmd: &mut CommandBuffer,
    world: &mut SubWorld,
    (a, b, c, d): (&A, &mut B, Option<&C>, Option<&D>),
    (time, renderer): (&Time, &mut Renderer)| {
        ...
    }
);

It might, for consistency, make sense to then also put the SystemBuilder constructor under System::builder().

[kabergstrom]

I really think a struct is needed here. Closures are just not enough to provide multiple hooks as per #164 while still having named variables shared across these. If a macro is needed to make it reasonable to define, I think that should be considered. It's better than having to hack around closure limitations.

[chemicstry]

@kabergstrom I highly agree with you. I've been trying various alternatives to struct and nothing felt good enough.

The main limitation with struct systems is query filtering. Currently legion queries are implemented to be setup via runtime expression and this makes typed filters really inconvenient to use. Ideally this should be fixed in the filter types, but macro is also possible without much trouble. Maybe @TomGillen can comment on this.

I was also trying to merge QueryData and ResourceData into single SystemData tuple (like in specs), but this inevitably introduces lifetime for System without GATs. And I'm not sure if that's any better.

[TomGillen]

I have been working on this over the weekend (very nearly done, just need to sort supporting systems with generic arguments), but for handling system state it looks a bit like this:

/// declare a system with mutable counter state
#[system]
fn stateful(#[state] counter: &mut usize) {
    *counter += 1;
    println!("state: {}", counter);
}

fn build_schedule() -> Schedule {
    Schedule::builder()
         // initialize state when you construct the system
        .add_system(stateful_system(5usize))
        .build()
}

Other examples of what you can do:

#[system(for_each)]
fn update_positions(position: &mut Position, velocity: &Velocity, #[resource] time: &Time) {
    *position += *velocity * time.seconds();
}

#[system(par_for_each)]
#[filter(maybe_changed::<Position>())]
fn transform(position: &Position, rotation: Option<&Quaternion>, transform: &mut mat4x4) {
    *transform = Transform::calculate(position, rotation);
}

#[system]
#[read_component(Velocity)]
#[write_component(Position)]
fn custom(world: &mut SubWorld, #[resource] time: &Time) {
    let mut query = <(Write<Position>, Read<Velocity>)>::query();
    for (pos, vel) in query.iter_mut(world) {
        *pos += *vel * time.seconds();
    }
}

The functions that are annotated with the #[system] attribute are left in place as normal functions, so there are no issues with IDE support and auto-complete etc. It generates a new function which constructs a system which will call your function. It supports just about everything you can do with SystemBuilder, although systems with multiple queries are still a little more efficient than using a "basic" system, because SystemBuilder can declare all the queries that will be used ahead of time (which allows caching some data between runs and the scheduler has more flexibility).

You even get nicer compile errors in many cases, because we can emit custom errors with the proc macro. e.g.:

Where that usize is squigglied with:

system function parameters must be `CommandBuffer` or `SubWorld` references,[optioned] component references, state references, or resource references

[TomGillen]

Just got generic systems working:

#[system(for_each)]
fn print_component<T: Component + Display>(component: &T) {
    println!("{}", component);
}

fn build_schedule() -> Schedule {
    Schedule::builder()
        .add_system(print_component_system::<Position>())
        .build()
}