Repository: mlange-42/arche
Pull request #340 (2024-01-18 01:22:51)

Fix relation events, reset target on relation change, document subscriptions
* Target is reset to zero when (ex)-changing the relation component
* `RelationChanged` always triggers `TargetChanged`
* Document subscription logic

coveralls: ## Pull Request Test Coverage Report for [Build 7568749038](https://coveralls.io/builds/65147759)


* **0** of **0**   changed or added relevant lines in **0** files are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage remained the same at **100.0%**

---



|  Totals | [![Coverage Status](https://coveralls.io/builds/65147759/badge)](https://coveralls.io/builds/65147759) |
| :-- | --: |
| Change from base [Build 7563483577](https://coveralls.io/builds/65139483): |  0.0% |
| Covered Lines: | 5242 |
| Relevant Lines: | 5242 |

---
##### 💛  - [Coveralls](https://coveralls.io)


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index dd8ef2c..aa2cfe9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,7 +7,7 @@
 * Arche supports full world serialization and deserialization, in conjunction with [arche-serde](https://github.com/mlange-42/arche-serde) (#319)
 * Supports 256 instead of 128 component types as well as resource types and engine locks (#313)
 * Generic API supports up to 12 instead of 8 component types (#324)
-* Reworked event system with granular subscription to different event types and components (#333, #334, #335)
+* Reworked event system with granular subscription to different event types and components (#333, #334, #335, #337, #340)
 
 ### Breaking changes
 
@@ -32,7 +32,7 @@ This change was necessary to get the same performance as before, despite the mor
 ### Performance
 
 * Reduces archetype memory footprint by using a dynamically sized slice for storage lookup (#327)
-* Reduces event listener overhead through granular subscriptions and elimination of a heap allocation (#333, #334, #335, #337)
+* Reduces event listener overhead through granular subscriptions and elimination of a heap allocation (#333, #334, #335, #337, #340)
 
 ### Other
 
diff --git a/ecs/batch.go b/ecs/batch.go
index e708a74..b8dbae1 100644
--- a/ecs/batch.go
+++ b/ecs/batch.go
@@ -86,6 +86,9 @@ func (b *Batch) SetRelationQ(filter Filter, comp ID, target Entity) Query {
 
 // Exchange exchanges components for many entities, matching a filter.
 //
+// When a [Relation] component is removed and another one is added,
+// the target entity of the relation is reset to zero.
+//
 // Panics:
 //   - when called with components that can't be added or removed because they are already present/not present, respectively.
 //   - when called on a locked world. Do not use during [Query] iteration!
@@ -98,6 +101,9 @@ func (b *Batch) Exchange(filter Filter, add []ID, rem []ID) {
 // ExchangeQ exchanges components for many entities, matching a filter.
 // It returns a query over the affected entities.
 //
+// When a [Relation] component is removed and another one is added,
+// the target entity of the relation is reset to zero.
+//
 // Panics:
 //   - when called with components that can't be added or removed because they are already present/not present, respectively.
 //   - when called on a locked world. Do not use during [Query] iteration!
diff --git a/ecs/event.go b/ecs/event.go
index 8042116..6c0f705 100644
--- a/ecs/event.go
+++ b/ecs/event.go
@@ -6,13 +6,19 @@ import "github.com/mlange-42/arche/ecs/event"
 //
 // To receive change events, register a [Listener] with [World.SetListener].
 //
+// # Event types & subscriptions
+//
 // Events notified are entity creation and removal, component addition and removal,
 // and change of relations and their targets.
 //
-// Event types that are subscribed are determined by [Listener.Subscriptions].
+// Event types that are subscribed are determined by [Listener].Subscriptions.
 // Events that cover multiple types (e.g. entity creation and component addition) are only notified once.
 // Field EventTypes contains the [event.Subscription] bits of covered event types.
 //
+// See module [event] and the [event.Subscription] constants for subscription logic.
+//
+// # Event scheduling
+//
 // Events are emitted immediately after the change is applied.
 //
 // Except for removed entities, events are always fired when the [World] is in an unlocked state.
@@ -41,14 +47,23 @@ func (e *EntityEvent) Contains(bit event.Subscription) bool {
 //
 // A listener can be added to a [World] with [World.SetListener].
 //
-// See [EntityEvent] for details.
+// # Subscriptions
+//
+// Listeners can subscribe to one or more event types via method Subscriptions.
+// Further, subscriptions can be restricted to one or more components via method Components.
+//
+// See module [event] and the [event.Subscription] constants for subscription logic.
+//
+// # See also
+//
+// See [EntityEvent] for more details.
 // See package [github.com/mlange-42/arche/listener] for Listener implementations.
 type Listener interface {
 	// Notify the listener about a subscribed event.
 	Notify(world *World, evt EntityEvent)
 	// Subscriptions to event types.
 	Subscriptions() event.Subscription
-	// Components the listener subscribes to.
+	// Components the listener subscribes to. No component restrictions are indicated by nil.
 	Components() *Mask
 }
 
diff --git a/ecs/event/event.go b/ecs/event/event.go
index 5ac49e1..812cd4e 100644
--- a/ecs/event/event.go
+++ b/ecs/event/event.go
@@ -3,19 +3,64 @@
 // See also ecs.Listener and ecs.EntityEvent.
 package event
 
-// Subscription bits for individual events
+// Subscription bits for individual events.
 const (
-	// EntityCreated subscription bit
+	// EntityCreated subscription bit.
+	//
+	// Without component subscription:
+	//   - Creation of an entity with or without any components
+	// With component subscription:
+	//   - Creation of an entity with any of the given components
 	EntityCreated Subscription = 1
-	// EntityRemoved subscription bit
+
+	// EntityRemoved subscription bit.
+	//
+	// Without component subscription:
+	//   - Removal of an entity, with or without any components
+	// With component subscription:
+	//   - Removal of an entity with any of the given components
 	EntityRemoved Subscription = 1 << 1
-	// ComponentAdded subscription bit
+
+	// ComponentAdded subscription bit.
+	//
+	// Without component subscription:
+	//   - Addition of any component(s) to an entity
+	//   - Creation of an entity with any components
+	// With component subscription:
+	//   - Addition of any of the given components to an entity
+	//   - Creation of an entity with any of the given components
 	ComponentAdded Subscription = 1 << 2
-	// ComponentRemoved subscription bit
+
+	// ComponentRemoved subscription bit.
+	//
+	// Without component subscription:
+	//   - Removal of any component(s) from an entity
+	//   - Removal of an entity with any components
+	// With component subscription:
+	//   - Removal of any of the given components from an entity
+	//   - Removal of an entity with any of the given components
 	ComponentRemoved Subscription = 1 << 3
-	// RelationChanged subscription bit
+
+	// RelationChanged subscription bit.
+	//
+	// Without component subscription:
+	//   - Addition of a relation component
+	//   - Removal of a relation component
+	//   - Exchange of a relation component with another
+	// With component subscription:
+	//   - Addition of any of the given relation components
+	//   - Removal of any of the given relation components
+	//   - Exchange if any of the two is among the given components
 	RelationChanged Subscription = 1 << 4
-	// TargetChanged subscription bit
+
+	// TargetChanged subscription bit.
+	//
+	// Without component subscription:
+	//   - Whenever RelationChanged is triggered
+	//   - Change of the target entity of any relation component
+	// With component subscription:
+	//   - Whenever RelationChanged is triggered
+	//   - Change of the target entity of any of the given (relation) components
 	TargetChanged Subscription = 1 << 5
 )
 
diff --git a/ecs/world.go b/ecs/world.go
index 28b7041..7a2e29d 100644
--- a/ecs/world.go
+++ b/ecs/world.go
@@ -212,7 +212,7 @@ func (w *World) NewEntity(comps ...ID) Entity {
 		if arch.HasRelationComponent {
 			newRel = &arch.RelationComponent
 		}
-		bits := subscription(true, false, len(comps) > 0, false, newRel != nil, false)
+		bits := subscription(true, false, len(comps) > 0, false, newRel != nil, newRel != nil)
 		trigger := w.listener.Subscriptions() & bits
 		if trigger != 0 && subscribes(trigger, &arch.Mask, nil, w.listener.Components(), nil, newRel) {
 			w.listener.Notify(w, EntityEvent{Entity: entity, Added: arch.Mask, AddedIDs: comps, NewRelation: newRel, EventTypes: bits})
@@ -261,7 +261,7 @@ func (w *World) NewEntityWith(comps ...Component) Entity {
 		if arch.HasRelationComponent {
 			newRel = &arch.RelationComponent
 		}
-		bits := subscription(true, false, len(comps) > 0, false, newRel != nil, false)
+		bits := subscription(true, false, len(comps) > 0, false, newRel != nil, newRel != nil)
 		trigger := w.listener.Subscriptions() & bits
 		if trigger != 0 && subscribes(trigger, &arch.Mask, nil, w.listener.Components(), nil, newRel) {
 			w.listener.Notify(w, EntityEvent{Entity: entity, Added: arch.Mask, AddedIDs: ids, NewRelation: newRel, EventTypes: bits})
@@ -294,7 +294,7 @@ func (w *World) RemoveEntity(entity Entity) {
 			oldIds = oldArch.node.Ids
 		}
 
-		bits := subscription(false, true, false, len(oldIds) > 0, oldRel != nil, !oldArch.RelationTarget.IsZero())
+		bits := subscription(false, true, false, len(oldIds) > 0, oldRel != nil, oldRel != nil)
 		trigger := w.listener.Subscriptions() & bits
 		if trigger != 0 && subscribes(trigger, nil, &oldArch.Mask, w.listener.Components(), oldRel, nil) {
 			lock := w.lock()
@@ -450,7 +450,7 @@ func (w *World) Remove(entity Entity, comps ...ID) {
 // This is more efficient than subsequent use of [World.Add] and [World.Remove].
 //
 // When a [Relation] component is removed and another one is added,
-// the target entity of the relation remains unchanged.
+// the target entity of the relation is reset to zero.
 //
 // Panics:
 //   - when called for a removed (and potentially recycled) entity.
diff --git a/ecs/world_internal.go b/ecs/world_internal.go
index baa58b9..e06fc2e 100644
--- a/ecs/world_internal.go
+++ b/ecs/world_internal.go
@@ -30,7 +30,7 @@ func (w *World) newEntityTarget(targetID ID, target Entity, comps ...ID) Entity
 	}
 
 	if w.listener != nil {
-		bits := subscription(true, false, len(comps) > 0, false, true, !target.IsZero())
+		bits := subscription(true, false, len(comps) > 0, false, true, true)
 		trigger := w.listener.Subscriptions() & bits
 		if trigger != 0 && subscribes(trigger, &arch.Mask, nil, w.listener.Components(), nil, &targetID) {
 			w.listener.Notify(w, EntityEvent{Entity: entity, Added: arch.Mask, AddedIDs: comps, NewRelation: &targetID, EventTypes: bits})
@@ -67,7 +67,7 @@ func (w *World) newEntityTargetWith(targetID ID, target Entity, comps ...Compone
 	}
 
 	if w.listener != nil {
-		bits := subscription(true, false, len(comps) > 0, false, true, !target.IsZero())
+		bits := subscription(true, false, len(comps) > 0, false, true, true)
 		trigger := w.listener.Subscriptions() & bits
 		if trigger != 0 && subscribes(trigger, &arch.Mask, nil, w.listener.Components(), nil, &targetID) {
 			w.listener.Notify(w, EntityEvent{Entity: entity, Added: arch.Mask, AddedIDs: ids, NewRelation: &targetID, EventTypes: bits})
@@ -86,7 +86,7 @@ func (w *World) newEntities(count int, targetID ID, hasTarget bool, target Entit
 		if arch.HasRelationComponent {
 			newRel = &arch.RelationComponent
 		}
-		bits := subscription(true, false, len(comps) > 0, false, newRel != nil, !target.IsZero())
+		bits := subscription(true, false, len(comps) > 0, false, newRel != nil, newRel != nil)
 		trigger := w.listener.Subscriptions() & bits
 		if trigger != 0 && subscribes(trigger, &arch.Mask, nil, w.listener.Components(), nil, newRel) {
 			cnt := uint32(count)
@@ -131,7 +131,7 @@ func (w *World) newEntitiesWith(count int, targetID ID, hasTarget bool, target E
 		if arch.HasRelationComponent {
 			newRel = &arch.RelationComponent
 		}
-		bits := subscription(true, false, len(comps) > 0, false, newRel != nil, !target.IsZero())
+		bits := subscription(true, false, len(comps) > 0, false, newRel != nil, newRel != nil)
 		trigger := w.listener.Subscriptions() & bits
 		if trigger != 0 && subscribes(trigger, &arch.Mask, nil, w.listener.Components(), nil, newRel) {
 			var i uint32
@@ -202,7 +202,7 @@ func (w *World) removeEntities(filter Filter) int {
 			if len(arch.node.Ids) > 0 {
 				oldIds = arch.node.Ids
 			}
-			bits = subscription(false, true, false, len(oldIds) > 0, oldRel != nil, !arch.RelationTarget.IsZero())
+			bits = subscription(false, true, false, len(oldIds) > 0, oldRel != nil, oldRel != nil)
 			trigger := w.listener.Subscriptions() & bits
 			listen = trigger != 0 && subscribes(trigger, nil, &arch.Mask, w.listener.Components(), oldRel, nil)
 		}
@@ -281,6 +281,15 @@ func (w *World) exchange(entity Entity, add []ID, rem []ID, relation ID, hasRela
 		}
 	} else {
 		target = oldArch.RelationTarget
+		if !oldArch.RelationTarget.IsZero() && oldArch.Mask.ContainsAny(&w.registry.IsRelation) {
+			for _, id := range rem {
+				// Removing a relation
+				if w.registry.IsRelation.Get(id) {
+					target = Entity{}
+					break
+				}
+			}
+		}
 	}
 
 	oldIDs := oldArch.Components()
@@ -322,7 +331,7 @@ func (w *World) exchange(entity Entity, add []ID, rem []ID, relation ID, hasRela
 		}
 		targChanged := oldTarget != arch.RelationTarget
 
-		bits := subscription(false, false, len(add) > 0, len(rem) > 0, relChanged, targChanged)
+		bits := subscription(false, false, len(add) > 0, len(rem) > 0, relChanged, relChanged || targChanged)
 		trigger := w.listener.Subscriptions() & bits
 		if trigger != 0 {
 			changed := oldMask.Xor(&arch.Mask)
@@ -419,7 +428,19 @@ func (w *World) exchangeBatchNoNotify(filter Filter, add []ID, rem []ID, batches
 func (w *World) exchangeArch(oldArch *archetype, oldArchLen uint32, add []ID, rem []ID) (*archetype, uint32) {
 	mask := w.getExchangeMask(oldArch.Mask, add, rem)
 	oldIDs := oldArch.Components()
-	arch := w.findOrCreateArchetype(oldArch, add, rem, oldArch.RelationTarget)
+
+	target := oldArch.RelationTarget
+	if !target.IsZero() && oldArch.Mask.ContainsAny(&w.registry.IsRelation) {
+		for _, id := range rem {
+			// Removing a relation
+			if w.registry.IsRelation.Get(id) {
+				target = Entity{}
+				break
+			}
+		}
+	}
+
+	arch := w.findOrCreateArchetype(oldArch, add, rem, target)
 
 	startIdx := arch.Len()
 	count := oldArchLen
@@ -1048,7 +1069,7 @@ func (w *World) notifyQuery(batchArch *batchArchetypes) {
 			event.OldRelation = oldRel
 		}
 
-		bits := subscription(oldArch == nil, false, len(batchArch.Added) > 0, len(batchArch.Removed) > 0, relChanged, targChanged)
+		bits := subscription(oldArch == nil, false, len(batchArch.Added) > 0, len(batchArch.Removed) > 0, relChanged, relChanged || targChanged)
 		event.EventTypes = bits
 
 		trigger := w.listener.Subscriptions() & bits


Test patch:
diff --git a/ecs/world_event_test.go b/ecs/world_event_test.go
index bbd73b2..c957910 100644
--- a/ecs/world_event_test.go
+++ b/ecs/world_event_test.go
@@ -60,7 +60,7 @@ func TestWorldListener(t *testing.T) {
 		Added:       All(posID, velID, relID),
 		AddedIDs:    []ID{posID, velID, relID},
 		NewRelation: &relID,
-		EventTypes:  event.EntityCreated | event.ComponentAdded | event.RelationChanged,
+		EventTypes:  event.EntityCreated | event.ComponentAdded | event.RelationChanged | event.TargetChanged,
 	}, events[len(events)-1])
 
 	w.Add(e0, rotID)
@@ -132,7 +132,7 @@ func TestWorldListenerBuilder(t *testing.T) {
 		Added:       All(posID, relID),
 		AddedIDs:    []ID{posID, relID},
 		NewRelation: &relID,
-		EventTypes:  event.EntityCreated | event.ComponentAdded | event.RelationChanged,
+		EventTypes:  event.EntityCreated | event.ComponentAdded | event.RelationChanged | event.TargetChanged,
 	}, events[len(events)-1])
 
 	query := builder.NewBatchQ(10)
@@ -144,7 +144,7 @@ func TestWorldListenerBuilder(t *testing.T) {
 		Added:       All(posID, relID),
 		AddedIDs:    []ID{posID, relID},
 		NewRelation: &relID,
-		EventTypes:  event.EntityCreated | event.ComponentAdded | event.RelationChanged,
+		EventTypes:  event.EntityCreated | event.ComponentAdded | event.RelationChanged | event.TargetChanged,
 	}, events[len(events)-1])
 
 	builder.NewBatch(10, parent)
@@ -183,7 +183,7 @@ func TestWorldListenerBuilder(t *testing.T) {
 		Added:       All(posID, relID),
 		AddedIDs:    []ID{posID, relID},
 		NewRelation: &relID,
-		EventTypes:  event.EntityCreated | event.ComponentAdded | event.RelationChanged,
+		EventTypes:  event.EntityCreated | event.ComponentAdded | event.RelationChanged | event.TargetChanged,
 	}, events[len(events)-1])
 
 	query = builder.NewBatchQ(10)
@@ -195,7 +195,7 @@ func TestWorldListenerBuilder(t *testing.T) {
 		Added:       All(posID, relID),
 		AddedIDs:    []ID{posID, relID},
 		NewRelation: &relID,
-		EventTypes:  event.EntityCreated | event.ComponentAdded | event.RelationChanged,
+		EventTypes:  event.EntityCreated | event.ComponentAdded | event.RelationChanged | event.TargetChanged,
 	}, events[len(events)-1])
 
 	builder.NewBatch(10, parent)
diff --git a/ecs/world_test.go b/ecs/world_test.go
index 960abf7..d499eee 100644
--- a/ecs/world_test.go
+++ b/ecs/world_test.go
@@ -864,7 +864,7 @@ func TestWorldRelationSet(t *testing.T) {
 		RemovedIDs:  []ID{relID},
 		OldRelation: &relID,
 		NewRelation: nil,
-		EventTypes:  event.ComponentRemoved | event.RelationChanged,
+		EventTypes:  event.ComponentRemoved | event.RelationChanged | event.TargetChanged,
 	}, events[len(events)-1])
 
 	assert.Panics(t, func() { world.Relations().Get(e2, relID) })
@@ -1246,7 +1246,9 @@ func TestWorldRelationMove(t *testing.T) {
 	world.SetListener(&listener)
 
 	posID := ComponentID[Position](&world)
+	velID := ComponentID[Velocity](&world)
 	relID := ComponentID[testRelationA](&world)
+	rel2ID := ComponentID[testRelationB](&world)
 
 	target1 := world.NewEntity()
 	target2 := world.NewEntity()
@@ -1277,6 +1279,9 @@ func TestWorldRelationMove(t *testing.T) {
 		world.Remove(e, posID)
 	}
 
+	world.Batch().Add(All(relID), posID)
+	world.Batch().Exchange(All(relID), []ID{velID}, []ID{posID})
+
 	for i, e := range entities {
 		trg := world.Relations().Get(e, relID)
 
@@ -1287,8 +1292,15 @@ func TestWorldRelationMove(t *testing.T) {
 		}
 	}
 
+	world.Batch().Exchange(All(relID), []ID{rel2ID}, []ID{relID})
+
+	for _, e := range entities {
+		trg := world.Relations().Get(e, rel2ID)
+		assert.Equal(t, Entity{}, trg)
+	}
+
 	for _, e := range entities {
-		world.Remove(e, relID)
+		world.Remove(e, rel2ID)
 	}
 }
 
diff --git a/listener/dispatch_test.go b/listener/dispatch_test.go
index 40fa2c3..e04d488 100644
--- a/listener/dispatch_test.go
+++ b/listener/dispatch_test.go
@@ -93,23 +93,23 @@ func TestDispatchRelations(t *testing.T) {
 	builder1.NewBatch(10, parent1)
 
 	assert.Equal(t, 20, len(h1.events))
-	assert.Equal(t, 10, len(h2.events))
+	assert.Equal(t, 20, len(h2.events))
 
 	builder2.NewBatch(10)
 	builder2.NewBatch(10, parent2)
 
 	assert.Equal(t, 20, len(h1.events))
-	assert.Equal(t, 10, len(h2.events))
+	assert.Equal(t, 20, len(h2.events))
 
 	world.Batch().SetRelation(ecs.All(rel1ID), rel1ID, parent2)
 
 	assert.Equal(t, 20, len(h1.events))
-	assert.Equal(t, 30, len(h2.events))
+	assert.Equal(t, 40, len(h2.events))
 
 	world.Batch().RemoveEntities(ecs.All(rel1ID))
 
 	assert.Equal(t, 40, len(h1.events))
-	assert.Equal(t, 50, len(h2.events))
+	assert.Equal(t, 60, len(h2.events))
 }
 
 func TestDispatchAllComps(t *testing.T) {