feat: dynamic lights #234
feat: dynamic lights #234
Conversation
Test this pull request
|
There was a problem hiding this comment.
Looks great! Some minor changes
What are hard/soft shadows?
I would recommend to copy some comments or use similar architecture from:
https://github.com/decentraland/protocol/blob/591a12ff93bb27fbda09c333764a83852adbfafa/proto/decentraland/sdk/components/light.proto
Also, can we update the light source ADR, at least with the parts that affects it?
decentraland/adr#276
| bool active = 4; // default = true | ||
| decentraland.common.Color3 color = 1; // default = Color.white | ||
| float brightness = 2; // range from 1 (dim) to 100,000 (very bright), expressed in Lumens for Point and Spot | ||
| float range = 3; // default = 10, expressed in meters | ||
| ShadowType shadow = 5; // default = ShadowType.ST_NONE |
There was a problem hiding this comment.
We believe that the default value of range should be based on brightness. The default could be equation using brightness as a value.
Additionally, if you want to implement this with a "default" value, you should mark it as optional float range = 3.
Only "optional" values can have default values, which are not required to be specified in the SDK (you will receive undefined or null in Unity, and you need to interpret that as the default).
@kuruk-mm Can you re-review the documentation I added? Is that clear enough? |
Related PRs
SDK PR: decentraland/js-sdk-toolchain#1049
E@ PR: decentraland/unity-explorer#3113