Skip to content

Commit

Permalink
fix: Issue where removing and re-adding an actor would cause subseque…
Browse files Browse the repository at this point in the history
…nt children to not function properly
  • Loading branch information
eonarheim committed May 28, 2024
1 parent 60616a2 commit fc3ab68
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 12 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).

- Fixed issue where `ex.SpriteFont` did not respect scale when measuring text
- Fixed issue where negative transforms would cause collision issues because polygon winding would change.
- Fixed issue where removing and re-adding an actor would cause subsequent children added not to function properly with regards to their parent/child transforms

### Updates

Expand Down
36 changes: 24 additions & 12 deletions src/engine/EntityComponentSystem/EntityManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import { Scene } from '../Scene';
export class EntityManager {
public entities: Entity[] = [];
public _entityIndex: { [entityId: string]: Entity } = {};
private _childAddedHandlerMap = new Map<Entity, (entity: Entity) => void>();
private _childRemovedHandlerMap = new Map<Entity, (entity: Entity) => void>();

constructor(private _world: World) {}

Expand All @@ -33,6 +35,14 @@ export class EntityManager {
}
}

private _createChildAddedHandler = () => (e: Entity) => {
this.addEntity(e);
};

private _createChildRemovedHandler = () => (e: Entity) => {
this.removeEntity(e, false);
};

/**
* Adds an entity to be tracked by the EntityManager
* @param entity
Expand All @@ -50,16 +60,12 @@ export class EntityManager {
c.scene = entity.scene;
this.addEntity(c);
});
entity.childrenAdded$.register({
notify: (e) => {
this.addEntity(e);
}
});
entity.childrenRemoved$.register({
notify: (e) => {
this.removeEntity(e, false);
}
});
const childAdded = this._createChildAddedHandler();
this._childAddedHandlerMap.set(entity, childAdded);
const childRemoved = this._createChildRemovedHandler();
this._childRemovedHandlerMap.set(entity, childRemoved);
entity.childrenAdded$.subscribe(childAdded);
entity.childrenRemoved$.subscribe(childRemoved);
}
}

Expand Down Expand Up @@ -93,8 +99,14 @@ export class EntityManager {
c.scene = null;
this.removeEntity(c, deferred);
});
entity.childrenAdded$.clear();
entity.childrenRemoved$.clear();
const childAddedHandler = this._childAddedHandlerMap.get(entity);
if (childAddedHandler) {
entity.childrenAdded$.unsubscribe(childAddedHandler);
}
const childRemovedHandler = this._childRemovedHandlerMap.get(entity);
if (childRemovedHandler) {
entity.childrenRemoved$.unsubscribe(childRemovedHandler);
}

// stats
if (this._world?.scene?.engine) {
Expand Down
17 changes: 17 additions & 0 deletions src/spec/EntityManagerSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,4 +64,21 @@ describe('An EntityManager', () => {
expect(entityManager.entities.length).toBe(0);
expect((entityManager as any)._entitiesToRemove.length).toBe(0);
});

it('should only remove the handlers that the manager added on removal', () => {
const entityManager = new ex.EntityManager(new ex.World(null));
const entity = new ex.Entity([new ex.TransformComponent()], 'some-e');
// Transform component listens to child add/remove
const entity2 = new ex.Entity();
entityManager.addEntity(entity);
entityManager.addEntity(entity2);

expect(entity.childrenAdded$.subscriptions.length).toBe(2);
expect(entity2.childrenAdded$.subscriptions.length).toBe(1);

entityManager.removeEntity(entity, false);
entityManager.removeEntity(entity2, false);
expect(entity.childrenAdded$.subscriptions.length).toBe(1);
expect(entity2.childrenAdded$.subscriptions.length).toBe(0);
});
});
20 changes: 20 additions & 0 deletions src/spec/TransformComponentSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,26 @@ describe('A TransformComponent', () => {
expect(child2.get(TransformComponent).get().parent).toBe(null);
});

it('can be parented to a previously deleted entity with transform component', () => {
const entityManager = new ex.EntityManager(new ex.World(null));

const child1 = new ex.Entity([new TransformComponent()]);
const child2 = new ex.Entity([new TransformComponent()]);
const parent = new ex.Entity([new TransformComponent()]);

parent.get(TransformComponent).pos = ex.vec(100, 500);
parent.addChild(child1);

entityManager.addEntity(parent);
entityManager.removeEntity(parent, false);

entityManager.addEntity(parent);
parent.addChild(child2);

expect(child1.get(TransformComponent).globalPos).toBeVector(ex.vec(100, 500));
expect(child2.get(TransformComponent).globalPos).toBeVector(ex.vec(100, 500));
});

it('children inherit the top most parent coordinate plane', () => {
const logger = ex.Logger.getInstance();
spyOn(logger, 'warn');
Expand Down

0 comments on commit fc3ab68

Please sign in to comment.