Skip to content

Commit

Permalink
Fix hooks not notifying (#103)
Browse files Browse the repository at this point in the history 
* Fix world_has_any checking for inverse statement

* invoke onAdd for set
  • Loading branch information
@Ukendio
Ukendio authored Aug 16, 2024
1 parent 6d45af9 commit 4e5a40f
Show file tree
Hide file tree
Showing 2 changed files with 137 additions and 38 deletions.
75 changes: 37 additions & 38 deletions src/init.luau
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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]
Expand All @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -1464,52 +1469,48 @@ 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>(t_world) -> Entity<T>,
component: <T>(self: World) -> Entity<T>,
--- 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: <T>(world: t_world, id: Entity, component: Entity<T>) -> (),
add: <T>(self: World, id: Entity, component: Entity<T>) -> (),
--- Assigns a value to a component on the given entity
set: <T>(world: World, id: Entity, component: Entity<T>, data: T) -> (),
set: <T>(self: World, id: Entity, component: Entity<T>, 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: (<A>(t_world, id: any, Entity<A>) -> A)
& (<A, B>(t_world, id: Entity, Entity<A>, Entity<B>) -> (A, B))
& (<A, B, C>(t_world, id: Entity, Entity<A>, Entity<B>, Entity<C>) -> (A, B, C))
& <A, B, C, D>(t_world, id: Entity, Entity<A>, Entity<B>, Entity<C>, Entity<D>) -> (A, B, C, D),
get: (<A>(self: World, id: any, Entity<A>) -> A)
& (<A, B>(self: World, id: Entity, Entity<A>, Entity<B>) -> (A, B))
& (<A, B, C>(self: World, id: Entity, Entity<A>, Entity<B>, Entity<C>) -> (A, B, C))
& <A, B, C, D>(self: World, id: Entity, Entity<A>, Entity<B>, Entity<C>, Entity<D>) -> (A, B, C, D),

--- Searches the world for entities that match a given query
query: (<A>(t_world, Entity<A>) -> Query<A>)
& (<A, B>(t_world, Entity<A>, Entity<B>) -> Query<A, B>)
& (<A, B, C>(t_world, Entity<A>, Entity<B>, Entity<C>) -> Query<A, B, C>)
& (<A, B, C, D>(t_world, Entity<A>, Entity<B>, Entity<C>, Entity<D>) -> Query<A, B, C, D>)
query: (<A>(self: World, Entity<A>) -> Query<A>)
& (<A, B>(self: World, Entity<A>, Entity<B>) -> Query<A, B>)
& (<A, B, C>(self: World, Entity<A>, Entity<B>, Entity<C>) -> Query<A, B, C>)
& (<A, B, C, D>(self: World, Entity<A>, Entity<B>, Entity<C>, Entity<D>) -> Query<A, B, C, D>)
& (<A, B, C, D, E>(
t_world,
self: World,
Entity<A>,
Entity<B>,
Entity<C>,
Entity<D>,
Entity<E>
) -> Query<A, B, C, D, E>)
& (<A, B, C, D, E, F>(
t_world,
self: World,
Entity<A>,
Entity<B>,
Entity<C>,
Expand All @@ -1518,7 +1519,7 @@ export type t_world = typeof(setmetatable(
Entity<F>
) -> Query<A, B, C, D, E, F>)
& (<A, B, C, D, E, F, G>(
t_world,
self: World,
Entity<A>,
Entity<B>,
Entity<C>,
Expand All @@ -1528,7 +1529,7 @@ export type t_world = typeof(setmetatable(
Entity<G>
) -> Query<A, B, C, D, E, F, G>)
& (<A, B, C, D, E, F, G, H>(
t_world,
self: World,
Entity<A>,
Entity<B>,
Entity<C>,
Expand All @@ -1539,12 +1540,10 @@ export type t_world = typeof(setmetatable(
Entity<H>,
...Entity<any>
) -> Query<A, B, C, D, E, F, G, H>),
}, {}
))

}

return {
World = World :: { new: () -> t_world },
World = World :: { new: () -> World },

OnAdd = EcsOnAdd :: Entity,
OnRemove = EcsOnRemove :: Entity,
Expand Down
100 changes: 100 additions & 0 deletions test/tests.luau
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<T> = () -> (Entity, T?, T?)
export type Destructor = () -> ()

-- Helpers

type ValuesMap<T> = { [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<T>(component): (Iterator<T>, Destructor)
local values: ValuesMap<T> = {}
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<T> = 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()

0 comments on commit 4e5a40f

Please sign in to comment.