Add queryFirstWithTraits

[mdirolf]

This is mostly just a proof of concept - obviously doesn't make sense unless you're satisfied with the api @krispya

[mdirolf]

I added readFirst to QueryResults just to make this easier to implement. If preferable we could ditch the queryFirstWithTraits part and just stick with readFirst only. If you think it's better could also potentially change the readFirst return type to match a bit closer to the callback params for readEach (i.e. [ [...traits], Entity] | [[...undefined], undefined]).

LMK if either of those are preferable to this and I'd be happy to change this PR to match.

[krispya]

I forgot about this, my apologies.

I can see what we are trying to avoid is this kind of pattern.

const entity = world.queryFirst(Position, Name)
if (!entity) return // What if no entity exists?
const name = entity.get(Name)!
const position = entity.get(Position)!

And while something this API looks ergonomic:

const [entity, position, name] = world.readFirst(Position, Name)

It starts to look a lot less ergonomic to me once we consider the what happens when the entity does not exist?

// We are destructuring so all of the array members are no filled with `undefined`
const [entity, position, name] = world.readFirst(Position, Name)
// And we have to check before moving forward anwyays
if (!entity) return

So it looks like what we save is all the get calls. But what if you could just batch get?

const entity = world.queryFirst(Position, Name)
if (!entity) return
const [position, name] = entity.get(Position, Name)

Now this looks equally ergonomic again. So, I am not convinced the kind of proposed API is actually helpful. It is probably have the strict boundary check be "does my entity exist?" and not "does my data exist?"

[mdirolf]

What I'm mostly trying to avoid are the non-null assertions, or the requirement to check each trait individually. I like the readFirst API even though it still requires the existence check for entity because that simultaneously guarantees existence for position and name. But I understand wanting to think carefully about your API surface so I'll leave it up to you.

[krispya]

Hmm, I don't think the readFirst API saves you from assertions because the type system still cannot know at compile time if the the data is there or not, even if the entity is checked. It does not know that the entity being defined means the data is also defined.

const [entity, position, name] = world.readFirst(Position, Name) // All types are T | undefined
if (!entity) return
position!.x // This still requires an assertion or a check above with entity

So it looks like the same ergonomics to me with less API surface if you can batch get all values at once. Am I wrong?

const entity = world.queryFirst(Position, Name)
if (!entity) return
const [position, name] = entity.get(Position, Name)

The only way to avoid assertion is actually to make another change -- allow the Entity type to keep a generic of the traits it has on it -- at least in the block scope.

[mdirolf]

The type

export type QueryFirstWithTraitsResult<T extends QueryParameter[] = QueryParameter[]> =
    | [Entity, ...InstancesFromParameters<T>]
    | [undefined, ...{ [K in keyof T]: undefined }];

Guarantees that all elements in the return tuple are either undefined or present. So checking the entity is enough for the type checker to know that the traits are all defined. There is a test to that effect in the PR as well.

[krispya]

Ah hah, I see the vision now. I did not know you could do this! I am going to reconsider.