Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 49 additions & 7 deletions newIDE/app/src/EditorFunctions/EditorFunctions.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 }]);
});

Expand Down Expand Up @@ -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 () => {
Expand Down
30 changes: 26 additions & 4 deletions newIDE/app/src/EditorFunctions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -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).'
}`
);
}

Expand Down Expand Up @@ -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 || [
Expand Down Expand Up @@ -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).'
}`
);
}

Expand Down
Loading