-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add static scriptName property to multiple script classes #7633
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a static scriptName property to multiple script classes and updates corresponding Jsdoc examples to comply with the new specification.
- Adds static scriptName properties to various ESM script classes in both production and documentation examples.
- Ensures consistency in how scripts are identified across the codebase.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/framework/script/script.js | Updated Jsdoc examples with static scriptName properties; multiple examples for different events. |
scripts/esm/xr-navigation.mjs | Added static scriptName property for the XR navigation script. |
scripts/esm/xr-controllers.mjs | Added static scriptName property for the XR controllers script. |
scripts/esm/shadow-catcher.mjs | Added static scriptName property for the shadow catcher script. |
scripts/esm/grid.mjs | Added static scriptName property for the grid script. |
scripts/esm/first-person-controller.mjs | Added static scriptName property for the first person controller script. |
scripts/esm/camera-frame.mjs | Added static scriptName property for the camera frame script. |
scripts/esm/camera-controls.mjs | Added static scriptName property for the camera controls script. |
examples/assets/scripts/misc/rotator.mjs | Added static scriptName property for the rotator script example. |
Comments suppressed due to low confidence (2)
src/framework/script/script.js:53
- [nitpick] Multiple Jsdoc examples in this file use the same 'player-controller' value for scriptName; consider using varied names in examples to better illustrate different scenarios and reduce potential confusion.
static scriptName = 'player-controller';
src/framework/script/script.js:31
- [nitpick] Ensure that the scriptName values in documentation examples clearly reflect the intended use cases of each script for better clarity in learning and usage.
static scriptName = 'entity-rotator';
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you maybe modify one of the examples to show how to create the script using that given name.
For script names, camelCase is used in multiple places, would be great to be consistent here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a new static property, scriptName, to multiple script classes and updates associated JSDoc examples to reflect this change.
- Updated JSDoc examples in src/framework/script/script.js with static scriptName entries
- Added static scriptName properties for various scripts in scripts/esm (xr-navigation, xr-controllers, shadow-catcher, grid, first-person-controller, camera-frame, and camera-controls)
- Added a static scriptName for a sample script in examples/assets/scripts/misc/rotator.mjs
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/framework/script/script.js | Updated JSDoc examples with static scriptName properties (e.g., 'entityRotator', 'playerController') |
scripts/esm/xr-navigation.mjs | Added static scriptName ('xrNavigation') to support XR navigation |
scripts/esm/xr-controllers.mjs | Added static scriptName ('xrControllers') for XR controllers |
scripts/esm/shadow-catcher.mjs | Added static scriptName ('shadowCatcher') for the shadow catcher script |
scripts/esm/grid.mjs | Introduced static scriptName ('grid') for the grid script |
scripts/esm/first-person-controller.mjs | Added static scriptName ('firstPersonController') for the first-person controller script |
scripts/esm/camera-frame.mjs | Inserted static scriptName ('cameraFrame') for the camera frame script |
scripts/esm/camera-controls.mjs | Added static scriptName ('cameraControls') for the camera controls script |
examples/assets/scripts/misc/rotator.mjs | Added static scriptName ('rotator') for the sample rotator script |
Comments suppressed due to low confidence (1)
src/framework/script/script.js:31
- [nitpick] In the JSDoc examples, the added static scriptName values use different identifiers (for instance, 'entityRotator' and several instances of 'playerController'). Consider clarifying these examples either by using distinct names for each example or by adding a comment that explains these are separate sample cases.
static scriptName = 'entityRotator';
Updates Scripts in
scripts/esm/
to use includescriptName
field as per #7593. Also updates Jsdoc examples.I confirm I have read the contributing guidelines and signed the Contributor License Agreement.