fix(composer): drag root node crashes scene (#372)

awslabs · Nov 16, 2022 · ca01c40 · ca01c40
1 parent 69fc869
commit ca01c40
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 11 deletions.
diff --git a/...s/scene-composer/src/components/panels/SceneHierarchyPanel/SceneHierarchyDataProvider.tsx b/...s/scene-composer/src/components/panels/SceneHierarchyPanel/SceneHierarchyDataProvider.tsx
@@ -105,7 +105,7 @@ const SceneHierarchyDataProvider: FC<SceneHierarchyDataProviderProps> = ({ selec
   const sceneComposerId = useSceneComposerId();
   const { document, removeSceneNode } = useStore(sceneComposerId)((state) => state);
   const { isEditing } = useStore(sceneComposerId)((state) => state);
-  const { updateSceneNodeInternal } = useSceneDocument(sceneComposerId);
+  const { updateSceneNodeInternal, updateDocumentInternal } = useSceneDocument(sceneComposerId);
   const selectedSceneNodeRef = useStore(sceneComposerId)((state) => state.selectedSceneNodeRef);
   const getSceneNodeByRef = useStore(sceneComposerId)((state) => state.getSceneNodeByRef);
   const getObject3DBySceneNodeRef = useStore(sceneComposerId)((state) => state.getObject3DBySceneNodeRef);
@@ -190,15 +190,20 @@ const SceneHierarchyDataProvider: FC<SceneHierarchyDataProviderProps> = ({ selec
         const oldParent = getSceneNodeByRef(oldParentRef);
         const oldParentChildren = oldParent?.childRefs.filter((child) => child !== objectToMoveRef);
         // remove child ref from parent
-        updateSceneNodeInternal(oldParentRef, { childRefs: oldParentChildren });
+        if (!oldParentRef) {
+          const newRoots = document.rootNodeRefs.filter((ref) => ref !== objectRef);
+          updateDocumentInternal({ rootNodeRefs: newRoots });
+        } else {
+          updateSceneNodeInternal(oldParentRef, { childRefs: oldParentChildren });
+        }
         // update node to have new parent
         updateSceneNodeInternal(objectToMoveRef, { parentRef: newParentRef });
         // update new parent to have new child
         updateSceneNodeInternal(newParentRef, { childRefs: [...newParent!.childRefs, objectRef] });
         // TODO: create single call to handle this
       }
     },
-    [updateSceneNodeInternal, getSceneNodeByRef, nodeMap],
+    [updateSceneNodeInternal, updateDocumentInternal, getSceneNodeByRef, nodeMap, document],
   );
 
   const show = useCallback(

diff --git a/packages/scene-composer/src/store/helpers/serializationHelpers.ts b/packages/scene-composer/src/store/helpers/serializationHelpers.ts
@@ -1,3 +1,5 @@
+import { isNumber } from 'lodash';
+
 import DebugLogger from '../../logger/DebugLogger';
 import {
   DEFAULT_DISTANCE_UNIT,
@@ -632,15 +634,15 @@ function convertNodes(
     const exportedNode = exportedNodes[index];
     if (node.childRefs && node.childRefs.length > 0) {
       // we know the indexMap will not return undefined
-      exportedNode.children = node.childRefs.map((ref) => nodeRefToIndexMap[ref]) as number[];
+      exportedNode.children = node.childRefs.map((ref) => nodeRefToIndexMap[ref]).filter((index) => isNumber(index));
     }
   }
 
   return exportedNodes;
 }
 
 function convertNodeIndexes(nodeRefs: string[], indexMap: Record<string, number>): number[] {
-  return nodeRefs.map((ref) => indexMap[ref]) as number[];
+  return nodeRefs.map((ref) => indexMap[ref]).filter((index) => isNumber(index));
 }
 
 // Component is really flexible so what this function does is just a shape change

diff --git a/packages/scene-composer/src/store/slices/SceneDocumentSlice.ts b/packages/scene-composer/src/store/slices/SceneDocumentSlice.ts
@@ -37,7 +37,7 @@ export interface ISceneDocumentSlice {
   getSceneNodesByRefs(refs: (string | undefined)[]): (ISceneNodeInternal | undefined)[];
   appendSceneNodeInternal(node: ISceneNodeInternal, parentRef?: string): void;
   updateSceneNodeInternal(ref: string, partial: RecursivePartial<ISceneNodeInternal>, isTransient?: boolean): void;
-  updateDocumentInternal(partial: RecursivePartial<Pick<ISceneDocumentInternal, 'unit'>>): void;
+  updateDocumentInternal(partial: RecursivePartial<Pick<ISceneDocumentInternal, 'unit' | 'rootNodeRefs'>>): void;
   listSceneRuleMapIds(): string[];
   getSceneRuleMapById(id?: string): Readonly<IRuleBasedMapInternal> | undefined;
   updateSceneRuleMapById(id: string, ruleMap: IRuleBasedMapInternal): void;

diff --git a/packages/scene-composer/stories/SceneComposer.stories.tsx b/packages/scene-composer/stories/SceneComposer.stories.tsx
@@ -144,14 +144,14 @@ const knobsConfigurationDecorator = [
     args.config = {
       featureConfig: {
         [COMPOSER_FEATURES.SceneHierarchyRedesign]: true, // New Scene Hierarchy Panel
-        [COMPOSER_FEATURES.SceneHierarchySearch]: false, // Entity Search
+        [COMPOSER_FEATURES.SceneHierarchySearch]: true, // Entity Search
         [COMPOSER_FEATURES.SceneHierarchyMultiSelect]: false, // MultiSelect disabled, not sure if we will support this.
-        [COMPOSER_FEATURES.SceneHierarchyReorder]: false, // Drag/Drop Reordering
-        [COMPOSER_FEATURES.SubModelSelection]: false,
+        [COMPOSER_FEATURES.SceneHierarchyReorder]: true, // Drag/Drop Reordering
+        [COMPOSER_FEATURES.SubModelSelection]: true,
         [COMPOSER_FEATURES.ENHANCED_EDITING]: true,
         [COMPOSER_FEATURES.CameraView]: true,
         [COMPOSER_FEATURES.EnvironmentModel]: false,
-        [COMPOSER_FEATURES.TagResize]: true,
+        [COMPOSER_FEATURES.TagResize]: false,
         [COMPOSER_FEATURES.SubModelMovement]: false,
         ...args.config.featureConfig,
       },
@@ -266,7 +266,7 @@ Default.decorators = knobsConfigurationDecorator;
 Default.args = {
   onSceneUpdated: (e) => {
     // empty to avoid state being printed out
-    // console.log('document changed', e.serialize('1'));
+    // console.log('document changed', e.serialize('1.0'));
   },
   dataStreams: convertDataInputToDataStreams(getTestDataInputContinuous()),
   viewport: {