Components are added/removed immediately, listeners are notified afte…

…r system update. Fixes issue #186
libgdx · Oct 24, 2015 · 84de679 · 84de679
1 parent 5550b65
commit 84de679
Show file tree

Hide file tree

Showing 5 changed files with 180 additions and 48 deletions.
diff --git a/ashley/src/com/badlogic/ashley/core/Engine.java b/ashley/src/com/badlogic/ashley/core/Engine.java
@@ -432,8 +432,12 @@ private void processComponentOperations() {
 			ComponentOperation operation = componentOperations.get(i);
 
 			switch(operation.type) {
-				case Add: operation.entity.addInternal(operation.component); break;
-				case Remove: operation.entity.removeInternal(operation.componentClass); break;
+				case Add:
+					operation.entity.notifyComponentAdded();
+					break;
+				case Remove:
+					operation.entity.notifyComponentRemoved();
+					break;
 				default: break;
 			}
 
@@ -463,25 +467,25 @@ public ComponentOperationHandler(Engine engine) {
 			this.engine = engine;
 		}
 
-		public void add(Entity entity, Component component) {
+		public void add(Entity entity) {
 			if (engine.updating) {
 				ComponentOperation operation = engine.componentOperationsPool.obtain();
-				operation.makeAdd(entity, component);
+				operation.makeAdd(entity);
 				engine.componentOperations.add(operation);
 			}
 			else {
-				entity.addInternal(component);
+				entity.notifyComponentAdded();
 			}
 		}
 
-		public void remove(Entity entity, Class<? extends Component> componentClass) {
+		public void remove(Entity entity) {
 			if (engine.updating) {
 				ComponentOperation operation = engine.componentOperationsPool.obtain();
-				operation.makeRemove(entity, componentClass);
+				operation.makeRemove(entity);
 				engine.componentOperations.add(operation);
 			}
 			else {
-				entity.removeInternal(componentClass);
+				entity.notifyComponentRemoved();
 			}
 		}
 	}
@@ -494,27 +498,20 @@ public enum Type {
 
 		public Type type;
 		public Entity entity;
-		public Component component;
-		public Class<? extends Component> componentClass;
 
-		public void makeAdd(Entity entity, Component component) {
+		public void makeAdd(Entity entity) {
 			this.type = Type.Add;
 			this.entity = entity;
-			this.component = component;
-			this.componentClass = null;
 		}
 
-		public void makeRemove(Entity entity, Class<? extends Component> componentClass) {
+		public void makeRemove(Entity entity) {
 			this.type = Type.Remove;
 			this.entity = entity;
-			this.component = null;
-			this.componentClass = componentClass;
 		}
 
 		@Override
 		public void reset() {
 			entity = null;
-			component = null;
 		}
 	}
 

diff --git a/ashley/src/com/badlogic/ashley/core/Entity.java b/ashley/src/com/badlogic/ashley/core/Entity.java
@@ -62,11 +62,15 @@ public Entity () {
 	 * @return The Entity for easy chaining
 	 */
 	public Entity add (Component component) {
-		if (componentOperationHandler != null) {
-			componentOperationHandler.add(this, component);
-		} else {
-			addInternal(component);
+		if (addInternal(component)) {
+			if (componentOperationHandler != null) {
+				componentOperationHandler.add(this);
+			}
+			else {
+				notifyComponentAdded();
+			}
 		}
+
 		return this;
 	}
 
@@ -80,10 +84,13 @@ public Component remove (Class<? extends Component> componentClass) {
 		int componentTypeIndex = componentType.getIndex();
 		Component removeComponent = components.get(componentTypeIndex);
 
-		if (componentOperationHandler != null) {
-			componentOperationHandler.remove(this, componentClass);
-		} else {
-			removeInternal(componentClass);
+		if (removeComponent != null && removeInternal(componentClass)) {
+			if (componentOperationHandler != null) {
+				componentOperationHandler.remove(this);
+			}
+			else {
+				notifyComponentRemoved();
+			}
 		}
 
 		return removeComponent;
@@ -92,7 +99,7 @@ public Component remove (Class<? extends Component> componentClass) {
 	/** Removes all the {@link Component}'s from the Entity. */
 	public void removeAll () {
 		while (componentsArray.size > 0) {
-			removeInternal(componentsArray.get(0).getClass());
+			remove(componentsArray.get(0).getClass());
 		}
 	}
 
@@ -129,15 +136,13 @@ <T extends Component> T getComponent (ComponentType componentType) {
 	}
 
 	/**
-	 * Internal use.
 	 * @return Whether or not the Entity has a {@link Component} for the specified class.
 	 */
 	boolean hasComponent (ComponentType componentType) {
 		return componentBits.get(componentType.getIndex());
 	}
 
 	/**
-	 * Internal use.
 	 * @return This Entity's component bits, describing all the {@link Component}s it contains.
 	 */
 	Bits getComponentBits () {
@@ -149,31 +154,35 @@ Bits getFamilyBits () {
 		return familyBits;
 	}
 
-	Entity addInternal (Component component) {
+	/**
+	 * @param component
+	 * @return whether or not the component was added.
+	 */
+	boolean addInternal (Component component) {
 		Class<? extends Component> componentClass = component.getClass();
-
 		Component oldComponent = getComponent(componentClass);
 
 		if (component == oldComponent) {
-			return this;
+			return false;
 		}
 
 		if (oldComponent != null) {
 			removeInternal(componentClass);
 		}
 
 		int componentTypeIndex = ComponentType.getIndexFor(componentClass);
-
 		components.set(componentTypeIndex, component);
 		componentsArray.add(component);
-
 		componentBits.set(componentTypeIndex);
-
-		componentAdded.dispatch(this);
-		return this;
+
+		return true;
 	}
 
-	Component removeInternal (Class<? extends Component> componentClass) {
+	/**
+	 * @param componentClass
+	 * @return whether or not a component with the specified class was found and removed.
+	 */
+	boolean removeInternal (Class<? extends Component> componentClass) {
 		ComponentType componentType = ComponentType.getFor(componentClass);
 		int componentTypeIndex = componentType.getIndex();
 		Component removeComponent = components.get(componentTypeIndex);
@@ -182,11 +191,19 @@ Component removeInternal (Class<? extends Component> componentClass) {
 			components.set(componentTypeIndex, null);
 			componentsArray.removeValue(removeComponent, true);
 			componentBits.clear(componentTypeIndex);
-
-			componentRemoved.dispatch(this);
+			
+			return true;
 		}
-
-		return removeComponent;
+
+		return false;
+	}
+
+	void notifyComponentAdded() {
+		componentAdded.dispatch(this);
+	}
+
+	void notifyComponentRemoved() {
+		componentRemoved.dispatch(this);
 	}
 
 	/** @return true if the entity is scheduled to be removed */

diff --git a/ashley/src/com/badlogic/ashley/core/PooledEngine.java b/ashley/src/com/badlogic/ashley/core/PooledEngine.java
@@ -93,8 +93,8 @@ protected void removeEntityInternal (Entity entity) {
 
 	private class PooledEntity extends Entity implements Poolable {
 		@Override
-		Component removeInternal (Class<? extends Component> componentType) {
-			Component component = super.removeInternal(componentType);
+		public Component remove (Class<? extends Component> componentClass) {
+			Component component = super.remove(componentClass);
 
 			if (component != null) {
 				componentPools.free(component);

diff --git a/ashley/tests/com/badlogic/ashley/core/EngineTests.java b/ashley/tests/com/badlogic/ashley/core/EngineTests.java
@@ -20,6 +20,7 @@
 
 import org.junit.Test;
 
+import com.badlogic.ashley.systems.IteratingSystem;
 import com.badlogic.ashley.utils.ImmutableArray;
 import com.badlogic.gdx.utils.Array;
 import com.badlogic.gdx.utils.Bits;
@@ -548,6 +549,125 @@ public void entitySystemRemovalWhileIterating () {
 			assertEquals(1, entities.get(i).getComponent(CounterComponent.class).counter);
 		}
 	}
+
+	public class ComponentAddSystem extends IteratingSystem {
+		private ComponentAddedListener listener; 
+
+		public ComponentAddSystem (ComponentAddedListener listener) {
+			super(Family.all().get());
+			this.listener = listener;
+		}
+
+		@Override
+		protected void processEntity (Entity entity, float deltaTime) {
+			assertNull(entity.getComponent(ComponentA.class));
+			entity.add(new ComponentA());
+			assertNotNull(entity.getComponent(ComponentA.class));
+			listener.checkEntityListenerUpdate();
+		}
+	}
+
+	public class ComponentRemoveSystem extends IteratingSystem {
+		private ComponentRemovedListener listener; 
+
+		public ComponentRemoveSystem (ComponentRemovedListener listener) {
+			super(Family.all().get());
+			this.listener = listener;
+		}
+
+		@Override
+		protected void processEntity (Entity entity, float deltaTime) {
+			assertNotNull(entity.getComponent(ComponentA.class));
+			entity.remove(ComponentA.class);
+			assertNull(entity.getComponent(ComponentA.class));
+			listener.checkEntityListenerUpdate();
+		}
+	}
+
+	public class ComponentAddedListener implements EntityListener {
+		int addedCalls;
+		int numEntities;
+
+		public ComponentAddedListener(int numEntities) {
+			this.numEntities = numEntities;
+		}
+
+		@Override
+		public void entityAdded (Entity entity) {
+			addedCalls++;
+		}
+
+		@Override
+		public void entityRemoved (Entity entity) {
+
+		}
+
+		public void checkEntityListenerNonUpdate() {
+			assertEquals(numEntities, addedCalls);
+			addedCalls = 0;
+		}
+
+		public void checkEntityListenerUpdate() {
+			assertEquals(0, addedCalls);
+		}
+	}
+
+	public class ComponentRemovedListener implements EntityListener {
+		int removedCalls;
+		int numEntities;
+
+		public ComponentRemovedListener(int numEntities) {
+			this.numEntities = numEntities;
+		}
+
+		@Override
+		public void entityAdded (Entity entity) {
+
+		}
+
+		@Override
+		public void entityRemoved (Entity entity) {
+			removedCalls++;
+		}
+
+		public void checkEntityListenerNonUpdate() {
+			assertEquals(numEntities, removedCalls);
+			removedCalls = 0;
+		}
+
+		public void checkEntityListenerUpdate() {
+			assertEquals(0, removedCalls);
+		}
+	}
+
+	@Test
+	public void entityAddRemoveComponentWhileIterating() {
+		int numEntities = 20;
+		Engine engine = new Engine();
+		ComponentAddedListener addedListener = new ComponentAddedListener(numEntities);
+		ComponentAddSystem addSystem = new ComponentAddSystem(addedListener);
+
+		ComponentRemovedListener removedListener = new ComponentRemovedListener(numEntities);
+		ComponentRemoveSystem removeSystem = new ComponentRemoveSystem(removedListener);
+
+		for (int i = 0; i < numEntities; ++i) {
+			Entity entity = new Entity();
+			engine.addEntity(entity);
+		}
+
+		engine.addEntityListener(Family.all(ComponentA.class).get(), addedListener);
+		engine.addEntityListener(Family.all(ComponentA.class).get(), removedListener);
+
+		engine.addSystem(addSystem);
+		engine.update(deltaTime);
+		addedListener.checkEntityListenerNonUpdate();
+		engine.removeSystem(addSystem);
+
+		engine.addSystem(removeSystem);
+		engine.update(deltaTime);
+		removedListener.checkEntityListenerNonUpdate();
+		engine.removeSystem(removeSystem);
+	}
 
 	@Test
 	public void familyListener () {

diff --git a/ashley/tests/com/badlogic/ashley/core/PooledEngineTests.java b/ashley/tests/com/badlogic/ashley/core/PooledEngineTests.java
@@ -179,9 +179,8 @@ public void resetEntityCorrectly () {
 
 	@Test
 	public void recycleEntity () {
-		PooledEngine engine = new PooledEngine();
-
-		int numEntities = 200;
+		int numEntities = 5;
+		PooledEngine engine = new PooledEngine(numEntities, 100, 0, 100);
 		Array<Entity> entities = new Array<Entity>();
 
 		for (int i = 0; i < numEntities; ++i) {
@@ -200,8 +199,7 @@ public void recycleEntity () {
 		for (int i = 0; i < numEntities; ++i) {
 			Entity entity = engine.createEntity();
 			assertEquals(0, entity.flags);
-			engine.addEntity(entity);
-			entities.add(entity);
+			assertTrue(entities.contains(entity, true));
 		}
 	}