Tighten up types for Entity API

[willeastcott]

At the moment, the Entity API has reasonably weak typings. This PR improves the situation, particularly for addComponent.

I confirm I have read the contributing guidelines and signed the Contributor License Agreement.

[Copilot]

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

[hmans]

Hi! As promised, a couple of suggestions. I think this PR is already great in how it establishes a list of supported internal components, mapping them to their data object types, and requiring addComponent to accept one of the official component names:

The big drawback of the PR in its current state is that custom components will always yield a type error (the error around "bad" here was due to me using double quotes, but note how the component name is yielding an error, too):

I think generally with an API like this we want to achieve three things:

1. auto-suggest and -complete known component names (the first argument)
2. auto-suggest etc. and validate the shape of the second argument, if known
3. but also allow custom component names and their data objects

I think one way forward into this direction would be to introduce several type overloads. I've prototyped this on my machine, but instead of opening a PR-into-the-PR or similar, I'm just going to post some code here, if that's alright.

Also, please note that I've never used jsdoc types before (I'm very much a TS person), so chances are this can be cleaned up or otherwise optimized further.

Basically, the idea is to provide two overloads for addEntity:

• one accepts a known internal entity name and its associated data object
• the other accepts just any random string together with a data object typed unknown.

This allows both cases to be supported:

This also successfully gives rich help for all involved properties:

There is one big caveat though -- and I haven't been able to solve it, but maybe someone else has an idea: when you use an internal component but mis-type a property name, the tooling will immediately switch to the "anonymous" overload, instead of marking the unknown property as an error:

I think even with this caveat this is already pretty nice, and if there is no way to solve it, then it might be an acceptable trade-off (compared to just out-right erroring all custom components.)

Implementation

Basically, I just added these two overloads to the codedoc of addComponent:

     *
     * @overload
     * @param {T} type
     * @param {Partial<ComponentMap[T]>} [data]
     *
     * @overload
     * @param {string} type
     * @param {unknown} [data]

Some open ends:

• I'm not familiar with jsdoc types at all, so maybe something can be expressed in a leaner fashion
• Probably need to check if documentation still renders correctly in tooling (what I've seen so far looked OK)
• removeComponent and maybe others probably need a similar treatment

I hope this helps a little. Either way, looking forward to the improved typings!

[willeastcott]

Wow @hmans - we make a great team. 😄 Thanks for your help! Check out the updated PR. Seems to work great.

entity-typings.mp4

Here are the docs:

What do you think?

[LeXXik]

This is pretty close to my land. I do have custom physics components that get introduced once you import our Jolt physics library:

github.com/gamebop/physics

It will create multiple new components, like 'body'. I just had to suck up the errors, since, I concured I am in minority of users who introduce new components to the system. However, I am aware of the strides towards supporting custom addons/components and was holding off till closed beta is out.

My library will definitely benefit from this PR. Good motion!

Also worth mentioning that a factory component could add Debug asserts for supplied properties. We do it to spot any unrecognized properties, mostly due to typos. A component may throw a warning if a property is not recognized, or values are out of acceptable range.

[willeastcott]

@LeXXik Wow, that physics library looks really cool! Nice job!

@hmans I do have one concern about something. Before adding support for custom components, if you tried to add an invalid property to the options object, IntelliSense would show a red line under that property and log an error in the PROBLEMS panel. Now, if you do that, IntelliSense thinks it's just a custom component. Hmmm. I'm wondering if we can say a custom component is a string but NOT any of the keyof ComponentMap values.

[hmans]

Before adding support for custom components, if you tried to add an invalid property to the options object, IntelliSense would show a red line under that property and log an error in the PROBLEMS panel. Now, if you do that, IntelliSense thinks it's just a custom component.

Yeah, this is the one caveat I was writing about in my post. I have some more ideas for stuff I want to try to improve this. I won't be able to get to it until later tonight, but maybe we can figure something out.

My main issue that's slowing me down right now is that I'm having a hard time figuring out a speedy development/testing flow for this. I'm diving into this codebase for the first time, and it's also the first time I am working with jsdoc types (as opposed to plain old TS, which I'm very familiar with.)

So far, the only way I've found to get my changes to entity.js to register was to npm run build the entire engine and then use the build bundle in a new example within the examples package. Is there a more direct way to approach this?

Right now I'm trying the changes from within the test scripts in the engine project, but there these changes don't seem to register at all. - this works now. A restart of VS Code seems to have fixed it. Technology! So... stay tuned. =)

[kungfooman]

Technically we should have everything that's needed from this point:

createOptions.componentSystems = [
    pc.RenderComponentSystem,
    pc.CameraComponentSystem,
    pc.LightComponentSystem,
    pc.ScriptComponentSystem,
    pc.AnimComponentSystem
];

Then just do some type voodoo. Minimal example with a new component:

class OceanComponent {}
class OceanComponentData {}
class OceanComponentSystem extends ComponentSystem {
    /** @type {'ocean'} */
    id = 'ocean';

    ComponentType = OceanComponent;

    DataType = OceanComponentData;
}

const systems = [OceanComponentSystem];
console.log("systems", systems);

/**
 * @typedef {{
 *   [S in (typeof systems)[number] as InstanceType<S>['id']]: InstanceType<InstanceType<S>['ComponentType']>
 * }} ComponentMap
 */

So we get the actual types we are using:

Instead of simply registering every PC system. Why would I even want types for e.g. ModelComponentSystem when I specifically only added RenderComponentSystem? Funnily enough the @typedef for this ComponentMap is way shorter than the first one based on the pc.ComponentSystemRegistry class.