From 0adf687ad30e775f83390d5cf1e2dd16a324b696 Mon Sep 17 00:00:00 2001 From: Anton Soloviev Date: Sat, 17 Oct 2026 21:06:43 -0700 Subject: [PATCH 1/3] Prevent empty registry rows from retaining cleared entities Read optional components without allocating sparse-set entries, and avoid creating rows when removals fail. Clear empty component and tag rows so unused entities can be collected while incoming relations and external references retain their existing ownership semantics. Preserve existing query behavior and the save schema. --- CHANGELOG.md | 3 ++ tcod/ecs/entity.py | 43 ++++++++++++------- tests/test_entity_lifecycle.py | 76 ++++++++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+), 14 deletions(-) create mode 100644 tests/test_entity_lifecycle.py diff --git a/CHANGELOG.md b/CHANGELOG.md index d7eb0d2..f125862 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Component reads and tag cleanup no longer retain unused entities through empty registry rows. +- Failed component removals no longer create empty rows, and clearing removes existing empty rows. + - Replacing component relations from an inherited view now preserves values which depend on the destination. - Removing a relation tag or component now preserves query membership when the other relation still exists. - Queries collecting component rows now resolve inherited components instead of raising `KeyError` for prefab instances. diff --git a/tcod/ecs/entity.py b/tcod/ecs/entity.py index c498c45..790e897 100644 --- a/tcod/ecs/entity.py +++ b/tcod/ecs/entity.py @@ -420,9 +420,12 @@ def __getitem__(self, key: ComponentKey[T]) -> T: assert self.__assert_key(key) _components_by_entity = self.entity.registry._components_by_entity for entity in _traverse_entities(self.entity, self.traverse): + components = _components_by_entity.get(entity) + if components is None: + continue try: - return _components_by_entity[entity][key] # type: ignore[no-any-return] - except KeyError: # noqa: PERF203 + return components[key] # type: ignore[no-any-return] + except KeyError: pass raise KeyError(key) @@ -444,10 +447,13 @@ def __delitem__(self, key: TypeForm[object] | tuple[object, TypeForm[object]]) - """Delete a directly held component from an entity.""" assert self.__assert_key(key) - old_value = self.entity.registry._components_by_entity[self.entity].get(key) + components = self.entity.registry._components_by_entity.get(self.entity) + if components is None: + raise KeyError(key) + old_value = components.get(key) - del self.entity.registry._components_by_entity[self.entity][key] - if not self.entity.registry._components_by_entity[self.entity]: + del components[key] + if not components: del self.entity.registry._components_by_entity[self.entity] del self.entity.registry._components_by_type[key][self.entity] @@ -589,8 +595,10 @@ def pop( def clear(self) -> None: """Remove any components stored directly in this entity.""" if self.traverse: - return self(traverse=()).clear() - return super().clear() + self(traverse=()).clear() + return + super().clear() + self.entity.registry._components_by_entity.pop(self.entity, None) @attrs.define(eq=False, frozen=True, weakref_slot=False) @@ -626,7 +634,7 @@ def add(self, tag: object) -> None: def discard(self, tag: object) -> None: """Discard a tag directly held by an entity.""" - if tag not in self.entity.registry._tags_by_entity[self.entity]: + if tag not in self.entity.registry._tags_by_entity.get(self.entity, ()): return # Already doesn't have tag tcod.ecs.query._touch_tag(self.entity.registry, tag) # Tag removed @@ -648,8 +656,9 @@ def remove(self, tag: object) -> None: def clear(self) -> None: """Remove all tags directly in this entity.""" _tags_by_entity = self.entity.registry._tags_by_entity - while _tags_by_entity[self.entity]: - self.discard(next(iter(_tags_by_entity[self.entity]))) + while tags := _tags_by_entity.get(self.entity): + self.discard(next(iter(tags))) + _tags_by_entity.pop(self.entity, None) def __contains__(self, x: object) -> bool: """Return True if this entity has the given tag.""" @@ -996,10 +1005,16 @@ def __setitem__(self, target: Entity, component: T) -> None: def __delitem__(self, target: Entity) -> None: """Delete a component assigned to the target entity.""" registry = self.entity.registry - del registry._relation_components_by_entity[self.entity][self.key][target] - if not registry._relation_components_by_entity[self.entity][self.key]: - del registry._relation_components_by_entity[self.entity][self.key] - if not registry._relation_components_by_entity[self.entity]: + by_entity = registry._relation_components_by_entity.get(self.entity) + if by_entity is None: + raise KeyError(target) + by_key = by_entity.get(self.key) + if by_key is None: + raise KeyError(target) + del by_key[target] + if not by_key: + del by_entity[self.key] + if not by_entity: del registry._relation_components_by_entity[self.entity] _relations_lookup_discard(registry, self.entity, self.key, target) diff --git a/tests/test_entity_lifecycle.py b/tests/test_entity_lifecycle.py new file mode 100644 index 0000000..985f156 --- /dev/null +++ b/tests/test_entity_lifecycle.py @@ -0,0 +1,76 @@ +"""Regression tests for unreferenced entity lifetimes.""" + +from __future__ import annotations + +import gc +import weakref + +import pytest + +from tcod.ecs import Registry + + +@pytest.mark.parametrize( + "operation", + [ + "clear", + "clear_empty", + "clear_tags", + "discard_missing_tag", + "read_missing_component", + "read_inherited_component", + "delete_missing_component", + "delete_missing_relation_component", + ], +) +def test_unused_entity_is_released(operation: str) -> None: + """Reads and cleanup must leave no registry reference to an unused entity.""" + registry = Registry() + prefab = registry["prefab"] + prefab_value = 7 + prefab.components[int] = prefab_value + entity = prefab.instantiate() if operation == "read_inherited_component" else registry.new_entity() + if operation == "clear": + entity.components[int] = 3 + entity.tags.add("inventory") + entity.clear() + elif operation == "clear_empty": + entity.clear() + elif operation == "clear_tags": + entity.tags.add("inventory") + entity.tags.clear() + elif operation == "discard_missing_tag": + entity.tags.discard("inventory") + elif operation == "read_missing_component": + assert entity.components.get(str) is None + elif operation == "read_inherited_component": + assert entity.components[int] == prefab_value + entity.relation_tags_many.clear() + elif operation == "delete_missing_component": + with pytest.raises(KeyError, match="str"): + del entity.components[str] + else: + assert operation == "delete_missing_relation_component" + with pytest.raises(KeyError, match="target"): + del entity.relation_components[str][registry["target"]] + reference = weakref.ref(entity) + del entity + gc.collect() + assert reference() is None + assert registry["prefab"] is prefab + assert prefab.components[int] == prefab_value + + +def test_incoming_relation_retains_cleared_entity() -> None: + """An incoming relation intentionally keeps its target alive until removed.""" + registry = Registry() + owner, target = registry["owner"], registry.new_entity() + owner.relation_tag["target"] = target + reference = weakref.ref(target) + target.clear() + del target + gc.collect() + assert reference() is not None + del owner.relation_tag["target"] + gc.collect() + assert reference() is None From 63acd025c7de60b9c92807b7867788333f2b3010 Mon Sep 17 00:00:00 2001 From: Anton Soloviev Date: Sat, 17 Oct 2026 21:21:34 -0700 Subject: [PATCH 2/3] Cover missing relation type deletion on populated entities --- tests/test_entity_lifecycle.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/test_entity_lifecycle.py b/tests/test_entity_lifecycle.py index 985f156..3d48f75 100644 --- a/tests/test_entity_lifecycle.py +++ b/tests/test_entity_lifecycle.py @@ -21,6 +21,7 @@ "read_inherited_component", "delete_missing_component", "delete_missing_relation_component", + "delete_missing_relation_component_type", ], ) def test_unused_entity_is_released(operation: str) -> None: @@ -49,10 +50,16 @@ def test_unused_entity_is_released(operation: str) -> None: elif operation == "delete_missing_component": with pytest.raises(KeyError, match="str"): del entity.components[str] + elif operation == "delete_missing_relation_component": + with pytest.raises(KeyError, match="target"): + del entity.relation_components[str][registry["target"]] else: - assert operation == "delete_missing_relation_component" + assert operation == "delete_missing_relation_component_type" + entity.relation_components[int][prefab] = 3 with pytest.raises(KeyError, match="target"): del entity.relation_components[str][registry["target"]] + assert entity.relation_components[int] == {prefab: 3} + del entity.relation_components[int][prefab] reference = weakref.ref(entity) del entity gc.collect() From 28fa94d9265c09cc790ae479a59d8ca88a86901d Mon Sep 17 00:00:00 2001 From: Anton Soloviev Date: Sun, 18 Oct 2026 06:43:39 -0700 Subject: [PATCH 3/3] Explain cleanup of existing empty component rows --- CHANGELOG.md | 1 - tcod/ecs/entity.py | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f125862..d2f8d6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Component reads and tag cleanup no longer retain unused entities through empty registry rows. - Failed component removals no longer create empty rows, and clearing removes existing empty rows. - - Replacing component relations from an inherited view now preserves values which depend on the destination. - Removing a relation tag or component now preserves query membership when the other relation still exists. - Queries collecting component rows now resolve inherited components instead of raising `KeyError` for prefab instances. diff --git a/tcod/ecs/entity.py b/tcod/ecs/entity.py index 790e897..08a548d 100644 --- a/tcod/ecs/entity.py +++ b/tcod/ecs/entity.py @@ -598,6 +598,8 @@ def clear(self) -> None: self(traverse=()).clear() return super().clear() + # MutableMapping.clear() only calls __delitem__ for existing components, + # so it leaves an already empty registry row behind. self.entity.registry._components_by_entity.pop(self.entity, None)