Remove side effects from components

packages/editor/src/classes/TransformGizmoControlComponent.ts

@@ -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)

packages/editor/src/classes/TransformGizmoControlledComponent.ts

@@ -56,9 +56,6 @@ export const TransformGizmoControlledComponent = defineComponent({
       controller: UndefinedEntity
     }
   },
-  onRemove: (entity, component) => {
-    component.controller.set(UndefinedEntity)
-  },
 
   reactor: function (props) {
     const entity = useEntityContext()

packages/editor/src/classes/TransformGizmoVisualComponent.ts

@@ -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)
@@ -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])
         }
       }
     }, [])

packages/engine/src/avatar/components/AvatarAnimationComponent.ts

@@ -30,14 +30,12 @@ import { AnimationAction, Group, Matrix4, SkeletonHelper, Vector3 } from 'three'
 import {
   defineComponent,
   getComponent,
-  removeComponent,
   setComponent,
   useComponent,
   useOptionalComponent
 } from '@etherealengine/ecs/src/ComponentFunctions'
-import { Entity } from '@etherealengine/ecs/src/Entity'
 import { createEntity, entityExists, removeEntity, useEntityContext } from '@etherealengine/ecs/src/EntityFunctions'
-import { getMutableState, matches, none, useHookstate } from '@etherealengine/hyperflux'
+import { getMutableState, matches, useHookstate } from '@etherealengine/hyperflux'
 import { NameComponent } from '@etherealengine/spatial/src/common/NameComponent'
 import { addObjectToGroup } from '@etherealengine/spatial/src/renderer/components/GroupComponent'
 import { setObjectLayers } from '@etherealengine/spatial/src/renderer/components/ObjectLayerComponent'
@@ -102,7 +100,6 @@ export const AvatarRigComponent = defineComponent({
       rawRig: null! as VRMHumanBones,
       /** contains ik solve data */
       ikMatrices: {} as Record<VRMHumanBoneName, Matrices>,
-      helperEntity: null as Entity | null,
       /** The VRM model */
       vrm: null! as VRM,
       avatarURL: null as string | null
@@ -117,11 +114,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)
@@ -144,7 +136,6 @@ export const AvatarRigComponent = defineComponent({
       const helperEntity = createEntity()
       setVisibleComponent(helperEntity, true)
       addObjectToGroup(helperEntity, helper)
-      rigComponent.helperEntity.set(helperEntity)
       setComponent(helperEntity, NameComponent, helper.name)
       setObjectLayers(helper, ObjectLayers.AvatarHelper)
 
@@ -158,7 +149,6 @@ export const AvatarRigComponent = defineComponent({
 
       return () => {
         removeEntity(helperEntity)
-        rigComponent.helperEntity.set(none)
       }
     }, [visible, debugEnabled, pending, rigComponent.normalizedRig])
 

packages/engine/src/gltf/GLTFState.test.tsx

@@ -373,8 +373,14 @@ describe('GLTFState', () => {
 
     assert.equal(getComponent(nodeEntity!, VisibleComponent), true)
     assert(getComponent(nodeEntity!, HemisphereLightComponent))
-    assert.equal(getComponent(nodeEntity!, HemisphereLightComponent).skyColor.getHex(), new Color('green').getHex())
-    assert.equal(getComponent(nodeEntity!, HemisphereLightComponent).groundColor.getHex(), new Color('purple').getHex())
+    assert.equal(
+      new Color(getComponent(nodeEntity!, HemisphereLightComponent).skyColor).getHex(),
+      new Color('green').getHex()
+    )
+    assert.equal(
+      new Color(getComponent(nodeEntity!, HemisphereLightComponent).groundColor).getHex(),
+      new Color('purple').getHex()
+    )
     assert.equal(getComponent(nodeEntity!, HemisphereLightComponent).intensity, 0.5)
   })
 

packages/engine/src/scene/components/HyperspaceTagComponent.ts

@@ -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'
@@ -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)
@@ -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)
@@ -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])
@@ -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)

packages/engine/src/scene/components/MediaComponent.ts

@@ -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'
@@ -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']
@@ -156,10 +163,6 @@ export const MediaComponent = defineComponent({
     }
   },
 
-  onRemove: (entity, component) => {
-    removeComponent(entity, MediaElementComponent)
-  },
-
   toJSON: (entity, component) => {
     return {
       controls: component.controls.value,
@@ -288,6 +291,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)
     }
   }, [])
 

packages/engine/src/scene/components/SourceComponent.ts

@@ -23,9 +23,10 @@ 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 { hookstate, none, useImmediateEffect } from '@etherealengine/hyperflux'
 
 const entitiesBySource = {} as Record<string, Entity[]>
 
@@ -38,26 +39,32 @@ export const SourceComponent = defineComponent({
     if (typeof src !== 'string') throw new Error('SourceComponent expects a non-empty string')
 
     component.set(src)
-
-    const exists = SourceComponent.entitiesBySource[src]
-    const entitiesBySourceState = SourceComponent.entitiesBySourceState[src]
-    if (exists) {
-      if (exists.includes(entity)) return
-      entitiesBySourceState.merge([entity])
-    } else {
-      entitiesBySourceState.set([entity])
-    }
   },
 
-  onRemove: (entity, component) => {
-    const src = component.value
+  reactor: () => {
+    const entity = useEntityContext()
+    const sourceComponent = useComponent(entity, SourceComponent)
+
+    useImmediateEffect(() => {
+      const source = sourceComponent.value
+      const entitiesBySourceState = SourceComponent.entitiesBySourceState[source]
+      if (!entitiesBySourceState.value) {
+        entitiesBySourceState.set([entity])
+      } else {
+        entitiesBySourceState.merge([entity])
+      }
+
+      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)
+        }
+      }
+    }, [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)
-    }
+    return null
   },
 
   entitiesBySourceState: hookstate(entitiesBySource),

packages/spatial/src/camera/components/CameraComponent.ts

@@ -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
@@ -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
   }
 })