From 4e5a40f75b5f89e99c2fa32a9454c6faf3b74393 Mon Sep 17 00:00:00 2001 From: Marcus Date: Sat, 17 Aug 2024 01:10:35 +0200 Subject: [PATCH] Fix hooks not notifying (#103) * Fix world_has_any checking for inverse statement * invoke onAdd for set --- src/init.luau | 75 ++++++++++++++++++------------------ test/tests.luau | 100 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 137 insertions(+), 38 deletions(-) diff --git a/src/init.luau b/src/init.luau index 51a5f53..51d7fe2 100644 --- a/src/init.luau +++ b/src/init.luau @@ -365,7 +365,7 @@ local function world_has_any(world: World, entity: number, ...: i53): boolean local records = archetype.records for i = 1, select("#", ...) do - if not records[select(i, ...)] then + if records[select(i, ...)] then return true end end @@ -442,7 +442,7 @@ end local function archetype_create(world: World, types: { i24 }, prev: Archetype?): Archetype local ty = hash(types) - local id = world.nextArchetypeId + 1 + local id = (world.nextArchetypeId :: number) + 1 world.nextArchetypeId = id local length = #types @@ -494,7 +494,7 @@ local function archetype_create(world: World, types: { i24 }, prev: Archetype?): end local function world_entity(world: World): i53 - local entityId = world.nextEntityId + 1 + local entityId = (world.nextEntityId :: number) + 1 world.nextEntityId = entityId return entity_index_new_id(world.entityIndex, entityId + EcsRest) end @@ -605,7 +605,8 @@ end -- Symmetric like `World.add` but idempotent local function world_set(world: World, entity: i53, id: i53, data: unknown) - local record = world.entityIndex.sparse[entity] + local entityIndex = world.entityIndex + local record = entityIndex.sparse[entity] local from = record.archetype local to = archetype_traverse_add(world, id, from) local idr = world.componentIndex[id] @@ -625,7 +626,7 @@ local function world_set(world: World, entity: i53, id: i53, data: unknown) if from then -- If there was a previous archetype, then the entity needs to move the archetype - entity_move(world.entityIndex, entity, record, to) + entity_move(entityIndex, entity, record, to) else if #to.types > 0 then -- When there is no previous archetype it should create the archetype @@ -634,15 +635,19 @@ local function world_set(world: World, entity: i53, id: i53, data: unknown) end local tr = to.records[id] - to.columns[tr.column][record.row] = data + local column = to.columns[tr.column] - if has_hooks then + if not has_hooks then + column[record.row] = data + else + invoke_hook(world, EcsOnAdd, id, entity, data) + column[record.row] = data invoke_hook(world, EcsOnSet, id, entity, data) end end local function world_component(world: World): i53 - local componentId = world.nextComponentId + 1 + local componentId = (world.nextComponentId :: number) + 1 if componentId > HI_COMPONENT_ID then -- IDs are partitioned into ranges because component IDs are not nominal, -- so it needs to error when IDs intersect into the entity range. @@ -1407,7 +1412,7 @@ World.target = world_target World.parent = world_parent World.contains = world_contains -function World.new(): t_world +function World.new() local self = setmetatable({ archetypeIndex = {} :: { [string]: Archetype }, archetypes = {} :: Archetypes, @@ -1464,44 +1469,40 @@ type World = { nextComponentId: number, nextEntityId: number, nextArchetypeId: number, -} - -export type t_world = typeof(setmetatable( - {} :: { - +} & { --- Creates a new entity - entity: (t_world) -> Entity, + entity: (self: World) -> Entity, --- Creates a new entity located in the first 256 ids. --- These should be used for static components for fast access. - component: (t_world) -> Entity, + component: (self: World) -> Entity, --- Gets the target of an relationship. For example, when a user calls --- `world:target(id, ChildOf(parent))`, you will obtain the parent entity. - target: (t_world, id: Entity, relation: Entity) -> Entity?, + target: (self: World, id: Entity, relation: Entity) -> Entity?, --- Deletes an entity and all it's related components and relationships. - delete: (t_world, id: Entity) -> (), + delete: (self: World, id: Entity) -> (), --- Adds a component to the entity with no value - add: (world: t_world, id: Entity, component: Entity) -> (), + add: (self: World, id: Entity, component: Entity) -> (), --- Assigns a value to a component on the given entity - set: (world: World, id: Entity, component: Entity, data: T) -> (), + set: (self: World, id: Entity, component: Entity, data: T) -> (), -- Clears an entity from the world - clear: (t_world, id: Entity) -> (), + clear: (self: World, id: Entity) -> (), --- Removes a component from the given entity - remove: (t_world, id: Entity, component: Entity) -> (), + remove: (self: World, id: Entity, component: Entity) -> (), --- Retrieves the value of up to 4 components. These values may be nil. - get: ((t_world, id: any, Entity) -> A) - & ((t_world, id: Entity, Entity, Entity) -> (A, B)) - & ((t_world, id: Entity, Entity, Entity, Entity) -> (A, B, C)) - & (t_world, id: Entity, Entity, Entity, Entity, Entity) -> (A, B, C, D), + get: ((self: World, id: any, Entity) -> A) + & ((self: World, id: Entity, Entity, Entity) -> (A, B)) + & ((self: World, id: Entity, Entity, Entity, Entity) -> (A, B, C)) + & (self: World, id: Entity, Entity, Entity, Entity, Entity) -> (A, B, C, D), --- Searches the world for entities that match a given query - query: ((t_world, Entity) -> Query) - & ((t_world, Entity, Entity) -> Query) - & ((t_world, Entity, Entity, Entity) -> Query) - & ((t_world, Entity, Entity, Entity, Entity) -> Query) + query: ((self: World, Entity) -> Query) + & ((self: World, Entity, Entity) -> Query) + & ((self: World, Entity, Entity, Entity) -> Query) + & ((self: World, Entity, Entity, Entity, Entity) -> Query) & (( - t_world, + self: World, Entity, Entity, Entity, @@ -1509,7 +1510,7 @@ export type t_world = typeof(setmetatable( Entity ) -> Query) & (( - t_world, + self: World, Entity, Entity, Entity, @@ -1518,7 +1519,7 @@ export type t_world = typeof(setmetatable( Entity ) -> Query) & (( - t_world, + self: World, Entity, Entity, Entity, @@ -1528,7 +1529,7 @@ export type t_world = typeof(setmetatable( Entity ) -> Query) & (( - t_world, + self: World, Entity, Entity, Entity, @@ -1539,12 +1540,10 @@ export type t_world = typeof(setmetatable( Entity, ...Entity ) -> Query), - }, {} -)) - + } return { - World = World :: { new: () -> t_world }, + World = World :: { new: () -> World }, OnAdd = EcsOnAdd :: Entity, OnRemove = EcsOnRemove :: Entity, diff --git a/test/tests.luau b/test/tests.luau index 23a6aa2..30fc3f4 100644 --- a/test/tests.luau +++ b/test/tests.luau @@ -1142,5 +1142,105 @@ TEST("Hooks", function() CHECK(not world:has(e1, A)) end + do CASE "the filip incident" + local world = jecs.World.new() + + export type Iterator = () -> (Entity, T?, T?) + export type Destructor = () -> () + + -- Helpers + + type ValuesMap = { [Entity]: T? } + type ChangeSet = { [Entity]: true? } + type ChangeSets = { [ChangeSet]: true? } + type ChangeSetsCache = { + Added: ChangeSets, + Changed: ChangeSets, + Removed: ChangeSets, + } + + local cachedChangeSets = {} + local function getChangeSets(component): ChangeSetsCache + if cachedChangeSets[component] == nil then + local changeSetsAdded: ChangeSets = {} + local changeSetsChanged: ChangeSets = {} + local changeSetsRemoved: ChangeSets = {} + world:set(component, jecs.OnAdd, function(id) + for set in changeSetsAdded do + set[id] = true + end + end) + world:set(component, jecs.OnSet, function(id) + for set in changeSetsChanged do + set[id] = true + end + end) + world:set(component, jecs.OnRemove, function(id) + for set in changeSetsRemoved do + set[id] = true + end + end) + cachedChangeSets[component] = { + Added = changeSetsAdded, + Changed = changeSetsChanged, + Removed = changeSetsRemoved, + } + end + return cachedChangeSets[component] + end + + local function ChangeTracker(component): (Iterator, Destructor) + local values: ValuesMap = {} + local changeSet: ChangeSet = {} + + for id in world:query(component) do + changeSet[id] = true + end + + local changeSets = getChangeSets(component) + changeSets.Added[changeSet] = true + changeSets.Changed[changeSet] = true + changeSets.Removed[changeSet] = true + + local id: Entity? = nil + local iter: Iterator = function() + id = next(changeSet) + if id then + changeSet[id] = nil + local old: T? = values[id] + local new: T? = world:get(id, component) + if old ~= nil and new == nil then + -- Old value but no new value = removed + values[id] = nil + else + -- Old+new value or just new value = new becomes old + values[id] = new + end + return id, old, new + end + return nil :: any, nil, nil + end + + local destroy: Destructor = function() + changeSets.Added[changeSet] = nil + changeSets.Changed[changeSet] = nil + changeSets.Removed[changeSet] = nil + end + + return iter, destroy + end + + local Transform = world:component() + local iter, destroy = ChangeTracker(Transform) + + local e1 = world:entity() + world:set(e1, Transform, {1,1}) + local counter = 0 + for _ in iter do + counter += 1 + end + CHECK(counter == 1) + end + end) FINISH()