From 967ba553077f2814eb99ddd9cb2c63c07697f8bf Mon Sep 17 00:00:00 2001 From: Gonzalo DCL Date: Tue, 28 Nov 2023 15:33:49 -0300 Subject: [PATCH] Fix delete network entity with childrens --- packages/@dcl/ecs/src/engine/index.ts | 31 +++++----------- packages/@dcl/ecs/src/engine/types.ts | 13 ------- packages/@dcl/ecs/src/runtime/helpers/tree.ts | 37 +++++++++++++++++-- packages/@dcl/ecs/src/systems/crdt/index.ts | 2 + .../etc/playground-assets.api.md | 8 +--- .../sdk/src/internal/transports/logger.ts | 7 +++- .../src/create-cube.ts | 9 ++++- 7 files changed, 59 insertions(+), 48 deletions(-) diff --git a/packages/@dcl/ecs/src/engine/index.ts b/packages/@dcl/ecs/src/engine/index.ts index 27efb7225..9a6fa2e96 100644 --- a/packages/@dcl/ecs/src/engine/index.ts +++ b/packages/@dcl/ecs/src/engine/index.ts @@ -17,6 +17,7 @@ import { ValueSetOptions } from './grow-only-value-set-component-definition' import { removeEntityWithChildren as removeEntityWithChildrenEngine } from '../runtime/helpers/tree' +import { NetworkEntity } from '..' export * from './input' export * from './readonly' export * from './types' @@ -26,7 +27,6 @@ function preEngine(): PreEngine { const entityContainer = EntityContainer() const componentsDefinition = new Map>() const systems = SystemContainer() - let networkManager: ReturnType let sealed = false @@ -38,19 +38,6 @@ function preEngine(): PreEngine { return systems.remove(selector) } - function getNetworkManager() { - if (!networkManager) throw new Error('Network manager not initialized. Start CRDT Server') - return networkManager - } - - function addNetworkManager(reservedLocalEntities: number, range: [number, number]) { - entityContainer.setNetworkEntitiesRange(reservedLocalEntities, range) - networkManager = { - addEntity: () => entityContainer.generateEntity(true) - } - return networkManager - } - function addEntity() { const entity = entityContainer.generateEntity() return entity @@ -58,6 +45,9 @@ function preEngine(): PreEngine { function removeEntity(entity: Entity) { for (const [, component] of componentsDefinition) { + // TODO: hack for the moment. It should be enough to delete the entity, but the renderer is not cleaning the components. + // So we still need the NetworkEntity to forward this message to the SyncTransport. + if (component.componentId === NetworkEntity.componentId) continue component.entityDeleted(entity, true) } @@ -65,7 +55,10 @@ function preEngine(): PreEngine { } function removeEntityWithChildren(entity: Entity) { - return removeEntityWithChildrenEngine({ removeEntity, defineComponentFromSchema, getEntitiesWith }, entity) + return removeEntityWithChildrenEngine( + { removeEntity, defineComponentFromSchema, getEntitiesWith, defineComponent }, + entity + ) } function registerComponentDefinition( @@ -253,9 +246,7 @@ function preEngine(): PreEngine { registerComponentDefinition, entityContainer, componentsIter, - seal, - addNetworkManager, - getNetworkManager + seal } } /** @@ -311,8 +302,6 @@ export function Engine(options?: IEngineOptions): IEngine { getEntityState: partialEngine.entityContainer.getEntityState, addTransport: crdtSystem.addTransport, - entityContainer: partialEngine.entityContainer, - addNetworkManager: partialEngine.addNetworkManager, - getNetworkManager: partialEngine.getNetworkManager + entityContainer: partialEngine.entityContainer } } diff --git a/packages/@dcl/ecs/src/engine/types.ts b/packages/@dcl/ecs/src/engine/types.ts index 276bbaa53..4fc136d84 100644 --- a/packages/@dcl/ecs/src/engine/types.ts +++ b/packages/@dcl/ecs/src/engine/types.ts @@ -62,8 +62,6 @@ export type PreEngine = Pick< | 'seal' | 'entityContainer' | 'getEntityOrNullByName' - | 'addNetworkManager' - | 'getNetworkManager' > & { getSystems: () => SystemItem[] } @@ -300,15 +298,4 @@ export interface IEngine { * Entity container with custom methods to update their state. */ entityContainer: EntityContainer - - /** - * @alpha - * Initialize network manager - */ - addNetworkManager(reservedLocalEntities: number, range: [number, number]): { addEntity: IEngine['addEntity'] } - /** - * @alpha - * Get netowrk manager to create entities. - */ - getNetworkManager(): ReturnType } diff --git a/packages/@dcl/ecs/src/runtime/helpers/tree.ts b/packages/@dcl/ecs/src/runtime/helpers/tree.ts index 19fe43677..fd0ef4c8c 100644 --- a/packages/@dcl/ecs/src/runtime/helpers/tree.ts +++ b/packages/@dcl/ecs/src/runtime/helpers/tree.ts @@ -1,6 +1,7 @@ import * as components from '../../components' -import { Entity } from '../../engine/entity' +import { Entity, EntityState } from '../../engine/entity' import { ComponentDefinition, IEngine } from '../../engine' +import { NetworkParent } from '../..' function* genEntityTree(entity: Entity, entities: Map): Generator { // This avoid infinite loop when there is a cyclic parenting @@ -45,6 +46,28 @@ export function getComponentEntityTree( return genEntityTree(entity, entities) } +function removeNetworkEntityChildrens( + engine: Pick, + parent: Entity +): void { + const NetworkParent = components.NetworkParent(engine) + const NetworkEntity = components.NetworkEntity(engine) + + // Remove parent + engine.removeEntity(parent) + + // Remove childs + const network = NetworkEntity.getOrNull(parent) + if (network) { + for (const [entity, parent] of engine.getEntitiesWith(NetworkParent)) { + if (parent.entityId === network.entityId && parent.networkId === network.networkId) { + removeNetworkEntityChildrens(engine, entity) + } + } + } + return +} + /** * Remove all components of each entity in the tree made with Transform parenting * @param engine - the engine running the entities @@ -52,11 +75,17 @@ export function getComponentEntityTree( * @public */ export function removeEntityWithChildren( - engine: Pick, + engine: Pick, entity: Entity ) { const Transform = components.Transform(engine) - for (const _entity of getComponentEntityTree(engine, entity, Transform)) { - engine.removeEntity(_entity) + const NetworkEntity = components.NetworkEntity(engine) + + if (NetworkEntity.has(entity)) { + return removeNetworkEntityChildrens(engine, entity) + } + + for (const ent of getComponentEntityTree(engine, entity, Transform)) { + engine.removeEntity(ent) } } diff --git a/packages/@dcl/ecs/src/systems/crdt/index.ts b/packages/@dcl/ecs/src/systems/crdt/index.ts index f16439297..51284162f 100644 --- a/packages/@dcl/ecs/src/systems/crdt/index.ts +++ b/packages/@dcl/ecs/src/systems/crdt/index.ts @@ -234,11 +234,13 @@ export function crdtSceneSystem(engine: PreEngine, onProcessEntityComponentChang for (const entityId of entitiesDeletedThisTick) { const offset = buffer.currentWriteOffset() DeleteEntity.write(entityId, buffer) + crdtMessages.push({ type: CrdtMessageType.DELETE_ENTITY, entityId, messageBuffer: buffer.buffer().subarray(offset, buffer.currentWriteOffset()) }) + onProcessEntityComponentChange && onProcessEntityComponentChange(entityId, CrdtMessageType.DELETE_ENTITY) } diff --git a/packages/@dcl/playground-assets/etc/playground-assets.api.md b/packages/@dcl/playground-assets/etc/playground-assets.api.md index a603d49d5..03ee9d684 100644 --- a/packages/@dcl/playground-assets/etc/playground-assets.api.md +++ b/packages/@dcl/playground-assets/etc/playground-assets.api.md @@ -1144,10 +1144,6 @@ export type GSetComponentGetter; readonly PlayerEntity: Entity; registerComponentDefinition(componentName: string, componentDefinition: ComponentDefinition): ComponentDefinition; // (undocumented) @@ -3544,7 +3538,7 @@ export namespace Rect { } // @public -export function removeEntityWithChildren(engine: Pick, entity: Entity): void; +export function removeEntityWithChildren(engine: Pick, entity: Entity): void; // Warning: (ae-missing-release-tag) "RESERVED_LOCAL_ENTITIES" is part of the package's API, but it is missing a release tag (@alpha, @beta, @public, or @internal) // diff --git a/packages/@dcl/sdk/src/internal/transports/logger.ts b/packages/@dcl/sdk/src/internal/transports/logger.ts index a901fcbd8..02a6a4526 100644 --- a/packages/@dcl/sdk/src/internal/transports/logger.ts +++ b/packages/@dcl/sdk/src/internal/transports/logger.ts @@ -1,4 +1,4 @@ -import { IEngine, CrdtMessage, CrdtMessageType } from '@dcl/ecs' +import { IEngine, CrdtMessage, CrdtMessageType, CRDT_MESSAGE_HEADER_LENGTH } from '@dcl/ecs' import { ReadWriteByteBuffer } from '@dcl/ecs/dist/serialization/ByteBuffer' import { readMessage } from '@dcl/ecs/dist/serialization/crdt/message' @@ -32,7 +32,10 @@ export function* serializeCrdtMessages(prefix: string, data: Uint8Array, engine: } catch { yield `${preface} c=${componentId} t=${timestamp} data=?` } - } else if (message.type === CrdtMessageType.DELETE_ENTITY) { + } else if ( + message.type === CrdtMessageType.DELETE_ENTITY || + message.type === CrdtMessageType.DELETE_ENTITY_NETWORK + ) { yield preface } else { yield `${preface} Unknown CrdtMessageType` diff --git a/test/build-ecs/fixtures/sdk7-humming-birds-sync/src/create-cube.ts b/test/build-ecs/fixtures/sdk7-humming-birds-sync/src/create-cube.ts index c31928711..271cc5e57 100644 --- a/test/build-ecs/fixtures/sdk7-humming-birds-sync/src/create-cube.ts +++ b/test/build-ecs/fixtures/sdk7-humming-birds-sync/src/create-cube.ts @@ -58,7 +58,11 @@ export function createCube(x: number, y: number, z: number, sync: boolean = true eventType: PointerEventType.PET_DOWN, eventInfo: { button: InputAction.IA_POINTER, hoverText: 'Change Color' } }, - { eventType: PointerEventType.PET_DOWN, eventInfo: { button: InputAction.IA_PRIMARY, hoverText: 'Grab Cube' } } + { eventType: PointerEventType.PET_DOWN, eventInfo: { button: InputAction.IA_PRIMARY, hoverText: 'Grab Cube' } }, + { + eventType: PointerEventType.PET_DOWN, + eventInfo: { button: InputAction.IA_SECONDARY, hoverText: 'Remove Cube' } + } ] }) @@ -114,6 +118,9 @@ export function cubeSystem() { if (inputSystem.isTriggered(InputAction.IA_POINTER, PointerEventType.PET_DOWN, entity)) { Material.setPbrMaterial(entity, { albedoColor: Color4.fromHexString(getRandomHexColor()) }) } + if (inputSystem.isTriggered(InputAction.IA_SECONDARY, PointerEventType.PET_DOWN, entity)) { + engine.removeEntityWithChildren(entity) + } if ( inputSystem.isTriggered(InputAction.IA_PRIMARY, PointerEventType.PET_DOWN, entity) && !Grabbed.getOrNull(entity)?.userId