Engine.getEntitiesFor(Family) returning non-matching entities

[Barryrowe]

I've been working with Ashley 1.7.0 for a while and it's been running smooth. I just upgraded to 1.7.2 and now I am getting a NullPointer in the code below. When I get the FollowerComponent from an entity in the list of engine.getEntitiesFor(Family.all(FollowerComponent.class).get()), the FollowerCompnent is null.

This shouldn't be possible correct? All of the entities in the returned array should be guaranteed to have the component's required by the Family.

 EntityListener el;
    @Override
    public void addedToEngine(Engine engine) {
        super.addedToEngine(engine);

        final Engine eg = engine;
        if(el == null){
            el = new EntityListener() {
                private ComponentMapper<FollowerComponent> fm = ComponentMapper.getFor(FollowerComponent.class);

                public void entityAdded(Entity entity) {

                }

                @Override
                public void entityRemoved(Entity entity) {
                    for (Entity follower : eg.getEntitiesFor(Family.all(FollowerComponent.class).get())) {
                        FollowerComponent fc = fm.get(follower);
                       if (fc.target == entity) {  ///<<<<------ error occurs here
                            fc.target = null;
                            follower.removeAll();
                            eg.removeEntity(follower);
                        }
                    }

                }
            };
        }

        engine.addEntityListener(Family.all(EnemyComponent.class).get(), el);
    }

[dsaltares]

Could you please provide a jUnit test that highlights the issue? That would make it a lot easier to reproduce and fix. Thanks a lot!

[Barryrowe]

I'll work on a repro for this soon. I wish I hadn't found it in the midst of "Jam Code" so it was easier to extract :)

[dsaltares]

If you don't mind and having limited time these days, I will wait for the test case for this potential bug. Thanks for reporting it though!

[Barryrowe]

No problem. I'll try to get a unit test or at least a small example project that presents the issue this weekend.

The issue exists and is reproducible in this project, but there's quite a bit of code in there, and as the name shows it was started for libgdxjam, so a lot of it is probably a mess:
https://github.com/RoaringCatGames/libgdxjam-2015/tree/laxversion

The System where I encountered the bug is:
https://github.com/RoaringCatGames/libgdxjam-2015/blob/laxversion/core/src/com/roaringcatgames/libgdxjam/systems/FollowerSystem.java

Line 52 is where I had to null check to protect against this issue after the upgrade.

[Barryrowe]

I was able to create a unit test to highlight this. PR #217

If I call Entity.removeAll(), and then engine.remove(entity) in that block, I get instances where the engine.getEntitiesFor(Family) returns a component that shouldn't match the family any longer.

I don't know if this counts as a bug now, or just bad usage on my part. It seems Entity.removeAll is redundant if you are removing the entity anyway, but not knowing for certain, I was trying to be safe.

If I comment out the line that removes the components, I don't get any bad hits. The unit test I've created will have the line commented out so that all tests pass for anyone else that might get this while it's being worked.

[dsaltares]

I think this is related, although maybe not the same as #161, which has been around for a while, removeAll() is not atomic and can cause issues if combined with other entity operations before the next time pending operations are processed.

No one has come up with a nice, clean solution so far. I'm happy to hear new proposals though, as I'd like Ashley to be as robust as possible.