Skip to content

Commit 0b36e63

Browse files
Copilotcodehz
andauthored
Fix wildcard queries for dontFragment relations (#5)
* Initial plan * Add test cases to reproduce wildcard query bug with dontFragment components Co-authored-by: codehz <13158903+codehz@users.noreply.github.com> * Fix wildcard query issue with dontFragment components - Added hasRelationWithComponentId method to Archetype to check for relations including dontFragment - Updated getMatchingArchetypes to use new method for wildcard relation filtering - Modified Query.getEntities to filter entities with wildcard relations at entity level for dontFragment components - Enhanced buildWildcardRelationValue to include dontFragment relations in iteration - Added comprehensive tests for wildcard queries with dontFragment components Co-authored-by: codehz <13158903+codehz@users.noreply.github.com> * Address code review feedback - Add null safety check for wildcard relation results in Query.getEntities - Improve error message with component ID and entity ID for better debugging Co-authored-by: codehz <13158903+codehz@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: codehz <13158903+codehz@users.noreply.github.com>
1 parent 07baed9 commit 0b36e63

File tree

4 files changed

+194
-21
lines changed

4 files changed

+194
-21
lines changed

src/archetype.ts

Lines changed: 84 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -457,10 +457,11 @@ export class Archetype {
457457
componentTypes: T,
458458
componentDataSources: (any[] | EntityId<any>[] | undefined)[],
459459
entityIndex: number,
460+
entityId: EntityId,
460461
): ComponentTuple<T> {
461462
return componentDataSources.map((dataSource, i) => {
462463
const compType = componentTypes[i]!;
463-
return this.buildSingleComponent(compType, dataSource, entityIndex);
464+
return this.buildSingleComponent(compType, dataSource, entityIndex, entityId);
464465
}) as ComponentTuple<T>;
465466
}
466467

@@ -471,12 +472,19 @@ export class Archetype {
471472
compType: ComponentType<any>,
472473
dataSource: any[] | EntityId<any>[] | undefined,
473474
entityIndex: number,
475+
entityId: EntityId,
474476
): any {
475477
const optional = isOptionalEntityId(compType);
476478
const actualType = optional ? compType.optional : compType;
477479

478480
if (getIdType(actualType) === "wildcard-relation") {
479-
return this.buildWildcardRelationValue(dataSource, entityIndex, optional);
481+
return this.buildWildcardRelationValue(
482+
actualType as WildcardRelationId<any>,
483+
dataSource,
484+
entityIndex,
485+
entityId,
486+
optional,
487+
);
480488
} else {
481489
return this.buildRegularComponentValue(dataSource, entityIndex, optional);
482490
}
@@ -486,27 +494,54 @@ export class Archetype {
486494
* Build wildcard relation value from matching relations
487495
*/
488496
private buildWildcardRelationValue(
497+
wildcardRelationType: WildcardRelationId<any>,
489498
dataSource: any[] | EntityId<any>[] | undefined,
490499
entityIndex: number,
500+
entityId: EntityId,
491501
optional: boolean,
492502
): any {
493-
if (dataSource === undefined) {
494-
if (optional) {
495-
return undefined;
496-
}
497-
throw new Error(`No matching relations found for mandatory wildcard relation component type`);
498-
}
499-
500-
const matchingRelations = dataSource as EntityId<any>[];
503+
const matchingRelations = (dataSource as EntityId<any>[]) || [];
501504
const relations: [EntityId<unknown>, any][] = [];
502505

506+
// Add regular archetype relations
503507
for (const relType of matchingRelations) {
504508
const dataArray = this.getComponentData(relType);
505509
const data = dataArray[entityIndex];
506510
const decodedRel = decodeRelationId(relType as RelationId<any>);
507511
relations.push([decodedRel.targetId, data === MISSING_COMPONENT ? undefined : data]);
508512
}
509513

514+
// Add dontFragment relations for this entity
515+
// Get the component ID from the wildcard relation type
516+
const wildcardDecoded = decodeRelationId(wildcardRelationType);
517+
const targetComponentId = wildcardDecoded.componentId;
518+
519+
const dontFragmentData = this.dontFragmentRelations.get(entityId);
520+
if (dontFragmentData) {
521+
// Check dontFragment relations for matching component ID
522+
for (const [relType, data] of dontFragmentData) {
523+
const relDetailed = getDetailedIdType(relType);
524+
if (
525+
(relDetailed.type === "entity-relation" || relDetailed.type === "component-relation") &&
526+
relDetailed.componentId === targetComponentId
527+
) {
528+
relations.push([relDetailed.targetId, data]);
529+
}
530+
}
531+
}
532+
533+
// If no relations found and not optional, this entity doesn't match
534+
if (relations.length === 0) {
535+
if (!optional) {
536+
const wildcardDecoded = decodeRelationId(wildcardRelationType);
537+
throw new Error(
538+
`No matching relations found for mandatory wildcard relation component ${wildcardDecoded.componentId} on entity ${entityId}`,
539+
);
540+
}
541+
// For optional, return undefined when there are no relations
542+
return undefined;
543+
}
544+
510545
return optional ? { value: relations } : relations;
511546
}
512547

@@ -570,7 +605,7 @@ export class Archetype {
570605
for (let entityIndex = 0; entityIndex < this.entities.length; entityIndex++) {
571606
const entity = this.entities[entityIndex]!;
572607

573-
const components = this.buildComponentsForIndex(componentTypes, componentDataSources, entityIndex);
608+
const components = this.buildComponentsForIndex(componentTypes, componentDataSources, entityIndex, entity);
574609

575610
yield [entity, ...components];
576611
}
@@ -593,7 +628,7 @@ export class Archetype {
593628
const entity = this.entities[entityIndex]!;
594629

595630
// Direct array access for each component type using pre-cached sources
596-
const components = this.buildComponentsForIndex(componentTypes, componentDataSources, entityIndex);
631+
const components = this.buildComponentsForIndex(componentTypes, componentDataSources, entityIndex, entity);
597632

598633
callback(entity, ...components);
599634
}
@@ -613,4 +648,41 @@ export class Archetype {
613648
callback(this.entities[i]!, components);
614649
}
615650
}
651+
652+
/**
653+
* Check if any entity in this archetype has a relation matching the given component ID
654+
* This includes both regular relations in componentTypes and dontFragment relations
655+
* @param componentId The component ID to match
656+
* @returns true if any entity has a matching relation
657+
*/
658+
hasRelationWithComponentId(componentId: EntityId<any>): boolean {
659+
// Check regular archetype components
660+
for (const componentType of this.componentTypes) {
661+
const detailedType = getDetailedIdType(componentType);
662+
if (
663+
(detailedType.type === "entity-relation" || detailedType.type === "component-relation") &&
664+
detailedType.componentId === componentId
665+
) {
666+
return true;
667+
}
668+
}
669+
670+
// Check dontFragment relations for any entity in this archetype
671+
for (const entityId of this.entities) {
672+
const entityDontFragmentRelations = this.dontFragmentRelations.get(entityId);
673+
if (entityDontFragmentRelations) {
674+
for (const relationType of entityDontFragmentRelations.keys()) {
675+
const detailedType = getDetailedIdType(relationType);
676+
if (
677+
(detailedType.type === "entity-relation" || detailedType.type === "component-relation") &&
678+
detailedType.componentId === componentId
679+
) {
680+
return true;
681+
}
682+
}
683+
}
684+
}
685+
686+
return false;
687+
}
616688
}

src/dont-fragment.test.ts

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,4 +222,73 @@ describe("DontFragment Relations", () => {
222222
expect(archetypes2.length).toBe(1);
223223
expect(archetypes2[0].size).toBe(5);
224224
});
225+
226+
it("should query entities with wildcard relation on dontFragment component using createQuery", () => {
227+
const world = new World();
228+
229+
const PositionId = component();
230+
const ChildOf = component({ dontFragment: true });
231+
232+
const parent1 = world.new();
233+
const parent2 = world.new();
234+
235+
const child1 = world.new();
236+
world.set(child1, PositionId);
237+
world.set(child1, relation(ChildOf, parent1));
238+
239+
const child2 = world.new();
240+
world.set(child2, PositionId);
241+
world.set(child2, relation(ChildOf, parent2));
242+
243+
world.sync();
244+
245+
// Try to query entities with wildcard ChildOf relation
246+
const wildcardChildOf = relation(ChildOf, "*");
247+
const query = world.createQuery([wildcardChildOf]);
248+
const entities = query.getEntities();
249+
250+
// This should find both child1 and child2
251+
expect(entities.length).toBe(2);
252+
expect(entities).toContain(child1);
253+
expect(entities).toContain(child2);
254+
});
255+
256+
it("should query entities with wildcard relation + other components on dontFragment", () => {
257+
const world = new World();
258+
259+
const PositionId = component();
260+
const VelocityId = component();
261+
const ChildOf = component({ dontFragment: true });
262+
263+
const parent1 = world.new();
264+
const parent2 = world.new();
265+
266+
const child1 = world.new();
267+
world.set(child1, PositionId);
268+
world.set(child1, VelocityId);
269+
world.set(child1, relation(ChildOf, parent1));
270+
271+
const child2 = world.new();
272+
world.set(child2, PositionId);
273+
world.set(child2, VelocityId);
274+
world.set(child2, relation(ChildOf, parent2));
275+
276+
// Entity without ChildOf relation
277+
const child3 = world.new();
278+
world.set(child3, PositionId);
279+
world.set(child3, VelocityId);
280+
281+
world.sync();
282+
283+
// Query for entities with wildcard ChildOf relation AND Position
284+
const wildcardChildOf = relation(ChildOf, "*");
285+
const query = world.createQuery([wildcardChildOf, PositionId]);
286+
const entities = query.getEntities();
287+
288+
// Should find child1 and child2, but not child3 (no ChildOf relation)
289+
expect(entities.length).toBe(2);
290+
expect(entities).toContain(child1);
291+
expect(entities).toContain(child2);
292+
expect(entities).not.toContain(child3);
293+
});
225294
});

src/query.ts

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { Archetype } from "./archetype";
2-
import type { EntityId } from "./entity";
2+
import type { EntityId, WildcardRelationId } from "./entity";
3+
import { getDetailedIdType } from "./entity";
34
import { matchesComponentTypes, matchesFilter, type QueryFilter } from "./query-filter";
45
import type { ComponentTuple, ComponentType } from "./types";
56
import type { World } from "./world";
@@ -38,9 +39,44 @@ export class Query {
3839
getEntities(): EntityId[] {
3940
this.ensureNotDisposed();
4041
const result: EntityId[] = [];
41-
for (const archetype of this.cachedArchetypes) {
42-
result.push(...archetype.getEntities());
42+
43+
// Check if any component types are wildcard relations
44+
const hasWildcardRelations = this.componentTypes.some((ct) => {
45+
const detailed = getDetailedIdType(ct);
46+
return detailed.type === "wildcard-relation";
47+
});
48+
49+
// If there are wildcard relations, we need to filter entities that actually have them
50+
// This is necessary for dontFragment components where an archetype can contain entities
51+
// with and without the relation
52+
if (hasWildcardRelations) {
53+
for (const archetype of this.cachedArchetypes) {
54+
for (const entity of archetype.getEntities()) {
55+
// Check if entity has all required wildcard relations
56+
let hasAllRelations = true;
57+
for (const componentType of this.componentTypes) {
58+
const detailed = getDetailedIdType(componentType);
59+
if (detailed.type === "wildcard-relation") {
60+
// Check if entity has at least one relation matching this wildcard
61+
const relations = archetype.get(entity, componentType as WildcardRelationId<any>);
62+
if (!relations || relations.length === 0) {
63+
hasAllRelations = false;
64+
break;
65+
}
66+
}
67+
}
68+
if (hasAllRelations) {
69+
result.push(entity);
70+
}
71+
}
72+
}
73+
} else {
74+
// No wildcard relations, can just return all entities from matching archetypes
75+
for (const archetype of this.cachedArchetypes) {
76+
result.push(...archetype.getEntities());
77+
}
4378
}
79+
4480
return result;
4581
}
4682

src/world.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -573,13 +573,9 @@ export class World<UpdateParams extends any[] = []> {
573573

574574
// Filter by wildcard relations
575575
for (const wildcard of wildcardRelations) {
576-
// Keep only archetypes that have the component
576+
// Keep only archetypes that have the component (including dontFragment relations)
577577
matchingArchetypes = matchingArchetypes.filter((archetype) =>
578-
archetype.componentTypes.some((archetypeType) => {
579-
if (!isRelationId(archetypeType)) return false;
580-
const decoded = decodeRelationId(archetypeType);
581-
return decoded.componentId === wildcard.componentId;
582-
}),
578+
archetype.hasRelationWithComponentId(wildcard.componentId),
583579
);
584580
}
585581

0 commit comments

Comments
 (0)