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

Calling destroy twice has very odd effects #66

Closed
JackStouffer opened this issue Jan 28, 2021 · 1 comment
Closed

Calling destroy twice has very odd effects #66

JackStouffer opened this issue Jan 28, 2021 · 1 comment

Comments

@JackStouffer
Copy link

I was accidentally calling world.removeEntity and then entity.destroy in sequence.

function removeEntity(ecs: World, entity: Entity) {
    const graphicData = entity.getOne(GraphicsComponent);
    // delete the sprite from memory here

    ecs.removeEntity(entity);
    entity.destroy();
}

I noticed that doing this led to the entity's components not being cleared before it was entered into the entity pool. As I was stepping through the code I could clearly see that my entities were in the pool and all of its components were still defined.

Later on in the code, I called createEntity and the latest destroyed entity would be popped from the pool, with all of its original components. So the two component definitions would be overlaid on top of each other.

This is very surprising. I would expect the second call would be a no-op or would throw an error saying this entity is already destroyed.

@fritzy
Copy link
Owner

fritzy commented Feb 2, 2021

I wasn't able to reproduce this with a test.

ape-ecs/tests/index.ts

Lines 1918 to 1970 in 9dcbcd2

it('#66 Calling destroy twice has very odd effects', () => {
const world = new World({ entityPool: 1 });
class TestA extends Component {
static properties = {
greeting: "Hi",
a: 1
};
static typeName = 'TestA';
greeting: string;
a: number;
}
class TestB extends Component {
static properties = {
greeting: "Hi",
a: 1
};
static typeName = 'TestB';
greeting: string;
a: number;
}
world.registerComponent(TestA);
world.registerComponent(TestB);
const e = world.createEntity({
c: {
TestA: {
greeting: "What",
a: 2
}
}
});
world.removeEntity(e);
e.destroy();
const e2 = world.createEntity({
c: {
TestB: {
greeting: "No",
a: 3
}
}
});
expect(e2.has('TestA')).to.be.false;
expect(e2.has('TestB')).to.be.true;
});

However, you're right, that destroy() should no-op the second time, so I made that fix (world.removeEntity(e) also calls destroy() on the entity).

Is there some other way that I can reproduce the problem in the previous commit reliably?

In any case, ape-ecs@1.3.1 has been published.

@fritzy fritzy closed this as completed Feb 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants