diff --git a/addons/ob.luau b/addons/ob.luau index fa6873b..781636d 100755 --- a/addons/ob.luau +++ b/addons/ob.luau @@ -2,15 +2,12 @@ local jecs = require("@jecs") type World = jecs.World -type Query = jecs.Query type Id = jecs.Id -export type Iter = (Observer) -> () -> (jecs.Entity, T...) - -export type Observer = { - disconnect: (Observer) -> (), +export type Observer = { + disconnect: () -> (), } export type Monitor = { @@ -19,13 +16,13 @@ export type Monitor = { removed: ((jecs.Entity) -> ()) -> () } -local function observers_new( - query: Query, +local function observers_new( + query: jecs.Query<...any>, callback: (jecs.Entity) -> () -): Observer +): Observer local cachedquery = query:cached() - local world = (cachedquery :: Query & { world: World }).world + local world = (cachedquery :: jecs.Query & { world: World }).world callback = callback local archetypes = cachedquery.archetypes_map @@ -35,7 +32,9 @@ local function observers_new( local function emplaced( entity: jecs.Entity, - id: jecs.Id + id: jecs.Id, + value: a, + oldarchetype: jecs.Archetype ) local r = entity_index.sparse_array[jecs.ECS_ID(entity)] @@ -54,7 +53,7 @@ local function observers_new( local tgt = jecs.ECS_PAIR_SECOND(term) local wc = tgt == jecs.w - local onadded = world:added(rel, function(entity, id) + local function emplaced_w_pair(entity, id, value, oldarchetype: jecs.Archetype) if not wc and id ~= term then return end @@ -62,8 +61,12 @@ local function observers_new( if archetypes[r.archetype.id] then callback(entity) end - end) + end + + local onadded = world:added(rel, emplaced_w_pair) + local onchanged = world:changed(rel, emplaced_w_pair) table.insert(cleanup, onadded) + table.insert(cleanup, onchanged) else local onadded = world:added(term, emplaced) local onchanged = world:changed(term, emplaced) @@ -124,10 +127,10 @@ local function observers_new( return observer end -local function monitors_new(query: Query<...any>): Monitor +local function monitors_new(query: jecs.Query<...any>): Monitor local cachedquery = query:cached() - local world = (cachedquery :: Query<...any> & { world: World }).world :: jecs.World + local world = (cachedquery :: jecs.Query<...any> & { world: World }).world :: jecs.World local archetypes = cachedquery.archetypes_map local terms = cachedquery.filter_with :: { jecs.Id } @@ -142,10 +145,14 @@ local function monitors_new(query: Query<...any>): Monitor local callback_added: ((jecs.Entity) -> ())? local callback_removed: ((jecs.Entity) -> ())? - -- NOTE(marcus): Track the last old archetype we processed to detect bulk operations. - -- We can detect this pattern by checking if we've seen this old archetype - -- before with a component in the terms list. + -- NOTE(marcus): Track the last (entity, old archetype) pair we processed to detect bulk operations. + -- During bulk_insert from ROOT_ARCHETYPE, the entity is moved to the target archetype first, + -- then all on_add callbacks fire sequentially with the same oldarchetype for the same entity. + -- We track both entity and old archetype to distinguish between: + -- 1. Same entity, same old archetype (bulk operation - skip) + -- 2. Different entity, same old archetype (separate operation - don't skip) local last_old_archetype: jecs.Archetype? = nil + local last_entity: jecs.Entity? = nil local function emplaced( entity: jecs.Entity, @@ -157,39 +164,65 @@ local function monitors_new(query: Query<...any>): Monitor return end - -- NOTE(marcus): Skip if we've seen this old archetype before AND - -- this component is in the query's terms. The component-in-terms - -- check ensures we don't skip legitimate separate operations. - if last_old_archetype == oldarchetype and terms_lookup[id] then - return - end - local r = jecs.entity_index_try_get_fast( entity_index, entity :: any) :: jecs.Record if not archetypes[oldarchetype.id] and archetypes[r.archetype.id] then + -- NOTE(marcus): Skip if we've seen this exact (entity, old archetype) combination before + -- AND this component is in the query's terms. This detects bulk operations where + -- the same entity transitions with multiple components, while allowing different + -- entities to trigger even if they share the same old archetype. + if last_old_archetype == oldarchetype and last_entity == entity and terms_lookup[id] then + return + end last_old_archetype = oldarchetype + last_entity = entity callback_added(entity) else -- NOTE(marcus): Clear tracking when we see a different transition pattern last_old_archetype = nil + last_entity = nil end end + -- Track which entity we've already processed for deletion to avoid duplicate callbacks + -- during bulk deletion where multiple components are removed with delete=true + local last_deleted_entity: jecs.Entity? = nil + local function removed(entity: jecs.Entity, component: jecs.Component, delete:boolean?) - if delete then - return - end if callback_removed == nil then return end + + if delete then + -- Deletion is a bulk removal - all components are removed with delete=true + -- We should only trigger the callback once per entity, not once per component + if last_deleted_entity == entity then + return + end + + local r = jecs.record(world, entity) + if r and r.archetype and archetypes[r.archetype.id] then + -- Entity was in the monitor before deletion + last_deleted_entity = entity + -- Clear tracking when entity is deleted + last_old_archetype = nil + last_entity = nil + callback_removed(entity) + end + return + end + local r = jecs.record(world, entity) local src = r.archetype local dst = jecs.archetype_traverse_remove(world, component, src) if not archetypes[dst.id] then + -- Clear tracking when entity leaves the monitor to allow re-entry last_old_archetype = nil + last_entity = nil + last_deleted_entity = nil callback_removed(entity) end end @@ -206,9 +239,6 @@ local function monitors_new(query: Query<...any>): Monitor if callback_added == nil then return end - if last_old_archetype == oldarchetype and terms_lookup[id] then - return - end if not wc and id ~= term then return @@ -218,11 +248,22 @@ local function monitors_new(query: Query<...any>): Monitor entity_index, entity :: any) :: jecs.Record if not archetypes[oldarchetype.id] and archetypes[r.archetype.id] then + -- NOTE(marcus): Skip if we've seen this exact (entity, old archetype) combination before + -- AND this component is in the query's terms. + if last_old_archetype == oldarchetype and last_entity == entity and terms_lookup[id] then + return + end + last_old_archetype = oldarchetype + last_entity = entity callback_added(entity) + else + -- Clear tracking when we see a different transition pattern + last_old_archetype = nil + last_entity = nil end end) - local onremoved = world:removed(rel, function(entity, id) + local onremoved = world:removed(rel, function(entity, id, deleted) if callback_removed == nil then return end @@ -307,7 +348,7 @@ local function monitors_new(query: Query<...any>): Monitor table.insert(cleanup, onadded) table.insert(cleanup, onremoved) else - local onadded = world:added(term, function(entity, id, _, oldarchetype) + local onadded = world:added(term, function(entity, id, _, oldarchetype: jecs.Archetype) if callback_removed == nil then return end @@ -379,5 +420,6 @@ end return { monitor = monitors_new, - observer = observers_new + observer = observers_new, + test = function(q: jecs.Query<...any>) end } diff --git a/test/addons/ob.luau b/test/addons/ob.luau index aad9f25..18e0953 100755 --- a/test/addons/ob.luau +++ b/test/addons/ob.luau @@ -24,7 +24,6 @@ TEST("addons/ob::observer", function() world:add(e, jecs.pair(A, B)) CHECK(c==2) world:add(e, jecs.pair(A, C)) - print(c) CHECK(c==2) end @@ -220,12 +219,79 @@ TEST("addons/ob::observer", function() world:add(e, jecs.pair(A, e2)) CHECK(callcount == 1 + 2 * 2) end + + do CASE "observer with pair should handle remove and re-add correctly" + local A = world:component() + local B = world:component() + local e1 = world:entity() + + local c = 0 + ob.observer(world:query(jecs.pair(A, e1)), function() + c += 1 + end) + + local e = world:entity() + world:add(e, jecs.pair(A, e1)) + CHECK(c == 1) + world:remove(e, jecs.pair(A, e1)) + CHECK(c == 1) + world:add(e, jecs.pair(A, e1)) + CHECK(c == 2) + world:remove(e, jecs.pair(A, e1)) + CHECK(c == 2) + end + + do CASE "observer with pair should handle set operations correctly" + local A = world:component() + local B = world:component() + local e1 = world:entity() + + local c = 0 + ob.observer(world:query(jecs.pair(A, e1)), function() + c += 1 + end) + + local e = world:entity() + world:set(e, jecs.pair(A, e1), true) + CHECK(c == 1) + world:set(e, jecs.pair(A, e1), false) + CHECK(c == 2) + world:set(e, jecs.pair(A, e1), true) + CHECK(c == 3) + world:remove(e, jecs.pair(A, e1)) + CHECK(c == 3) + world:set(e, jecs.pair(A, e1), true) + CHECK(c == 4) + end + + do CASE "observer with wildcard pair should handle operations correctly" + local A = world:component() + local B = world:component() + local e1 = world:entity() + local e2 = world:entity() + + local c = 0 + ob.observer(world:query(A, jecs.pair(B, jecs.w)), function() + c += 1 + end) + + local e = world:entity() + world:add(e, A) + CHECK(c == 0) + world:set(e, jecs.pair(B, e1), true) + CHECK(c == 1) + world:set(e, jecs.pair(B, e2), false) + CHECK(c == 2) + world:remove(e, jecs.pair(B, e1)) + CHECK(c == 2) + world:remove(e, jecs.pair(B, e2)) + CHECK(c == 2) + end end) TEST("addons/ob::monitor", function() local world = jecs.world() - do CASE [[should not invoke monitor.added callback multiple times in a bulk_move ]] local A = world:component() @@ -241,7 +307,6 @@ TEST("addons/ob::monitor", function() local e = world:entity() jecs.bulk_insert(world, e, { A, B, C }, { 1, 2, 3 }) CHECK(c==1) - print(c) end do CASE [[should not invoke monitor.added callback unless it wasn't apart @@ -273,15 +338,146 @@ TEST("addons/ob::monitor", function() CHECK(c==2) end + do CASE "different entities bulk_insert with same old archetype" + local A = world:component() + local B = world:component() + local C = world:component() + + local monitor = ob.monitor(world:query(A, B, C)) + local c = 0 + monitor.added(function() + c += 1 + end) + + local e1 = world:entity() + local e2 = world:entity() + + -- First entity: ROOT_ARCHETYPE -> ABC (triggers, sets last_old_archetype = ROOT_ARCHETYPE) + jecs.bulk_insert(world, e1, { A, B, C }, { 1, 2, 3 }) + CHECK(c == 1) + + -- Second entity: ROOT_ARCHETYPE -> ABC (same old archetype, but different entity) + -- Should still trigger even though last_old_archetype == ROOT_ARCHETYPE + jecs.bulk_insert(world, e2, { A, B, C }, { 4, 5, 6 }) + CHECK(c == 2) + end + + do CASE "multiple entities bulk_insert pairs with same old archetype" + local A = world:component() + local B = world:component() + local e1 = world:entity() + local e2 = world:entity() + + local monitor = ob.monitor(world:query(jecs.pair(A, e1), jecs.pair(A, e2))) + local c = 0 + monitor.added(function() + c += 1 + end) + + local entity1 = world:entity() + local entity2 = world:entity() + + -- First entity: ROOT_ARCHETYPE -> pairs (triggers) + jecs.bulk_insert(world, entity1, { jecs.pair(A, e1), jecs.pair(A, e2) }, { true, true }) + CHECK(c == 1) + + -- Second entity: ROOT_ARCHETYPE -> pairs (same old archetype, different entity) + -- Should still trigger + jecs.bulk_insert(world, entity2, { jecs.pair(A, e1), jecs.pair(A, e2) }, { true, true }) + CHECK(c == 2) + end + + do CASE "entities transition sequentially with same old archetype" + local A = world:component() + local B = world:component() + local C = world:component() + + local monitor = ob.monitor(world:query(A, B, C)) + local c = 0 + monitor.added(function() + c += 1 + end) + + local e1 = world:entity() + local e2 = world:entity() + local e3 = world:entity() + + -- All three entities: ROOT_ARCHETYPE -> ABC + -- Each should trigger independently + jecs.bulk_insert(world, e1, { A, B, C }, { 1, 2, 3 }) + CHECK(c == 1) + + jecs.bulk_insert(world, e2, { A, B, C }, { 4, 5, 6 }) + CHECK(c == 2) + + jecs.bulk_insert(world, e3, { A, B, C }, { 7, 8, 9 }) + CHECK(c == 3) + end + + do CASE "entity transitions after another entity with same old archetype" + local A = world:component() + local B = world:component() + + local monitor = ob.monitor(world:query(A, B)) + local c = 0 + monitor.added(function() + c += 1 + end) + + local e1 = world:entity() + local e2 = world:entity() + + -- First entity: ROOT_ARCHETYPE -> AB (triggers, sets last_old_archetype = ROOT_ARCHETYPE) + jecs.bulk_insert(world, e1, { A, B }, { 1, 2 }) + CHECK(c == 1) + + -- Remove e1 from monitor (should clear last_old_archetype) + world:remove(e1, A) + + -- Second entity: ROOT_ARCHETYPE -> AB (should trigger even though old archetype matches) + jecs.bulk_insert(world, e2, { A, B }, { 3, 4 }) + CHECK(c == 2) + + -- Re-add e1: ROOT_ARCHETYPE -> AB again (should trigger) + world:add(e1, A) + CHECK(c == 3) + end + + do CASE "mixed bulk_insert and regular add with same old archetype" + local A = world:component() + local B = world:component() + local C = world:component() + + local monitor = ob.monitor(world:query(A, B, C)) + local c = 0 + monitor.added(function() + c += 1 + end) + + local e1 = world:entity() + local e2 = world:entity() + + -- First entity: bulk_insert from ROOT_ARCHETYPE (sets last_old_archetype = ROOT_ARCHETYPE, last_entity = e1) + jecs.bulk_insert(world, e1, { A, B, C }, { 1, 2, 3 }) + CHECK(c == 1) + + -- Second entity: regular add from ROOT_ARCHETYPE + -- Adding components one by one - callback should trigger when entity matches query + world:add(e2, A) + CHECK(c == 1) -- Entity doesn't match query yet (needs A, B, C) + world:add(e2, B) + CHECK(c == 1) -- Still doesn't match + world:add(e2, C) + CHECK(c == 2) -- Now matches query, should trigger (different entity, so not skipped) + end + do CASE "deleted entity should only exit monitor once" local A = world:component() local B = world:component() - local Destroy = world:entity() - local c = 0 - local monitor = ob.monitor(world:query(A, B):without(Destroy)) + local monitor = ob.monitor(world:query(A, B)) monitor.added(function() print("enter") @@ -290,15 +486,13 @@ TEST("addons/ob::monitor", function() c += 1 end) - local e = world:entity() world:add(e, A) world:add(e, B) - - world:add(e, Destroy) - + print("---world:delete---") world:delete(e) - print(c) + print("-----") + print(c, "c") CHECK(c==1) end