Remove amethyst_ecs sub crate

[nchashch]

Changes:

• There is no more amethyst_ecs sub crate.
• specs is now used directly.
• Processors are now called Systems.

Update:

• Add specs_batteries module.
• Move world_resources to specs_batteries::resources.
• Move components to specs_batteries::components.
• Move systems to specs_batteries::systems.

Update 2:

• Rename specs_batteries to ecs.

[kvark]

I think the original idea behind wrapping specs was to be able to swap it for something else. But I do welcome this change!

[Emilgardis]

What are the benefits for doing this?

[nchashch]

The only thing amethyst_ecs currently does is renaming System to Processor. So if we remove amethyst_ecs and use specs directly, we would have consistent naming between specs and amethyst, using specs would be explicit, and there would be less working directory clutter (no more amethyst_ecs sub crate).

[Emilgardis]

Can we add the current usage example to the specs repo, I feel like our example is good for explaining in a short way the possibilities specs provide.

[ebkalderon]

I think this crate should stay if and only if the default Amethyst components and systems from the core crate are moved into this one. Having all this ECS-related functionality in the top-level crate seems somewhat "messy" to me.

[kvark]

the use path appear quite long, perhaps change specs_batteries to just ecs?. Otherwise looks good.

[Emilgardis]

Just noticed the name aswell, I'd prefer something else.

[kvark]

@nchashch thanks! please rebase and make sure CI passes

[ebkalderon]

Didn't notice this before, but why are we implementing Component and VecStorage over here? Since the renderer is meant to be usable outside of the engine and specs in general, this functionality should be moved up to the top-level crate under amethyst::ecs.

[nchashch]

It is done in order to have a Light Component without a wrapper type (the way it was implemented before context rewrite PR), since it isn't possible to impl traits for types defined in an outside crate.

So it is either an amethyst::ecs::components::Light Component, which wraps a amethyst_renderer::Light and no specs import in amethyst_renderer or a plain amethyst_renderer::Light Component with a specs import.

I guess returning back to a wrapper type would work, but it is less convenient to access the Light Component this way. Instead of getting say light.color you would have to get light.light.color.

[ebkalderon]

But do we really need to wrap it? Can't we implement the Component trait directly onto the Light from inside the engine crate instead of the renderer? It seems like the current method causes a circular dependency, which defeats the purpose of having amethyst_renderer as a separate, modular crate entirely.

[nchashch]

Although I think if we wanted to return Light wrapper type (or PointLight/DirectionalLight wrappers since #166) it would be better to do this in a separate PR.

[nchashch]

But do we really need to wrap it? Can't we implement the Component trait directly onto the Light from inside the engine crate instead of the renderer? It seems like the current method causes a circular dependency, which defeats the purpose of having amethyst_renderer as a separate, modular crate entirely.

Currently it is impossible to directly impl an external trait for an external type (there is rust-lang/rfcs#493, so this might change for private trait impls in the future). AFAIK the only way to do this is to use a wrapper.

[ebkalderon]

I've messed around with implementing external traits on external types, and there doesn't seem to be any hacks or workarounds. Thanks for directing me to rust-lang/rfcs#493; I'll be following the issue very closely! In the meantime, I don't want to hold up the review process. We'll hopefully revisit this later in the future if and when Rust catches up. You've got my approval! 😄

[Emilgardis]

Wouldn't DerefMut fix this?

[ebkalderon]

@Emilgardis Indeed, implementing DerefMut would add some syntactic sugar that makes the code look a little prettier. However, it does not resolve the core problem that we are pulling in specs as a dependency of amethyst_renderer when it really doesn't need to be. The only reason the renderer is being kept outside the top-level crate is because it's intended to be usable outside of amethyst and specs. But that's out of the scope of this PR, so I'll open a new issue about it.