Create a ESM Script with attributes

[marklundin]

Scripts components have a create() method for instantiating scripts with attributes.

scripts.create('scriptName', { attributes: { myNum: 1 })`

This depends on an attribute schema existing. ESM Scripts use JSDoc tags for their schema which is unavailable in engine only environments, meaning attributes cannot be set on a script

For ESM scripts in engine only, properties must be set after a script is instantiated:

const script = scripts.create('scriptName')`
script.myNum = 1

Ideally properties can also be set in-line on a script instance, at the same time as it's created.

Possible solutions

1. Re-use attribute property

In the editor, a schema will exist, so it will be used to create the attributes, when it doesn't, the values are simply assigned as properties onto the script instance

scripts.create('scriptName', { attributes: { myNum: 1 }})

• Pros Simple. Re-uses existing api.

  • Cons Inconsistent behaviour between editor/engine. - In editor, this will validate and deep copy values. In engine, it will not. semantics Attributes don't inherently exist in engine only, so this is overloading the term.

2. Separate method for engine only

Create a new method eg scripts.createWithProperties('scriptName', { myNum: 1 }) (TBD) that assigns values when instantiated

scripts.createWithProperties('scriptName', { myNum: 1 })

• Pros Clear and separate behaviour. Does not overload the attributes property

  • Cons Creates an additional method which could lead to confusion. create() still exists and would work for instantiating a script with no props/attributes

3. Create an additional property

Attributes are applied first if they and schema exist, and properties are then assigned after.

scripts.create('scriptName', { properties: { myNum: 1 }})

• Pros No additional method. Clear distinction between behaviour of attributes and properties (attributes require schema, props do not)

• Cons Adds additional property. ??

Any thoughts @mvaligursky @willeastcott @Maksims @slimbuck

[mvaligursky]

How about options 4:

• use the function you proposed in option 1 for the engine users:

  scripts.create('scriptName', { attributes: { myNum: 1 }})

• this way we preserve API compatibility with scripts 2.0

• this could just assign the attributes to properties in the class, but ideally it would do a deep copy. It should be reasonably easy to have a generic deep copy function which handles numbers, strings, classes, objects, arrays and similar.

• create a new function for use by the Editor where the schema is used, for example

  scripts.createWithSchema('scriptName', ...);

• this is never used by the engine users, and Editor users never call this manually, it's just called behind the scenes for them

• so this function would not be exposed as a public engine API

• this would use the schema in the editor

[marklundin]

With scripts 2.0, create('script', {attributes}) will set those attributes according to the schema (resolve entities, map arrays to Vecs/Color etc) and not assign those without a schema. For ESM this would behave differently depending on whether a schema was present or not. This is confusing and could cause issues if you were instantiating a script from within a script (I believe @Maksims mentioned he does this?)

I think that attributes are separate from properties and should only be set if a schema exists

scripts.create('scriptName', { attributes: { myNum: 1 }}) // Warns if no schema exists

scripts.create('scriptName', { properties: { myNum: 1 }}) // Always assigns/copies regardless of attributes and/or schema

[mvaligursky]

Hey that makes sense.

So how about the engine users have access to scripts.create('scriptName', { properties: { myNum: 1 }})

and the Editor uses internal API scripts.createWithAttributes('scriptName', { attributes: { myNum: 1 }})

when the engine users pass attributes to .create function, it should error out. This would help them port code from 2.0 to ESM.

Regarding deep copy. That would still be great for Vec3 and similar types, but not for Entities and similar .. not sure what would be the best here.

[marklundin]

So how about the engine users have access to scripts.create('scriptName', { properties: { myNum: 1 }})

and the Editor uses internal API scripts.createWithAttributes('scriptName', { attributes: { myNum: 1 }})

This makes sense, but is't it more straightforward to have a single create('X', { attributes, properties }) with values both optional. If attributes exists without a schema it errors. If props exists then copy/assign.

Regarding deep copy, I feel we should provide options here as I can see many different valid use cases. Maybe you can optionally provide a function at the end, which defaults to Object.assign

function create(scriptName, { attributes, properties }, operation = Object.assign){}

And if you wanted to deep copy you just override

create("scriptName", { properties }, deepCopy)

At least then it's clear what's happening

[mvaligursky]

Single function is fine too I guess .. as long as when we don't have a schema, we Debug.warn or similar when the attributes are used, instead of just quietly ignoring them.

I don't like the extra parameter that handles cloning. That's equivalent of not cloning internally and just asking user to clone. They can do it on their side completely in that case. We either clone by default, or let them clone. Helpers are not needed?

[marklundin]

Single function is fine too I guess .. as long as when we don't have a schema, we Debug.warn or similar when the attributes are used, instead of just quietly ignoring them.

Yep agreed.

I don't like the extra parameter that handles cloning. That's equivalent of not cloning internally and just asking user to clone. They can do it on their side completely in that case. We either clone by default, or let them clone. Helpers are not needed?

Yep, but this would happen before initialize() so you can ensure things are setup. The rationale behind providing options is that users might want to shallow clone, or use something else, so it's it's better to give options whilst defaulting to the cheapest option?

So create becomes

create(name, { properties }, operation = Object.assign) {
   script = new Script(name)
   operation(script, properties)
   script.initialize()
}

[mvaligursky]

The user can clone properties any way it wants to before passing it to .create function. Not sure what the advantage is in cloning them for them within the function using a callback they provide?

create(name, operation( { properties} ));

[marklundin]

Yep, that's fair, if we don't clone internally and just make it clear that the properties are assigned

[Maksims]

The important aspect of this is how engine-only users can easily save/load attribute data from the entity's script component. Here is an example of ordinary script attributes on entity:

{
    "attrs": [
        {
            "name": "vertex_position",
            "semantic": "POSITION"
        },
        {
            "name": "vertex_normal",
            "semantic": "NORMAL"
        },
        {
            "name": "vertex_uv0",
            "semantic": "TEXCOORD0"
        },
        {
            "name": "vertex_bone_index",
            "semantic": "BLENDINDICES"
        }
    ],
    "vertex": 177287037,
    "fragment": 177287044,
    "textures": [
        {
            "name": "texture_albedo",
            "asset": 182217219
        }
    ],
    "depthTest": true,
    "depthWrite": true
}

And script 2.0 has this attribute definition:

CustomMaterial.attributes.add('vertex', { type: 'asset', assetType: 'shader' });
CustomMaterial.attributes.add('fragment', { type: 'asset', assetType: 'shader' });

CustomMaterial.attributes.add('depthTest', { type: 'boolean', default: true });
CustomMaterial.attributes.add('depthWrite', { type: 'boolean', default: true });

CustomMaterial.attributes.add('attrs', {
    type: 'json',
    array: true,
    schema: [{
        name: 'name',
        type: 'string'
    }, {
        name: 'semantic',
        type: 'string',
        enum: [
            { 'position': pc.SEMANTIC_POSITION },
            { 'normal': pc.SEMANTIC_NORMAL },
            { 'color': pc.SEMANTIC_COLOR },
            { 'uv0': pc.SEMANTIC_TEXCOORD0 },
            { 'blend-indices': pc.SEMANTIC_BLENDINDICES },
            { 'blend-weight': pc.SEMANTIC_BLENDWEIGHT },
            { '12': pc.SEMANTIC_ATTR12 },
            { '13': pc.SEMANTIC_ATTR13 },
            { '14': pc.SEMANTIC_ATTR14 },
            { '15': pc.SEMANTIC_ATTR15 }
        ]
    }]
});

CustomMaterial.attributes.add('textures', {
    type: 'json',
    array: true,
    schema: [{
        name: 'name',
        type: 'string'
    }, {
        name: 'asset',
        type: 'asset',
        assetType: 'texture'
    }]
});

Will the user be able to simply add script to entity and provide JSON as an attribute argument from above with a new system, and expect attributes to be in correct (by the schema) types?

[marklundin]

I'm oversimplifying here, but ignoring events + asset/entity resolution etc, the attribute system...

1. Exposes class members to the editor
2. Validates type

In an Engine only scenario, 1 is unnecessary. For 2, we should really lean on early in-editor type validation, so that type errors can be flagged before running. One issue with scripts.create(Class, { data }) is that the editor can't validate the data against the Class types. You lose any type information, whereas with something like scripts.add(new Class(data)) any type errors get flagged ahead-of-time in editor.

I'd need to check what the implications of this would be, but it feels like a valid option