Skip to content
This repository has been archived by the owner on Apr 18, 2022. It is now read-only.

META: transition to legion #2278

Closed
chemicstry opened this issue May 7, 2020 · 2 comments
Closed

META: transition to legion #2278

chemicstry opened this issue May 7, 2020 · 2 comments

Comments

@chemicstry
Copy link
Member

chemicstry commented May 7, 2020

This is a meta issue for transition from specs to legion. Continuation of amethyst/rfcs#22

Thank you @jaynus for your work on the legion. Since the legion branch was more than 150 commits behind master and had different folder structure due to initial specs and legion coexistance it was almost impossible to merge. So I manually ported most of the changes on top of fresh master and work continues here https://github.com/amethyst/amethyst/commits/legion_v2.

Progress tracking is done in legion project https://github.com/amethyst/amethyst/projects/21

Current status is similar to the one described in the RFC issue. Core, assets, rendy are ported and can be tested by the two window and material examples.

Feel free to pick any issue from the project and contribute 😃

@chemicstry chemicstry added this to To do in Legion integration via automation May 7, 2020
@chemicstry
Copy link
Member Author

chemicstry commented May 12, 2020

So there are some architectural issues that need to be discussed before continuing with the rest of the port. I will try to overview them to get some feedback.

System syntax

One of the main differences with legion is that systems use closures instead of traits with run function (as in specs). This seems fine for small isolated examples in the legion repository, but for more complicated systems syntax gets really annoying. For example, currently specs systems are initialized in two stages. One is System::new() which is used to provide user data to the system, other is SystemDesc::build(self, world: &mut World) trait which allows to initialize resources in the world for the system (such as registering even channel reader). So the whole system implementation looks really neat with just 3 functions new, build and run. With legion, however, we can't have system as a struct because automatic type deduction only works on closure (correct me if I'm wrong) and it's impossible to define run without lengthy expressions. This forces system syntax to look like this:

pub fn build_my_system(config: MyConfig) ->
    Box<dyn FnOnce(&mut World, &mut Resources) -> Box<dyn Schedulable>>
{
    Box::new(|world, resources| {
        // Initialize resources
        resources.insert(SomeResource::default());
        SystemBuilder::<()>::new("MySystem")
            .write_resource::<SomeResource>()
            .build(move |commands, world, res, query| {
                // do smth with config and SomeResource
            })
    })
}

As you can see the syntax is quite ugly. First function gets called by the user to provide Config when building dispatcher. This closure is then called by DispatcherBuilder during build to setup resources and produces a Schedulable system. There are some attempts to improve system API here and here. The first solution in my opinion introduces too much custom non-rust syntax which is undesirable. The second solution looks quite nice and would potentially allow retaining specs like system definitions (?).

Dispatcher

Second major part is dispatcher. Since we no longer have system dependencies as in specs (although it could be implemented, but I don't think it was a good concept), a staged executor concept was introduced in legion rfc. Stages are mostly needed by system bundles to insert multiple systems in the correct order. Although, I have not yet seen any bundle that would have non-sequential dependencies in the engine. The downside of this approach is that it makes really hard to understand system execution order from code as you have to look at each bundle implementation what stage each system is assigned to.

I think a much better approach is current legion's scheduler. You basically add all systems in order of execution and they are parallelised based on their data dependencies. This provides a really nice deterministic control over system execution order. However, it puts the burden of correctly ordering core systems (input, transform, rendering) on the user, otherwise delays could be introduced (not a big deal?).

There were also ideas for dynamic dispatcher, so that you could add and remove systems on the fly. This would technically only be possible with the staged dispatcher, as it would allow inserting systems at a specific order. But for legion's scheduler, I think this could be implemented as dynamic bundles. You would basically query for bundle in dispatcher (by name?) and add/remove/clear systems in it. This could also be recursively composable. Pausing bundles is also possible (yay!).

@chemicstry
Copy link
Member Author

Closing this as we have #2309

Legion integration automation moved this from To do to Done Aug 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
No open projects
Development

No branches or pull requests

2 participants