Merge branch 'Ukendio:main' into docs/studio

examples/hooks/cleanup.luau

@@ -13,8 +13,9 @@ world:set(Model, jecs.OnRemove, function(entity)
 	model:Destroy()
 end)
 
-world:set(Model, jecs.OnSet, function(entity, model)
+world:set(Model, jecs.OnAdd, function(entity, id, model)
-	-- OnSet is invoked after the data has been assigned.
+	-- OnAdd is invoked after the data has been assigned.
+	-- This hook only fires the first time the component is added.
 	-- It also returns the data for faster access.
 	-- There may be some logic to do some side effects on reassignments
 	model:SetAttribute("entityId", entity)

how_to/100_cleanup_traits.luau

@@ -71,7 +71,7 @@ print(world:has(e1, T1))
 	Cascading deletion, dangerous.
 ]]
 
-world:add(T2, pair(jecs.OnDelete, jecs.Remove))
+world:add(T2, pair(jecs.OnDelete, jecs.Delete))
 
 local e2 = world:entity()
 world:add(e2, T2)

src/jecs.luau

@@ -29,7 +29,7 @@ export type QueryInner = {
 	filter_with: { Component },
 	filter_without: { Component },
 	next: () -> (Entity, ...any),
-	world: World,
+	-- world: World,
 }
 
 type function ecs_entity_t(ty: type)
@@ -72,7 +72,7 @@ type function ecs_id_t(first: type, second: type)
 	return p
 end
 
-export type Entity<T = any> = { __T: T }
+export type Entity<T = nil> = { __T: T }
 export type Id<T = any> = { __T: T }
 export type Pair<First=any, Second=any> = ecs_pair_t<Entity<First>, Entity<Second>>
 export type Component<T=any> = { __T: T }
@@ -82,15 +82,12 @@ export type Item<T...> = (self: Query<T...>) -> (Entity, T...)
 export type Iter<T...> = (query: Query<T...>) -> () -> (Entity, T...)
 export type CachedIter<T...> = (query: CachedQuery<T...>) -> () -> (Entity, T...)
 
-type TypePack<T...> = {
+type TypePack<T...> = (T...) -> never
-	__phantomdata: () -> (T...)
-}
 
 export type CachedQuery<T...> = typeof(setmetatable(
 	{} :: {
 		iter: CachedIter<T...>,
-		archetypes: (self: CachedQuery<T...>) -> { Archetype },
+		archetypes: (CachedQuery<T...>) -> { Archetype },
-		cached: (self: CachedQuery<T...>) -> CachedQuery<T...>,
 		has: (CachedQuery<T...>, Entity) -> boolean,
 		ids: { Id<any> },
 		filter_with: { Id<any> }?,
@@ -108,8 +105,8 @@ export type Query<T...> = typeof(setmetatable(
 		iter: Iter<T...>,
 		with: ((Query<T...>, ...Component) -> Query<T...>),
 		without: ((Query<T...>, ...Component) -> Query<T...>),
-		archetypes: (self: Query<T...>) -> { Archetype },
+		archetypes: (Query<T...>) -> { Archetype },
-		cached: (self: Query<T...>) -> CachedQuery<T...>,
+		cached: (Query<T...>) -> CachedQuery<T...>,
 		has: (Query<T...>, Entity) -> boolean,
 		ids: { Id<any> },
 		filter_with: { Id<any> }?,
@@ -246,14 +243,14 @@ export type World = {
 	component: <T>(self: World) -> Entity<T>,
 	--- Gets the target of an relationship. For example, when a user calls
 	--- `world:target(id, ChildOf(parent), 0)`, you will obtain the parent entity.
-	target: <T, a>(self: World, id: Entity<T>, relation: ecs_entity_t<Component>, index: number?) -> Entity?,
+	target: <T, a>(self: World, id: Entity<T>, relation: ecs_entity_t<Component>, index: number?) -> Entity<unknown>?,
 	--- Deletes an entity and all it's related components and relationships.
 	delete: <T>(self: World, id: Entity<T>) -> (),
 
 	--- Adds a component to the entity with no value
 	add: <a>(
 		self: World,
-		id: ecs_entity_t<Entity>,
+		id: ecs_entity_t<Entity<any>>,
 		component: Component<a>
 	) -> (),
 
@@ -267,10 +264,10 @@ export type World = {
 	--- Removes a component from the given entity
 	remove: <T, a>(self: World, id: Entity<T>, component: Component<a>) -> (),
 	--- Retrieves the value of up to 4 components. These values may be nil.
-	get: & (<T, a>(World, Entity<T>, Component<a>) -> a?)
+	get: & (<T, a>(World, Entity<T> | number, Component<a>) -> a?)
-		& (<T, a, b>(World, Entity<T>, Component<a>, Component<b>) -> (a?, b?))
+		& (<T, a, b>(World, Entity<T> | number, Component<a>, Component<b>) -> (a?, b?))
-		& (<T, a, b, c>(World, Entity<T>, Component<a>, Component<b>, Component<c>) -> (a?, b?, c?))
+		& (<T, a, b, c>(World, Entity<T> | number, Component<a>, Component<b>, Component<c>) -> (a?, b?, c?))
-		& (<T, a, b, c, d>(World, Entity<T>, Component<a>, Component<b>, Component<c>, Component<d>) -> (a?, b?, c?, d?)),
+		& (<T, a, b, c, d>(World, Entity<T> | number, Component<a>, Component<b>, Component<c>, Component<d>) -> (a?, b?, c?, d?)),
 
 	--- Returns whether the entity has the ID.
 	has: (<T, a>(World, Entity<T>, Component<a>) -> boolean)
@@ -1704,7 +1701,7 @@ local function query_cached(query: QueryInner)
 
 	local compatible_archetypes = archetypes :: { Archetype }
 
-	local world = query.world
+	local world = (query :: { world: World }).world
 	-- Only need one observer for EcsArchetypeCreate and EcsArchetypeDelete respectively
 	-- because the event will be emitted for all components of that Archetype.
 	local observable = world.observable
@@ -2128,7 +2125,7 @@ local function query_cached(query: QueryInner)
 end
 
 local function query_has(query: QueryInner, entity: i53)
-	local world = query.world :: world
+	local world = (query::any).world :: world
 	local r = entity_index_try_get(world.entity_index, entity)
 	if not r then
 		return false
@@ -3086,11 +3083,13 @@ local function world_new()
 		end
 
 		local archetype = record.archetype
-
 		if archetype then
+			-- NOTE(marcus): It is important to remove the data and invoke
+			-- the hooks before the archetype and certain component records are
+			-- invalidated or else it will have a nasty runtime error.
 			for _, id in archetype.types do
-				local idr = component_index[id]
+				local cr = component_index[id]
-				local on_remove = idr.on_remove
+				local on_remove = cr.on_remove
 				if on_remove then
 					on_remove(entity, id, true)
 				end
@@ -3107,6 +3106,28 @@ local function world_new()
 		local idr = component_index[entity]
 		local idr_r = component_index[rel]
 
+		--[[
+			It is important to note that `world_delete` uses a depth-first
+			traversal that prunes the children of the entity before their
+			parents. archetypes can be destroyed and removed from component
+			records while we're still iterating over those records. The
+			recursive nature of this function entails that archetype ids can be
+			removed from component records (idr_t.records, idr.records and
+			idr_r.records) while that collection is still being iterated over.
+			If we try to look up an archetype by ID after it has been destroyed,
+			we get nil. This is hard to debug because the removal happens deep
+			in the opaque call stack. Essentially the entry is removed on a
+			first come first serve basis.
+
+			The solution is to separate processing from cleanup. We first iterate
+			over the archetypes to process entities (move them, call hooks, etc.),
+			but do not destroy the archetypes yet. Then we iterate again to destroy
+			the archetypes, but check if they still exist (archetypes[archetype_id]
+			is not nil) before destroying. This handles the case where recursive
+			world_delete calls have already destroyed some archetypes.
+
+										- Marcus
+		]]
 		if idr then
 			local flags = idr.flags
 			if (bit32.btest(flags, ECS_ID_DELETE) == true) then
@@ -3200,10 +3221,13 @@ local function world_new()
 						end
 					end
 				end
-			end
 
+			end
 			for archetype_id in archetype_ids do
-				archetype_destroy(world, archetypes[archetype_id])
+				local idr_t_archetype = archetypes[archetype_id]
+				if idr_t_archetype then
+					archetype_destroy(world, idr_t_archetype)
+				end
 			end
 		end
 
@@ -3226,34 +3250,44 @@ local function world_new()
 				local records = idr_r.records
 				for archetype_id in archetype_ids do
 					local idr_r_archetype = archetypes[archetype_id]
-					local node = idr_r_archetype
+					-- local node = idr_r_archetype
 					local entities = idr_r_archetype.entities
 					local tr = records[archetype_id]
 					local tr_count = counts[archetype_id]
 					local idr_r_types = idr_r_archetype.types
+					local dst = table.clone(idr_r_types)
 					for i = tr, tr + tr_count - 1 do
 						local id = idr_r_types[i]
-						node = archetype_traverse_remove(world, id, node)
+						local at = table.find(dst, id)
+						if at then
+							table.remove(dst, at)
+						end
+						-- node = archetype_traverse_remove(world, id, node)
 						local on_remove = component_index[id].on_remove
 						if on_remove then
+							-- NOTE(marcus): Since hooks can move the entities
+							-- assumptions about which archetype it jumps to is
+							-- diminished. We assume that people who delete
+							-- relation will never have hooks on them.
 							for _, entity in entities do
 								on_remove(entity, id)
 							end
 						end
 					end
 
+					local node = archetype_ensure(world, dst)
+
 					for i = #entities, 1, -1 do
 						local e = entities[i]
 						local r = entity_index_try_get_unsafe(e) :: record
 						inner_entity_move(e, r, node)
 					end
+
+					archetype_destroy(world, idr_r_archetype)
+				end
+			end
 		end
 
-				for archetype_id in archetype_ids do
-					archetype_destroy(world, archetypes[archetype_id])
-				end
-			end
-		end
 
 
 		local dense = record.dense
@@ -3457,7 +3491,7 @@ return {
 	archetype_append_to_records = archetype_append_to_records,
 	id_record_ensure = id_record_ensure :: (World, Component) -> ComponentRecord,
 	component_record = id_record_get :: (World, Component) -> ComponentRecord?,
-	record = ecs_entity_record :: (World, Entity) -> Record,
+	record = ecs_entity_record :: (World, Entity<any>) -> Record,
 
 	archetype_create = archetype_create :: (World, { Component }, string) -> Archetype,
 	archetype_ensure = archetype_ensure :: (World, { Component }) -> Archetype,

test/benches/visual/query.bench.luau

@@ -2,11 +2,32 @@
 --!native
 
 local ReplicatedStorage = game:GetService("ReplicatedStorage")
+local Matter = require(ReplicatedStorage.DevPackages.Matter)
+local ecr = require(ReplicatedStorage.DevPackages.ecr)
+local newWorld = Matter.World.new()
 
 local jecs = require(ReplicatedStorage.Lib:Clone())
-local chrono = require(ReplicatedStorage.chronoecs:Clone())
+local mirror = require(ReplicatedStorage.mirror:Clone())
+local mcs = mirror.world()
 local ecs = jecs.world()
-local ccs = chrono.new()
+
+local A1 = Matter.component()
+local A2 = Matter.component()
+local A3 = Matter.component()
+local A4 = Matter.component()
+local A5 = Matter.component()
+local A6 = Matter.component()
+local A7 = Matter.component()
+local A8 = Matter.component()
+
+local B1 = ecr.component()
+local B2 = ecr.component()
+local B3 = ecr.component()
+local B4 = ecr.component()
+local B5 = ecr.component()
+local B6 = ecr.component()
+local B7 = ecr.component()
+local B8 = ecr.component()
 
 local D1 = ecs:component()
 local D2 = ecs:component()
@@ -17,74 +38,107 @@ local D6 = ecs:component()
 local D7 = ecs:component()
 local D8 = ecs:component()
 
-local E1 = ccs:component()
+local E1 = mcs:component()
-local E2 = ccs:component()
+local E2 = mcs:component()
-local E3 = ccs:component()
+local E3 = mcs:component()
-local E4 = ccs:component()
+local E4 = mcs:component()
-local E5 = ccs:component()
+local E5 = mcs:component()
-local E6 = ccs:component()
+local E6 = mcs:component()
-local E7 = ccs:component()
+local E7 = mcs:component()
-local E8 = ccs:component()
+local E8 = mcs:component()
 
-local d_components = {}
+local registry2 = ecr.registry()
-local e_components = {}
-
-for i = 1, 150 do
-	ecs:component()
-	ccs:component()
-end
 
 local function flip()
-	return math.random() >= 0.5
+	return math.random() >= 0.25
 end
 
 local N = 2 ^ 16 - 2
+local archetypes = {}
 
+local hm = 0
 for i = 1, N do
+	local id = registry2.create()
+	local combination = ""
+	local n = newWorld:spawn()
 	local entity = ecs:entity()
-	local m = ccs:entity()
+	local m = mcs:entity()
 
 	if flip() then
-		ecs:set(entity, D1, true)
+		registry2:set(id, B1, { value = true })
-		ccs:add(m, E1)
+		ecs:set(entity, D1, { value = true })
-		ccs:set(m, E1, true)
+		newWorld:insert(n, A1({ value = true }))
+		mcs:set(m, E1, { value = 2 })
 	end
 	if flip() then
-		ecs:set(entity, D2, true)
+		combination ..= "B"
-		ccs:add(m, E2)
+		registry2:set(id, B2, { value = true })
-		ccs:set(m, E2, true)
+		ecs:set(entity, D2, { value = true })
+		mcs:set(m, E2, { value = 2 })
+		newWorld:insert(n, A2({ value = true }))
 	end
 	if flip() then
-		ecs:set(entity, D3, true)
+		combination ..= "C"
-		ccs:add(m, E3)
+		registry2:set(id, B3, { value = true })
-		ccs:set(m, E3, true)
+		ecs:set(entity, D3, { value = true })
+		mcs:set(m, E3, { value = 2 })
+		newWorld:insert(n, A3({ value = true }))
 	end
 	if flip() then
-		ecs:set(entity, D4, true)
+		combination ..= "D"
-		ccs:add(m, E4)
+		registry2:set(id, B4, { value = true })
-		ccs:set(m, E4, true)
+		ecs:set(entity, D4, { value = true })
+		mcs:set(m, E4, { value = 2 })
+
+		newWorld:insert(n, A4({ value = true }))
 	end
 	if flip() then
-		ecs:set(entity, D5, true)
+		combination ..= "E"
-		ccs:add(m, E4)
+		registry2:set(id, B5, { value = true })
-		ccs:set(m, E5, true)
+		ecs:set(entity, D5, { value = true })
+		mcs:set(m, E5, { value = 2 })
+
+		newWorld:insert(n, A5({ value = true }))
 	end
 	if flip() then
-		ecs:set(entity, D6, true)
+		combination ..= "F"
-		ccs:add(m, E6)
+		registry2:set(id, B6, { value = true })
-		ccs:set(m, E6, true)
+		ecs:set(entity, D6, { value = true })
+		mcs:set(m, E6, { value = 2 })
+		newWorld:insert(n, A6({ value = true }))
 	end
 	if flip() then
-		ecs:set(entity, D7, true)
+		combination ..= "G"
-		ccs:add(m, E7)
+		registry2:set(id, B7, { value = true })
-		ccs:set(m, E7, true)
+		ecs:set(entity, D7, { value = true })
+		mcs:set(m, E7, { value = 2 })
+		newWorld:insert(n, A7({ value = true }))
 	end
 	if flip() then
-		ccs:add(m, E8)
+		combination ..= "H"
-		ecs:set(entity, D8, true)
+		registry2:set(id, B8, { value = true })
-		ccs:set(m, E8, true)
+		newWorld:insert(n, A8({ value = true }))
+		ecs:set(entity, D8, { value = true })
+		mcs:set(m, E8, { value = 2 })
 	end
+
+	if combination:find("BCDF") then
+		if not archetypes[combination] then
+			print(combination)
+		end
+		hm += 1
+	end
+	archetypes[combination] = true
 end
+print("TEST", hm)
+
+local count = 0
+
+for _, archetype in ecs:query(D2, D4, D6, D8):archetypes() do
+	count += #archetype.entities
+end
+
+print(count)
 
 return {
 	ParameterGenerator = function()
@@ -92,21 +146,21 @@ return {
 	end,
 
 	Functions = {
-		-- Matter = function()
+		Matter = function()
-		-- 	for entityId, firstComponent in newWorld:query(A2, A4, A6, A8) do
+			for entityId, firstComponent in newWorld:query(A2, A4, A6, A8) do
-		-- 	end
-		-- end,
-
-		-- ECR = function()
-		-- 	for entityId, firstComponent in registry2:view(B2, B4, B6, B8) do
-		-- 	end
-		-- end,
-		--
-		chrono = function()
-			for entityId, firstComponent in ccs:view(E2, E4, E6, E8) do
 			end
 		end,
 
+		ECR = function()
+			for entityId, firstComponent in registry2:view(B2, B4, B6, B8) do
+			end
+		end,
+
+		-- Mirror = function()
+		-- 	for entityId, firstComponent in mcs:query(E2, E4, E6, E8) do
+		-- 	end
+		-- end,
+
 		Jecs = function()
 			for entityId, firstComponent in ecs:query(D2, D4, D6, D8) do
 			end

test/tests.luau

@@ -24,6 +24,145 @@ type Id<T=unknown> = jecs.Id<T>
 local entity_visualiser = require("@modules/entity_visualiser")
 local dwi = entity_visualiser.stringify
 
+TEST("optimize idr_r removal", function()
+
+	local pair = jecs.pair
+    local world = jecs.world()
+    local rel = world:component()
+    local A = world:component()
+    local B = world:component()
+
+    local t1 = world:entity()
+    local t2 = world:entity()
+
+    local entities = {} :: { jecs.Entity }
+
+    for i = 1, 10 do
+
+	    local e1 = world:entity()
+        local e2 = world:entity()
+
+        world:set(e1, A, true)
+        world:set(e2, A, true)
+        world:add(e1, pair(B, t1))
+        world:add(e1, pair(B, t2))
+        world:add(e2, pair(B, t1))
+        world:add(e2, pair(B, t2))
+
+        table.insert(entities, e1)
+        table.insert(entities, e2)
+    end
+
+    local e1 = world:entity()
+    local e2 = world:entity()
+
+    table.insert(entities, e1)
+	table.insert(entities, e2)
+
+    world:set(e1, A, true)
+    world:set(e2, A, true)
+    world:add(e1, pair(B, t1))
+    world:add(e1, pair(B, t2))
+    world:add(e2, pair(B, t1))
+    world:add(e2, pair(B, t2))
+
+    BENCH("delete B", function()
+    	world:delete(B)
+    end)
+
+    for _, e in entities do
+	    CHECK(world:has(e, A))
+        CHECK(not world:target(e, B))
+        CHECK(not world:target(e, B))
+    end
+
+end)
+TEST("deleting t1's archetype before invoking its onremove hooks", function()
+    local pair = jecs.pair
+    local world = jecs.world()
+    local rel = world:component()
+
+    local t1 = world:entity()
+    local t2 = world:entity()
+
+    --[[
+    weirdly enough if i do this (only when adding childof relation after adding (rel, t2) to t1) it does not error. Probably a red herring
+
+    world:add(t2, pair(rel, t1))
+    world:add(t1, pair(rel, t2))
+    world:add(t2, pair(jecs.ChildOf, t1))
+    --]]
+
+    -- this causes world:delete to error
+    world:add(t2, pair(jecs.ChildOf, t1))
+    world:add(t1, pair(rel, t2))
+
+    world:delete(t1)
+end)
+TEST("reproduce idr_t nil archetype bug", function()
+	local world = jecs.world()
+
+	local cts = {
+	    Humanoid = world:component(),
+	    Animator = world:component(),
+	    VelocitizeAnimationWeight = world:component(),
+	}
+
+	local char = world:entity()
+
+	-- REMOVING ONE OF THESE THESE OFFSETS i BY +1
+	world:set(char, cts.Humanoid, 0)
+	world:set(char, cts.Animator, 0)
+	--
+
+	world:added(cts.Humanoid, function() end) -- REMOVING THIS OFFSETS i BY +1 TOO
+	world:removed(cts.Animator, function(entity, id)
+	    local r = jecs.record(world, entity)
+	    local src = r.archetype
+
+	    --REMOVING THIS jecs.archetype_traverse_remove CALL STOPS IT FROM HAPPENING
+	    local dst = src and jecs.archetype_traverse_remove(world, id, src)
+	end)
+
+	local batches = 10
+	local batchSize = 20
+
+	local trackedEntities: { [number]: { parentId: number? } } = {}
+
+	for batch = 1, batches do
+	    for i = 1, batchSize do
+	        local root = world:entity()
+	        world:add(root, jecs.pair(jecs.ChildOf, char))
+
+	        -- Removing animator from trackEntity1 causes it to stop happening
+	        local trackEntity1 = world:entity()
+	        world:set(trackEntity1, cts.Animator, 0)
+	        world:add(trackEntity1, jecs.pair(jecs.ChildOf, root))
+	        trackedEntities[trackEntity1] = { parentId = root }
+
+	        -- Removing animator from trackEntity2 causes it to happen less frequently
+	        local trackEntity2 = world:entity()
+	        world:set(trackEntity2, cts.Animator, 0)
+	        world:add(trackEntity2, jecs.pair(jecs.ChildOf, root))
+	        trackedEntities[trackEntity2] = { parentId = root }
+
+	        -- Removing this, but keeping Animator on the other 2 causes it to stop happening
+	        world:set(trackEntity1, cts.VelocitizeAnimationWeight, 0)
+
+	        for entityId, info in trackedEntities do
+	            if world:contains(entityId) and not world:parent(entityId :: any) then
+	                print(`bugged entity found: {entityId}`)
+	                print(`original parent: {info.parentId}`)
+	                print(`batch = {batch}, i = {i}`)
+	                print("==========================================")
+	                trackedEntities[entityId] = nil
+					world:delete(entityId)
+	            end
+	        end
+	    end
+	end
+end)
+
 TEST("Ensure archetype edges get cleaned", function()
 	local A = jecs.component()
 	local B = jecs.component()