Skip to content

Commit 7b69914

Browse files
committed
fix(exclusive): prevent wildcard marker removal when setting same dontFragment exclusive relation
When repeatedly setting the same dontFragment exclusive relation, the wildcard marker was incorrectly removed from the archetype. This happened because removeMatchingRelations() was deleting all relations matching the baseComponentId, including the wildcard marker itself (since its componentId also matches). The fix adds a check to skip wildcard markers in removeMatchingRelations(), ensuring they are only removed by maybeRemoveWildcardMarker() when appropriate (i.e., when the last concrete relation is removed). Added comprehensive tests covering: - Repeated setting of same exclusive relation - Changing exclusive relation targets - Non-exclusive dontFragment relations with repeated setting - Wildcard queries with has() checks for specific targets - Rapid target switching scenarios
1 parent 2183c8c commit 7b69914

File tree

2 files changed

+235
-0
lines changed

2 files changed

+235
-0
lines changed

src/__tests__/dont-fragment-query-notification.test.ts

Lines changed: 232 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,4 +232,236 @@ describe("DontFragment Query Notification Issue", () => {
232232
expect(queryB.getEntities()).not.toContain(entity1);
233233
expect(queryB.getEntities()).toContain(entity2);
234234
});
235+
236+
it("should handle repeated setting of the same dontFragment exclusive relation", () => {
237+
const world = new World();
238+
239+
const PositionId = component();
240+
const ChildOf = component({ dontFragment: true, exclusive: true });
241+
242+
const parent = world.new();
243+
const entity = world.new();
244+
world.set(entity, PositionId);
245+
246+
// Create a wildcard query before setting relations
247+
const wildcardChildOf = relation(ChildOf, "*");
248+
const query = world.createQuery([wildcardChildOf, PositionId]);
249+
250+
// Initially, should not match
251+
expect(query.getEntities()).not.toContain(entity);
252+
253+
// Set the relation for the first time
254+
world.set(entity, relation(ChildOf, parent));
255+
world.sync();
256+
257+
// Should now match
258+
expect(query.getEntities()).toContain(entity);
259+
expect(query.getEntities().length).toBe(1);
260+
261+
// Repeat setting the same relation (second time)
262+
world.set(entity, relation(ChildOf, parent));
263+
world.sync();
264+
265+
// Should still match (not lost due to exclusive handling)
266+
expect(query.getEntities()).toContain(entity);
267+
expect(query.getEntities().length).toBe(1);
268+
269+
// Set it one more time to be sure
270+
world.set(entity, relation(ChildOf, parent));
271+
world.sync();
272+
273+
// Should still match
274+
expect(query.getEntities()).toContain(entity);
275+
expect(query.getEntities().length).toBe(1);
276+
});
277+
278+
it("should handle changing exclusive relation target (replacing one relation with another)", () => {
279+
const world = new World();
280+
281+
const PositionId = component();
282+
const ChildOf = component({ dontFragment: true, exclusive: true });
283+
284+
const parent1 = world.new();
285+
const parent2 = world.new();
286+
const entity = world.new();
287+
world.set(entity, PositionId);
288+
289+
const wildcardChildOf = relation(ChildOf, "*");
290+
const query = world.createQuery([wildcardChildOf, PositionId]);
291+
292+
// Set relation to parent1
293+
world.set(entity, relation(ChildOf, parent1));
294+
world.sync();
295+
296+
expect(query.getEntities()).toContain(entity);
297+
expect(world.has(entity, relation(ChildOf, parent1))).toBe(true);
298+
expect(world.has(entity, relation(ChildOf, parent2))).toBe(false);
299+
300+
// Change relation to parent2 (exclusive should remove parent1 relation)
301+
world.set(entity, relation(ChildOf, parent2));
302+
world.sync();
303+
304+
expect(query.getEntities()).toContain(entity);
305+
expect(query.getEntities().length).toBe(1);
306+
expect(world.has(entity, relation(ChildOf, parent1))).toBe(false);
307+
expect(world.has(entity, relation(ChildOf, parent2))).toBe(true);
308+
});
309+
310+
it("should handle specific relation query when target changes (non-dontFragment)", () => {
311+
const world = new World();
312+
313+
const PositionId = component();
314+
// Note: Using non-dontFragment exclusive relation for specific queries
315+
const ChildOf = component({ exclusive: true });
316+
317+
const parent1 = world.new();
318+
const parent2 = world.new();
319+
const entity = world.new();
320+
world.set(entity, PositionId);
321+
322+
// Create specific queries for each parent
323+
const queryParent1 = world.createQuery([relation(ChildOf, parent1), PositionId]);
324+
const queryParent2 = world.createQuery([relation(ChildOf, parent2), PositionId]);
325+
326+
// Set relation to parent1
327+
world.set(entity, relation(ChildOf, parent1));
328+
world.sync();
329+
330+
expect(queryParent1.getEntities()).toContain(entity);
331+
expect(queryParent2.getEntities()).not.toContain(entity);
332+
333+
// Change to parent2
334+
world.set(entity, relation(ChildOf, parent2));
335+
world.sync();
336+
337+
expect(queryParent1.getEntities()).not.toContain(entity);
338+
expect(queryParent2.getEntities()).toContain(entity);
339+
340+
// Change back to parent1
341+
world.set(entity, relation(ChildOf, parent1));
342+
world.sync();
343+
344+
expect(queryParent1.getEntities()).toContain(entity);
345+
expect(queryParent2.getEntities()).not.toContain(entity);
346+
});
347+
348+
it("should handle non-exclusive dontFragment relations with repeated setting", () => {
349+
const world = new World();
350+
351+
const PositionId = component();
352+
const TaggedWith = component({ dontFragment: true }); // non-exclusive
353+
354+
const tag1 = world.new();
355+
const tag2 = world.new();
356+
const entity = world.new();
357+
world.set(entity, PositionId);
358+
359+
const wildcardTagged = relation(TaggedWith, "*");
360+
const query = world.createQuery([wildcardTagged, PositionId]);
361+
362+
// Add first tag
363+
world.set(entity, relation(TaggedWith, tag1));
364+
world.sync();
365+
366+
expect(query.getEntities()).toContain(entity);
367+
368+
// Add second tag (non-exclusive allows multiple)
369+
world.set(entity, relation(TaggedWith, tag2));
370+
world.sync();
371+
372+
expect(query.getEntities()).toContain(entity);
373+
expect(world.has(entity, relation(TaggedWith, tag1))).toBe(true);
374+
expect(world.has(entity, relation(TaggedWith, tag2))).toBe(true);
375+
376+
// Re-set first tag (should not affect second tag)
377+
world.set(entity, relation(TaggedWith, tag1));
378+
world.sync();
379+
380+
expect(query.getEntities()).toContain(entity);
381+
expect(world.has(entity, relation(TaggedWith, tag1))).toBe(true);
382+
expect(world.has(entity, relation(TaggedWith, tag2))).toBe(true);
383+
});
384+
385+
it("should handle wildcard queries with has() checks for specific targets", () => {
386+
const world = new World();
387+
388+
const PositionId = component();
389+
const ChildOf = component({ dontFragment: true, exclusive: true });
390+
391+
const parent1 = world.new();
392+
const parent2 = world.new();
393+
const entity1 = world.new();
394+
const entity2 = world.new();
395+
396+
world.set(entity1, PositionId);
397+
world.set(entity2, PositionId);
398+
399+
// For dontFragment relations, use wildcard query and filter with has()
400+
const wildcardChildOf = relation(ChildOf, "*");
401+
const wildcardQuery = world.createQuery([wildcardChildOf, PositionId]);
402+
403+
// Set entity1 -> parent1, entity2 -> parent2
404+
world.set(entity1, relation(ChildOf, parent1));
405+
world.set(entity2, relation(ChildOf, parent2));
406+
world.sync();
407+
408+
expect(wildcardQuery.getEntities().length).toBe(2);
409+
expect(wildcardQuery.getEntities()).toContain(entity1);
410+
expect(wildcardQuery.getEntities()).toContain(entity2);
411+
412+
// Filter for specific parent using has()
413+
const entitiesWithParent1 = wildcardQuery.getEntities().filter((e) => world.has(e, relation(ChildOf, parent1)));
414+
expect(entitiesWithParent1.length).toBe(1);
415+
expect(entitiesWithParent1).toContain(entity1);
416+
417+
// Change entity1 from parent1 to parent2
418+
world.set(entity1, relation(ChildOf, parent2));
419+
world.sync();
420+
421+
expect(wildcardQuery.getEntities().length).toBe(2);
422+
const entitiesWithParent1After = wildcardQuery
423+
.getEntities()
424+
.filter((e) => world.has(e, relation(ChildOf, parent1)));
425+
expect(entitiesWithParent1After.length).toBe(0);
426+
427+
// Change entity1 back to parent1
428+
world.set(entity1, relation(ChildOf, parent1));
429+
world.sync();
430+
431+
expect(wildcardQuery.getEntities().length).toBe(2);
432+
const entitiesWithParent1Final = wildcardQuery
433+
.getEntities()
434+
.filter((e) => world.has(e, relation(ChildOf, parent1)));
435+
expect(entitiesWithParent1Final.length).toBe(1);
436+
expect(entitiesWithParent1Final).toContain(entity1);
437+
});
438+
439+
it("should preserve wildcard marker when switching between targets rapidly", () => {
440+
const world = new World();
441+
442+
const PositionId = component();
443+
const ChildOf = component({ dontFragment: true, exclusive: true });
444+
445+
const parents = [world.new(), world.new(), world.new()];
446+
const entity = world.new();
447+
world.set(entity, PositionId);
448+
449+
const wildcardChildOf = relation(ChildOf, "*");
450+
const query = world.createQuery([wildcardChildOf, PositionId]);
451+
452+
// Rapidly switch between different parents
453+
for (let i = 0; i < 10; i++) {
454+
const parent = parents[i % parents.length]!;
455+
world.set(entity, relation(ChildOf, parent));
456+
world.sync();
457+
458+
expect(query.getEntities()).toContain(entity);
459+
expect(query.getEntities().length).toBe(1);
460+
expect(world.has(entity, relation(ChildOf, parent))).toBe(true);
461+
462+
// Verify wildcard marker is still in archetype
463+
const archetype = (world as any).entityToArchetype.get(entity);
464+
expect(archetype.componentTypes).toContain(wildcardChildOf);
465+
}
466+
});
235467
});

src/core/world-commands.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,9 @@ export function removeMatchingRelations(
9191
): void {
9292
// Check archetype components
9393
for (const componentType of archetype.componentTypes) {
94+
// Skip wildcard markers - they should only be removed by maybeRemoveWildcardMarker
95+
if (isWildcardRelationId(componentType)) continue;
96+
9497
if (getComponentIdFromRelationId(componentType) === baseComponentId) {
9598
changeset.delete(componentType);
9699
}

0 commit comments

Comments
 (0)