[RFC] JSDoc Script Attributes

[marklundin]

In the current Script system we declare attributes using the static attributes property

Current Scripts

var Rotator = pc.createScript('rotator');
Rotator.attributes.add( 'speed', {
   type: 'number', 
   description: "Speed determines how fast to rotate things",
   defaultValue: 10 
})
Rotator.attributes.add( 'thingsToRotate', {
   type: 'entity', 
   description: "An array of Entities to rotate",
   array: true 
})

ScriptType class

Using class based syntax, this can be declared like this...

class Rotator extends ScriptType {
  static attributes = {
    speed: { 
        type: 'number', 
        description: "Speed determines how fast to rotate things",
        defaultValue: 10 
    },
    thingsToRotate: { 
        type: 'entity', 
        array: true 
    }
  }
}

Issues

• Incompatible with Intellisense, property members would have to be defined seperateley from the attributes tag in order for this to work
• There's some additional redundancy in declaring defaultValues and types both in attributes
• Many of the attribute properties min, max, precision, step are just for the editor to provide additional information around presenting UI. They are not required byt the class itself

Proposed inline attributes

This RFC proposes an alternative way of declaring attributes using JSDoc style comments

class Rotator extends ScriptType {
  /**
   * @attribute
   * Speed determines how fast to rotate things"
   */
   speed = new Vec3()

   /**
    * @attribute
    *  An array of Entities to rotate
    * 
    * @type {Entity[]}
    */
   thingsToRotate
}

There are a few things to note here:

• attributes are defined with the @attribute modifier tag
• An attributes type is inferred if it's initialized or via it's @type tag
• Description is take from the comment itself
• Other tags provide additional metadata such as @range, @title etc. A full example can be found here

Feedback

Any thoughts and feedback on this would be massively appreciated 🙏

[yaustar]

How would it work with things like default values, min/max and assetTypes? Can it also handle the JSON attribute?

There's also some requests with the existing system where users want to share common data between scripts like an enum listing of colours or a particular JSON format for JSON attributes. Would that work with the proposal?

[marklundin]

How would it work with things like default values, min/max and assetTypes? Can it also handle the JSON attribute?

Somethign along the lines of this

/** 
  * @attribute
  * There are additional tags an attribute can have to provide more context to the editor.
  * 
  * Range, step, precision specifies numerical constraints
  * @title The title tag is used as short label
  * @range [0.1, 10]
  * @precision 0.01
  * @step 0.005
  * @placeholder This is some placeholder text, used to populate input fields
  */
  velocity = 2

https://gist.github.com/marklundin/99ee9174bdb8fc37723df15e7d9ab826#file-my-class-js-L71-L76

There's also some requests with the existing system where users want to share common data between scripts like an enum listing of colours or a particular JSON format for JSON attributes. Would that work with the proposal?

You would be able to import enums and other types from other files in the asset registry

[AlexAPPi]

Hi what about TypeScript support ?

[marklundin]

Hi what about TypeScript support ?

yep, this will be compatible with TS.

[marklundin]

Any thoughts on this @kungfooman @Maksims

[kpal81xd]

@marklundin does this include the nested seralizable objects?

[marklundin]

@marklundin does this include the nested seralizable objects?

It does, but this isn't included in the example. Will update

[Maksims]

It does sound pretty nice, with a few caveats (good to mention in the first post as well), as drawbacks vs existing or other potential solutions - are as important as benefits when defining new APIs.

1. How a JSON type is defined?
2. What about events when attributes are changed?
3. What about the mutability of data to be consistent with other components?
4. Another drawback - it is much more verbose and does not guide you in defining it as autocomplete does currently. Unless this can be improved with IDE.
5. Some confusion with @serializable. enemy = new EnemyType() - from first glance would behave like so: you can select only entities with such with script EnemyType on it, and it would reference actual script, not an entity. Although this looks weird, as it instantiates a new instance of it, which is somewhat weird. I would expect it to be similar to what assets do: 'type: 'script', scriptType: EnemyType'.
6. How do asset attributes are defined with this style?
7. With enums, do you need to define an enum somewhere else?

[marklundin]

Thanks @Maksims, good to have your input.

1. How a JSON type is defined?

The JSON type becomes a complex type, this could be defined through the jsdoc @type {{ myValue: number, myString: string }} or via initialization ie myAttrbute = {myValue: 10, myString: 'mystring'}

2. What about events when attributes are changed?

Yep, events on attributes become opt-in and up to the user. This makes it clear and transparent to the user and solves the getter/setters issue. Additionally non-attributes may want events, so this separates the two.

3. What about the mutability of data to be consistent with other components?

Not sure I follow. Could you give an example?

4. Another drawback - it is much more verbose and does not guide you in defining it as autocomplete does currently. Unless this can be improved with IDE.

Yeah we'd definitely want to improve the experience in Monaco to add highlighting, rollover annotations etc. A key feature here is that we would win regular attribute intellisense/autocomplete

5. Some confusion with @serializable. enemy = new EnemyType() - from first glance would behave like so: you can select only entities with such with script EnemyType on it, and it would reference actual script, not an entity. Although this looks weird, as it instantiates a new instance of it, which is somewhat weird. I would expect it to be similar to what assets do: 'type: 'script', scriptType: EnemyType'.

This is simply to support json type uses cases, so a enemy attribute has the json properties defined in the EnemyType. Here, the @serializable tag opts in for the type to be used, however this may be unnecessary, and we could avoid the serializable tag altogether.

6. How do asset attributes are defined with this style?

Do you mean like assetType? These would be additional tags like @assetType {Texture}

7. With enums, do you need to define an enum somewhere else?

Yes enums would need to be defined elsewhere.

[LeXXik]

A side question, how will these attributes look in typedoc? Will they be among other properties/accessors?

[marklundin]

A side question, how will these attributes look in typedoc? Will they be among other properties/accessors?

Tags like type and enum are already so would be handled as-is. It looks like typedoc supports custom tags but not sure what control you have over generating output

[slimbuck]

To also respond to some of @Maksims questions:

2. What about events when attributes are changed?

Events become your responsibility to implement as and when you need them. Less custom magic under the hood - you work with plain old esm/es6 (which everyone knows and understands, hopefully).

4. Another drawback - it is much more verbose and does not guide you in defining it as autocomplete does currently. Unless this can be improved with IDE.

I don't imagine users struggling in the general case adding plain es6 members (though we might want to think about the custom jsdoc tags). In return for this change though, the script attributes themselves are included in auto completion and type checking - quite a big win.

[yaustar]

There's also some requests with the existing system where users want to share common data between scripts like an enum listing of colours or a particular JSON format for JSON attributes. Would that work with the proposal?

Coming back to this question :)

[LeXXik]

Coming back to this question :)

Hmm, I maybe missing something, but since this is a module, we can now just do

// settings.mjs
const vehicleOptions = { isDynamic: true };
export { vehicleOptions };

// script.mjs
import { vehicleOptions } from './settings.mjs';

Unless you mean something else.

[Maksims]

As a good exercise when designing APIs, please show code implementation of these examples:

Complex JSON

var Example = pc.createScript('example');

Example.attributes.add('chunks', {
    type: 'json',
    array: true,
    schema: [{
        name: 'name',
        type: 'string'
    }, {
        name: 'shader',
        type: 'asset',
        assetType: 'shader'
    }]
});

Example.prototype.initialize = function() {
    for(let i = 0; i < this.chunks.length; i++) {
        const chunk = this.chunks[i];
        this.material.chunks[chunk.name] = chunk.shader.resource;
    }
};

Enum and Attribute Events

var Example = pc.createScript('example');

Example.attributes.add('dimensions', {
    type: 'number',
    default: 1024,
    enum: [
        { '1k': 1024 },
        { '2k': 2048 },
        { '4k': 4096 }
    ]
});

Example.prototype.initialize = function() {
    this.on('attr:dimensions', () => {
        // regenerate texture with new dimensions
    });
};

@slimbuck how does an attribute event work with new attributes, please show an example.

[slimbuck]

@slimbuck how does an attribute event work with new attributes, please show an example.

Since there will be no built-in events user must add them if they need them.

Users can go as complex or as simple as required, but we imagine most devs will probably just implement an attribute setter and manually evoke an event, something like:

class A : extends EventHandler {
    _myVar = 1;

    set myVar(newValue) {
        this._myVar = newValue;
        this.fire('myVar', newValue);
    }

    get myVar() {
        return this._myVar;
    }
};

If this is ultimately too cumbersome, we can also consider adding helpers for creating standard getters and setters like the script system did before (but then you'll be left without auto completion and type checking). In any case, the user has control.

[Maksims]

@slimbuck code above does not have @attribute, where exactly you would add it?

[marklundin]

There's also some requests with the existing system where users want to share common data between scripts like an enum listing of colours or a particular JSON format for JSON attributes. Would that work with the proposal?

Coming back to this question :)

Yep this would work, enums and types can be exported and imported across files

[marklundin]

@Maksims Coming back to you earlier point;

var Example = pc.createScript('example');

Example.attributes.add('chunks', {
    type: 'json',
    array: true,
    schema: [{
        name: 'name',
        type: 'string'
    }, {
        name: 'shader',
        type: 'asset',
        assetType: 'shader'
    }]
});

can be represented as the following

/** @interface */
class Chunk {
  name = ''

  /** 
   * @type {Asset}
   * @assetType {Shader}
   */
   shader
}

export class Example {
  /**
   * @attribute
   * @type {Chunk[]}
   */ 
   chunks
}

The @interface tag is provides a concise way to define the shape of an object which is already supports standard across editor. Meaning it supports Intellisense out of the box this.chunks[0].name.... Keeping this as a type definition make it also compatible with using inline type declarations ie /** @type {{ name: string, asset: Asset}} */ where a user wants to declare complex types quickly. This also works with @typedef tags too.

[slimbuck]

@slimbuck code above does not have @attribute, where exactly you would add it?

Sorry @Maksims I forgot the @attribute tag on the setter.

[Maksims]

Looks like there is almost full feature parity between old-way and JSDoc-way. The only drawback - is how verbose and made of many concepts the new definition is. So it will be somewhat harder to learn and remember. As well as write.

Will it still be possible to use the old-way? As it will make code migration much easier and some still might prefer it.

[kungfooman]

I would like the JSDoc parsing using Babel and/or TS (for type compatibility with IntelliSense) to be open-source, but otherwise I like the JSDoc way!

/**
 * @type {Asset}
 * @assetType {Shader}
 */

For such simple types I would opt for one-liners:

/** @type {Asset<Shader>} */

• 4 lines vs 1 line
• not introducing too many custom JSDoc docs and it's the standard-way for typing container classes
• adding proper template types enriches IntelliSense for engine developers aswell

[Maksims]

For such simple types I would opt for one-liners:

assetType - is not a type, but a string enum, with these options:

"animation" | "container" | "font" | "audio" | "html" | "script" | "template" | "text" | "json" | "model" | "sprite" | "texture" | "textureatlas" | "css" | "cubemap" | "material" | "shader" | "binary" | "render"

[kungfooman]

@Maksims Good point and then it would be a string here too:

/**
 * @type {Asset}
 * @assetType {'shader'}
 */

My point is to have better typing overall aswell: TS Playground

Right now everything is just Asset... asset of what?

A little bit of typing and we could know exactly based on type inferring:

And auto-complete works aswell:

Considering type being a string, it is of course:

/** @type {Asset<'font'>} */

And people can write that using IntelliSense aswell:

IMO too good of a chance to miss out on here while we are already refactoring.

[Maksims]

Looking at JSDoc, these brackets: <> are used either for iterators (set, map) or promises. And the type inside of brackets - is an actual type of a thing. In PlayCanvas, Asset - is a type. And it has a type defined as a string, which is not a JS type, but just an enum property. The resource property of an Asset - is null by default, and will be defined as asset being loaded. And it does not correspond to property directly.

So the @assetType keyword, is purely instructional filter for Editor, so it can filter assets when picking using attribute. It always will be Asset, with whatever resource inside.

So it will be not within JSDoc style to do so.

[kungfooman]

Looking at JSDoc, these brackets: <> are used either for iterators (set, map) or promises.

I believe your argument is based on lack of type knowledge when it comes to template types or some confusion around it. Lets make things clear, a Map is not even an iterator either. It is a container which can be iterated.

Simple example:

const map = new Map([['a', 1], ['b', 2]]);
const mapIterator = map[Symbol.iterator]();
mapIterator;

There is your iterator. Since Map is a container, you basically just need to make the bridge to seeing that Asset is also a container, hence requiring a template type, just like a Map. A Map is a templated type already and Asset is not - it's a type flaw for years.

And the type inside of brackets - is an actual type of a thing.

The "actual type of a thing" is exactly what I used - a literal string type. That's as good as a type as any other type.

Example of two instrinsic string templates and the stock Record type using string arguments: TS Playground

/** @type {Uppercase<'hai'>|null} */
const test1 = 'HAI';

/** @type {Lowercase<'HAI'>|null} */
const test2 = null;

/** @type {Record<'someKey', 'someValue'>} */
const test3 = {someKey: 'someValue'};

[LeXXik]

Some guys write the code locally, using TS. After which the code gets compiled, minified and gets uploaded. The compiler will remove all those comments in that case.

[Maksims]

Asset.type - is not a js type of Asset.resource, nor it is 1:1 match even (text, css, html, types, etc). Nor it has resource available necessarily. We can dance around fancy coding styles and notations, but end of the day, it should be:

1. Easy to learn.
2. Easy to remember.
3. Easy to read.
4. Easy to write.

If some notation requires some niche knowledge of templating in JSDoc notations, or some other languages - this will keep users from using it.

Please, keep things simple.

[LeXXik]

Not sure, but the Asset<Shader> seems pretty normal to me. Isn't it the same as this?

https://github.com/playcanvas/engine/blob/f32ed0ad50f6fcad90b301211635e2e74087c28e/src/scene/shader-pass.js#L90

[kungfooman]

If some notation requires some niche knowledge of templating in JSDoc notations, or some other languages - this will keep users from using it.

Niche knowledge about a PlayCanvas-specific JSDoc tag spanning 4 lines is better, easier to read/write/learn than a one-liner using standard container typing?