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

Remove side effects from components #10979

Draft
wants to merge 16 commits into
base: dev
Choose a base branch
from
Draft
Original file line number Diff line number Diff line change
Expand Up @@ -121,12 +121,7 @@ export const TransformGizmoControlComponent = defineComponent({
if (typeof json.showY === 'number') component.showY.set(json.showY)
if (typeof json.showZ === 'number') component.showZ.set(json.showZ)
},
onRemove: (entity, component) => {
component.controlledEntities.set([])
component.visualEntity.set(UndefinedEntity)
component.planeEntity.set(UndefinedEntity)
component.pivotEntity.set(UndefinedEntity)
},

reactor: function (props) {
const gizmoControlEntity = useEntityContext()
const gizmoControlComponent = useComponent(gizmoControlEntity, TransformGizmoControlComponent)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,6 @@ export const TransformGizmoControlledComponent = defineComponent({
controller: UndefinedEntity
}
},
onRemove: (entity, component) => {
component.controller.set(UndefinedEntity)
},

reactor: function (props) {
const entity = useEntityContext()
Expand Down
13 changes: 6 additions & 7 deletions packages/editor/src/classes/TransformGizmoVisualComponent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,16 +108,11 @@ export const TransformGizmoVisualComponent = defineComponent({
}
return visual
},

onSet(entity, component, json) {
if (!json) return
},
onRemove: (entity, component) => {
for (const mode in TransformMode) {
removeEntity(component.gizmo[mode])
removeEntity(component.picker[mode])
removeEntity(component.helper[mode])
}
},

reactor: function (props) {
const gizmoVisualEntity = useEntityContext()
const visualComponent = useComponent(gizmoVisualEntity, TransformGizmoVisualComponent)
Expand Down Expand Up @@ -179,6 +174,10 @@ export const TransformGizmoVisualComponent = defineComponent({
cleanupGizmo(pickerObject[mode])
removeObjectFromGroup(helper[mode], helperObject[mode])
cleanupGizmo(helperObject[mode])

removeEntity(gizmo[mode])
removeEntity(picker[mode])
removeEntity(helper[mode])
}
}
}, [])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import { AnimationAction, Group, Matrix4, SkeletonHelper, Vector3 } from 'three'
import {
defineComponent,
getComponent,
removeComponent,
setComponent,
useComponent,
useOptionalComponent
Expand Down Expand Up @@ -117,11 +116,6 @@ export const AvatarRigComponent = defineComponent({
if (matches.string.test(json.avatarURL)) component.avatarURL.set(json.avatarURL)
},

onRemove: (entity, component) => {
// ensure synchronously removed
if (component.helperEntity.value) removeComponent(component.helperEntity.value, ComputedTransformComponent)
},

reactor: function () {
const entity = useEntityContext()
const debugEnabled = useHookstate(getMutableState(RendererState).avatarDebug)
Expand Down
29 changes: 14 additions & 15 deletions packages/engine/src/scene/components/HyperspaceTagComponent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ import { Engine } from '@etherealengine/ecs/src/Engine'
import { Entity, UndefinedEntity } from '@etherealengine/ecs/src/Entity'
import { createEntity, removeEntity, useEntityContext } from '@etherealengine/ecs/src/EntityFunctions'
import { useExecute } from '@etherealengine/ecs/src/SystemFunctions'
import { getMutableState, getState } from '@etherealengine/hyperflux'
import { getMutableState, getState, useHookstate } from '@etherealengine/hyperflux'
import { CameraComponent } from '@etherealengine/spatial/src/camera/components/CameraComponent'
import { ObjectDirection } from '@etherealengine/spatial/src/common/constants/MathConstants'
import { createTransitionState } from '@etherealengine/spatial/src/common/functions/createTransitionState'
Expand Down Expand Up @@ -172,26 +172,23 @@ export const HyperspaceTagComponent = defineComponent({
return {
// all internals
sceneVisible: true,
transition: createTransitionState(0.5, 'OUT'),
hyperspaceEffectEntity: UndefinedEntity,
ambientLightEntity: UndefinedEntity
transition: createTransitionState(0.5, 'OUT')
}
},

onRemove(entity, component) {
removeEntity(component.ambientLightEntity.value)
destroyEntityTree(component.hyperspaceEffectEntity.value)
},

reactor: () => {
const entity = useEntityContext()
const [galaxyTexture] = useTexture(
`${config.client.fileServer}/projects/default-project/assets/galaxyTexture.jpg`,
entity
)
const hyperspaceEffectEntityState = useHookstate(createEntity)
const ambientLightEntityState = useHookstate(createEntity)

useEffect(() => {
const hyperspaceEffectEntity = createEntity()
const hyperspaceEffectEntity = hyperspaceEffectEntityState.value
const ambientLightEntity = ambientLightEntityState.value

const hyperspaceEffect = new PortalEffect(hyperspaceEffectEntity)
addObjectToGroup(hyperspaceEffectEntity, hyperspaceEffect)
setObjectLayers(hyperspaceEffect, ObjectLayers.Portal)
Expand All @@ -200,7 +197,6 @@ export const HyperspaceTagComponent = defineComponent({
setComponent(hyperspaceEffectEntity, EntityTreeComponent, { parentEntity: entity })
setComponent(hyperspaceEffectEntity, VisibleComponent)

const ambientLightEntity = createEntity()
const light = new AmbientLight('#aaa')
light.layers.enable(ObjectLayers.Portal)
addObjectToGroup(ambientLightEntity, light)
Expand All @@ -222,14 +218,16 @@ export const HyperspaceTagComponent = defineComponent({
new Vector3(0, 0, 1).applyQuaternion(cameraTransform.rotation).setY(0).normalize()
)

getMutableComponent(entity, HyperspaceTagComponent).hyperspaceEffectEntity.set(hyperspaceEffectEntity)
getMutableComponent(entity, HyperspaceTagComponent).ambientLightEntity.set(ambientLightEntity)
return () => {
removeEntity(ambientLightEntity)
destroyEntityTree(hyperspaceEffectEntity)
}
}, [])

useEffect(() => {
if (!galaxyTexture) return

const hyperspaceEffectEntity = getComponent(entity, HyperspaceTagComponent).hyperspaceEffectEntity
const hyperspaceEffectEntity = hyperspaceEffectEntityState.value
const hyperspaceEffect = getComponent(hyperspaceEffectEntity, GroupComponent)[0] as any as PortalEffect
hyperspaceEffect.texture = galaxyTexture
}, [galaxyTexture])
Expand All @@ -238,8 +236,9 @@ export const HyperspaceTagComponent = defineComponent({
() => {
if (!hasComponent(entity, HyperspaceTagComponent)) return

const { transition, hyperspaceEffectEntity } = getComponent(entity, HyperspaceTagComponent)
const hyperspaceEffectEntity = hyperspaceEffectEntityState.value
if (!hyperspaceEffectEntity) return
const { transition } = getComponent(entity, HyperspaceTagComponent)

const hyperspaceEffect = getComponent(hyperspaceEffectEntity, GroupComponent)[0] as any as PortalEffect
const cameraTransform = getComponent(Engine.instance.cameraEntity, TransformComponent)
Expand Down
43 changes: 25 additions & 18 deletions packages/engine/src/scene/components/MediaComponent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ Ethereal Engine. All Rights Reserved.
*/

import type Hls from 'hls.js'
import { startTransition, useEffect } from 'react'
import { startTransition, useEffect, useLayoutEffect } from 'react'
import { DoubleSide, MeshBasicMaterial, PlaneGeometry, Vector3 } from 'three'

import { isClient } from '@etherealengine/common/src/utils/getEnvironment'
Expand Down Expand Up @@ -103,19 +103,26 @@ export const MediaElementComponent = defineComponent({
component.element.set(json.element as HTMLMediaElement)
},

onRemove: (entity, component) => {
const element = component.element.get({ noproxy: true }) as HTMLMediaElement
component.hls.value?.destroy()
component.hls.set(none)
const audioNodeGroup = AudioNodeGroups.get(element)
if (audioNodeGroup && audioNodeGroup.panner) removePannerNode(audioNodeGroup)
AudioNodeGroups.delete(element)
element.pause()
element.removeAttribute('src')
element.load()
element.remove()
component.element.set(none)
component.abortController.value.abort()
reactor: () => {
const entity = useEntityContext()
const mediaElementComponent = useComponent(entity, MediaElementComponent)

useLayoutEffect(() => {
return () => {
const element = mediaElementComponent.element.get({ noproxy: true }) as HTMLMediaElement
mediaElementComponent.hls.value?.destroy()
mediaElementComponent.hls.set(none)
const audioNodeGroup = AudioNodeGroups.get(element)
if (audioNodeGroup && audioNodeGroup.panner) removePannerNode(audioNodeGroup)
AudioNodeGroups.delete(element)
element.pause()
element.removeAttribute('src')
element.load()
element.remove()
mediaElementComponent.element.set(none)
mediaElementComponent.abortController.value.abort()
}
}, [])
},

errors: ['MEDIA_ERROR', 'HLS_ERROR']
Expand Down Expand Up @@ -156,10 +163,6 @@ export const MediaComponent = defineComponent({
}
},

onRemove: (entity, component) => {
removeComponent(entity, MediaElementComponent)
},

toJSON: (entity, component) => {
return {
controls: component.controls.value,
Expand Down Expand Up @@ -286,6 +289,10 @@ export function MediaReactor() {
document.body.removeEventListener('touchend', handleAutoplay)
renderer.domElement.removeEventListener('pointerup', handleAutoplay)
renderer.domElement.removeEventListener('touchend', handleAutoplay)

removeComponent(entity, BoundingBoxComponent)
removeComponent(entity, InputComponent)
removeComponent(entity, MediaElementComponent)
}
}, [])

Expand Down
28 changes: 19 additions & 9 deletions packages/engine/src/scene/components/SourceComponent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@ All portions of the code written by the Ethereal Engine team are Copyright © 20
Ethereal Engine. All Rights Reserved.
*/

import { defineComponent } from '@etherealengine/ecs/src/ComponentFunctions'
import { useEntityContext } from '@etherealengine/ecs'
import { defineComponent, useComponent } from '@etherealengine/ecs/src/ComponentFunctions'
import { Entity } from '@etherealengine/ecs/src/Entity'
import { hookstate, none } from '@etherealengine/hyperflux'
import { useLayoutEffect } from 'react'

const entitiesBySource = {} as Record<string, Entity[]>

Expand All @@ -49,15 +51,23 @@ export const SourceComponent = defineComponent({
}
},

onRemove: (entity, component) => {
const src = component.value
reactor: () => {
const entity = useEntityContext()
const sourceComponent = useComponent(entity, SourceComponent)

const entities = SourceComponent.entitiesBySource[src].filter((currentEntity) => currentEntity !== entity)
if (entities.length === 0) {
SourceComponent.entitiesBySourceState[src].set(none)
} else {
SourceComponent.entitiesBySourceState[src].set(entities)
}
useLayoutEffect(() => {
const source = sourceComponent.value
Copy link
Member

Choose a reason for hiding this comment

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

if the source changes after the component has been created, this will no longer have the same value. we should just move this line inside the returned function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh if the source can change this is setup incorrectly altogether and won't clean up properly, good catch

return () => {
const entities = SourceComponent.entitiesBySource[source].filter((currentEntity) => currentEntity !== entity)
if (entities.length === 0) {
SourceComponent.entitiesBySourceState[source].set(none)
} else {
SourceComponent.entitiesBySourceState[source].set(entities)
}
}
}, [])

return null
},

entitiesBySourceState: hookstate(entitiesBySource),
Expand Down
31 changes: 23 additions & 8 deletions packages/spatial/src/camera/components/CameraComponent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,16 @@ Ethereal Engine. All Rights Reserved.

import { ArrayCamera, PerspectiveCamera } from 'three'

import { defineComponent } from '@etherealengine/ecs/src/ComponentFunctions'
import { defineComponent, useComponent } from '@etherealengine/ecs/src/ComponentFunctions'

import { useEntityContext } from '@etherealengine/ecs'
import { useImmediateEffect } from '@etherealengine/hyperflux'
import { addObjectToGroup, removeObjectFromGroup } from '../../renderer/components/GroupComponent'

export const CameraComponent = defineComponent({
name: 'CameraComponent',
jsonID: 'EE_camera',

onInit: (entity) => {
const camera = new ArrayCamera()
camera.fov = 60
Expand All @@ -41,23 +44,35 @@ export const CameraComponent = defineComponent({
camera.cameras = [new PerspectiveCamera().copy(camera, false)]
return camera
},

onSet: (entity, component, json) => {
addObjectToGroup(entity, component.value as ArrayCamera)
if (!json) return
if (typeof json.fov === 'number') component.fov.set(json.fov)
if (typeof json.aspect === 'number') component.fov.set(json.aspect)
if (typeof json.near === 'number') component.fov.set(json.near)
if (typeof json.far === 'number') component.fov.set(json.far)
},
onRemove: (entity, component) => {
removeObjectFromGroup(entity, component.value as ArrayCamera)
if (typeof json.aspect === 'number') component.aspect.set(json.aspect)
if (typeof json.near === 'number') component.near.set(json.near)
if (typeof json.far === 'number') component.far.set(json.far)
},

toJSON: (entity, component) => {
return {
fov: component.fov.value,
aspect: component.aspect.value,
near: component.near.value,
far: component.far.value
}
},

reactor: () => {
const entity = useEntityContext()
const cameraComponent = useComponent(entity, CameraComponent)

useImmediateEffect(() => {
const camera = cameraComponent.value as ArrayCamera
addObjectToGroup(entity, camera)
return () => {
removeObjectFromGroup(entity, camera)
}
}, [])
return null
}
})
8 changes: 0 additions & 8 deletions packages/spatial/src/renderer/components/GroupComponent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,6 @@ export const GroupComponent = defineComponent({

onInit: (entity: Entity) => {
return [] as Object3D[]
},

onRemove: (entity, component) => {
for (const obj of component.value) {
if (obj.parent) {
obj.removeFromParent()
}
}
}
})

Expand Down
18 changes: 16 additions & 2 deletions packages/spatial/src/renderer/components/Object3DComponent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,9 @@ Ethereal Engine. All Rights Reserved.

import { Object3D } from 'three'

import { defineComponent, getComponent, hasComponent } from '@etherealengine/ecs'
import { defineComponent, useComponent, useEntityContext, useOptionalComponent } from '@etherealengine/ecs'

import { NO_PROXY, useImmediateEffect } from '@etherealengine/hyperflux'
import { NameComponent } from '../../common/NameComponent'

export const Object3DComponent = defineComponent({
Expand All @@ -36,7 +37,20 @@ export const Object3DComponent = defineComponent({
onInit: (entity) => null! as Object3D,
onSet: (entity, component, object3d: Object3D) => {
if (!object3d || !object3d.isObject3D) throw new Error('Object3DComponent: Invalid object3d')
if (hasComponent(entity, NameComponent)) object3d.name = getComponent(entity, NameComponent)
component.set(object3d)
},

reactor: () => {
const entity = useEntityContext()
const object3DComponent = useComponent(entity, Object3DComponent)
const nameComponent = useOptionalComponent(entity, NameComponent)

useImmediateEffect(() => {
if (!nameComponent) return
const object = object3DComponent.get(NO_PROXY) as Object3D
object.name = nameComponent.value
}, [nameComponent?.value])

return null
}
})
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import {
useOptionalComponent
} from '@etherealengine/ecs/src/ComponentFunctions'
import { useEntityContext } from '@etherealengine/ecs/src/EntityFunctions'
import { matches, useMutableState } from '@etherealengine/hyperflux'
import { matches, useImmediateEffect, useMutableState } from '@etherealengine/hyperflux'

import { mergeBufferGeometries } from '../../../common/classes/BufferGeometryUtils'
import { useDisposable } from '../../../resources/resourceHooks'
Expand Down Expand Up @@ -147,7 +147,7 @@ export const DirectionalLightComponent = defineComponent({
const [light] = useDisposable(DirectionalLight, entity)
const lightHelper = useOptionalComponent(entity, LineSegmentComponent)

useEffect(() => {
useImmediateEffect(() => {
setComponent(entity, LightTagComponent)
directionalLightComponent.light.set(light)
addObjectToGroup(entity, light)
Expand Down
Loading
Loading