Skip to content

Commit 429ece3

Browse files
authored
Adds Script scriptName field (#7593)
* Enhance script registration with static "scriptName" validation Added warnings for ESM Script classes lacking a static "scriptName" property. Updated the ScriptComponent and ScriptHandler to infer script names and log appropriate debug messages. Enhanced getScriptName function to return the scriptName if defined. Added tests to verify the new behavior. * Refine script registration validation and enhance test coverage Updated the ScriptComponent to ensure that a static "scriptName" property is required only for non-ScriptType instances. Modified tests to check for script existence using the `has` method and added a new test to confirm that no warnings are logged when a ScriptType is used. This improves the robustness of script registration and validation.
1 parent 6e4781d commit 429ece3

File tree

4 files changed

+73
-2
lines changed

4 files changed

+73
-2
lines changed

src/framework/components/script/component.js

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -712,7 +712,16 @@ class ScriptComponent extends Component {
712712
if (typeof scriptType === 'string') {
713713
scriptType = this.system.app.scripts.get(scriptType);
714714
} else if (scriptType) {
715-
scriptName = scriptType.__name ??= toLowerCamelCase(getScriptName(scriptType));
715+
716+
const inferredScriptName = getScriptName(scriptType);
717+
const lowerInferredScriptName = toLowerCamelCase(inferredScriptName);
718+
719+
if (!(scriptType.prototype instanceof ScriptType) && !scriptType.scriptName) {
720+
Debug.warnOnce(`The Script class "${inferredScriptName}" must have a static "scriptName" property: \`${inferredScriptName}.scriptName = "${lowerInferredScriptName}";\`. This will be an error in future versions of PlayCanvas.`);
721+
}
722+
723+
scriptType.__name ??= scriptType.scriptName ?? lowerInferredScriptName;
724+
scriptName = scriptType.__name;
716725
}
717726

718727
if (scriptType) {

src/framework/handlers/script.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { platform } from '../../core/platform.js';
2+
import { Debug } from '../../core/debug.js';
23
import { script } from '../script.js';
34
import { ScriptTypes } from '../script/script-types.js';
45
import { registerScript } from '../script/script-create.js';
@@ -137,7 +138,13 @@ class ScriptHandler extends ResourceHandler {
137138

138139
if (extendsScriptType) {
139140

140-
const scriptName = toLowerCamelCase(scriptClass.name);
141+
const lowerCamelCaseName = toLowerCamelCase(scriptClass.name);
142+
143+
if (!scriptClass.scriptName) {
144+
Debug.warnOnce(`The Script class "${scriptClass.name}" must have a static "scriptName" property: \`${scriptClass.name}.scriptName = "${lowerCamelCaseName}";\`. This will be an error in future versions of PlayCanvas.`);
145+
}
146+
147+
const scriptName = scriptClass.scriptName ?? lowerCamelCaseName;
141148

142149
// Register the script name
143150
registerScript(scriptClass, scriptName);

src/framework/script/script.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,7 @@ const funcNameRegex = /^\s*function(?:\s|\s*\/\*.*\*\/\s*)+([^(\s\/]*)\s*/;
347347
*/
348348
export function getScriptName(constructorFn) {
349349
if (typeof constructorFn !== 'function') return undefined;
350+
if (constructorFn.scriptName) return constructorFn.scriptName;
350351
if ('name' in Function.prototype) return constructorFn.name;
351352
if (constructorFn === Function || constructorFn === Function.prototype.constructor) return 'Function';
352353
const match = (`${constructorFn}`).match(funcNameRegex);

test/framework/components/script/component.test.mjs

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import { expect } from 'chai';
22

3+
import { Debug } from '../../../../src/core/debug.js';
34
import { Asset } from '../../../../src/framework/asset/asset.js';
45
import { Entity } from '../../../../src/framework/entity.js';
56
import { createScript } from '../../../../src/framework/script/script-create.js';
7+
import { Script } from '../../../../src/framework/script/script.js';
68
import { createApp } from '../../../app.mjs';
79
import { jsdomSetup, jsdomTeardown } from '../../../jsdom.mjs';
810

@@ -2925,4 +2927,56 @@ describe('ScriptComponent', function () {
29252927
expect(e.script.has(null)).to.equal(false);
29262928
});
29272929

2930+
it('warns when an ESM Script class does not have a static "scriptName" property', function () {
2931+
class TestScript extends Script {}
2932+
const a = new Entity();
2933+
a.addComponent('script', { enabled: true });
2934+
a.script.create(TestScript);
2935+
2936+
expect(Debug._loggedMessages.has(
2937+
'The Script class "TestScript" must have a static "scriptName" property: `TestScript.scriptName = "testScript";`. This will be an error in future versions of PlayCanvas.'
2938+
)).to.equal(true);
2939+
});
2940+
2941+
it('correctly registers an ESM script with its scriptName', function () {
2942+
class TestScript extends Script {
2943+
static scriptName = 'myTestScript';
2944+
}
2945+
const a = new Entity();
2946+
a.addComponent('script', { enabled: true });
2947+
a.script.create(TestScript);
2948+
2949+
expect(a.script.has('testScript')).to.equal(false);
2950+
expect(a.script.has('myTestScript')).to.equal(true);
2951+
});
2952+
2953+
it('falls back to camelCase script name if scriptName is not defined', function () {
2954+
class TestScript extends Script {}
2955+
const a = new Entity();
2956+
a.addComponent('script', { enabled: true });
2957+
a.script.create(TestScript);
2958+
2959+
expect(a.script.has('testScript')).to.equal(true);
2960+
expect(a.script.has('myTestScript')).to.equal(false);
2961+
});
2962+
2963+
it('does not warn when a ScriptType is used', function () {
2964+
Debug._loggedMessages.clear();
2965+
createScript('nullScript');
2966+
const e = new Entity();
2967+
e.addComponent('script', {
2968+
enabled: true,
2969+
order: ['nullScript'],
2970+
scripts: {
2971+
nullScript: {
2972+
enabled: true
2973+
}
2974+
}
2975+
});
2976+
app.root.addChild(e);
2977+
2978+
expect(Debug._loggedMessages.size).to.equal(0);
2979+
expect(e.script.has('nullScript')).to.equal(true);
2980+
});
2981+
29282982
});

0 commit comments

Comments
 (0)