Add more tests and fix edge cases with pairs

addons/ob.luau

@@ -2,15 +2,12 @@
 local jecs = require("@jecs")
 
 type World = jecs.World
-type Query<T...> = jecs.Query<T...>
 
 type Id<T=any> = jecs.Id<T>
 
 
-export type Iter<T...> = (Observer<T...>) -> () -> (jecs.Entity, T...)
-
-export type Observer<T...> = {
-	disconnect: (Observer<T...>) -> (),
+export type Observer = {
+	disconnect: () -> (),
 }
 
 export type Monitor = {
@@ -19,13 +16,13 @@ export type Monitor = {
 	removed: ((jecs.Entity) -> ()) -> ()
 }
 
-local function observers_new<T...>(
-	query: Query<T...>,
+local function observers_new(
+	query: jecs.Query<...any>,
 	callback: (jecs.Entity) -> ()
-): Observer<T...>
+): Observer
 	local cachedquery = query:cached()
 
-	local world = (cachedquery :: Query<any> & { world: World }).world
+	local world = (cachedquery :: jecs.Query<any> & { world: World }).world
 	callback = callback
 
 	local archetypes = cachedquery.archetypes_map
@@ -35,7 +32,9 @@ local function observers_new<T...>(
 
 	local function emplaced<a>(
 		entity: jecs.Entity,
-		id: jecs.Id<a>
+		id: jecs.Id<a>,
+		value: a,
+		oldarchetype: jecs.Archetype
 	)
 		local r = entity_index.sparse_array[jecs.ECS_ID(entity)]
 
@@ -54,7 +53,7 @@ local function observers_new<T...>(
 			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<T...>(
 				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<T...>(
     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<any> }
@@ -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<a>(
  		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
 }

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