From a7ab97aa9d8bdf66cfc414e3884ef7163d912fae Mon Sep 17 00:00:00 2001 From: Gon Pombo Date: Mon, 28 Nov 2022 22:31:23 -0300 Subject: [PATCH] fix: enable CRDT engine to work with unknown component IDs (#338) * fix missing components * update api-extractor * linter; * ignore statement in coverage * test coverage * test coverage Co-authored-by: menduz --- packages/@dcl/ecs/src/engine/index.ts | 12 ++++++++++++ packages/@dcl/ecs/src/engine/types.ts | 11 +++++++++++ packages/@dcl/ecs/src/systems/crdt/index.ts | 15 +++++++++++---- .../etc/playground-assets.api.md | 1 + packages/@dcl/playground-assets/package-lock.json | 4 ++-- scripts/prepare.spec.ts | 7 ++++++- .../fixtures/side-effect-free-build/src/index.ts | 3 ++- 7 files changed, 45 insertions(+), 8 deletions(-) diff --git a/packages/@dcl/ecs/src/engine/index.ts b/packages/@dcl/ecs/src/engine/index.ts index 388475faf..fac8ec68c 100644 --- a/packages/@dcl/ecs/src/engine/index.ts +++ b/packages/@dcl/ecs/src/engine/index.ts @@ -112,6 +112,16 @@ function preEngine() { return component } + function getComponentOrNull, V>( + componentId: number + ): ComponentDefinition | null { + return ( + componentsDefinition.get(componentId) ?? + /* istanbul ignore next */ + null + ) + } + function* getEntitiesWith< T extends [ ComponentDefinition, @@ -166,6 +176,7 @@ function preEngine() { defineComponentFromSchema, getEntitiesWith, getComponent, + getComponentOrNull, removeComponentDefinition } } @@ -249,6 +260,7 @@ export function Engine(): IEngine { defineComponentFromSchema: engine.defineComponentFromSchema, getEntitiesWith: engine.getEntitiesWith, getComponent: engine.getComponent, + getComponentOrNull: engine.getComponentOrNull, removeComponentDefinition: engine.removeComponentDefinition, update, RootEntity: 0 as Entity, diff --git a/packages/@dcl/ecs/src/engine/types.ts b/packages/@dcl/ecs/src/engine/types.ts index a3aef6154..41cef160a 100644 --- a/packages/@dcl/ecs/src/engine/types.ts +++ b/packages/@dcl/ecs/src/engine/types.ts @@ -135,6 +135,17 @@ export type IEngine = { */ getComponent(componentId: number): CompDef + /** + * Get the component definition from the component id. + * @param componentId - component number used to identify the component descriptor + * @returns the component definition or null if its not founded + * ```ts + * const StateComponentId = 10023 + * const StateComponent = engine.getComponent(StateComponentId) + * ``` + */ + getComponentOrNull(componentId: number): CompDef | null + /** * Get a iterator of entities that has all the component requested. * @param components - a list of component definitions diff --git a/packages/@dcl/ecs/src/systems/crdt/index.ts b/packages/@dcl/ecs/src/systems/crdt/index.ts index 9e77a4e44..24bc75aa8 100644 --- a/packages/@dcl/ecs/src/systems/crdt/index.ts +++ b/packages/@dcl/ecs/src/systems/crdt/index.ts @@ -7,7 +7,7 @@ import { ComponentOperation as Message } from '../../serialization/crdt/componen import WireMessage from '../../serialization/wireMessage' import { ReceiveMessage, TransportMessage, Transport } from './types' -export function crdtSceneSystem(engine: Pick) { +export function crdtSceneSystem(engine: Pick) { const transports: Transport[] = [] // CRDT Client @@ -81,9 +81,12 @@ export function crdtSceneSystem(engine: Pick) { data: data || null, timestamp: timestamp } - const component = engine.getComponent(componentId) + const component = engine.getComponentOrNull(componentId) const current = crdtClient.processMessage(crdtMessage) - + /* istanbul ignore next */ + if (!component) + // TODO: TEST + continue // CRDT outdated message. Resend this message through the wire if (crdtMessage !== current) { const type = component.has(entity) @@ -126,7 +129,11 @@ export function crdtSceneSystem(engine: Pick) { for (const [entity, componentsId] of dirtyMap) { for (const componentId of componentsId) { - const component = engine.getComponent(componentId) + const component = engine.getComponentOrNull(componentId) + /* istanbul ignore next */ + if (!component) + // TODO: test coverage + continue const entityComponent = component.has(entity) ? component.toBinary(entity).toBinary() : null diff --git a/packages/@dcl/playground-assets/etc/playground-assets.api.md b/packages/@dcl/playground-assets/etc/playground-assets.api.md index 74d80b85b..c0cc793b3 100644 --- a/packages/@dcl/playground-assets/etc/playground-assets.api.md +++ b/packages/@dcl/playground-assets/etc/playground-assets.api.md @@ -615,6 +615,7 @@ export type IEngine = { defineComponent>>(spec: T, componentId: number, constructorDefault?: ConstructorType): ComponentDefinition>, Partial>>; defineComponentFromSchema, ConstructorType>(spec: T, componentId: number, constructorDefault?: ConstructorType): ComponentDefinition; getComponent(componentId: number): ComponentDefinition; + getComponentOrNull(componentId: number): ComponentDefinition | null; getEntitiesWith, ...ComponentDefinition[]]>(...components: T): Iterable<[Entity, ...ReadonlyComponentSchema]>; update(deltaTime: number): void; readonly RootEntity: Entity; diff --git a/packages/@dcl/playground-assets/package-lock.json b/packages/@dcl/playground-assets/package-lock.json index bac74411a..02fbb11c3 100644 --- a/packages/@dcl/playground-assets/package-lock.json +++ b/packages/@dcl/playground-assets/package-lock.json @@ -20,7 +20,7 @@ "@dcl/ecs": "file:../ecs", "@dcl/ecs-math": "^2.0.1-20221108141807.commit-a1344cb", "@dcl/js-runtime": "file:../js-runtime", - "@dcl/kernel": "^2.0.0-3565941736.commit-96b03fd", + "@dcl/kernel": "^2.0.0-3568680164.commit-ec0d714", "@dcl/react-ecs": "file:../react-ecs", "@dcl/unity-renderer": "^1.0.64444-20221125155434.commit-fcef3c4" } @@ -38,7 +38,7 @@ "@dcl/ecs": "file:../ecs", "@dcl/ecs-math": "^2.0.1-20221108141807.commit-a1344cb", "@dcl/js-runtime": "file:../js-runtime", - "@dcl/kernel": "^2.0.0-3565941736.commit-96b03fd", + "@dcl/kernel": "^2.0.0-3568680164.commit-ec0d714", "@dcl/react-ecs": "file:../react-ecs", "@dcl/unity-renderer": "^1.0.64444-20221125155434.commit-fcef3c4" } diff --git a/scripts/prepare.spec.ts b/scripts/prepare.spec.ts index abae6b060..7743679b5 100644 --- a/scripts/prepare.spec.ts +++ b/scripts/prepare.spec.ts @@ -5,7 +5,8 @@ import { ROLLUP_CONFIG_PATH, JS_RUNTIME, ECS7_PATH, - REACT_ECS + REACT_ECS, + PLAYGROUND_ASSETS_PATH } from './common' import { @@ -25,6 +26,10 @@ flow('build-all', () => { itInstallsADependencyFromFolderAndCopiesTheVersion(SDK_PATH, ECS7_PATH) itInstallsADependencyFromFolderAndCopiesTheVersion(SDK_PATH, REACT_ECS) itInstallsADependencyFromFolderAndCopiesTheVersion(SDK_PATH, JS_RUNTIME) + itInstallsADependencyFromFolderAndCopiesTheVersion( + PLAYGROUND_ASSETS_PATH, + SDK_PATH + ) }) flow('pack every package', () => { diff --git a/test/build-ecs/fixtures/side-effect-free-build/src/index.ts b/test/build-ecs/fixtures/side-effect-free-build/src/index.ts index fa93cee4b..a68735d75 100644 --- a/test/build-ecs/fixtures/side-effect-free-build/src/index.ts +++ b/test/build-ecs/fixtures/side-effect-free-build/src/index.ts @@ -1,2 +1,3 @@ -// This exists to evaluate the final size of a side-effect free bundle +// This file exists to evaluate the final size of a side-effect free bundle +// which should be `"use strict";` only import '@dcl/ecs'