-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
LitOptions class to hold backend options #4900
Conversation
…hader. Also validates that options changed directly through the onUpdateShader so users get some feedback if they set a value moved to the backend options. Changed the name of ambientTint in the backend to be more explanatory, which coincidentally helps with the validation in this PR as well.
The docs for these should probably move to appropriate classes too engine/src/scene/materials/standard-material.js Lines 495 to 564 in 636571e
|
And also, if the option has moved to litOptions, we should generate some deprecated properties that warn user the option has moved. Those would ideally be added to deprecated.js |
Replacing the options with a class allows for hooking up setters and getters for the parameters that have been moved to the lit options. This allows for a more generic solution for validating option properties having been moved.
I added an options class which is used to hookup deprecation callbacks, and replaced the use of an anonymous options object with this new (albeit empty) one. |
Add refraction as a deprecated option as that is now known as useRefraction and resides in the lit options. Rename options to Standard Material Options.
… both option sets.
…ink to them from the standard material
…ons when we run our update refs.
… StandardMaterialOptions. Also updated the documentation to use properties annotations (with types) for typescript.
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.
Looks good to me. Please do test it with at least engine examples to make sure all is happy.
Description
Fixes #4871.
The backend options for the lit shader are now defined in their own class, which makes it possible to know what options are available.
Additionally, this allows us to validate options which might have moved to the backend object, and inform developers if they are still expecting the option to be there. This primarily pertains to options set through the onUpdateShader callback.