[Merged by Bors] - add default direction to DirectionalLight docs #5188
Conversation
crates/bevy_pbr/src/light.rs
Outdated
| @@ -92,6 +92,9 @@ impl Default for PointLightShadowMap { | |||
| /// approximation for light sources VERY far away, like the sun or | |||
| /// the moon. | |||
| /// | |||
| /// The light's default direction is negative-Z (0, 0, -1) and is | |||
| /// adjusted by the GlobalTransform. | |||
There was a problem hiding this comment.
| /// adjusted by the GlobalTransform. | |
| /// adjusted by the [`GlobalTransform`] of the entity that has this component. |
There was a problem hiding this comment.
I'd rather remove the mention of GlobalTransform as it may make people think they should edit it instead of Transform.
alice-i-cecile
added
C-Docs
crates/bevy_pbr/src/light.rs
Outdated
| @@ -92,6 +92,9 @@ impl Default for PointLightShadowMap { | |||
| /// approximation for light sources VERY far away, like the sun or | |||
| /// the moon. | |||
| /// | |||
| /// The light's default direction is negative-Z (0, 0, -1) and is | |||
There was a problem hiding this comment.
| /// The light's default direction is negative-Z (0, 0, -1) and is | |
| /// The light shines in the forward direction of the transform (negative z) and is |
It's true that that is the default direction, but I think it's better to explain why that is the case.
There was a problem hiding this comment.
@devil-ira Can you clarify this statement? Why it's negative-Z or what it has a default direction?
There was a problem hiding this comment.
I think it's important to explain "how is the direction inferred from the transform", and then tie that back to the default transform value.
Co-authored-by: Robert Swain <[email protected]>
crates/bevy_pbr/src/light.rs
Outdated
| @@ -92,6 +92,9 @@ impl Default for PointLightShadowMap { | |||
| /// approximation for light sources VERY far away, like the sun or | |||
| /// the moon. | |||
| /// | |||
| /// By default, the light shines along the negative-Z axis and the local | |||
| /// transform adjusts its direction. | |||
There was a problem hiding this comment.
| /// transform adjusts its direction. | |
| /// transform adjusts its direction. | |
| /// The light always points in the "forward" direction of its local rotation. |
There was a problem hiding this comment.
The problem with this is that people don’t always know that negative z is forward.
There was a problem hiding this comment.
It's explained above, so I think this is fine.
There was a problem hiding this comment.
Oh, I misread the diff in the email and thought you were replacing the text. I see now you added to it. Apologies.
|
Are we good to merge this? |
|
I think my suggestion should be merged in first to clarify exactly what's going on. |
|
@alice-i-cecile, you mean this suggestion?
That seemed redundant by the previous line:
|
|
That suggestion yes. Feel free to tweak the phrasing, but it's not clear to new users exactly "how" the local transform adjusts its direction and we should try to be more precise. |
Co-authored-by: Robert Swain <[email protected]>
# Objective - Adds a default direction to the documentation of DirectionalLight ## Solution Suggestion from Q&A answer: #5186 (reply in thread)
|
Pull request successfully merged into main. Build succeeded: |
bors
bot
changed the title
# Objective - Adds a default direction to the documentation of DirectionalLight ## Solution Suggestion from Q&A answer: bevyengine#5186 (reply in thread)
# Objective - Adds a default direction to the documentation of DirectionalLight ## Solution Suggestion from Q&A answer: bevyengine#5186 (reply in thread)
Objective
Solution
Suggestion from Q&A answer:
#5186 (reply in thread)