Stricter types for `Query` and `QueryBuilder`?

[marionebl]

This one is more of a directional question to ask if you'd want a contribution of this kind.

When using the query system there is a number of invariants checked during runtime, resulting in errors being thrown if violated. Some examples (as per this CodeSandbox)

world.createEntity(A);

/* ... */

const q = this.query(b => b.current.with(A).using(C));

execute = () => {
  // Query 'added' not configured ...
  this.q.added.forEach(() => {});

  this.q.current.forEach((e) => {
    // System didn't mark component B as readable
    const b = e.read(B);
    
    // System didn't mark component A as writable
    const a = e.write(A);
    
    // Entity doesn't have a C component
    const c = e.read(C);
  });
}

While learning the query system and bumping into those I figured they could be mostly covered by the type system - is this something you'd be interested to receive a contribution for?

I've given stricter types for the sub-systems at hand a first shot via this TypeScript playground example - the types are definitely not trivial so maintenance vs. value considerations might come into play.

The suggested types would bump the error site for the cases above up to compile time, e.g.

world.createEntity(A);

/* ... */

const q = this.query(b => b.current.with(A).using(C));

execute = () => {
  // Property added might be undefined
  this.q.added.forEach(() => {});

  this.q.current.forEach((e) => {
    // Argument of type 'typeof B' is not assignable to parameter of type 'typeof A'.
    const b = e.read(B);
    
    // Property 'write' does not exist on type 'ReadableEntity<typeof A>'.
    const a = e.write(A);
    
    // Not covered with types (yet)
    // const c = e.read(C);
  });
}

[pkaminski]

Hey Mario, thanks for suggesting this idea and taking the time to write a proof of concept! I'm generally in favor of tightening up types and I think the query surface API is now stable enough that the maintenance load should be acceptable. I'm concerned that TypeScript isn't sufficiently expressive to correctly represent the API, though. I couldn't quite figure out how to make the playground work for me so here's a few questions:

1. Can you support multi-flavor queries? Doing something like q.added.changed.removed.with(Foo) is recommended and more efficient than three separate queries.
2. Do you lose type safety when the number of component types in with exceeds the pre-expanded method type signatures? If so, I think it's a bit risky to provide type safety most of the time only to drop it silently in corner cases.
3. Can this approach support multiple with, without, and using clauses, some with write and some (implicitly or explicitly) just read? It looks to me like right now it applies the write modifier to all component types.
4. Can you enforce at least one track being required if there are any changed-related flavors, and prohibited otherwise?

I'm also willing to consider tweaking the query language to make strong types easier to create but don't really want to get too far from the fluent API approach.

[marionebl]

Awesome, looking forward to contributing this. I've iterated a bit further, doing away with pre-expansion (link to playground).

1. Can you support multi-flavor queries? Doing something like q.added.changed.removed.with(Foo) is recommended and more efficient than three separate queries.

Yes, the current proof of concept supports this

2. Do you lose type safety when the number of component types in with exceeds the pre-expanded method type signatures? If so, I think it's a bit risky to provide type safety most of the time only to drop it silently in corner cases.

Agreed, I've figured out how to avoid the method overrides to avoid this

3. Can this approach support multiple with, without, and using clauses, some with write and some (implicitly or explicitly) just read? It looks to me like right now it applies the write modifier to all component types.

I think so! We'd have to come up with a full set of type tests to verify this - I plan to make that test suite the first step of the contribution, we can then work on filling out the gaps as you see them.

4. Can you enforce at least one track being required if there are any changed-related flavors, and prohibited otherwise?

Not intuitively, but I'll try to come up with something. Would you want this to be part of the first iteration?

---

Another thing I haven't covered yet is Entity.prototype.read throwing when a non-existent component matched via .using is read. I'm not too sure about how to support that either - could this be a candidate for an API change, e.g. by making it return undefined in those cases with Entity.prototype.read giving a way to narrow from T | undefined to T ?

[pkaminski]

Can this approach support multiple with, without, and using clauses, some with write and some (implicitly or explicitly) just read? It looks to me like right now it applies the write modifier to all component types.

I think so! We'd have to come up with a full set of type tests to verify this - I plan to make that test suite the first step of the contribution, we can then work on filling out the gaps as you see them.

That would be most impressive!

Can you enforce at least one track being required if there are any changed-related flavors, and prohibited otherwise?

Not intuitively, but I'll try to come up with something. Would you want this to be part of the first iteration?

We'll never be able to enforce everything via types (e.g., you must drop references to a read (write) component before you request the next read (write) component of the same type), so I'm fine with a "best effort" approach. My only concern, as noted earlier, is that I'd prefer not to have "partial enforcement" of a rule -- if it's handled at compile time, there should be no way for that rule to throw a runtime exception. But I could imagine even making exceptions on this point.

Another thing I haven't covered yet is Entity.prototype.read throwing when a non-existent component matched via .using is read. I'm not too sure about how to support that either - could this be a candidate for an API change, e.g. by making it return undefined in those cases with Entity.prototype.read giving a way to narrow from T | undefined to T ?

Hmm, I'm not sure what you have in mind for narrowing the type? The current API is this way for two reasons: 1) to avoid having to suffix every component read/write with ! in TS when you know the component exists (even though it may not be required by the query), and 2) to throw a more informative exception on failure, so you don't need to guess which component was missing if the argument wasn't a class literal.

[marionebl]

Very good, I give it a go then. let's defer outstanding questions to when we have the type tests as a means of communication.

[marionebl]

First draft of how type tests could look like here https://github.com/LastOliveGames/becsy/pull/8/files, let me know what you think.

[Calamari]

Heyho. Just saw this project in the search for a less typescripty awkward/verbose alternative to ecsy. How is the state of your type-enhancing going? Looks promising so far as I read.

[pkaminski]

I'm not sure how @marionebl is doing, but note that the library is very much usable in TypeScript as-is: this is just trying to push static typing that little bit further, but runtime checks will remain necessary for a bunch of constraints that just can't be expressed in the type system. I'd encourage you to give it a try!

[marionebl]

Not making much progress, busy with other things!