junkdog / artemis-odb

A continuation of the popular Artemis ECS framework
BSD 2-Clause "Simplified" License
776 stars 111 forks source link

Request: entity.addComponents(...) #75

Closed DaanVanYperen closed 10 years ago

DaanVanYperen commented 10 years ago

Easy one this time. Could we get a variable length utility method for adding components to entities?

public void addComponents(Component... components)

junkdog commented 10 years ago

Variadic methods allocate a temp array on the heap each time it is invoked.

Where are you getting the components from? Would addComponents(List<Component>) and addComponents(Bag<Component>) work?

DaanVanYperen commented 10 years ago

Dang your focus on performance! I want my jam games slow and convenient to write! ;)

junkdog commented 10 years ago

I'm a hardliner ;) You can already chain addComponent, not exactly what you're looking for maybe, but I'd rather not introduce more GC-inflicting stuff into artemis.

Not saying it can't be done though... libGDX, for these kind of usecases, sometimes provide overloaded methods, one method for each for 1-n arguments and a last variadic method.

DaanVanYperen commented 10 years ago

I already chain it. I'm just really really lazy!

LibGDX's implementation seems pragmatic and performant! ;)

junkdog commented 10 years ago

Hmm... I'm closing this as a "won't fix" now - if you really, really want it. Re-open it and I'll add it (or send a PR).

DaanVanYperen commented 10 years ago

Nah you've got the right balance for artemis-odb going, this is far from a make or break feature for me ;)

Made a Builder that wraps creation, tagging and grouping for my own purposes.

/**
 * Entity creation helper.
 *
 * Please call artemis directly if you need high performance creation.
 *
 * Instance the builder once, call .create() per entity, chain steps, then .build()
 *
 * <p/>
 * myEntityBuilder
 * .in(world)
 * .with(Pos.class, Anim.class)
 * .tag("boss")
 * .group("enemies")
 * .build()
 * .addToWorld();
 *
 * @author Daan van Yperen
 */
public class EntityBuilder {

    private final World world;
    private Entity entity;
    private TagManager tagManager;
    private GroupManager groupManager;

    public EntityBuilder( World world ) {
        this.world = world;
    }

    /** Begin building new entity. Instead of instancing the builder over and over, we call this instead. */
    public EntityBuilder createEntity() {
        entity = world.createEntity();
        return this;
    }

    /** Add component to entity. */
    public EntityBuilder with(Component component) {
        entity.addComponent(component);
        return this;
    }

    /** Add components to entity. */
    public EntityBuilder with(Component component1, Component component2) {
        entity.addComponent(component1);
        entity.addComponent(component2);
        return this;
    }

    /** Add components to entity. */
    public EntityBuilder with(Component component1, Component component2, Component component3) {
        entity.addComponent(component1);
        entity.addComponent(component2);
        entity.addComponent(component3);
        return this;
    }

    /** Add components to entity. */
    public EntityBuilder with(Component component1, Component component2, Component component3, Component component4) {
        entity.addComponent(component1);
        entity.addComponent(component2);
        entity.addComponent(component3);
        entity.addComponent(component4);
        return this;
    }

    /** Add components to entity. */
    public EntityBuilder with(Component component1, Component component2, Component component3, Component component4, Component component5) {
        entity.addComponent(component1);
        entity.addComponent(component2);
        entity.addComponent(component3);
        entity.addComponent(component4);
        entity.addComponent(component5);
        return this;
    }

    /** Add components to entity. */
    public EntityBuilder with(Component... components) {
        for (int i = 0, n = components.length; i < n; i++) {
            entity.addComponent(components[i]);
        }
        return this;
    }

    /** Add artemis managed components to entity. */
    public EntityBuilder with(Class<Component> component) {
        entity.createComponent(component);
        return this;
    }

    /** Add artemis managed components to entity. */
    public EntityBuilder with(Class<Component> component1, Class<Component> component2) {
        entity.createComponent(component1);
        entity.createComponent(component2);
        return this;
    }

    /** Add artemis managed components to entity. */
    public EntityBuilder with(Class<Component> component1, Class<Component> component2, Class<Component> component3) {
        entity.createComponent(component1);
        entity.createComponent(component2);
        entity.createComponent(component3);
        return this;
    }

    /** Add artemis managed components to entity. */
    public EntityBuilder with(Class<Component> component1, Class<Component> component2, Class<Component> component3, Class<Component> component4) {
        entity.createComponent(component1);
        entity.createComponent(component2);
        entity.createComponent(component3);
        entity.createComponent(component4);
        return this;
    }

    /** Add artemis managed components to entity. */
    public EntityBuilder with(Class<Component> component1, Class<Component> component2, Class<Component> component3, Class<Component> component4, Class<Component> component5) {
        entity.createComponent(component1);
        entity.createComponent(component2);
        entity.createComponent(component3);
        entity.createComponent(component4);
        entity.createComponent(component5);
        return this;
    }

    /** Add artemis managed components to entity. */
    public EntityBuilder with(Class<Component>... components) {
        for (int i = 0, n = components.length; i < n; i++) {
            entity.createComponent(components[i]);
        }
        return this;
    }

    /** Register entity with tag. Requires registered TagManager */
    public EntityBuilder tag(String tag) {
        if (tagManager == null) {
            // resolve tagmanager.
            this.tagManager = world.getManager(TagManager.class);
            if ( tagManager == null )
            throw new RuntimeException("Register TagManager with your artemis world.");
        }
        tagManager.register(tag, entity);
        return this;
    }

    /** Register entity with group. Requires registered TagManager */
    public EntityBuilder group(String group) {
        if (groupManager == null) {
            // resolve groupmanager.
            this.groupManager = world.getManager(GroupManager.class);
            if ( groupManager == null )
                throw new RuntimeException("Register GroupManager with your artemis world.");
        }
        groupManager.add(entity, group);
        return this;
    }

    /** Assemble, but do not add to world yet. */
    public Entity build() {
        final Entity result = this.entity;
        this.entity = null;
        return result;
    }

    /** Assemble, add to world */
    public Entity addToWorld()
    {
        final Entity result = build();
        result.addToWorld();
        return result;
    }
}
junkdog commented 10 years ago

That looks pretty convenient - how about adding it under the com.artemis.utils package?

DaanVanYperen commented 10 years ago

Sure! It's copied from some uncommitted work on libgdx-artemis-quickstart (github), so might want to give it a proofread. ;)

junkdog commented 10 years ago

I changed the error message in your comment:

I personally would prefer if build added the entity to the world, as a sane default.

DaanVanYperen commented 10 years ago

I personally would prefer if build added the entity to the world, as a sane default.

Sounds like a good idea. I used the distinction to operate on the entity directly before shooting it into the world but for the life of me I can't think up a clear use-case for it.

Other than that, the builder instancing via createEntity is a bit of a compromise, typically a builder would be instanced constantly but this seemed cheaper ;) Maybe you know of a neater way to do it.

junkdog commented 10 years ago

Sounds like a good idea. I used the distinction to operate on the entity directly before shooting it into the world but for the life of me I can't think up a clear use-case for it.

Yeah, I know - it has occurred in my mind too, but never in actual code.

Other than that, the builder instancing via createEntity is a bit of a compromise, typically a builder would be instanced constantly but this seemed cheaper ;) Maybe you know of a neater way to do it.

No point in re-instancing the builder each time; although semantically, with might suggest that is the case. Otoh, I can't come up with a better alternative to with - it's short and to the point.

How about instancing the Entity inside the constructor? The createEntity method seems superfluous and would cause a NPE if it's accidentally omitted.

DaanVanYperen commented 10 years ago

How about instancing the Entity inside the constructor? The createEntity method seems superfluous and would cause a NPE if it's accidentally omitted.

Have the builder handle entity creation autonomously? seems fine, we could just return the current and prep the next build instance when build() gets called. Provided that doesn't cause engine issues.

I'm all for less typing ;)

junkdog commented 10 years ago

Ah... I see, it's actually a reusable builder. In that case we might need to keep the createEntity regardless, else we end up with leaking entities. Alternatively, build() allocates the entity.

DaanVanYperen commented 10 years ago

Yeah that's the compromise I was talking about. Unless instancing the builder over and over isn't a performance issue. You run benchmarks as a hobby, so figured you'd know. ;)

making build instance would be cleaner, but require the builder to track all choices. Could just do an instance check in each with, tag, etc XD.

junkdog commented 10 years ago

Hmm... doubt it'd cause a problem (it's a small, short-lived class - mainly intended for quick prototyping). An alternative would be to create the entity on demand - keeping two separate Bags for the components; class and instance types - but I have a feeling that would be more expensive for the typical scenario.

On Tue, May 6, 2014 at 1:15 AM, Daan van Yperen notifications@github.comwrote:

Yeah that's the compromise I was talking about. Unless instancing the builder over and over isn't a performance issue. You run benchmarks as a hobby, so figured you'd know. ;)

making build instance would be cleaner, but require the builder to track all choices. Could just do an instance check in each with, tag, etc XD.

— Reply to this email directly or view it on GitHubhttps://github.com/junkdog/artemis-odb/issues/75#issuecomment-42251973 .

DaanVanYperen commented 10 years ago

If the performance impact is minimal a non reusable builder would be less convoluted. new EntityBuilder(world).with(..).tag(..).build(); Could do away with manager fields if that makes a difference.

Maybe add tags(...) and groups(...) as well? More incidental perhaps but this is a convenience class after all.

junkdog commented 10 years ago

Sure thing, I'll add it later today.

junkdog commented 10 years ago

Entities can only have a single tag - there are two maps for tracking entity<->tag. Added the groups method though.