Repository: benmoran56/esper

Issue:
`remove_component()` method removes entity if no more components left
**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**
```python3
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:**
 - Python 3.11
 - Esper 2.4

**Additional context**
I will create unit tests for it and fix if approved for implementation


Discussion:
gretkierewicz: @benmoran56  check this out pls 

I'm using pop() instead of del to return removed component but it should not have big performance impact overall

Patch:
diff --git a/esper/__init__.py b/esper/__init__.py
index 5ac17a7..1c843e7 100644
--- a/esper/__init__.py
+++ b/esper/__init__.py
@@ -1,7 +1,9 @@
+import inspect as _inspect
 import time as _time
 
 from types import MethodType as _MethodType
 
+from typing import cast as _cast
 from typing import Iterable as _Iterable
 from typing import List as _List
 from typing import Optional as _Optional
@@ -317,10 +319,10 @@ class World:
         self._entities[entity][component_type] = component_instance
         self.clear_cache()
 
-    def remove_component(self, entity: int, component_type: _Type[_C]) -> int:
+    def remove_component(self, entity: int, component_type: _Type[_C]) -> _C:
         """Remove a Component instance from an Entity, by type.
 
-        A Component instance can be removed by providing its type.
+        A Component instance can only be removed by providing its type.
         For example: world.delete_component(enemy_a, Velocity) will remove
         the Velocity instance from the Entity enemy_a.
 
@@ -332,13 +334,8 @@ class World:
         if not self._components[component_type]:
             del self._components[component_type]
 
-        del self._entities[entity][component_type]
-
-        if not self._entities[entity]:
-            del self._entities[entity]
-
         self.clear_cache()
-        return entity
+        return self._entities[entity].pop(component_type)
 
     def _get_component(self, component_type: _Type[_C]) -> _Iterable[_Tuple[int, _C]]:
         entity_db = self._entities


Test patch:
diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml
index b01379b..b9db2ce 100644
--- a/.github/workflows/unit-tests.yml
+++ b/.github/workflows/unit-tests.yml
@@ -11,10 +11,10 @@ jobs:
     strategy:
       matrix:
         os: [ 'ubuntu-latest', 'macos-latest', 'windows-latest' ]
-        python-version: [ '3.8', '3.9', '3.10', 'pypy-3.7' ]
+        python-version: [ '3.8', '3.9', '3.10', '3.11', '3.12-dev', 'pypy-3.7' ]
     steps:
       - name: Python ${{ matrix.python-version }} ${{ matrix.os }}
-        uses: actions/checkout@v2
+        uses: actions/checkout@v3
       - name: Set up Python ${{ matrix.python-version }}
         uses: actions/setup-python@v4
         with:
diff --git a/tests/test_world.py b/tests/test_world.py
index 126102c..db99d96 100644
--- a/tests/test_world.py
+++ b/tests/test_world.py
@@ -278,6 +278,55 @@ class TestEntityExists:
         assert world.entity_exists(entity_with_component)
 
 
+class TestRemoveComponent:
+    def test_remove_from_not_existing_entity_raises_key_error(self, world):
+        with pytest.raises(KeyError):
+            world.remove_component(123, ComponentA)
+
+    def test_remove_not_existing_component_raises_key_error(self, world):
+        entity = world.create_entity(ComponentB())
+
+        with pytest.raises(KeyError):
+            world.remove_component(entity, ComponentA)
+
+    def test_remove_component_with_object_raises_key_error(self, populated_world):
+        entity = 2
+        component = ComponentD()
+
+        assert populated_world.has_component(entity, type(component))
+        with pytest.raises(KeyError):
+            populated_world.remove_component(entity, component)
+
+    def test_remove_component_returns_removed_instance(self, world):
+        component = ComponentA()
+        entity = world.create_entity(component)
+
+        result = world.remove_component(entity, type(component))
+
+        assert result is component
+
+    def test_remove_last_component_leaves_empty_entity(self, world):
+        entity = world.create_entity()
+        world.add_component(entity, ComponentA())
+
+        world.remove_component(entity, ComponentA)
+
+        assert not world.has_component(entity, ComponentA)
+        assert world.entity_exists(entity)
+
+    def test_removing_one_component_leaves_other_intact(self, world):
+        component_a = ComponentA()
+        component_b = ComponentB()
+        component_c = ComponentC()
+        entity = world.create_entity(component_a, component_b, component_c)
+
+        world.remove_component(entity, ComponentB)
+
+        assert world.component_for_entity(entity, ComponentA) is component_a
+        assert not world.has_component(entity, ComponentB)
+        assert world.component_for_entity(entity, ComponentC) is component_c
+
+
 def test_event_dispatch_no_handlers():
     esper.dispatch_event("foo")
     esper.dispatch_event("foo", 1)