Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue Deleting Entities During System Update and Using Listeners in 1.2 #64

Closed
Dano4Droid opened this issue Sep 17, 2014 · 13 comments
Closed
Labels

Comments

@Dano4Droid
Copy link

Hi,

I have an unusual issue discovered in Ashley 1.2. I discovered it while trying to incorporate the new Family option for the Listeners. I started getting NullPointerExceptions when the Entity entered the entityRemoved() method of the Listener. I later determined that a component was always missing if the Entity was deleted during the update. I am not sure if this is a bug - but it does appear to be inconsistent -since the component is present when deleted outside of the System update.

Here is an example of the issue:

package com.badlogic.ashley.tests;

import com.badlogic.ashley.core.ComponentMapper;
import com.badlogic.ashley.core.Engine;
import com.badlogic.ashley.core.Entity;
import com.badlogic.ashley.core.EntityListener;
import com.badlogic.ashley.core.EntitySystem;
import com.badlogic.ashley.core.Family;
import com.badlogic.ashley.core.PooledEngine;
import com.badlogic.ashley.tests.components.MovementComponent;
import com.badlogic.ashley.tests.components.PositionComponent;
import com.badlogic.ashley.utils.ImmutableArray;
import com.badlogic.gdx.utils.Array;


public class DeleteDuringUpdateListenerTest
{


    public static void main(String[] args)
    {
        PooledEngine engine = new PooledEngine();

        CombinedSystem combinedSystem = new CombinedSystem(engine);

        engine.addSystem(combinedSystem);
        engine.addEntityListener(Family
                .getFor(PositionComponent.class), new MyPositionListener());


        for (int i = 0; i < 10; i++)
        {
            Entity entity = engine.createEntity();
            entity.add(new PositionComponent(10, 0));
            entity.add(new MovementComponent(10, 2));

            engine.addEntity(entity);

        }

        log("system has: " + combinedSystem.allEntities.size() + " entities.");

        for (int i = 0; i < 10; i++)
        {
            engine.update(0.25f);

        }
        engine.removeAllEntities();
    }

    public static class MyPositionListener implements EntityListener
    {
        public static ComponentMapper<PositionComponent> positionMapper = ComponentMapper.getFor(PositionComponent.class);

        int counter = 0;

        @Override
        public void entityAdded(Entity entity)
        {

        }

        @Override
        public void entityRemoved(Entity entity)
        {

            PositionComponent position = positionMapper.get(entity);
            if (position == null)
            {
                log("there is no position component while removing, is this a bug?");
            }
            else
            {
                log("position component detected while removing");
            }

        }

    }

    public static class CombinedSystem extends EntitySystem
    {
        PooledEngine engine;
        protected CombinedSystem(PooledEngine engine)
        {
            this.engine = engine;
        }

        public ImmutableArray<Entity> allEntities;
        int counter = 0;


        @Override
        public void addedToEngine(Engine engine)
        {
            allEntities = engine.getEntitiesFor(Family
                    .getFor(PositionComponent.class));
        }

        @Override
        public void removedFromEngine(Engine engine)
        {
        }

        @Override
        public void update(float deltaTime)
        {
            //delete some of the entities during the update - note: this is when the 
            //bug (or feature) occurs!
            if (counter >= 6 && counter <=8)
            {
                engine.removeEntity(allEntities.get(2));
            }
            counter++;   
        }  
    }

    public static void log(String string)
    {
        System.out.println(string);
    }
}

output:
system has: 10 entities.
there is no position component while removing, is this a bug?
there is no position component while removing, is this a bug?
there is no position component while removing, is this a bug?
position component detected while removing
position component detected while removing
position component detected while removing
position component detected while removing
position component detected while removing
position component detected while removing
position component detected while removing

Thanks,

Dana

@Dano4Droid Dano4Droid changed the title Deleting Entities during a System update may cause some inconsistencies with Entity groups within Listeners Issue Deleting Entities During System Update and Using Listeners in 1.2 Sep 17, 2014
@dsaltares
Copy link
Member

I believe this could be related to issue #44. Can you please try the same but using the nightly builds or source rather than the 1.2.0 release? I suspect this is now fixed.

Thanks!

@Dano4Droid
Copy link
Author

If you made changes after the release, then it probably is fixed. I'll test it later this week when I get chance to get the most recent source.

Thanks,

Dana

@Dano4Droid
Copy link
Author

I just tested this with 1.2.1-SNAPSHOT. Right now this test results in a never ending loop as soon as the first remove is executed. The only output is "system has: 10 entities." Tested on 9-18-2014.

Thanks

Dana

@dsaltares
Copy link
Member

Will add a unit test and take a look at this over the weekend.

Thanks.

@dsaltares dsaltares added the bug label Sep 19, 2014
dsaltares added a commit that referenced this issue Sep 21, 2014
@dsaltares
Copy link
Member

Just added a test (as you can see in the issue updates) but it's passing just fine. I made the test from your code. Can you please check it and make sure it's testing what you wanted?

@Dano4Droid
Copy link
Author

Hi,

My test still fails with the infinite loop issue. One thing I noticed that is different with your test is that it does not test the PooledEngine. I think this issue is specific with the PooledEngine, and deletion of entities during the update.

Thanks,

Dana

@dsaltares
Copy link
Member

Actually, the test IS using PooledEngine.

Engine engine = new PooledEngine();

@Dano4Droid
Copy link
Author

In your test - I noticed earlier that you were creating the entity with new Entity() instead of using engine.createEntity(). That probably caused the reset() method and freeing of the Entity in the Poolable code to also be skipped.

Thanks,

Dana

dsaltares added a commit that referenced this issue Sep 21, 2014
@dsaltares
Copy link
Member

Alright, got it now, I believe it's now fixed. Could you please pull from master and test?

Thanks.

@Dano4Droid
Copy link
Author

Great! I'll test it tomorrow. Thanks

junkdog added a commit to junkdog/entity-system-benchmarks that referenced this issue Sep 22, 2014
@Dano4Droid
Copy link
Author

Hi,

I confirmed the test was fixed. But now I am getting a different issue. Will debug it further to determine if it is an issue with Ashley or my game code.

Thanks,

Dana

@dsaltares
Copy link
Member

Awesome, I will close this issue, you can open a new one if you discover a new bug :-).

@Dano4Droid
Copy link
Author

The issue was in my code. The latest Ashley snapshot version appears to be working great.

Thanks,
Dana

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants