Improvement when positioning objects with brush none

Author: ClementPasteau

newIDE/app/src/EditorFunctions/EditorFunctions.spec.js

@@ -2824,22 +2824,33 @@ describe('editorFunctions', () => {
       expect(getInstancePositions(testScene)).toEqual([{ x: 300, y: 400 }]);
     });
 
-    // The none brush must still leave existing instances where they are.
-    it('does not move an existing instance edited with the none brush', async () => {
+    // A position passed with the none brush would be silently ignored, which
+    // agents misread as a move failure: the call must fail explicitly.
+    it('fails when a brush_position is passed with the none brush', async () => {
       await putInstances({
         brush_kind: 'point',
         brush_position: '100,200',
         new_instances_count: 1,
       });
       const [created] = getInstances(testScene);
 
-      await putInstances({
-        brush_kind: 'none',
-        brush_position: '640,360',
-        existing_instance_ids: created.uuid,
-        instances_size: '48,48',
+      const result = await editorFunctions.put_2d_instances.launchFunction({
+        ...makeFakeLaunchFunctionOptionsWithProject(project),
+        args: {
+          scene_name: 'TestScene',
+          object_name: 'Player',
+          layer_name: '',
+          brush_kind: 'none',
+          brush_position: '640,360',
+          existing_instance_ids: created.uuid,
+          instances_size: '48,48',
+        },
       });
 
+      expect(result.success).toBe(false);
+      expect(result.message).toEqual(
+        expect.stringContaining('never moves instances')
+      );
       expect(getInstancePositions(testScene)).toEqual([{ x: 100, y: 200 }]);
     });
 
@@ -3370,6 +3381,37 @@ describe('editorFunctions', () => {
       expect(getInstancePositions(testScene)).toEqual([]);
     });
 
+    // A position passed with the none brush would be silently ignored, which
+    // agents misread as a move failure: the call must fail explicitly.
+    it('fails when a brush_position is passed with the none brush', async () => {
+      await putInstances({
+        brush_kind: 'point',
+        brush_position: '10,20,30',
+        new_instances_count: 1,
+      });
+      const [created] = getInstances(testScene);
+
+      const result = await editorFunctions.put_3d_instances.launchFunction({
+        ...makeFakeLaunchFunctionOptionsWithProject(project),
+        args: {
+          scene_name: 'TestScene',
+          object_name: 'Player',
+          layer_name: '',
+          brush_kind: 'none',
+          brush_position: '10,20,150',
+          existing_instance_ids: created.uuid,
+        },
+      });
+
+      expect(result.success).toBe(false);
+      expect(result.message).toEqual(
+        expect.stringContaining('never moves instances')
+      );
+      expect(getInstancePositions(testScene)).toEqual([
+        { x: 10, y: 20, z: 30 },
+      ]);
+    });
+
     // The created id in the message closes the loop "create then adjust":
     // it must be directly usable as `existing_instance_ids`.
     it('reports the created instance id, usable to modify it in a follow-up call', async () => {

newIDE/app/src/EditorFunctions/index.js

@@ -3497,6 +3497,13 @@ const put2dInstances: EditorFunction = {
             : `A valid \`brush_position\` is required for the "${brush_kind}" brush (or use the "none" brush to modify existing instances without moving them).`
         );
       }
+      // The "none" brush never applies a position: fail instead of silently
+      // ignoring it, so the caller switches to a placement brush to move.
+      if (brush_kind === 'none' && parsedBrushPosition) {
+        return makeGenericFailure(
+          'The "none" brush never moves instances: remove `brush_position`, or use the "point" brush to move (it sets the exact position of every matched instance — one call per target position).'
+        );
+      }
       // After the guard, a missing position can only happen when nothing is
       // created nor moved ("none" brush only): the fallback is never used.
       const brushPosition: [number, number] = parsedBrushPosition || [0, 0];
@@ -3879,14 +3886,18 @@ const put2dInstances: EditorFunction = {
           return makeGenericFailure(
             `Matched ${matchedCount} existing instance${
               matchedCount > 1 ? 's' : ''
-            } but no change was requested — provide a value to modify (see the tool parameters for what can be edited).`
+            } but no change was requested — provide a value to modify, or use the "point" brush with \`brush_position\` to move.`
           );
         }
 
         return makeGenericFailure(
           `Matched ${matchedCount} existing instance${
             matchedCount > 1 ? 's' : ''
-          } but the requested values are identical to their current ones, so nothing changed.`
+          } but the requested values are identical to their current ones, so nothing changed.${
+            hasPositionBrush
+              ? ''
+              : ' To move instances, use the "point" brush with `brush_position` (the "none" brush never changes position).'
+          }`
         );
       }
 
@@ -4241,6 +4252,13 @@ const put3dInstances: EditorFunction = {
             : `A valid \`brush_position\` is required for the "${brush_kind}" brush (or use the "none" brush to modify existing instances without moving them).`
         );
       }
+      // The "none" brush never applies a position: fail instead of silently
+      // ignoring it, so the caller switches to a placement brush to move.
+      if (brush_kind === 'none' && parsedBrushPosition) {
+        return makeGenericFailure(
+          'The "none" brush never moves instances: remove `brush_position`, or use the "point" brush to move (it sets the exact X, Y and Z of every matched instance — one call per target position).'
+        );
+      }
       // After the guard, a missing position can only happen when nothing is
       // created nor moved ("none" brush only): the fallback is never used.
       const brushPosition: [number, number, number] = parsedBrushPosition || [
@@ -4565,14 +4583,18 @@ const put3dInstances: EditorFunction = {
           return makeGenericFailure(
             `Matched ${matchedCount} existing instance${
               matchedCount > 1 ? 's' : ''
-            } but no change was requested — provide a value to modify (see the tool parameters for what can be edited).`
+            } but no change was requested — provide a value to modify, or use the "point" brush with \`brush_position\` to move.`
           );
         }
 
         return makeGenericFailure(
           `Matched ${matchedCount} existing instance${
             matchedCount > 1 ? 's' : ''
-          } but the requested values are identical to their current ones, so nothing changed.`
+          } but the requested values are identical to their current ones, so nothing changed.${
+            hasPositionBrush
+              ? ''
+              : ' To move instances, use the "point" brush with `brush_position` (the "none" brush never changes position).'
+          }`
         );
       }