Some editor fixes (#954)

* add unique names for new Nodes

* add free movement gizmo to toolbar

* add tests

packages/@dcl/inspector/src/components/Toolbar/Gizmos/Gizmos.css

@@ -23,10 +23,17 @@
 }
 
 .Gizmos .gizmo.scale {
+  margin-left: 1px;
+  margin-right: 0;
+  border-radius: 0;
+  background-image: url(./icons/scale.svg);
+}
+
+.Gizmos .gizmo.free {
   margin-left: 1px;
   border-top-left-radius: 0px;
   border-bottom-left-radius: 0px;
-  background-image: url(./icons/scale.svg);
+  background-image: url(./icons/free.svg);
 }
 
 .Gizmos .open-panel {
@@ -86,4 +93,4 @@
 .Gizmos .panel .alignment.disabled .icon {
   cursor: not-allowed;
   opacity: 0.5;
-}
+}

packages/@dcl/inspector/src/components/Toolbar/Gizmos/Gizmos.tsx

@@ -41,13 +41,12 @@ export const Gizmos = withSdk(({ sdk }) => {
     [selection, setSelection]
   )
 
-  useHotkey(['M'], handlePositionGizmo)
-  useHotkey(['R'], handleRotationGizmo)
-  useHotkey(['X'], handleScaleGizmo)
+  const handleFreeGizmo = useCallback(() => setSelection({ gizmo: GizmoType.FREE }), [selection, setSelection])
 
   useHotkey(['M'], handlePositionGizmo)
   useHotkey(['R'], handleRotationGizmo)
   useHotkey(['X'], handleScaleGizmo)
+  useHotkey(['F'], handleFreeGizmo)
 
   const {
     isPositionGizmoWorldAligned,
@@ -89,6 +88,12 @@ export const Gizmos = withSdk(({ sdk }) => {
         onClick={handleScaleGizmo}
         title="Scaling tool"
       />
+      <ToolbarButton
+        className={cx('gizmo free', { active: selection?.gizmo === GizmoType.FREE })}
+        disabled={disableGizmos}
+        onClick={handleFreeGizmo}
+        title="Free movement tool"
+      />
       <BsCaretDown className="open-panel" onClick={handleTogglePanel} />
       <div className={cx('panel', { visible: showPanel })}>
         <div className="title">

packages/@dcl/inspector/src/lib/sdk/nodes.ts

@@ -25,7 +25,7 @@ export function getRoot(entity: Entity, nodes: DeepReadonlyObject<Node[]>) {
   return root
 }
 
-function getNodes(engine: IEngine): readonly DeepReadonlyObject<Node>[] {
+export function getNodes(engine: IEngine): readonly DeepReadonlyObject<Node>[] {
   const Nodes = engine.getComponent(EditorComponentNames.Nodes) as EditorComponents['Nodes']
   return Nodes.getOrNull(engine.RootEntity)?.value || []
 }

packages/@dcl/inspector/src/lib/sdk/operations/add-child.spec.ts

@@ -0,0 +1,145 @@
+import { Engine, Entity, IEngine } from '@dcl/ecs'
+
+import addChild, { generateUniqueName, getSuffixDigits } from './add-child'
+import { createComponents, createEditorComponents, SdkComponents } from '../components'
+
+describe('generateUniqueName', () => {
+  let engine: IEngine
+  let Name: SdkComponents['Name']
+  let _addChild: (parent: Entity, name: string) => Entity
+
+  beforeEach(() => {
+    engine = Engine()
+    const coreComponents = createComponents(engine)
+    createEditorComponents(engine)
+    Name = coreComponents.Name
+    _addChild = addChild(engine)
+  })
+
+  it('should return the base name when there are no existing nodes', () => {
+    const result = generateUniqueName(engine, Name, 'SomeName')
+
+    expect(result).toBe('SomeName')
+  })
+
+  it('should return the base name with _1 when the base name already exists', () => {
+    _addChild(engine.RootEntity, 'SomeName')
+
+    const result = generateUniqueName(engine, Name, 'SomeName')
+
+    expect(result).toBe('SomeName_1')
+  })
+
+  it('should return the base name with the next incremented suffix', () => {
+    _addChild(engine.RootEntity, 'SomeName')
+    _addChild(engine.RootEntity, 'SomeName_1')
+    _addChild(engine.RootEntity, 'SomeName_2')
+
+    const result = generateUniqueName(engine, Name, 'SomeName')
+
+    expect(result).toBe('SomeName_3')
+  })
+
+  it('should not match names with multiple underscore-separated numbers', () => {
+    _addChild(engine.RootEntity, 'SomeName_123_1')
+
+    const result = generateUniqueName(engine, Name, 'SomeName')
+
+    expect(result).toBe('SomeName')
+  })
+
+  it('should handle mixed case names correctly', () => {
+    _addChild(engine.RootEntity, 'someName')
+    _addChild(engine.RootEntity, 'SomeName_1')
+
+    const result = generateUniqueName(engine, Name, 'SomeName')
+
+    expect(result).toBe('SomeName_2')
+  })
+
+  it('should return the base name with _1 when the base name exists in mixed case', () => {
+    _addChild(engine.RootEntity, 'SomeName')
+    _addChild(engine.RootEntity, 'SOMENAME_1')
+
+    const result = generateUniqueName(engine, Name, 'SomeName')
+
+    expect(result).toBe('SomeName_2')
+  })
+
+  it('should handle names with an underscore and valid digits', () => {
+    _addChild(engine.RootEntity, 'SomeName_1')
+    _addChild(engine.RootEntity, 'SomeName_2')
+    _addChild(engine.RootEntity, 'SomeName_3')
+
+    const result = generateUniqueName(engine, Name, 'SomeName')
+
+    expect(result).toBe('SomeName_4')
+  })
+
+  it('should handle names with only digits after underscore', () => {
+    _addChild(engine.RootEntity, 'SomeName')
+    _addChild(engine.RootEntity, 'SomeName_10')
+
+    const result = generateUniqueName(engine, Name, 'SomeName')
+
+    expect(result).toBe('SomeName_11')
+  })
+
+  it('should handle names with no digits after underscore', () => {
+    _addChild(engine.RootEntity, 'SomeName_')
+
+    const result = generateUniqueName(engine, Name, 'SomeName')
+
+    expect(result).toBe('SomeName')
+  })
+})
+
+describe('getSuffixDigits', () => {
+  it('should return -1 if there is no underscore', () => {
+    expect(getSuffixDigits('SomeName')).toBe(-1)
+  })
+
+  it('should return -1 if the string ends with an underscore', () => {
+    expect(getSuffixDigits('SomeName_')).toBe(-1)
+  })
+
+  it('should return the digits after the last underscore if they are valid numbers', () => {
+    expect(getSuffixDigits('SomeName_123')).toBe(123)
+  })
+
+  it('should return -1 if the part after the last underscore is not a valid number', () => {
+    expect(getSuffixDigits('SomeName_abc')).toBe(-1)
+  })
+
+  it('should return -1 if there is only one underscore without digits', () => {
+    expect(getSuffixDigits('SomeName_')).toBe(-1)
+  })
+
+  it('should return the correct number for multiple underscores with valid digits at the end', () => {
+    expect(getSuffixDigits('SomeName_1_2_3')).toBe(3)
+  })
+
+  it('should return -1 if there are multiple underscores and the last part is not a valid number', () => {
+    expect(getSuffixDigits('SomeName_1_2_abc')).toBe(-1)
+  })
+
+  it('should return -1 for an empty string', () => {
+    expect(getSuffixDigits('')).toBe(-1)
+  })
+
+  it('should return -1 if the string is just an underscore', () => {
+    expect(getSuffixDigits('_')).toBe(-1)
+  })
+
+  it('should return -1 if there are no digits after the underscore', () => {
+    expect(getSuffixDigits('SomeName_')).toBe(-1)
+  })
+
+  it('should handle strings with digits but no underscore correctly', () => {
+    expect(getSuffixDigits('123')).toBe(-1)
+  })
+
+  it('should return -1 if there are multiple underscores with no digits after the last underscore', () => {
+    expect(getSuffixDigits('SomeName_1_')).toBe(-1)
+  })
+})

packages/@dcl/inspector/src/lib/sdk/operations/add-child.ts

@@ -1,6 +1,7 @@
-import { Entity, IEngine, Transform as TransformEngine, Name as NameEngine } from '@dcl/ecs'
+import { Entity, IEngine, Transform as TransformEngine, Name as NameEngine, NameComponent } from '@dcl/ecs'
+
 import { EditorComponentNames, EditorComponents } from '../components'
-import { pushChild } from '../nodes'
+import { getNodes, pushChild } from '../nodes'
 
 export function addChild(engine: IEngine) {
   return function addChild(parent: Entity, name: string): Entity {
@@ -10,7 +11,7 @@ export function addChild(engine: IEngine) {
     const Name = engine.getComponent(NameEngine.componentName) as typeof NameEngine
 
     // create new child components
-    Name.create(child, { value: name })
+    Name.create(child, { value: generateUniqueName(engine, Name, name) })
     Transform.create(child, { parent })
     // update Nodes component
     Nodes.createOrReplace(engine.RootEntity, { value: pushChild(engine, parent, child) })
@@ -19,4 +20,34 @@ export function addChild(engine: IEngine) {
   }
 }
 
+export function generateUniqueName(engine: IEngine, Name: NameComponent, value: string): string {
+  const pattern = new RegExp(`^${value.toLowerCase()}(_\\d+)?$`, 'i')
+  const nodes = getNodes(engine)
+
+  let isFirst = true
+  let max = 0
+  for (const $ of nodes) {
+    const name = (Name.getOrNull($.entity)?.value || '').toLowerCase()
+    if (pattern.test(name)) {
+      isFirst = false
+      const suffix = getSuffixDigits(name)
+      if (suffix !== -1) {
+        max = Math.max(max, suffix)
+      }
+    }
+  }
+
+  const suffix = isFirst ? '' : `_${max + 1}`
+
+  return `${value}${suffix}`
+}
+
+export function getSuffixDigits(name: string): number {
+  const underscoreIndex = name.lastIndexOf('_')
+  if (underscoreIndex === -1 || underscoreIndex === name.length - 1) return -1
+
+  const digits = name.slice(underscoreIndex + 1)
+  return /^\d+$/.test(digits) ? parseInt(digits, 10) : -1
+}
+
 export default addChild

packages/@dcl/inspector/src/lib/sdk/operations/duplicate-entity.spec.ts

@@ -59,7 +59,7 @@ describe('duplicateEntity', () => {
     expect(duplicateChild).not.toBe(original)
     expect(duplicateChild).not.toBe(originalChild)
     expect(duplicateChild).not.toBe(duplicate)
-    expect(NameComponent.get(duplicateChild!).value).toBe(NameComponent.get(originalChild).value)
+    expect(NameComponent.get(duplicateChild!).value).toBe(`${NameComponent.get(originalChild).value}_1`)
     expect(Nodes.get(ROOT).value).toStrictEqual(nodesAfter)
   })
 })

packages/@dcl/inspector/src/lib/sdk/operations/duplicate-entity.ts

@@ -1,14 +1,22 @@
-import { Entity, IEngine, Transform as TransformEngine, NetworkEntity as NetworkEntityEngine } from '@dcl/ecs'
+import {
+  Entity,
+  IEngine,
+  Transform as TransformEngine,
+  NetworkEntity as NetworkEntityEngine,
+  Name as NameEngine
+} from '@dcl/ecs'
 import { clone } from '@dcl/asset-packs'
 import { EditorComponentNames, EditorComponents } from '../components'
 import { pushChild } from '../nodes'
 import updateSelectedEntity from './update-selected-entity'
+import { generateUniqueName } from './add-child'
 
 export function duplicateEntity(engine: IEngine) {
   return function duplicateEntity(entity: Entity) {
     const Transform = engine.getComponent(TransformEngine.componentId) as typeof TransformEngine
     const Nodes = engine.getComponent(EditorComponentNames.Nodes) as EditorComponents['Nodes']
     const Triggers = engine.getComponent(EditorComponentNames.Triggers) as EditorComponents['Triggers']
+    const Name = engine.getComponent(NameEngine.componentName) as typeof NameEngine
     const NetworkEntity = engine.getComponent(NetworkEntityEngine.componentId) as typeof NetworkEntityEngine
 
     const { entities } = clone(entity, engine as any, Transform as any, Triggers as any) as {
@@ -20,6 +28,9 @@ export function duplicateEntity(engine: IEngine) {
         NetworkEntity.createOrReplace(duplicate, { entityId: duplicate, networkId: 0 })
       }
 
+      const originalName = Name.getOrNull(original)?.value
+      Name.createOrReplace(duplicate, { value: generateUniqueName(engine, Name, originalName || '') })
+
       const transform = Transform.getMutableOrNull(duplicate)
       if (transform === null || !transform.parent) {
         Nodes.createOrReplace(engine.RootEntity, { value: pushChild(engine, engine.RootEntity, duplicate) })