Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scripts 2.1 (Class support) #5910

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions examples/src/examples/misc/index.mjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export * from "./hello-world.mjs";
export * from "./mini-stats.mjs";
export * from "./spineboy.mjs";
export * from "./scripts.mjs";
export * from "./gizmos.mjs";
export * from "./multi-app.mjs";
106 changes: 106 additions & 0 deletions examples/src/examples/misc/scripts.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
import * as pc from 'playcanvas';

/**
* @param {import('../../options.mjs').ExampleOptions} options - The example options.
* @returns {Promise<pc.AppBase>} The example application.
*/
async function example({ canvas, deviceType, glslangPath, twgslPath }) {
// define script component
class Rotator extends pc.ScriptType {
// name of a script
static name = 'rotator';

// property
speed = 42;

// this method is executed once the object and script becomes
// available for the execution (enabled)
initialize() {
this.entity.rotate(Math.random() * 360, Math.random() * 360, Math.random() * 360);
}

// this method is executed on every frame
update(dt) {
this.entity.rotate(dt * this.speed, 0, 0);
}
}

const gfxOptions = {
deviceTypes: [deviceType],
glslangUrl: glslangPath + 'glslang.js',
twgslUrl: twgslPath + 'twgsl.js'
};

const device = await pc.createGraphicsDevice(canvas, gfxOptions);
const createOptions = new pc.AppOptions();
createOptions.graphicsDevice = device;

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

const app = new pc.AppBase(canvas);
app.init(createOptions);
app.start();

// add script to script registry
app.scripts.add(Rotator);

// Set the canvas to fill the window and automatically change resolution to be the same as the canvas size
app.setCanvasFillMode(pc.FILLMODE_FILL_WINDOW);
app.setCanvasResolution(pc.RESOLUTION_AUTO);

// Ensure canvas is resized when window changes size
const resize = () => app.resizeCanvas();
window.addEventListener('resize', resize);
app.on('destroy', () => {
window.removeEventListener('resize', resize);
});

// create box entity
const box = new pc.Entity('cube');
box.addComponent('render', {
type: 'box'
});
// add script component
box.addComponent('script');
// add rotator script to the entity
box.script?.create('rotator');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use ?. here? Isn't the script component guaranteed to be present?


// clone box multiple times
// arranged in a circle
for(let i = 0; i < 16; i++) {
const entity = box.clone();
const x = Math.sin(Math.PI * 2 * (i / 16));
const y = Math.cos(Math.PI * 2 * (i / 16));
entity.setLocalPosition(x * 2, y * 2, 0);
app.root.addChild(entity);
}

// create camera entity
const camera = new pc.Entity('camera');
camera.addComponent('camera', {
clearColor: new pc.Color(0.5, 0.6, 0.9)
});
app.root.addChild(camera);
camera.setPosition(0, 0, 10);

// create directional light entity
const light = new pc.Entity('light');
light.addComponent('light');
app.root.addChild(light);
light.setEulerAngles(45, 0, 0);

return app;
}

class ScriptsExample {
static CATEGORY = 'Misc';
static WEBGPU_ENABLED = true;
static example = example;
}

export { ScriptsExample };
2 changes: 2 additions & 0 deletions src/framework/components/script/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,8 @@ class ScriptComponent extends Component {
if (typeof scriptType === 'string') {
scriptType = this.system.app.scripts.get(scriptType);
} else if (scriptType) {
if (scriptType.__name === null)
scriptType.__name = scriptType.name;
Copy link
Member

@marklundin marklundin Jan 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing; the Class.name can be mangled in bundling and shouldn't be relied upon to be stable.

Mozilla: Minifiers and bundling name prop

Ideally we don't rely on name. Can __name be a required member instead of relying on the inferred name? Does this have any implication for existing scripts?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minifier will be impactful when static name is not set on a class.
Basically there are multiple ways to specify a name for a script:

Current:

let Test = pc.createScript('test');

Using a static name property:

class Test extends pc.ScriptType {
    static name = 'test';
}

Using pc.registerScript:

class Test extends pc.ScriptType {
    
}
pc.registerScript(Test, 'test');

And this also works even right now, but not pretty:

class Test extends pc.ScriptType {
    static __name = 'test';
}

We could define a different name for the static property to avoid collision with built-in property name on the prototype. E.g. scriptName.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can check if the name property was explicitly defined as a static member rather than inferred using getOwnPropertyDescriptor, which could be useful, but would need to verify. I wonder if for ES Modules, which are more likely to be minified/mangled, that we require/enforce that a name or similar is specifically defined 🤔

scriptName = scriptType.__name;
}

Expand Down
3 changes: 3 additions & 0 deletions src/framework/script/script-registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ class ScriptRegistry extends EventHandler {
* console.log(app.scripts.has('playerController')); // outputs true
*/
add(script) {
if (script.__name === null)
script.__name = script.name;

const scriptName = script.__name;

if (this._scripts.hasOwnProperty(scriptName)) {
Expand Down
2 changes: 1 addition & 1 deletion src/framework/script/script-type.js
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ class ScriptType extends EventHandler {
} else if (!this.__attributes.hasOwnProperty(key)) {
if (this.__scriptType.attributes.index[key].hasOwnProperty('default')) {
this[key] = this.__scriptType.attributes.index[key].default;
} else {
} else if (!this.hasOwnProperty(key)) {
this[key] = null;
}
}
Expand Down