Skip to content
This repository has been archived by the owner on Aug 21, 2024. It is now read-only.

Re-add castShadow property to light components and default to false #7559

Merged
merged 7 commits into from
Feb 12, 2023

Conversation

dinomut1
Copy link
Member

For performance reasons we want to have lights not cast shadows by default

@dinomut1 dinomut1 self-assigned this Feb 11, 2023
@dinomut1 dinomut1 changed the title re-add castShadow property and default to false Re-add castShadow property to light components and default to false Feb 11, 2023
Copy link
Member

@HexaField HexaField left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason we can't use a ShadowComponent with cast set to false? That was the intention of removing the property previously

@@ -11,6 +11,19 @@ import { ObjectLayers } from '../constants/ObjectLayers'
import { setObjectLayers } from '../functions/setObjectLayers'
import { addObjectToGroup, removeObjectFromGroup } from './GroupComponent'

export type PointLightComponentType = {
Copy link
Member

@HexaField HexaField Feb 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use ComponentType<typeof PointLightComponent> for these types

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not put the castShadow property with all of the other shadow properties that are in the light components?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, the lights need different default values for castShadow than meshes and whatnot (lights false, meshes true)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not put the castShadow property with all of the other shadow properties that are in the light components?

That's a good point, in this case we should remove remove castShadow from ShadowComponent and turn it into a tag component

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@HexaField what about receiving shadows? You want a tag component for casting and receiving each?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, yes, I mean put castShadow on light components, and have receiveShadow be set only via the presence of a tag ShadowComponent

@dinomut1 dinomut1 merged commit 9d88004 into dev Feb 12, 2023
@dinomut1 dinomut1 deleted the fix-lights branch February 12, 2023 19:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants