Ir 3866 remove side effects from UUID component on set on remove

.vscode/launch.json

@@ -113,6 +113,12 @@
             "request": "launch",
             "type": "node-terminal",
         },
+        {
+            "command": "cd packages/network && npm run test",
+            "name": "npm run test - network",
+            "request": "launch",
+            "type": "node-terminal",
+        },
         {
             "command": "cd packages/engine && npm run test",
             "name": "npm run test - engine",

packages/ecs/src/ComponentFunctions.ts

@@ -359,10 +359,11 @@ export const setComponent = <C extends Component>(
     root['component'] = Component.name
     Component.reactorMap.set(entity, root)
     return getComponent(entity, Component) as ComponentType<C>
+  } else {
+    const root = Component.reactorMap.get(entity)
+    root?.run()
   }
 
-  const root = Component.reactorMap.get(entity)
-  root?.run()
   return getComponent(entity, Component) as ComponentType<C>
 }
 

packages/ecs/src/UUIDComponent.test.tsx

@@ -27,6 +27,7 @@ import { act, render } from '@testing-library/react'
 import assert from 'assert'
 import React, { useEffect } from 'react'
 
+import { ReactorRoot, startReactor } from '@etherealengine/hyperflux'
 import {
   ComponentMap,
   getComponent,
@@ -38,7 +39,7 @@ import {
 } from './ComponentFunctions'
 import { createEngine, destroyEngine } from './Engine'
 import { Entity, EntityUUID, UndefinedEntity } from './Entity'
-import { createEntity, removeEntity } from './EntityFunctions'
+import { EntityContext, createEntity, removeEntity } from './EntityFunctions'
 import { UUIDComponent } from './UUIDComponent'
 
 describe('UUIDComponent', () => {
@@ -84,10 +85,16 @@ describe('UUIDComponent', () => {
     })
 
     it('Should throw an error when the UUID is already in use for another entity', () => {
+      let root = undefined as ReactorRoot | undefined
       setComponent(entity1, UUIDComponent, TestUUID)
-      assert.throws(() => {
-        setComponent(entity2, UUIDComponent, TestUUID)
-      }, Error)
+      setComponent(entity2, UUIDComponent, TestUUID)
+      const reactor = UUIDComponent.reactorMap.get(entity2)?.Reactor
+      root = startReactor(() => {
+        return React.createElement(EntityContext.Provider, { value: entity2 }, React.createElement(reactor!, {}))
+      }) as ReactorRoot
+
+      assert(root?.errors.length)
+      root?.stop()
     })
 
     it('should remove the old uuid from the entity', () => {
@@ -114,14 +121,14 @@ describe('UUIDComponent', () => {
     it('should remove the component from the entity', () => {
       setComponent(entity1, UUIDComponent, TestUUID)
       removeComponent(entity1, UUIDComponent)
-      assert.equal(UndefinedEntity, UUIDComponent.entitiesByUUIDState[TestUUID].value)
+      assert.equal(UndefinedEntity, UUIDComponent.getEntityByUUID(TestUUID))
       assert.equal(false, hasComponent(entity1, UUIDComponent))
       assert.equal(getOptionalComponent(entity1, UUIDComponent), undefined)
     })
 
     it('should do nothing if the entity does not have the component', () => {
       removeComponent(entity1, UUIDComponent)
-      assert.equal(UndefinedEntity, UUIDComponent.entitiesByUUIDState[TestUUID].value)
+      assert.equal(UndefinedEntity, UUIDComponent.getEntityByUUID(TestUUID))
       assert.equal(getOptionalComponent(entity1, UUIDComponent), undefined)
     })
   })
@@ -130,7 +137,7 @@ describe('UUIDComponent', () => {
     it('should return the correct entity', () => {
       setComponent(entity1, UUIDComponent, TestUUID)
       const testEntity = UUIDComponent.getEntityByUUID(TestUUID)
-      assert.equal(testEntity, UUIDComponent.entitiesByUUIDState[TestUUID].value)
+      assert.equal(testEntity, UUIDComponent.getEntityByUUID(TestUUID))
       assert.equal(testEntity, entity1)
     })
 
@@ -139,12 +146,12 @@ describe('UUIDComponent', () => {
       removeComponent(entity1, UUIDComponent)
       setComponent(entity1, UUIDComponent, TestUUID2)
       const testEntity = UUIDComponent.getEntityByUUID(TestUUID2)
-      assert.equal(testEntity, UUIDComponent.entitiesByUUIDState[TestUUID2].value)
+      assert.equal(testEntity, UUIDComponent.getEntityByUUID(TestUUID2))
     })
 
     it('should return UndefinedEntity when the UUID has not been added to any entity', () => {
       const testEntity = UUIDComponent.getEntityByUUID(TestUUID)
-      assert.equal(testEntity, UUIDComponent.entitiesByUUIDState[TestUUID].value)
+      assert.equal(testEntity, UUIDComponent.getEntityByUUID(TestUUID))
       assert.equal(testEntity, UndefinedEntity)
     })
   })
@@ -153,14 +160,14 @@ describe('UUIDComponent', () => {
     it('should return the correct entity when it exists', () => {
       setComponent(entity1, UUIDComponent, TestUUID)
       const testEntity = UUIDComponent.getOrCreateEntityByUUID(TestUUID)
-      assert.equal(testEntity, UUIDComponent.entitiesByUUIDState[TestUUID].value)
+      assert.equal(testEntity, UUIDComponent.getEntityByUUID(TestUUID))
       assert.equal(testEntity, entity1)
     })
 
     it("should create a new entity when the UUID hasn't been added to any entity", () => {
       setComponent(entity1, UUIDComponent, TestUUID)
       const testEntity = UUIDComponent.getOrCreateEntityByUUID(TestUUID2)
-      assert.equal(testEntity, UUIDComponent.entitiesByUUIDState[TestUUID2].value)
+      assert.equal(testEntity, UUIDComponent.getEntityByUUID(TestUUID2))
       assert.notEqual(testEntity, entity1)
     })
   })
@@ -246,7 +253,7 @@ describe('UUIDComponent Hooks', async () => {
       assert.equal(counter, 1, `The reactor has run an incorrect number of times: ${counter}`)
       assert.notEqual(result, undefined, "The result data didn't get assigned.")
       assert.equal(result, ExpectedValue, `Did not return the correct data. result = ${result}`)
-      assert.equal(testEntity, UUIDComponent.entitiesByUUIDState[TestUUID].value)
+      assert.equal(testEntity, UUIDComponent.getEntityByUUID(TestUUID))
       assert.equal(testEntity, ExpectedValue)
       unmount()
     })

packages/ecs/src/UUIDComponent.ts

@@ -25,11 +25,16 @@ Ethereal Engine. All Rights Reserved.
 
 import { MathUtils } from 'three'
 
-import { hookstate, NO_PROXY_STEALTH, State, useHookstate } from '@etherealengine/hyperflux'
+import { defineState, getState, useImmediateEffect, useMutableState } from '@etherealengine/hyperflux'
 
-import { defineComponent, setComponent } from './ComponentFunctions'
+import { defineComponent, setComponent, useComponent } from './ComponentFunctions'
 import { Entity, EntityUUID, UndefinedEntity } from './Entity'
-import { createEntity } from './EntityFunctions'
+import { createEntity, useEntityContext } from './EntityFunctions'
+
+export const UUIDState = defineState({
+  name: 'UUIDState',
+  initial: () => ({}) as Record<EntityUUID, Entity>
+})
 
 export const UUIDComponent = defineComponent({
   name: 'UUIDComponent',
@@ -39,64 +44,58 @@ export const UUIDComponent = defineComponent({
 
   onSet: (entity, component, uuid: EntityUUID) => {
     if (!uuid) throw new Error('UUID cannot be empty')
-
     if (component.value === uuid) return
-
-    // throw error if uuid is already in use
-    const currentEntity = UUIDComponent.getEntityByUUID(uuid)
-    if (currentEntity !== UndefinedEntity && currentEntity !== entity) {
-      throw new Error(`UUID ${uuid} is already in use`)
-    }
-
-    // remove old uuid
-    if (component.value) {
-      const currentUUID = component.value
-      _getUUIDState(currentUUID).set(UndefinedEntity)
-    }
-
-    // set new uuid
     component.set(uuid)
-    _getUUIDState(uuid).set(entity)
   },
 
   toJSON(entity, component) {
     return component.value
   },
 
-  onRemove: (entity, component) => {
-    const uuid = component.value
-    _getUUIDState(uuid).set(UndefinedEntity)
-  },
+  reactor: () => {
+    const entity = useEntityContext()
+    const uuidComponent = useComponent(entity, UUIDComponent)
+    const uuidState = useMutableState(UUIDState)
+
+    useImmediateEffect(() => {
+      const uuid = uuidComponent.value
+
+      // throw error if uuid is already in use
+      const currentEntity = uuidState[uuid].value
+      if (currentEntity && currentEntity !== entity) {
+        throw new Error(`UUID ${uuid} is already in use for entity ${currentEntity}, cannot use for entity ${entity}`)
+      }
+
+      // set new uuid
+      uuidState.merge({ [uuid]: entity })
 
-  entitiesByUUIDState: {} as Record<EntityUUID, State<Entity>>,
+      return () => {
+        // remove old uuid
+        uuidState.merge({ [uuid]: UndefinedEntity })
+      }
+    }, [uuidComponent.value])
+
+    return null
+  },
 
   useEntityByUUID(uuid: EntityUUID) {
-    return useHookstate(_getUUIDState(uuid)).value
+    return useMutableState(UUIDState)[uuid].value || UndefinedEntity
   },
 
   getEntityByUUID(uuid: EntityUUID) {
-    return _getUUIDState(uuid).get(NO_PROXY_STEALTH)
+    return getState(UUIDState)[uuid] || UndefinedEntity
   },
 
   getOrCreateEntityByUUID(uuid: EntityUUID) {
-    const state = _getUUIDState(uuid)
-    if (!state.value) {
-      const entity = createEntity()
+    let entity = getState(UUIDState)[uuid]
+    if (!entity) {
+      entity = createEntity()
       setComponent(entity, UUIDComponent, uuid)
     }
-    return state.value
+    return entity
   },
 
   generateUUID() {
     return MathUtils.generateUUID() as EntityUUID
   }
 })
-
-function _getUUIDState(uuid: EntityUUID) {
-  let entityState = UUIDComponent.entitiesByUUIDState[uuid]
-  if (!entityState) {
-    entityState = hookstate(UndefinedEntity)
-    UUIDComponent.entitiesByUUIDState[uuid] = entityState
-  }
-  return entityState
-}

packages/engine/src/assets/loaders/gltf/extensions/EEECSImporterExtension.ts

@@ -64,7 +64,7 @@ export default class EEECSImporterExtension extends ImporterExtension implements
         if (component === UUIDComponent) {
           const uuid = ecsExtensions[jsonID]
           //check if uuid already exists
-          if (UUIDComponent.entitiesByUUIDState[uuid]?.value) {
+          if (UUIDComponent.getEntityByUUID(uuid)) {
             //regenerate uuid if it already exists
             ecsExtensions[jsonID] = generateEntityUUID()
           }

packages/engine/src/scene/materials/systems/MaterialLibrarySystem.tsx

@@ -51,6 +51,7 @@ import {
   updateMaterialPrototype
 } from '@etherealengine/spatial/src/renderer/materials/materialFunctions'
 
+import { defineState, getState, useImmediateEffect } from '@etherealengine/hyperflux'
 import { MeshComponent } from '@etherealengine/spatial/src/renderer/components/MeshComponent'
 import {
   MaterialInstanceComponent,
@@ -61,7 +62,7 @@ import { Material, MeshBasicMaterial } from 'three'
 import { SourceComponent } from '../../components/SourceComponent'
 
 const reactor = (): ReactElement => {
-  useEffect(() => {
+  useImmediateEffect(() => {
     MaterialPrototypeDefinitions.map((prototype: MaterialPrototypeDefinition, uuid) =>
       createMaterialPrototype(prototype)
     )
@@ -90,12 +91,13 @@ const MeshReactor = () => {
     if (source) setComponent(materialEntity, SourceComponent, source)
   }
 
-  useEffect(() => {
+  useImmediateEffect(() => {
     if (materialComponent) return
     const material = meshComponent.material.value as Material
     if (!isArray(material)) createAndSourceMaterial(material)
     else for (const mat of material) createAndSourceMaterial(mat)
   }, [])
+
   return null
 }
 
@@ -135,8 +137,16 @@ const MaterialInstanceReactor = () => {
   return null
 }
 
+export const MaterialLibraryReactorState = defineState({
+  name: 'ee.engine.scene.MaterialLibrarySystem',
+  initial: () => false,
+  reactor
+})
+
 export const MaterialLibrarySystem = defineSystem({
   uuid: 'ee.engine.scene.MaterialLibrarySystem',
   insert: { after: PresentationSystemGroup },
-  reactor
+  execute: () => {
+    getState(MaterialLibraryReactorState)
+  }
 })