fix(composer): Fixes the Duplication of sub models on scene reload by…

… adding persistant identifier to the sub model (#356)
awslabs · Nov 11, 2022 · 446a4ca · 446a4ca
1 parent 7b952b9
commit 446a4ca
Show file tree

Hide file tree

Showing 5 changed files with 71 additions and 11 deletions.
diff --git a/packages/scene-composer/package.json b/packages/scene-composer/package.json
@@ -154,10 +154,10 @@
   "jest": {
     "coverageThreshold": {
       "global": {
-        "lines": 77.49,
-        "statements": 76.63,
-        "functions": 77.01,
-        "branches": 63.2,
+        "lines": 77.51,
+        "statements": 76.64,
+        "functions": 77.1,
+        "branches": 63.32,
         "branchesTrue": 100
       }
     }

diff --git a/...onents/panels/SceneHierarchyPanel/components/SubModelTree/__tests__/SubModelTree.spec.tsx b/...onents/panels/SceneHierarchyPanel/components/SubModelTree/__tests__/SubModelTree.spec.tsx
@@ -21,7 +21,10 @@ jest.mock('../SubModelTreeItemLabel', () => (props) => <div data-mocked='SubMode
 
 jest.mock('../../../../../../store', () => ({
   ...jest.requireActual('../../../../../../store'),
-  useSceneDocument: jest.fn(() => ({ appendSceneNodeInternal: jest.fn() })),
+  useSceneDocument: jest.fn(() => ({
+    appendSceneNodeInternal: jest.fn(),
+    document: { nodeMap: {} },
+  })),
   useEditorState: jest.fn(() => ({ setSceneNodeObject3DMapping: jest.fn() })),
 }));
 
@@ -148,7 +151,10 @@ describe('SubModelTree', () => {
 
       const parentRef = '112';
 
-      (useSceneDocument as jest.Mock).mockImplementation(() => ({ appendSceneNodeInternal }));
+      (useSceneDocument as jest.Mock).mockImplementation(() => ({
+        appendSceneNodeInternal,
+        document: { nodeMap: {} },
+      }));
 
       (useEditorState as jest.Mock).mockImplementation(() => ({ setSceneNodeObject3DMapping }));
 
@@ -164,5 +170,46 @@ describe('SubModelTree', () => {
       expect(setSceneNodeObject3DMapping).toBeCalled();
       expect(appendSceneNodeInternal.mock.calls[0]).toMatchSnapshot();
     });
+
+    it('should not append duplicate node onCreate', () => {
+      const appendSceneNodeInternal = jest.fn();
+      const setSceneNodeObject3DMapping = jest.fn();
+      const object = {
+        name: 'RootObject',
+        userData: { isOriginal: true },
+        position: { x: 1, y: 1, z: 1 },
+        rotation: { x: 1, y: 1, z: 1 },
+        scale: { x: 1, y: 1, z: 1 },
+        children: [] as unknown as Object3D<Event>[],
+      } as unknown as Object3D<Event>;
+
+      const parentRef = '112';
+
+      (useSceneDocument as jest.Mock).mockImplementation(() => ({
+        appendSceneNodeInternal,
+        document: {
+          nodeMap: {
+            RootObject: {
+              properties: {
+                subModelId: 'RootObject',
+              },
+            },
+          },
+        },
+      }));
+
+      (useEditorState as jest.Mock).mockImplementation(() => ({ setSceneNodeObject3DMapping }));
+
+      (useMaterialEffect as jest.Mock).mockImplementation(() => [jest.fn(), jest.fn()]);
+
+      render(<SubModelTree parentRef={parentRef} object3D={object} />);
+
+      // Act
+      const [, onCreate] = callbacks;
+      onCreate();
+
+      expect(appendSceneNodeInternal).not.toBeCalled();
+      expect(setSceneNodeObject3DMapping).not.toBeCalled();
+    });
   });
 });
diff --git a/...HierarchyPanel/components/SubModelTree/__tests__/__snapshots__/SubModelTree.spec.tsx.snap b/...HierarchyPanel/components/SubModelTree/__tests__/__snapshots__/SubModelTree.spec.tsx.snap
@@ -14,7 +14,9 @@ Array [
     ],
     "name": "RootObject",
     "parentRef": "112",
-    "properties": Array [],
+    "properties": Object {
+      "subModelId": "RootObject",
+    },
     "ref": "112#undefined",
     "transform": Object {
       "position": Array [

diff --git a/...cene-composer/src/components/panels/SceneHierarchyPanel/components/SubModelTree/index.tsx b/...cene-composer/src/components/panels/SceneHierarchyPanel/components/SubModelTree/index.tsx
@@ -27,7 +27,7 @@ const SubModelTree: FC<SubModelTreeProps> = ({
   visible: defaultVisible = true,
 }) => {
   const sceneComposerId = useSceneComposerId();
-  const { appendSceneNodeInternal } = useSceneDocument(sceneComposerId);
+  const { appendSceneNodeInternal, document } = useSceneDocument(sceneComposerId);
   const { setSceneNodeObject3DMapping } = useEditorState(sceneComposerId);
   const [expanded, setExpanded] = useState(defaultExpanded);
   const [visible, setVisible] = useState(defaultVisible);
@@ -54,6 +54,15 @@ const SubModelTree: FC<SubModelTreeProps> = ({
   }, []);
 
   const onCreate = useCallback(() => {
+    // Prevent duplicates
+    const duplicates = Object.entries(document.nodeMap).filter(
+      ([name, node]) => node.properties?.subModelId === object3D.name,
+    );
+
+    if (duplicates.length > 0) {
+      return;
+    }
+
     const nodeRef = `${parentRef}#${object3D.id}`;
     const subModelComponent: ISubModelRefComponent = {
       type: KnownComponentType.SubModelRef,
@@ -76,12 +85,14 @@ const SubModelTree: FC<SubModelTreeProps> = ({
         snapToFloor: true,
       },
       childRefs: [],
-      properties: [],
+      properties: {
+        subModelId: object3D.name,
+      },
     } as ISceneNodeInternal;
 
     appendSceneNodeInternal(node);
     setSceneNodeObject3DMapping(nodeRef, object3D); // Cache Reference
-  }, [object3D]);
+  }, [object3D, document.nodeMap]);
 
   const onHover = useCallback((e) => {
     e.preventDefault();

diff --git a/packages/scene-composer/tests/scenes/scene_1.json b/packages/scene-composer/tests/scenes/scene_1.json
@@ -250,7 +250,7 @@
         }
       ],
       "properties": {
-
+        "subModelId": "Scene491"
       }
     },
     {