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 <[email protected]>
decentraland · Nov 29, 2022 · a7ab97a · a7ab97a
1 parent fb5b256
commit a7ab97a
Show file tree

Hide file tree

Showing 7 changed files with 45 additions and 8 deletions.
diff --git 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<T extends ISchema<V>, V>(
+    componentId: number
+  ): ComponentDefinition<T, V> | null {
+    return (
+      componentsDefinition.get(componentId) ??
+      /* istanbul ignore next */
+      null
+    )
+  }
+
   function* getEntitiesWith<
     T extends [
       ComponentDefinition<any, any>,
@@ -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
@@ -135,6 +135,17 @@ export type IEngine = {
    */
   getComponent<T extends ISchema>(componentId: number): CompDef<T>
 
+  /**
+   * 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<T extends ISchema>(componentId: number): CompDef<T> | 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
@@ -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<IEngine, 'getComponent'>) {
+export function crdtSceneSystem(engine: Pick<IEngine, 'getComponentOrNull'>) {
   const transports: Transport[] = []
 
   // CRDT Client
@@ -81,9 +81,12 @@ export function crdtSceneSystem(engine: Pick<IEngine, 'getComponent'>) {
           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<IEngine, 'getComponent'>) {
 
     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
@@ -615,6 +615,7 @@ export type IEngine = {
     defineComponent<T extends Spec, ConstructorType = Partial<Result<T>>>(spec: T, componentId: number, constructorDefault?: ConstructorType): ComponentDefinition<ISchema<Result<T>>, Partial<Result<T>>>;
     defineComponentFromSchema<T extends ISchema<ConstructorType>, ConstructorType>(spec: T, componentId: number, constructorDefault?: ConstructorType): ComponentDefinition<T, ConstructorType>;
     getComponent<T extends ISchema>(componentId: number): ComponentDefinition<T>;
+    getComponentOrNull<T extends ISchema>(componentId: number): ComponentDefinition<T> | null;
     getEntitiesWith<T extends [ComponentDefinition<any>, ...ComponentDefinition<any>[]]>(...components: T): Iterable<[Entity, ...ReadonlyComponentSchema<T>]>;
     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
diff --git 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
@@ -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'