-
Notifications
You must be signed in to change notification settings - Fork 855
Added public GetMainLight API for HDRP #5126
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
Hi! This comment will help you figure out which jobs to run before merging your PR. The suggestions are dynamic based on what files you have changed. HDRP Depending on the scope of your PR, you may need to run more jobs than what has been suggested. Please speak to your lead or a Graphics SDET (#devs-graphics-automation) if you are unsure. |
It appears that you made a non-draft PR! |
@adrien-de-tocqueville This is not just adding an api callback for GetMainLight but replaces/removes GetCurrentSunLight function. Adding a notice about "this replaces GetCurrentSunLight" to change log would be useful, unless adding a stub for
is an intention. Otherwise, it can be confusing for the people who upgrade from an earlier version. (Or for the asset devs) |
@merpheus-dev That's right, however, the function GetCurrentSunLight was marked as internal, so it was not accessible to users. So I don't think renaming the function will break users project |
Ah sorry, I missed that. |
/// Main directional Light for the HD Render Pipeline. | ||
/// </summary> | ||
/// <returns>The main directional Light.</returns> | ||
public Light GetMainLight() { return m_CurrentSunLight; } |
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.
Guess is weird to expose this Method inside the LightLoop class. Guess it should be in another location, maybe HDRenderPipeline. What are your thought?
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.
There isn't actually any lightloop class, it's just the name of the file. The function is inside the class HDRenderPipeline, which is partial
, so defined in multiple files.
For user it will be accessible this way:
var sunLight = HDRenderPipeline.currentPipeline.GetMainLight();
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.
ah right :)
Added a public function to get the sun light from script in HDRP:
or simply
Note: This will return null until hdrp renders a camera.
A SG node doing the same thing will be added by #5086