Skip to content

Commit

Permalink
Fixed: "no" query operation would not match Is relationships properly
Browse files Browse the repository at this point in the history 
Fixed: world::is and world::in did the opposite of what they should
  • Loading branch information
@richardbiely
richardbiely committed Sep 17, 2024
1 parent f4c23c7 commit 19f72e8
Show file tree
Hide file tree
Showing 5 changed files with 223 additions and 131 deletions.
33 changes: 17 additions & 16 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1113,33 +1113,34 @@ Entities can inherit from other entities by using the (Is, target) relationship.
```cpp
ecs::World w;
ecs::Entity animal = w.add();
ecs::Entity herbivore = w.add();
ecs::Entity rabbit = w.add();
ecs::Entity hare = w.add();
ecs::Entity wolf = w.add();
ecs::Entity wall = w.add();
// Make rabbit an animal
w.as(rabbit, animal); // equivalent of w.add(rabbit, ecs::Pair(ecs::Is, animal))
w.as(wolf, animal); // equivalent of w.add(wolf, ecs::Pair(ecs::Is, animal))
w.as(herbivore, animal); // equivalent of w.add(herbivore, ecs::Pair(ecs::Is, animal))
w.as(rabbit, herbivore); // equivalent of w.add(rabbit, ecs::Pair(ecs::Is, herbivore))
w.as(hare, herbivore); // equivalent of w.add(hare, ecs::Pair(ecs::Is, herbivore))
// Check if an entity is inheriting from something
bool animal_is_animal = w.is(animal, animal); // true
bool rabbit_is_animal = w.is(herbivore, animal); // true
bool wall_is_animal = w.is(wall, animal); // false
```

The Is relation ship can be very helpful when used in queries. However, before this feature can be properly utilized, one needs to make sure the entity in the Is relationship is treated as a type (has a separate archetype).

bool herbivore_is_animal = w.is(herbivore, animal); // true
bool rabbit_is_herbivore = w.is(rabbit, herbivore); // true
bool rabbit_is_animal = w.is(rabbit, animal); // true
bool animal_is_rabbit = w.is(animal, rabbit); // false
bool wolf_is_herbivore = w.is(wolf, herbivore); // false
bool wolf_is_animal = w.is(wolf, animal); // true
```cpp
// Make sure animal is treated as a type
w.add(animal, animal);

// Iterate everything that is animal
ecs::Query q = w.query().all(Pair(ecs::Is, animal));
q.each([](ecs::Entity entity) {
// runs for herbivore, rabbit, hare and wolf
// entity = animal, rabbit
});

// Iterate everything that is animal but skip wolfs
ecs::Query q2 = w.query().all(Pair(ecs::Is, animal)).no(wolf);
ecs::Query q2 = w.query().all(Pair(ecs::Is, animal)).no(animal);
q2.each([](ecs::Entity entity) {
// runs for herbivore, rabbit, hare
// entity = rabbit
});
```

Expand Down
115 changes: 68 additions & 47 deletions include/gaia/ecs/vm.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,16 +237,6 @@ namespace gaia {
return Op::eval(queryIdsCnt, matches);
}

inline GAIA_NODISCARD bool match_idQueryId_with_idArchetype(
const World& w, uint32_t mask, uint32_t idQueryIdx, EntityId idQuery, EntityId idArchetype) {
if ((mask & (1U << idQueryIdx)) == 0U)
return idQuery == idArchetype;

const auto qe = entity_from_id(w, idQuery);
const auto ae = entity_from_id(w, idArchetype);
return is(w, ae, qe);
};

struct IdCmpResult {
bool matched;
};
Expand Down Expand Up @@ -284,15 +274,16 @@ namespace gaia {

inline GAIA_NODISCARD IdCmpResult
cmp_ids_is(const World& w, const Archetype& archetype, Entity idInQuery, Entity idInArchetype) {
auto archetypeIds = archetype.ids_view();

// all(Pair<Is, X>)
if (idInQuery.pair() && idInQuery.id() == Is.id()) {
return {as_relations_trav_if(w, idInQuery, [&](Entity relation) {
const auto idx = core::get_index(archetypeIds, relation);
// Stop at the first match
return idx != BadIndex;
})};
auto archetypeIds = archetype.ids_view();
return {
idInQuery.gen() == idInArchetype.id() || // X vs Id
as_relations_trav_if(w, idInQuery, [&](Entity relation) {
const auto idx = core::get_index(archetypeIds, relation);
// Stop at the first match
return idx != BadIndex;
})};
}

// 1:1 match needed for non-pairs
Expand All @@ -301,8 +292,6 @@ namespace gaia {

inline GAIA_NODISCARD IdCmpResult
cmp_ids_is_pairs(const World& w, const Archetype& archetype, Entity idInQuery, Entity idInArchetype) {
auto archetypeIds = archetype.ids_view();

if (idInQuery.pair()) {
// all(Pair<All, All>) aka "any pair"
if (idInQuery == Pair(All, All))
Expand All @@ -314,19 +303,25 @@ namespace gaia {
if (idInArchetype == idInQuery)
return {true};

const auto e = entity_from_id(w, idInQuery.gen());
auto archetypeIds = archetype.ids_view();
const auto eQ = entity_from_id(w, idInQuery.gen());
if (eQ == idInArchetype)
return {true};

// If the archetype entity is an (Is, X) pair treat Is as X and try matching it with
// entities inheriting from e.
if (idInArchetype.id() == Is.id()) {
const auto e2 = entity_from_id(w, idInArchetype.gen());
return {as_relations_trav_if(w, e, [&](Entity relation) {
return e2 == relation;
const auto eA = entity_from_id(w, idInArchetype.gen());
if (eA == eQ)
return {true};

return {as_relations_trav_if(w, eQ, [eA](Entity relation) {
return eA == relation;
})};
}

// Archetype entity is generic, try matching it with entities inheriting from e.
return {as_relations_trav_if(w, e, [&](Entity relation) {
return {as_relations_trav_if(w, eQ, [&archetypeIds](Entity relation) {
// Relation does not necessary match the sorted order of components in the archetype
// so we need to search through all of its ids.
const auto idx = core::get_index(archetypeIds, relation);
Expand All @@ -346,6 +341,8 @@ namespace gaia {

// If there are any Is pairs on the archetype we need to check if we match them
if (archetype.pairs_is() > 0) {
auto archetypeIds = archetype.ids_view();

const auto e = entity_from_id(w, idInQuery.gen());
return {as_relations_trav_if(w, e, [&](Entity relation) {
// Relation does not necessary match the sorted order of components in the archetype
Expand Down Expand Up @@ -375,7 +372,7 @@ namespace gaia {
}

//! Tries to match entity ids in \param queryIds with those in \param archetype given
//! the comparison function \param func.
//! the comparison function \param func. Does not consider Is relationships.
//! \return True on the first match, false otherwise.
template <typename Op>
inline GAIA_NODISCARD bool match_res(const Archetype& archetype, EntitySpan queryIds) {
Expand All @@ -399,9 +396,11 @@ namespace gaia {
});
}

//! Tries to match entity ids in \param queryIds with those in \param archetype given
//! the comparison function \param func. Considers Is relationships.
//! \return True on the first match, false otherwise.
template <typename Op>
inline GAIA_NODISCARD bool
match_res_backtrack(const World& w, const Archetype& archetype, EntitySpan queryIds) {
inline GAIA_NODISCARD bool match_res_as(const World& w, const Archetype& archetype, EntitySpan queryIds) {
// Archetype has no pairs we can compare ids directly
if (archetype.pairs() == 0) {
return match_inter<Op>(
Expand Down Expand Up @@ -510,7 +509,7 @@ namespace gaia {

if (matchesSet.contains(pArchetype))
continue;
if (!match_res_backtrack<OpAll>(*ctx.pWorld, *pArchetype, ctx.idsToMatch))
if (!match_res_as<OpAll>(*ctx.pWorld, *pArchetype, ctx.idsToMatch))
continue;

matchesSet.emplace(pArchetype);
Expand Down Expand Up @@ -615,7 +614,7 @@ namespace gaia {

if (matchesSet.contains(pArchetype))
continue;
if (!match_res_backtrack<OpAny>(*ctx.pWorld, *pArchetype, ctx.idsToMatch))
if (!match_res_as<OpAny>(*ctx.pWorld, *pArchetype, ctx.idsToMatch))
continue;

matchesSet.emplace(pArchetype);
Expand All @@ -629,6 +628,8 @@ namespace gaia {
auto& matchesArr = *ctx.pMatchesArr;
auto& matchesSet = *ctx.pMatchesSet;

ctx.pMatchesSet->clear();

// For NO we need to search among all archetypes.
const auto cache_it = ctx.pLastMatchedArchetypeIdx_All->find(EntityBadLookupKey);
uint32_t lastMatchedIdx = 0;
Expand Down Expand Up @@ -669,14 +670,42 @@ namespace gaia {

if (matchesSet.contains(pArchetype))
continue;
if (!match_res_backtrack<OpNo>(*ctx.pWorld, *pArchetype, ctx.idsToMatch))
if (!match_res_as<OpNo>(*ctx.pWorld, *pArchetype, ctx.idsToMatch))
continue;

matchesSet.emplace(pArchetype);
matchesArr.emplace_back(pArchetype);
}
}

inline void match_archetype_no_2(MatchingCtx& ctx) {
// We had some matches already (with ALL or ANY). We need to remove those
// that match with the NO list. Remove them back-to-front.
for (uint32_t i = 0; i < ctx.pMatchesArr->size();) {
auto* pArchetype = (*ctx.pMatchesArr)[i];
if (match_res<OpNo>(*pArchetype, ctx.idsToMatch)) {
++i;
continue;
}

core::erase_fast(*ctx.pMatchesArr, i);
}
}

inline void match_archetype_no_as_2(MatchingCtx& ctx) {
// We had some matches already (with ALL or ANY). We need to remove those
// that match with the NO list. Remove them back-to-front.
for (uint32_t i = 0; i < ctx.pMatchesArr->size();) {
auto* pArchetype = (*ctx.pMatchesArr)[i];
if (match_res_as<OpNo>(*ctx.pWorld, *pArchetype, ctx.idsToMatch)) {
++i;
continue;
}

core::erase_fast(*ctx.pMatchesArr, i);
}
}

struct OpCodeBaseData {
EOpCode id;
};
Expand All @@ -693,10 +722,10 @@ namespace gaia {
// First viable item is not related to an Is relationship
if (ctx.as_mask_0 + ctx.as_mask_1 == 0U) {
match_archetype_all(ctx);
} else
}
// First viable item is related to an Is relationship.
// In this case we need to gather all related archetypes and evaluate one-by-one (backtracking).
{
// In this case we need to gather all related archetypes.
else {
match_archetype_all_as(ctx);
}

Expand Down Expand Up @@ -755,7 +784,7 @@ namespace gaia {

// First viable item is related to an Is relationship.
// In this case we need to gather all related archetypes.
if (match_res_backtrack<OpAny>(*ctx.pWorld, *pArchetype, ctx.idsToMatch))
if (match_res_as<OpAny>(*ctx.pWorld, *pArchetype, ctx.idsToMatch))
goto checkNextArchetype;
}

Expand All @@ -773,7 +802,7 @@ namespace gaia {

struct OpCodeNot {
static constexpr EOpCode Id = EOpCode::Not;

bool exec(const QueryCompileCtx& comp, MatchingCtx& ctx) {
GAIA_PROF_SCOPE(vm::op_not);

Expand All @@ -783,23 +812,15 @@ namespace gaia {
if (ctx.pMatchesArr->empty()) {
// If there are no previous matches (no ALL or ANY matches),
// we need to search among all archetypes.
ctx.pMatchesSet->clear();
if (ctx.as_mask_0 + ctx.as_mask_1 == 0U)
match_archetype_no(ctx);
else
match_archetype_no_as(ctx);
} else {
// We had some matches already (with ALL or ANY). We need to remove those
// that match with the NO list. Remove them back-to-front.
for (uint32_t i = 0; i < ctx.pMatchesArr->size();) {
auto* pArchetype = (*ctx.pMatchesArr)[i];
if (match_res<OpNo>(*pArchetype, ctx.idsToMatch)) {
++i;
continue;
}

core::erase_fast(*ctx.pMatchesArr, i);
}
if (ctx.as_mask_0 + ctx.as_mask_1 == 0U)
match_archetype_no_2(ctx);
else
match_archetype_no_as_2(ctx);
}

return true;
Expand Down
8 changes: 4 additions & 4 deletions include/gaia/ecs/world.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1098,15 +1098,15 @@ namespace gaia {
//! Checks if \param entity inherits from \param entityBase.
//! True if entity inherits from entityBase. False otherwise.
GAIA_NODISCARD bool is(Entity entity, Entity entityBase) const {
return is_inter<true>(entity, entityBase);
return is_inter<false>(entity, entityBase);
}

//! Checks if \param entity is located in \param entityBase.
//! This is almost the same as "is" with the exception that false is returned
//! if \param entity matches \param entityBase
//! True if entity is located in entityBase. False otherwise.
GAIA_NODISCARD bool in(Entity entity, Entity entityBase) const {
return is_inter<false>(entity, entityBase);
return is_inter<true>(entity, entityBase);
}

GAIA_NODISCARD bool is_base(Entity target) const {
Expand Down Expand Up @@ -3268,8 +3268,8 @@ namespace gaia {
}
}

//! If \tparam CheckIn is true, checks if \param entity inherits from \param entityBase.
//! If \tparam CheckIn is false, checks if \param entity is located in \param entityBase.
//! If \tparam CheckIn is true, checks if \param entity is located in \param entityBase.
//! If \tparam CheckIn is false, checks if \param entity inherits from \param entityBase.
//! True if \param entity inherits from/is located in \param entityBase. False otherwise.
template <bool CheckIn>
GAIA_NODISCARD bool is_inter(Entity entity, Entity entityBase) const {
Expand Down
Loading

0 comments on commit 19f72e8

Please sign in to comment.