Add register_components API for ergonomics

[ghost]

A suggestion for #1794.
If accepted, fixes #1794.

The linked issue original code was as follows:

world
    .register_component(ComponentDescriptor::new::<CombatState<Peaceful>>(
        StorageType::SparseSet,
    ))
    .unwrap();
world
    .register_component(ComponentDescriptor::new::<CombatState<Chasing>>(
        StorageType::SparseSet,
    ))
    .unwrap();
world
    .register_component(ComponentDescriptor::new::<CombatState<Searching>>(
        StorageType::SparseSet,
    ))
    .unwrap();

With the new API, this is now:

world
    .register_components(vec![
        ComponentDescriptor::new::<CombatState<Peaceful>>(StorageType::SparseSet)),
        ComponentDescriptor::new::<CombatState<Chasing>>(StorageType::SparseSet)),
        ComponentDescriptor::new::<CombatState<Searching>>(StorageType::SparseSet)),
    ]).unwrap();

Or perhaps simpler as done in the added docs+test:

fn cd<T: Component>() -> ComponentDescriptor {
    ComponentDescriptor::new::<T>(StorageType::SparseSet)
 }

let descriptors = vec![
    cd::<CombatState<Peaceful>>(),
    cd::<CombatState<Chasing>>(),
    cd::<CombatState<Searching>>(),
];
world.register_components(descriptors).unwrap();

[alice-i-cecile]

Registering components is a nuisance, but seems unavoidable here. For now, I think this is a nice ergonomics improvement with no downside.

[bjorn3]

Can you try to pre-allocate this vec based in the iterator size_hint? Or maybe replace the function body with descriptors.map(|descriptor| self.register_component(descriptor)).collect()? That shouls work because of the FromIterator<Result<T, E>> for Result<Vec<T>, E> impl.

[NathanSWard]

Yes, I agree with the use of collect() as it will allow the compiler to optimize this a little better.

[ghost]

Pushed the changes from the second suggestion.

Btw. if one were to use size_hint(), would it have been appropriate to unwrap the upper bound and use that?
Seems the upper bound can be None is some cases ("infinite" iterators, or user defined iters) but those seem nonsensical for this API (just curious).

[mockersf]

would it have been appropriate to unwrap the upper bound and use that?

Probably not, I could want to use my own iterator that doesn't implement size_hint well and that shouldn't crash

[cart]

Obligatory pushback on apis that add a new way to do the same thing: do we need this? I agree that the World api with unwrapping is a bit boilerplatey, but most users won't be using World directly.

When comparing the two options for AppBuilder (which the majority of people will use), I don't see a significant win:

// individual
app
    .register_component(ComponentDescriptor::new::<A>(StorageType::SparseSet))
    .register_component(ComponentDescriptor::new::<B>(StorageType::SparseSet))
    .register_component(ComponentDescriptor::new::<C>(StorageType::SparseSet))
// batch
app
    .register_components(vec![
        ComponentDescriptor::new::<A>(StorageType::SparseSet),
        ComponentDescriptor::new::<B>(StorageType::SparseSet),
        ComponentDescriptor::new::<C>(StorageType::SparseSet),
    ])

In general I want to push for "one way" to do things. With two apis, users will need to ask themselves "should I use a batch api here". Theres also the (very minor) issue of the additional allocations the batch api adds for the vec.

[alice-i-cecile]

When comparing the two options for AppBuilder (which the majority of people will use), I don't see a significant win

This is a more relevant comparison, and I see your point here. Ultimately I would love to have this defined at the type-level somehow (see #1843), which would help reduce the boilerplate substantially and encourage much more local code organization.

[ghost]

Obligatory pushback on apis that add a new way to do the same thing: do we need this? I agree that the World api with unwrapping is a bit boilerplatey, but most users won't be using World directly.

When comparing the two options for AppBuilder (which the majority of people will use), I don't see a significant win:

// individual
app
    .register_component(ComponentDescriptor::new::<A>(StorageType::SparseSet))
    .register_component(ComponentDescriptor::new::<B>(StorageType::SparseSet))
    .register_component(ComponentDescriptor::new::<C>(StorageType::SparseSet))
// batch
app
    .register_components(vec![
        ComponentDescriptor::new::<A>(StorageType::SparseSet),
        ComponentDescriptor::new::<B>(StorageType::SparseSet),
        ComponentDescriptor::new::<C>(StorageType::SparseSet),
    ])

In general I want to push for "one way" to do things. With two apis, users will need to ask themselves "should I use a batch api here". Theres also the (very minor) issue of the additional allocations the batch api adds for the vec.

I agree, this comparison shows that the added cognitive load of more APIs is likely not worth.
Closed.