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

Component/System/Resource Grouping #155

Closed
Aceeri opened this issue Jun 1, 2017 · 5 comments
Closed

Component/System/Resource Grouping #155

Aceeri opened this issue Jun 1, 2017 · 5 comments

Comments

@Aceeri
Copy link
Member

Aceeri commented Jun 1, 2017

Grouping systems together through a procedural macro that would allow calling a method with each type of component/system/resource. This would allow for static dispatch of serialization or anything dealing with groups of components, systems, or resources (like configurations).

Reasons

Ergonomic registering of components, systems, and resources
While registering components by hand takes dozens of lines depending on the size of the project and its dependencies, registering a group would take a single line of code:

world.register_components::<ComponentsGroup>();
world.register_systems::<SystemsGroup>();
world.register_resources::<ResourcesGroup>();

Modularization of dependencies
External users of specs will likely not be using barebones specs and only their own components, especially in the future. For example, Amethyst provides Transform, Renderable, Parent, and more. Currently either the third party would have to make a special registering method to register all their components together or leave it up to the user to register those themselves, which can be a very tedious task when given a large amount of components.

Similarly without a component group, serialization would require manually plucking component storages and serializing them manually (as it is currently not possible to get a trait object of a Serialize/Serializer/Deserialize/Deserializer from serde.

It is very likely that a third party would make a macro for similar tasks to avoid a lot of boilerplate and centralizing it to a singular macro would be extremely beneficial rather than having the user juggle different implementations of macros/methods for registering and serialization.

Implementation Details

Currently the groups are just defined by two traits:

/// Group of components. Can be subgrouped into other component groups.
pub trait ComponentGroup {
    /// Components defined in this group, not a subgroup.
    fn local_components() -> Vec<&'static str>;
    /// Components defined in this group along with subgroups.
    fn components() -> Vec<&'static str>;
    /// Subgroups included in this group.
    fn subgroups() -> Vec<&'static str>;
    /// Registers the components into the world.
    fn register(&mut World);
}

/// Group of serializable components.
#[cfg(feature="serialize")]
pub trait SerializeGroup: ComponentGroup {
    /// Serializes the group of components from the world.
    fn serialize_group<S: Serializer>(world: &World, serializer: S) -> Result<S::Ok, S::Error>;
    /// Helper method for serializing the world.
    fn serialize_subgroup<S: Serializer>(world: &World, map: &mut S::SerializeMap) -> Result<(), S::Error>;
    /// Deserializes the group of components into the world.
    fn deserialize_group<D: Deserializer>(world: &mut World, entities: &[Entity], deserializer: D) -> Result<(), D::Error>;
    /// Helper method for deserializing the world.
    fn deserialize_subgroup<V>(world: &mut World, entities: &[Entity], key: String, visitor: &mut V) -> Result<Option<()>, V::Error>
        where V: serde::de::MapVisitor;
}

The implementation for the groups are defined by a procedural macro. For example:

#[derive(ComponentGroup)]
struct SpecsGroup {
    // Components of the group.
    comp1: Comp1,
    comp2: Comp2,
    // Components with this attribute will be added to the list of serializable components.
    #[group(serialize)]
    comp3: Comp3,
    // Not all fields in a group have to be a component, you can define a subgroup with
    // an attribute like so which will include all the components in that subgroup
    // along with any recursive subgroups.
    #[group(subgroup)]
    subgroup1; Subgroup,
}

Expanding usage of the groups for external implementations
Currently the only way to add methods that take in components is to modify the procedural macro. I have an idea to generate an implementation for a trait like GroupLocals or GroupSubgroups.

trait GroupLocals {
    type A;
    type B;
    type C;
    ... // Lots more associated types.
    fn used() -> usize; // Associated items that are being used.
}

So an implementation for some group with 3 components, Comp1, Comp2, ... might look like:

// Note: I could *possibly* define a trait for each group that only has the amount
// of associated items necessary for how many components there are in the group.
impl GroupLocals for SomeGroup {
    type A = Comp1;
    type B = Comp2;
    type C = Comp3;
    type D = (); // The rest of the associated items would be defined as `()`.
    ...
    fn used() -> usize { 3 }
}

This would allow a macro of the form:

fn method<A, B, C, D>(a: A, b: B, d: D) { ... }
call!( local: SomeGroup => fn method [ G1, G2 ] in [ G3 ] ( arg1, arg2, arg3) );
// G1, G2, and G3 are defined of the method, something like a `String`, `u32`, etc.
// `in` defines where the component generic would go in the call.

// For example, the macro would be expanded into a bunch of calls like:
method::<G1, G2, Comp1, G3>(arg1, arg2, arg3);
method::<G1, G2, Comp2, G3>(arg1, arg2, arg3);
method::<G1, G2, Comp3, G3>(arg1, arg2, arg3);

Feel free to ask any questions or ideas for improvements.

@torkleyy
Copy link
Member

torkleyy commented Jun 1, 2017

Thanks for writing all this up.

world.register_systems::<SystemsGroup>();

Given that the World does not know anything about systems and these are only added to the DispatcherBuilder, groups for systems don't make any sense in this context (there is one kind of group that would be useful, but that's totally out of scope here).

as it is currently not possible to get a trait object of a Serialize/Serializer/Deserialize/Deserializer from serde

Looking at the serde implementation we have a trait like this:

pub trait Serialize {
    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
        where S: Serializer;
}

So a trait object for Serialize is not possible due to its method being generic over S, however we can in these cases always do the following:

pub trait AnySerialize {
    fn serialize(&self, ser: Box<AnySerializer>) -> Result<Box<Any>, Box<Error>>;
}

impl<T> AnySerialize for T 
    where T: Serialize
{
    fn serialize(&self, ser: Box<AnySerializer>) -> Result<Box<Any>, Box<Error>> {
        <T as Serialize>::serialize(self, ser).map(Box::new).map_err(Box::new)
    }
}

(It's of course obvious that your idea is much better, just wanted to note that it is possible)

Modularization of dependencies

Fully agree with that point.

Expanding usage of the groups for external implementations

I cannot really follow what you need this for or what it's really about. Could you please explain when you would use that?

pub trait ComponentGroup {
   /// Components defined in this group, not a subgroup.
   fn local_components() -> Vec<&'static str>;
   /// Components defined in this group along with subgroups.
   fn components() -> Vec<&'static str>;
   /// Subgroups included in this group.
   fn subgroups() -> Vec<&'static str>;
   /// Registers the components into the world.
   fn register(&mut World);
} ```

Would recommend renaming local_components to components and components to components_recursively. Also, you probably forgot &self as the first parameter.


It's a pretty complex topic, so I'm not sure about all the details yet. I'll think a bit more about this and come back here later.

@kvark
Copy link
Member

kvark commented Jun 1, 2017

@Aceeri great detail, thanks for the issue!

I don't buy the ergonomics argument. This is a one-time operations that engines hide from the users anyway: there is no reason why Amethyst, for example, should require users to register all the components manually. Right now they (as in - Amethyst devs) at least have flexibility for optionally registering some components/systems, where ComponentGroups look more fixed.

As for serialization, I haven't really looked into it. How difficult/error prone it is to serialize and deserialize the world today?

@Aceeri
Copy link
Member Author

Aceeri commented Jun 2, 2017

@torkleyy That AnySerialize is essentially what erased-serde is sadly it isn't that easy due to the Serializer associated types and bounds. Plus it uses a lot of dynamic dispatch. Every call down the chain is a dynamic dispatch which can really hurt performance over time.

Expanding usage just means allowing people who have access to the ComponentGroups to call a method with the components as a generic. The fundamental part of this is just removing the boilerplate of:

method::<Comp1>();
method::<Comp2>();
method::<Comp3>();
...

which that macro allows for even when you don't have access to the procedural macro implementation.

Since right now the way I do it is just:

quote! {
    #(
        method::<#component>();
    )*
};

in simplest terms.

The component group methods are mainly just for debugging purposes and aren't entirely necessary. Doesn't make much sense to make them non-static methods either since you shouldn't really be constructing a component group rather than just using it as a generic argument.

@kvark I'm not entirely sure what you mean by optional components, but you can always just group together components into sections like Physics2d, Physics3d, Scripting, SceneGraph, etc. and allow the user to choose and pick which ones.

Ignoring components in a group can also be added to component groups:

Subgroup ignoring components.:

#[derive(ComponentGroup)]
struct ParentGroup {
    comp1: Comp1,]

    #[group(subgroup)]
    #[group(ignore(Comp2))]
    subgroup: Subgroup,
}

#[derive(ComponentGroup)]
struct Subgroup {
    comp2: Comp2,
    comp3: Comp3,
}

Serialization/deserialization of the world is kind of tricky because it is very context specific. This ties back to the entity serialization/deserialization, since a component can depend on another entity. If a component does have reference to another entity then it doesn't make much sense to deserialize/serialize that component's storage alone.

@Rhuagh
Copy link
Member

Rhuagh commented May 9, 2018

Is this still relevant with saveload and setup?

@torkleyy
Copy link
Member

torkleyy commented May 9, 2018

No, don't think so. It would be overly complicated for little use.

@torkleyy torkleyy closed this as completed May 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants