Skip to content

Commit

Permalink
Adds runtime check to prevent query modification.
Browse files Browse the repository at this point in the history
  • Loading branch information
noahlange committed Feb 17, 2024
1 parent 4a6a491 commit 0542c5a
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 18 deletions.
7 changes: 2 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ for (const component of entity.components) {
}
```

A major footgun here is that the types of entities with removed components will not change during the current system's `tick()`. Unless you're paying close attention, you may find yourself accessing a non-existent component.
A potential footgun here is that the types of entities with removed components will not change during the current system's `tick()`. Unless you're paying close attention, you may find yourself accessing a non-existent component.

```typescript
for (const entity of ctx.query.components(A, B)) {
Expand Down Expand Up @@ -401,10 +401,7 @@ for (const { $ } of query) {

### Persistence

After a query has been constructed for the first time, it's cached.
Any query built with the same "signature"

will return the same query—so.the additional overhead you're introducing by creating a new query each tick isn't enormous. But by assigning the query to a property on the system or plugin instance, you can access and execute a query without being forced to rebuild it.
After a query has been constructed for the first time, it's cached. Any query built with the same "signature" will return the same query instance—so the additional overhead you're introducing by creating a new query each tick isn't enormous. But by assigning the query to a property on the system or plugin instance, you can access and execute a query without being forced to rebuild it.

## Saving & Loading

Expand Down
7 changes: 7 additions & 0 deletions src/lib/QueryBuilder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,3 +180,10 @@ describe('first()', () => {
expect(ctx.query.components(A).first()).toBe(a);
});
});

test('mutating a query throws a runtime error', async () => {
const { ctx } = await setup(5);
const hasA = ctx.query.components(A);
expect(hasA.get()).toHaveLength(15);
expect(() => hasA.all.components(B)).toThrow();
});
25 changes: 14 additions & 11 deletions src/lib/QueryBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,9 @@ export interface QueryState {
export class QueryBuilder<T extends BaseType = {}> implements QueryBuilderBase<T> {
protected key: string[] = [];
protected state!: QueryState;
protected resolved: Query<T> | null = null;
protected criteria: QueryStep[] = [];
protected ctx: Context;

protected get id(): string {
return this.key.join('␝');
}
protected id: string | null = null;

/**
* Mark query parameters as mandatory.
Expand Down Expand Up @@ -85,15 +81,15 @@ export class QueryBuilder<T extends BaseType = {}> implements QueryBuilderBase<T
public references(...entities: Entity[]): this {
this.state.tag = Constraint.IN;
this.state.ids = entities.map(entity => entity.id);
return this.reset();
return this.step();
}

/**
* Constrain results based on one or components.
*/
public components<A extends ComponentClass[]>(...components: A): QueryBuilder<T & KeyedByType<A>> {
this.state.ids.push(...components.map(c => c.type));
return this.reset() as unknown as QueryBuilder<T & KeyedByType<A>>;
return this.step() as unknown as QueryBuilder<T & KeyedByType<A>>;
}

/**
Expand All @@ -107,11 +103,12 @@ export class QueryBuilder<T extends BaseType = {}> implements QueryBuilderBase<T
}

this.state.ids = items.map(i => tags[i]);
return this.reset();
return this.step();
}

public get query(): Query<T, Entity<T>> {
return (this.resolved ??= this.ctx.manager.getQuery<T, Entity<T>>(this.criteria, this.id));
const id = (this.id ??= this.key.join('␝'));
return this.ctx.manager.getQuery<T, Entity<T>>(this.criteria, id);
}

/**
Expand All @@ -135,7 +132,11 @@ export class QueryBuilder<T extends BaseType = {}> implements QueryBuilderBase<T
return this.query[Symbol.iterator]();
}

protected reset(): this {
private step(): this {
if (this.id) {
throw new Error('Modifying a resolved query will return inaccurate result sets.');
}

if (this.state && this.state.tag !== Constraint.SOME) {
const constraint = this.state.tag ?? Constraint.ALL;
const key = constraint + '␞' + this.state.ids.join('␟');
Expand All @@ -144,12 +145,14 @@ export class QueryBuilder<T extends BaseType = {}> implements QueryBuilderBase<T
this.criteria.push({ constraint, ids: this.state.ids.slice() });
}
}

this.state = { tag: null, ids: [] };
this.id = null;
return this;
}

public constructor(ctx: Context<any>) {
this.ctx = ctx;
this.reset();
this.step();
}
}
3 changes: 1 addition & 2 deletions src/test/helpers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,12 @@ export function getContext(): Context<{}> {
return ctx;
}

export async function setup(): Promise<{
export async function setup(count: number = 5): Promise<{
ctx: Context<{}>;
ids: Identifier[];
}> {
const ctx = getContext();
const entities: Entity[] = [];
const count = 5;

await withTick(ctx, () => {
for (let i = 0; i < count; i++) {
Expand Down

0 comments on commit 0542c5a

Please sign in to comment.