Mechanism to elegantly cleanup components

[dsaltares]

Use case:

PhysicsComponent contains a Box2D body. Every time either a component of this class is removed from an entity or the entity itself is removed from the engine, we need to call world.destroyBody(component.body). It would be nice to have a mechanism to catch this event and act accordingly.

Options:

• EntityListener that processes entities in the family of entities with physics component that does the cleanup on entity removed from the family.
• Make ComponentListener a public class and have something like this:

public interface ComponentListener {
    public void componentAdded(Entity e, Component e);
    public void componentRemoved(Entity e, Component e);
}

public class Engine {
    public void addComponentListener(Class<? implements Component> componentClass, ComponentListener listener)
    public void removeComponentListener(Class<? implements Component> componentClass, ComponentListener listener);
}

• Somehow reuse the Entity signaling system that already notifies engine of these events.
• Component could implement Disposable and the Engine would call dispose() if it implements the interface. Nasty because it adds logic to components.

What do you guys think would be best?

[azakhary]

I am all for option one. Usually you want to keep components simple. And in rare cases when you need to do this, working with engine and listeners is the best option.

The other option would be to have a "garbadge" collecting system that iterates over all entities, and does it's thing, but. like.. why? :D

[antag99]

Option one does not work if the PhysicsComponent is removed individually, as the component is removed before notifications are carried out. This can be demonstrated with the following test case:

@Test
public void testResourceCleanup() {
    class Resource implements Component {
    }

    final Engine engine = new Engine();
    engine.addEntityListener(Family.all(Resource.class).get(), new EntityListener() {
        @Override
        public void entityAdded(Entity entity) {
            assertNotNull(entity.getComponent(Resource.class));
        }

        @Override
        public void entityRemoved(Entity entity) {
            assertNotNull(entity.getComponent(Resource.class));
        }
    });
    Entity entity = new Entity();
    entity.add(new Resource());
    engine.addEntity(entity);
    entity.remove(Resource.class);
}

It might be a good idea to carry out notifications before removing the component. Otherwise, it could be necessary to implement some cleanup mechanism, but that adds quite a bit to the API.

[azakhary]

You can remove your physics body on both events, component removed, and entity removed.

[azakhary]

Although yes, it will result in a much cleaner code to cleanup based on the component disposing, rather then having ton's of if statements in entityRemoved

[antag99]

It's not possible to remove the body when removing a single component. As demonstrated by the test case, entity.getComponent(PhysicsComponent.class will return null inside the entity listener.

[azakhary]

ou.. oups. I didn't realize that. this has to be done something about. That means I had this body leaks without knowing about it, because I had a null check before removing....

[tdgroot]

Since nothing really happened on this issue, I'll just drop my solution here:
https://gist.github.com/Desmaster/a05d669208a05f98e8074761af5aad98

Could be way more advanced, but it currently fits my needs.

[mgsx-dev]

I would go for solution 2 for many reasons :

• it address both component cleanup and component initialization. With the Box2D example, you may want to create body once component added. With this solution, both body creation/destruction code would be in the same listener.
• sometimes you need whole entity to cleanup/initialize component which is not possible with the dispose method.

For now my workaround is to use PooledEngine and implements Poolable interface on my components which works but it doesn't work with other engine implementation.

@saltares I think this topic should be reopened.