Skip to content

Conversation

@aidinabedi
Copy link
Contributor

Description

This PR does the following fixes to applySceneSettings's settings parameter:

  • Fixes type for settings.render.lightingCells from Vec3 to number[]
  • Fixes optional properties and documents the defaults
  • Adds missing properties with description and defaults

Checklist

  • I have read the contributing guidelines
  • My code follows the project's coding standards
  • This PR focuses on a single change

@aidinabedi aidinabedi changed the title Fix types and documentation for applySceneSettings [DOCS] Fix types and documentation for applySceneSettings Oct 30, 2025
@willeastcott willeastcott requested a review from Copilot November 4, 2025 10:36
@willeastcott willeastcott added the docs Documentation related label Nov 4, 2025
@willeastcott
Copy link
Contributor

Minor point, but the engine codebase keeps JSDoc blocks to 100 columns and most of these additions exceed that. Would be great if you could address that in this PR please. 🙏

Copy link
Contributor

Copilot AI left a 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 updates JSDoc documentation for the settings.render parameters to clarify their optional nature and provide default values. The changes enhance API documentation by making it explicit which parameters are optional and what their default values are.

  • Added optional markers [] to previously required-looking parameters
  • Added default value information to parameter descriptions
  • Added documentation for new sky mesh and lightmap filter parameters

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@mvaligursky mvaligursky merged commit 49bab3e into playcanvas:main Nov 4, 2025
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Documentation related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants