Entity is already registered

[carlislefox]

Hi guys,

I'm having a real headache here. I'm using the PooledEngine and I create and dispose all of my entities through my Engine instance as instructed, however It seems if I remove a load of entities in an update cycle, then after the engine update has completed I create and add new entities, it sporadically throws this exception.

java.lang.IllegalArgumentException: Entity is already registered com.badlogic.ashley.core.PooledEngine$PooledEntity@2d59b291
at com.badlogic.ashley.core.EntityManager.addEntityInternal(EntityManager.java:115)
at com.badlogic.ashley.core.EntityManager.addEntity(EntityManager.java:33)
at com.badlogic.ashley.core.Engine.addEntity(Engine.java:61)

My issue is, the code that's throwing that exception is the following:

Entity bloodEntity = createEntity();
bloodEntity.add(renderPositionComponent);
bloodEntity.add(spriteComponent);
bloodEntity.add(expiringComp);
addEntity(bloodEntity);

It's the final line that does it. My createEntity call there is directly to the engine, and this code is executing after a call to engine.update - I've spent two days trying to get to the bottom of this and I really can't, it only ever happens when stuff has been removed in the prior engine update call. The pooled engine has just passed me an entity via create, why is it immediately exploding and telling me it's already added?

In addition, sporadically it appears as though the engine is sometimes removing the wrong entities, I feel like this could somehow be related? Is it possible entities are being mistaken for each other? Could be a red herring.

Unfortunately my project is huge and fairly complex at this point so I can't exactly just throw a nice repro up, I was hoping you guys might have some ideas given this information that I could investigate further? Any clue why this would be happening?

[dsaltares]

It sounds like the PooledEntity objects are not being properly disposed when being returned to the pool. This is going to be very complicated to replicate. Could you please try to come up with a unit test that fails, exposing this issue?

I will give it a shot this weekend too.

[carlislefox]

I have to sheepishly admit I haven't written LibGDX unit tests before, I've been needing to get around to doing it for my own project anyway so I guess now's the time to figure that out :P

Through some further testing I can confirm this only occurs with the PooledEngine, I have reverted back to Engine and created my own 'create' methods as such (so as not to break the rest of my code in the mean while) that just return new instances...

    /**
     * Temporary due to Ashley bug with {@link PooledEngine}
     */
    public Entity createEntity () {
        return new Entity();
    }

    /**
     * Temporary due to Ashley bug with {@link PooledEngine}
     */
    public <T extends Component> T createComponent (Class<T> componentType) {
        try {
            return componentType.newInstance();
        } catch (InstantiationException e) {
            e.printStackTrace();
        } catch (IllegalAccessException e) {
            e.printStackTrace();
        }

        return null;
    }

...and everything is happy. This at least narrows it down specifically to the reuse of entities in the PooledEngine.

[dsaltares]

I have to sheepishly admit I haven't written LibGDX unit tests before, I've been needing to get around to doing it for my own project anyway so I guess now's the time to figure that out :P.

This is not Libgdx specific, we use the pretty standard JUnit testing framework. All of our unit tests are written using JUnit.

[lukz]

In my case it happens when accidentally I remove object two times (there is no check in Ashley if object is already removed when removing and putting it in free objects pool) so when you pick up objects from pool you will get same instance multiple times and when adding them to engine it will throw exception.

In my case it often happens when I remove objects at the end of tween (Animations, Particles etc.) but for some reason object is removed earlier. I have object hierarchy in which when parent is removed also children are removed. So for example I'm adding to weapon as child some animation and before it gets to the end weapon entity is removed with animation entity but TweenEngine continues and removes it again at the end of tween.

@saltares shouldn't we check if entity is already removed before removing it again in PooledEngine and throw some exception to simplify debugging?

[dsaltares]

@lukz your issue could potentially be the same as @carlislefox's. Arguably this is a bug in your code although it is true that the symptoms appear quite far away from the root cause, which is bad.

Will look into throwing an exception if the entity didn't belong to an engine, should be pretty easy I believe.

[dsaltares]

I believe this is only a problem with PooledEngine, so we'll throw the exception when trying to remove an entity that is not in the engine to the PooledEngine class. Otherwise, we may break existing code.

[lukz]

@saltares yeah, throwing an exception when removing already removed entity only in PooledEngine would simplify tracking those bugs.

It will make little harder to migrate from Engine to PooledEngine (tracking existing entity removal bugs if Engine is not throwing an exception) but we should be fine with that.

[carlislefox]

Is an exception necessary?

Say in the same logic loop a system decides it's time for two entities to die, but one entity just so happens to be linked to that other entity and also decides to 'clean it up' as part of it's own destruction, you would have two removal calls within one update cycle. I don't see this case actually being an error in execution, more unfortunate timing.

Are we considering this to be worthy of throwing an exception or is it something we should design for and perhaps throw some warning level logging in? Two removal calls to the same entity is fine when not pooling, should it not be the same with?

[lukz]

@carlislefox shouldn't that be handled by some system that will prepare unique list of entities to remove if you are implementing such hierarchy? This is how I'm doing that.

[carlislefox]

Could be. Still not sure enforcing one remove request per update cycle is really necessary though, feels like an artificial limitation. What do you think Saltares?

[dsaltares]

I fixed it by making the second removal operation from actually doing anything. This should stop the exceptions @carlislefox is getting and won't break anyone.

[carlislefox]

Thanks a lot @Salteres, and nice work figuring out the cause @lukz ! I'll test this out asap

[carlislefox]

Question - is it possible there is a similar bug when pushing component removal requests to entities i,e, entity.remove(MyComponent.class) ? I am still seeing a bit of weirdness very sporadically despite running with the latest snapshot (verified your fix is in it) and using a system to remove all flagged entities. I am no longer getting exceptions, but I'm convinced I've seen the wrong entity vanish, could this be a similar issue to as above but perhaps it's my sprite component that is having issues now rather than the entity itself?

Edit: Alright I am definitely still seeing incorrect entities vanishing as I am adding new ones, it's like it thinks the previous ones are free when they are not. Blaaaaaargh.

[dsaltares]

We have checks in Entity and PooledEntity to only return components to the pool if we actually removed a component.

We can rewrite entity.remove() so that it's more explicit, but I think the code is correct.

Do you have a failing unit test with the issue? Maybe later this evening I can try write one.

[carlislefox]

Arr, I think I've got to the bottom of my issue - I've been really dumb.

I have two threads, one for the networking code, one is the LibGDX thread, I am creating entities via PooledEngine.createEntity() on the networking thread as I pass my dataInputStream directly to my components (which are also created in the networking thread) so they can read themselves - this obviously is screwing things up as it exists outside of the render() loop and must be short circuiting the PooledEngine's entity / component management.

Until I have rewritten everything so this is no longer the case, I would close this ticket as I cannot guarantee there is actually an issue here and entity creation / removal being thread safe isn't a reasonable request given most of libgdx is not thread safe.

If I find it still to be an issue after I have sorted this tangle out, I will get a working test case before commenting again on the ticket :)

Liam

[dsaltares]

Cool, multithreading would explain this as none of Ashley is threadsafe, same with Libgdx itself.

[carlislefox]

Well, I've gotten everything into the update cycle now and I still have this issue, though it seems the symptoms are slightly different, it seems if I add an entity around the same time as I remove one, sometimes it seems to 'steal' the components from the last entity I added. It's like because I removed an entity with the same components it thinks that the last one with those components I added is free?

Either that, or it gives me the same entity twice - but I'd expect my subsequent add to throw an exception so it must be the components that are getting mixed up? This is really, really strange.

Could the removal of an entity somehow incorrectly shift an index or something somewhere leading to the same component being freed twice?

EDIT: Never mind I've found the issue, it's my own proprietary code - because there was also the removal issue earlier in the ticket it masked my own. Thanks for all your help Saltares.