benmoran56 / esper

An ECS (Entity Component System) for Python
MIT License
544 stars 69 forks source link

`remove_component()` method removes entity if no more components left #82

Closed gretkierewicz closed 1 year ago

gretkierewicz commented 1 year ago

Describe the bug As for Single Responsibility Principle remove component should do just that but it removes empty entity as well. As I'm creating tests for my own project, I need to populate entity with some dummy component to prevent that and test cleaning another component logic.

To Reproduce

import esper

class Component:
    ...

world = esper.World()
entity = world.create_entity(Component())

assert world.entity_exists(entity=entity)

world.remove_component(entity=entity, component_type=Component)

assert world.entity_exists(entity=entity)  # AssertionError

Expected behavior I assume that cleaning components should not clear entity. As removing and adding component should be independent from entity existence. Entity should be deleted by delete_entity() only and not under the hood so programmer has full control over it.

Development environment:

Additional context I will create unit tests for it and fix if approved for implementation

benmoran56 commented 1 year ago

Hi Piotr,

Yes, please go ahead and remove that. I think this is leftover from the earlier code, and I never really thought about it. Thanks for opening the ticket.

gretkierewicz commented 1 year ago

@benmoran56 I wonder why remove_component() returns entity. Maybe could be good idea to return removed component instance? Imo that would make much more sense if you would like to transfer it to some other entity later on.

benmoran56 commented 1 year ago

Good question. That's pointless since the entity id is already known.

Returning the entity instance could be useful, as long as it's not too expensive.