Skip to content

Commit

Permalink
Fix component subscription logic (#337)
Browse files Browse the repository at this point in the history 
* use saparate masks for component addition and removal -- was not taken into account correctly
* fix component subscription logic -- some relation subscriptions could get lost
* tweak docs of listener implementations

# Commits

* tweak docs of listener implementations
* fix and tweak conditions in determining listener subscription
* use only triggered event type bits for component subscription check
* move check for event type subscription into utility function
* use separate added and removed masks in events and subscription logic
* check for trigger before calling subscription logic
  • Loading branch information
@mlange-42
mlange-42 authored Jan 17, 2024
1 parent 0f5b5ea commit a7cde98
Show file tree
Hide file tree
Showing 13 changed files with 288 additions and 120 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,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)
* Reduces event listener overhead through granular subscriptions and elimination of a heap allocation (#333, #334, #335, #337)

### Other

Expand Down
4 changes: 2 additions & 2 deletions ecs/event.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ import "github.com/mlange-42/arche/ecs/event"
// This allows the [World] to be in an unlocked state, and notifies after potential entity initialization.
type EntityEvent struct {
Entity Entity // The entity that was changed.
Changed Mask // Mask indicating changed components (additions and removals).
Added, Removed []ID // Components added and removed. DO NOT MODIFY! Get the current components with [World.Ids].
Added, Removed Mask // Masks indicating changed components (additions and removals).
AddedIDs, RemovedIDs []ID // Components added and removed. DO NOT MODIFY! Get the current components with [World.Ids].
OldRelation, NewRelation *ID // Old and new relation component ID. No relation is indicated by nil.
OldTarget Entity // Old relation target entity. Get the new target with [World.Relations] and [Relations.Get].
EventTypes event.Subscription // Bit mask of event types. See [event.Subscription].
Expand Down
14 changes: 7 additions & 7 deletions ecs/event_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func BenchmarkEntityEventCreate(b *testing.B) {

b.StartTimer()
for i := 0; i < b.N; i++ {
event = ecs.EntityEvent{Entity: e, Changed: mask, Added: added, Removed: nil}
event = ecs.EntityEvent{Entity: e, Added: mask, Removed: mask, AddedIDs: added, RemovedIDs: nil}
}
b.StopTimer()
_ = event
Expand All @@ -68,7 +68,7 @@ func BenchmarkEntityEventHeapPointer(b *testing.B) {

b.StartTimer()
for i := 0; i < b.N; i++ {
event = &ecs.EntityEvent{Entity: e, Changed: mask, Added: added, Removed: nil}
event = &ecs.EntityEvent{Entity: e, Added: mask, Removed: mask, AddedIDs: added, RemovedIDs: nil}
}
b.StopTimer()
_ = event
Expand All @@ -78,13 +78,13 @@ func BenchmarkEntityEventCopy(b *testing.B) {
handler := eventHandler{}

for i := 0; i < b.N; i++ {
handler.ListenCopy(ecs.EntityEvent{Entity: ecs.Entity{}, Changed: ecs.Mask{}, Added: nil, Removed: nil})
handler.ListenCopy(ecs.EntityEvent{Entity: ecs.Entity{}, Added: ecs.Mask{}, Removed: ecs.Mask{}, AddedIDs: nil, RemovedIDs: nil})
}
}

func BenchmarkEntityEventCopyReuse(b *testing.B) {
handler := eventHandler{}
event := ecs.EntityEvent{Entity: ecs.Entity{}, Changed: ecs.Mask{}, Added: nil, Removed: nil}
event := ecs.EntityEvent{Entity: ecs.Entity{}, Added: ecs.Mask{}, Removed: ecs.Mask{}, AddedIDs: nil, RemovedIDs: nil}

for i := 0; i < b.N; i++ {
handler.ListenCopy(event)
Expand All @@ -95,13 +95,13 @@ func BenchmarkEntityEventPointer(b *testing.B) {
handler := eventHandler{}

for i := 0; i < b.N; i++ {
handler.ListenPointer(&ecs.EntityEvent{Entity: ecs.Entity{}, Changed: ecs.Mask{}, Added: nil, Removed: nil})
handler.ListenPointer(&ecs.EntityEvent{Entity: ecs.Entity{}, Added: ecs.Mask{}, Removed: ecs.Mask{}, AddedIDs: nil, RemovedIDs: nil})
}
}

func BenchmarkEntityEventPointerReuse(b *testing.B) {
handler := eventHandler{}
event := ecs.EntityEvent{Entity: ecs.Entity{}, Changed: ecs.Mask{}, Added: nil, Removed: nil}
event := ecs.EntityEvent{Entity: ecs.Entity{}, Added: ecs.Mask{}, Removed: ecs.Mask{}, AddedIDs: nil, RemovedIDs: nil}

for i := 0; i < b.N; i++ {
handler.ListenPointer(&event)
Expand All @@ -117,7 +117,7 @@ func ExampleEntityEvent() {
world.SetListener(&listener)

world.NewEntity()
// Output: {{1 0} {[0 0 0 0]} [] [] <nil> <nil> {0 0} 1}
// Output: {{1 0} {[0 0 0 0]} {[0 0 0 0]} [] [] <nil> <nil> {0 0} 1}
}

func ExampleEntityEvent_Contains() {
Expand Down
35 changes: 30 additions & 5 deletions ecs/util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,37 @@ func subscription(entityCreated, entityRemoved, componentAdded, componentRemoved
return bits
}

// Returns whether a listener that subscribes to an event is also interested in terms of component subscription.
func subscribes(evtType event.Subscription, changed *Mask, subs *Mask, oldRel *ID, newRel *ID) bool {
if event.Relations.Contains(evtType) {
return subs == nil || (oldRel != nil && subs.Get(*oldRel)) || (newRel != nil && subs.Get(*newRel))
// Returns whether a listener is interested in an event based on event type and component subscriptions.
//
// Argument trigger should only contain the subscription bits that triggered the event.
// I.e. subscriptions & evenTypes.
func subscribes(trigger event.Subscription, added *Mask, removed *Mask, subs *Mask, oldRel *ID, newRel *ID) bool {
if trigger == 0 {
return false
}
if subs == nil {
// No component subscriptions
return true
}
if trigger.ContainsAny(event.Relations) {
// Contains event.RelationChanged and/or event.TargetChanged
if (oldRel != nil && subs.Get(*oldRel)) || (newRel != nil && subs.Get(*newRel)) {
return true
}
}
if trigger.ContainsAny(event.EntityCreated | event.ComponentAdded) {
// Contains additions-like types
if added != nil && subs.ContainsAny(added) {
return true
}
}
if trigger.ContainsAny(event.EntityRemoved | event.ComponentRemoved) {
// Contains additions-like types
if removed != nil && subs.ContainsAny(removed) {
return true
}
}
return subs == nil || subs.ContainsAny(changed)
return false
}

func maskToTypes(mask Mask, reg *componentRegistry) []componentType {
Expand Down
43 changes: 35 additions & 8 deletions ecs/util_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,31 +70,58 @@ func TestSubscribes(t *testing.T) {
id2 := id(2)
id3 := id(3)

assert.False(t,
subscribes(0, all(id1), all(id2), all(id1, id2), nil, nil),
)

assert.True(t,
subscribes(event.ComponentAdded, all(id1), all(id1, id2), nil, nil),
subscribes(event.ComponentAdded, all(id1), nil, all(id1, id2), nil, nil),
)
assert.False(t,
subscribes(event.ComponentAdded, nil, all(id1), all(id1, id2), nil, nil),
)
assert.True(t,
subscribes(event.ComponentAdded, all(id1, id2), all(id2), nil, nil),
subscribes(event.ComponentAdded, all(id1, id2), nil, all(id2), nil, nil),
)
assert.False(t,
subscribes(event.ComponentAdded, all(id1, id2), all(id3), nil, nil),
subscribes(event.ComponentAdded, all(id1, id2), nil, all(id3), nil, nil),
)

assert.True(t,
subscribes(event.RelationChanged, &Mask{}, all(id1, id2), nil, &id1),
subscribes(event.ComponentRemoved, nil, all(id1), all(id1, id2), nil, nil),
)
assert.False(t,
subscribes(event.ComponentRemoved, all(id1), nil, all(id1, id2), nil, nil),
)
assert.True(t,
subscribes(event.ComponentRemoved, nil, all(id1, id2), all(id2), nil, nil),
)
assert.False(t,
subscribes(event.ComponentRemoved, nil, all(id1, id2), all(id3), nil, nil),
)

assert.True(t,
subscribes(event.RelationChanged, &Mask{}, &Mask{}, all(id1, id2), nil, &id1),
)
assert.True(t,
subscribes(event.RelationChanged, &Mask{}, &Mask{}, all(id1, id2), &id1, &id3),
)
assert.False(t,
subscribes(event.RelationChanged, &Mask{}, &Mask{}, all(id1), &id2, &id3),
)

assert.True(t,
subscribes(event.RelationChanged, &Mask{}, all(id1, id2), &id1, &id3),
subscribes(event.TargetChanged, &Mask{}, &Mask{}, all(id1, id2), &id1, &id1),
)
assert.False(t,
subscribes(event.RelationChanged, &Mask{}, all(id1), &id2, &id3),
subscribes(event.TargetChanged, &Mask{}, &Mask{}, all(id1, id2), &id3, &id3),
)

assert.True(t,
subscribes(event.TargetChanged, &Mask{}, all(id1, id2), &id1, &id1),
subscribes(event.ComponentAdded|event.ComponentRemoved|event.TargetChanged, all(id1, id2), all(id1, id2), all(id3), &id3, &id3),
)
assert.False(t,
subscribes(event.TargetChanged, &Mask{}, all(id1, id2), &id3, &id3),
subscribes(event.ComponentAdded|event.ComponentRemoved|event.TargetChanged, all(id1), all(id1), all(id3), &id2, &id2),
)
}

Expand Down
75 changes: 41 additions & 34 deletions ecs/world.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,9 +214,9 @@ func (w *World) NewEntity(comps ...ID) Entity {
newRel = &arch.RelationComponent
}
bits := subscription(true, false, len(comps) > 0, false, newRel != nil, false)
if w.listener.Subscriptions().ContainsAny(bits) &&
subscribes(bits, &arch.Mask, w.listener.Components(), nil, newRel) {
w.listener.Notify(w, EntityEvent{entity, arch.Mask, comps, nil, nil, newRel, Entity{}, bits})
trigger := w.listener.Subscriptions() & bits
if trigger != 0 && subscribes(trigger, &arch.Mask, nil, w.listener.Components(), nil, newRel) {
w.listener.Notify(w, EntityEvent{entity, arch.Mask, Mask{}, comps, nil, nil, newRel, Entity{}, bits})
}
}
return entity
Expand Down Expand Up @@ -263,9 +263,9 @@ func (w *World) NewEntityWith(comps ...Component) Entity {
newRel = &arch.RelationComponent
}
bits := subscription(true, false, len(comps) > 0, false, newRel != nil, false)
if w.listener.Subscriptions().ContainsAny(bits) &&
subscribes(bits, &arch.Mask, w.listener.Components(), nil, newRel) {
w.listener.Notify(w, EntityEvent{entity, arch.Mask, ids, nil, nil, newRel, Entity{}, bits})
trigger := w.listener.Subscriptions() & bits
if trigger != 0 && subscribes(trigger, &arch.Mask, nil, w.listener.Components(), nil, newRel) {
w.listener.Notify(w, EntityEvent{entity, arch.Mask, Mask{}, ids, nil, nil, newRel, Entity{}, bits})
}
}
return entity
Expand Down Expand Up @@ -294,9 +294,9 @@ 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())
if w.listener.Subscriptions().ContainsAny(bits) &&
subscribes(bits, &arch.Mask, w.listener.Components(), nil, &targetID) {
w.listener.Notify(w, EntityEvent{entity, arch.Mask, comps, nil, nil, &targetID, Entity{}, bits})
trigger := w.listener.Subscriptions() & bits
if trigger != 0 && subscribes(trigger, &arch.Mask, nil, w.listener.Components(), nil, &targetID) {
w.listener.Notify(w, EntityEvent{entity, arch.Mask, Mask{}, comps, nil, nil, &targetID, Entity{}, bits})
}
}
return entity
Expand Down Expand Up @@ -331,9 +331,9 @@ 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())
if w.listener.Subscriptions().ContainsAny(bits) &&
subscribes(bits, &arch.Mask, w.listener.Components(), nil, &targetID) {
w.listener.Notify(w, EntityEvent{entity, arch.Mask, ids, nil, nil, &targetID, Entity{}, bits})
trigger := w.listener.Subscriptions() & bits
if trigger != 0 && subscribes(trigger, &arch.Mask, nil, w.listener.Components(), nil, &targetID) {
w.listener.Notify(w, EntityEvent{entity, arch.Mask, Mask{}, ids, nil, nil, &targetID, Entity{}, bits})
}
}
return entity
Expand All @@ -350,14 +350,14 @@ func (w *World) newEntities(count int, targetID ID, hasTarget bool, target Entit
newRel = &arch.RelationComponent
}
bits := subscription(true, false, len(comps) > 0, false, newRel != nil, !target.IsZero())
if w.listener.Subscriptions().ContainsAny(bits) &&
subscribes(bits, &arch.Mask, w.listener.Components(), nil, newRel) {
trigger := w.listener.Subscriptions() & bits
if trigger != 0 && subscribes(trigger, &arch.Mask, nil, w.listener.Components(), nil, newRel) {
cnt := uint32(count)
var i uint32
for i = 0; i < cnt; i++ {
idx := startIdx + i
entity := arch.GetEntity(idx)
w.listener.Notify(w, EntityEvent{entity, arch.Mask, comps, nil, nil, newRel, Entity{}, bits})
w.listener.Notify(w, EntityEvent{entity, arch.Mask, Mask{}, comps, nil, nil, newRel, Entity{}, bits})
}
}
}
Expand Down Expand Up @@ -395,14 +395,14 @@ func (w *World) newEntitiesWith(count int, targetID ID, hasTarget bool, target E
newRel = &arch.RelationComponent
}
bits := subscription(true, false, len(comps) > 0, false, newRel != nil, !target.IsZero())
if w.listener.Subscriptions().ContainsAny(bits) &&
subscribes(bits, &arch.Mask, w.listener.Components(), nil, newRel) {
trigger := w.listener.Subscriptions() & bits
if trigger != 0 && subscribes(trigger, &arch.Mask, nil, w.listener.Components(), nil, newRel) {
var i uint32
cnt := uint32(count)
for i = 0; i < cnt; i++ {
idx := startIdx + i
entity := arch.GetEntity(idx)
w.listener.Notify(w, EntityEvent{entity, arch.Mask, ids, nil, nil, newRel, Entity{}, bits})
w.listener.Notify(w, EntityEvent{entity, arch.Mask, Mask{}, ids, nil, nil, newRel, Entity{}, bits})
}
}
}
Expand Down Expand Up @@ -453,10 +453,10 @@ func (w *World) RemoveEntity(entity Entity) {
}

bits := subscription(false, true, false, len(oldIds) > 0, oldRel != nil, !oldArch.RelationTarget.IsZero())
if w.listener.Subscriptions().ContainsAny(bits) &&
subscribes(bits, &oldArch.Mask, w.listener.Components(), oldRel, nil) {
trigger := w.listener.Subscriptions() & bits
if trigger != 0 && subscribes(trigger, nil, &oldArch.Mask, w.listener.Components(), oldRel, nil) {
lock := w.lock()
w.listener.Notify(w, EntityEvent{entity, oldArch.Mask, nil, oldIds, oldRel, nil, oldArch.RelationTarget, bits})
w.listener.Notify(w, EntityEvent{entity, Mask{}, oldArch.Mask, nil, oldIds, oldRel, nil, oldArch.RelationTarget, bits})
w.unlock(lock)
}
}
Expand Down Expand Up @@ -517,15 +517,15 @@ func (w *World) removeEntities(filter Filter) int {
oldIds = arch.node.Ids
}
bits = subscription(false, true, false, len(oldIds) > 0, oldRel != nil, !arch.RelationTarget.IsZero())
listen = w.listener.Subscriptions().ContainsAny(bits) &&
subscribes(bits, &arch.Mask, w.listener.Components(), oldRel, nil)
trigger := w.listener.Subscriptions() & bits
listen = trigger != 0 && subscribes(trigger, nil, &arch.Mask, w.listener.Components(), oldRel, nil)
}

var j uint32
for j = 0; j < ln; j++ {
entity := arch.GetEntity(j)
if listen {
w.listener.Notify(w, EntityEvent{entity, arch.Mask, nil, oldIds, oldRel, nil, Entity{}, bits})
w.listener.Notify(w, EntityEvent{entity, Mask{}, arch.Mask, nil, oldIds, oldRel, nil, Entity{}, bits})
}
index := &w.entities[entity.id]
index.arch = nil
Expand Down Expand Up @@ -778,10 +778,13 @@ 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)
if w.listener.Subscriptions().ContainsAny(bits) {
trigger := w.listener.Subscriptions() & bits
if trigger != 0 {
changed := oldMask.Xor(&arch.Mask)
if subscribes(bits, &changed, w.listener.Components(), oldRel, newRel) {
w.listener.Notify(w, EntityEvent{entity, changed, add, rem, oldRel, newRel, oldTarget, bits})
added := arch.Mask.And(&changed)
removed := oldMask.And(&changed)
if subscribes(trigger, &added, &removed, w.listener.Components(), oldRel, newRel) {
w.listener.Notify(w, EntityEvent{entity, added, removed, add, rem, oldRel, newRel, oldTarget, bits})
}
}
}
Expand Down Expand Up @@ -980,9 +983,11 @@ func (w *World) setRelation(entity Entity, comp ID, target Entity) {
oldTarget := oldArch.RelationTarget
w.cleanupArchetype(oldArch)

if w.listener != nil && w.listener.Subscriptions().Contains(event.TargetChanged) &&
subscribes(event.TargetChanged, nil, w.listener.Components(), &comp, &comp) {
w.listener.Notify(w, EntityEvent{entity, Mask{}, nil, nil, &comp, &comp, oldTarget, event.TargetChanged})
if w.listener != nil {
trigger := w.listener.Subscriptions() & event.TargetChanged
if trigger != 0 && subscribes(trigger, nil, nil, w.listener.Components(), &comp, &comp) {
w.listener.Notify(w, EntityEvent{entity, Mask{}, Mask{}, nil, nil, &comp, &comp, oldTarget, event.TargetChanged})
}
}
}

Expand Down Expand Up @@ -1691,7 +1696,7 @@ func (w *World) notifyQuery(batchArch *batchArchetypes) {
}

event := EntityEvent{
Entity{}, arch.Mask, batchArch.Added, batchArch.Removed,
Entity{}, arch.Mask, Mask{}, batchArch.Added, batchArch.Removed,
nil, newRel,
Entity{}, 0,
}
Expand All @@ -1710,16 +1715,18 @@ func (w *World) notifyQuery(batchArch *batchArchetypes) {
relChanged = (oldRel == nil) != (newRel == nil) || *oldRel != *newRel
}
targChanged = oldArch.RelationTarget != arch.RelationTarget
event.Changed = event.Changed.Xor(&oldArch.node.Mask)
changed := event.Added.Xor(&oldArch.node.Mask)
event.Added = changed.And(&event.Added)
event.Removed = changed.And(&oldArch.node.Mask)
event.OldTarget = oldArch.RelationTarget
event.OldRelation = oldRel
}

bits := subscription(oldArch == nil, false, len(batchArch.Added) > 0, len(batchArch.Removed) > 0, relChanged, targChanged)
event.EventTypes = bits

if w.listener.Subscriptions().ContainsAny(bits) &&
subscribes(bits, &event.Changed, w.listener.Components(), event.OldRelation, event.NewRelation) {
trigger := w.listener.Subscriptions() & bits
if trigger != 0 && subscribes(trigger, &event.Added, &event.Removed, w.listener.Components(), event.OldRelation, event.NewRelation) {
start, end := batchArch.StartIndex[i], batchArch.EndIndex[i]
var e uint32
for e = start; e < end; e++ {
Expand Down
2 changes: 1 addition & 1 deletion ecs/world_examples_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ func ExampleWorld_SetListener() {
world.SetListener(&listener)

world.NewEntity()
// Output: {{1 0} {[0 0 0 0]} [] [] <nil> <nil> {0 0} 1}
// Output: {{1 0} {[0 0 0 0]} {[0 0 0 0]} [] [] <nil> <nil> {0 0} 1}
}

func ExampleWorld_Stats() {
Expand Down
Loading

0 comments on commit a7cde98

Please sign in to comment.