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

Define behavior when redundantly adding/removing entities #173

Closed
antag99 opened this Issue Jun 8, 2015 · 2 comments

Comments

Projects
None yet
2 participants
@antag99
Copy link
Contributor

antag99 commented Jun 8, 2015

What should happen when an entity is added twice? The current behaviour isn't too pretty: the entity is added to two slots in the array. In addition to this, the current operation delay mechanism leads to bad behavior when adding/removing entities inside of listeners or when an EntitySystem is executing. (See #161 for details.) I created two test cases for the behaviour when adding an entity twice:

@Test
public void addEntityTwice () {
    Engine engine = new Engine();
    Entity entity = new Entity();
    engine.addEntity(entity);
    engine.addEntity(entity);
    assertEquals(1, engine.getEntities().size());
}

@Test
public void addEntityTwiceInListener () {
    final Entity entityA = new Entity();
    final Entity entityB = new Entity();
    final Engine engine = new Engine();

    engine.addEntityListener(new EntityListener() {
        @Override
        public void entityAdded (Entity entity) {
            engine.addEntity(entityB);
            engine.addEntity(entityB);
        }

        @Override
        public void entityRemoved (Entity entity) {
        }
    });

    engine.addEntity(entityA);

    assertEquals(2, engine.getEntities().size());
}

Also worth noting is that it is perfectly allowed to do something evil such as nesting Engine updates; this should throw an exception rather than silently allowing it.

@dsaltares

This comment has been minimized.

Copy link
Member

dsaltares commented Jun 8, 2015

Checking whether the entity is registered with an engine should be easy enough because the entityId is only set when that's the case.

  • Do we wanna throw when we try to add an entity that's registered with an engine already?
  • We can also throw when trying to do nesting engine updates.

We could very well say that adding an entity twice is undefined behavior and be done with it. However, I like trying to make Ashley hard to break, so we should probably cater for this problem.

@dsaltares dsaltares added the bug label Jun 8, 2015

@antag99

This comment has been minimized.

Copy link
Contributor

antag99 commented Jun 8, 2015

Do we wanna throw when we try to add an entity that's registered with an engine already?

Yes, I think so. This is probably always a mistake.

@dsaltares dsaltares closed this Jun 8, 2015

@SgtCoDFish SgtCoDFish referenced this issue Oct 29, 2017

Open

Feature Parity with Java Original #3

1 of 5 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment