junkdog / artemis-odb

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

[GroupManager] allow add entity to several groups at once. #485

Closed sonirico closed 7 years ago

sonirico commented 7 years ago

I noticed that much of my code looks like

   world.getSystem(GroupManager.class).add(e, "enemies");
   world.getSystem(GroupManager.class).add(e, "collidable-dynamic");
   world.getSystem(GroupManager.class).add(e, "big");

I think it could be pretty handy something like

   world.getSystem(GroupManager.class).add(e, "enemies").add(e, "big") ...

or even

   world.getSystem(GroupManager.class).add(e, new String [] { "enemies", "collidable", "big"});

It would just take a return this for the first option and a simple foreach for the second. This could also apply to remove methods if necessary.

sonirico commented 7 years ago

I could do it myself as soon as I am told which method you prefer, whether it is one of the enumerated above or not :)

DaanVanYperen commented 7 years ago

The odb-way to deal with this could be to inject GroupManager instead of resolving it each time, Another alternative is using components as flags (Big.class), and resolving sets using world.aspectSubscriptionManager(). But injecting GroupManager in each system gets messy and flags are not always static, so get where you are coming from!

For array methods that will likely get repeated a lot we typically make variants with up to ~3 parameters just so for most cases a loop/array creation can be avoided. Something like:

public GroupManager add(Entity e, String group);
public GroupManager add(Entity e, String group, String group2);
public GroupManager add(Entity e, String group, String group2, String group 3);
public GroupManager add(Entity e, String ... groups);

GroupManager has fallen behind a bit to begin with, could use add/remove variants for entityId as well.

sonirico commented 7 years ago

Mmm... Another couple of things I have just noticed.

DaanVanYperen commented 7 years ago

There could be useful the method isInEveryGroup(Entity, String...groups) to complement isInAnyone.

Bit of an edge case I'd say. There's certain a place in Artemis for a more practical and unified grouping and searching for entities.

Shouldn't GroupManager use LibGDX collections such as ObjectMap/IntMap instead of HashMaps?

Not every user wants LibGDX as a dependency for odb core modules, if they happen to be using a different library or running headless.

sonirico commented 7 years ago

Not every user wants LibGDX as a dependency for odb core modules

Yeah, I totally forgot!