From 1293df0ebfcced26e78130bc08cc9745c7dc6a81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Pasteau?= <4895034+ClementPasteau@users.noreply.github.com> Date: Fri, 9 Oct 2026 13:12:24 +0200 Subject: [PATCH] Fix correctly flagging non-existing behaviors for free behaviors property fields (#9203) --- Core/GDCore/IDE/InstructionValidator.cpp | 25 ++++++ GDevelop.js/__tests__/Core.js | 67 ++++++++++++++ .../ParameterFields/BehaviorField.js | 89 ++++++++----------- 3 files changed, 128 insertions(+), 53 deletions(-) diff --git a/Core/GDCore/IDE/InstructionValidator.cpp b/Core/GDCore/IDE/InstructionValidator.cpp index 8d68e00902..5d97ada227 100644 --- a/Core/GDCore/IDE/InstructionValidator.cpp +++ b/Core/GDCore/IDE/InstructionValidator.cpp @@ -10,6 +10,7 @@ #include "GDCore/Extensions/Metadata/BehaviorMetadata.h" #include "GDCore/Extensions/Metadata/InstructionMetadata.h" #include "GDCore/Extensions/Metadata/MetadataProvider.h" +#include "GDCore/Extensions/Metadata/ParameterMetadataTools.h" #include "GDCore/Extensions/Platform.h" #include "GDCore/IDE/Events/ExpressionValidator.h" #include "GDCore/IDE/VariableInstructionSwitcher.h" @@ -94,6 +95,29 @@ ParameterValidationResult InstructionValidator::ValidateParameter( parameterMetadata.GetExtraInfo()) && InstructionValidator::HasRequiredBehaviors( instruction, metadata, parameterIndex, objectsContainersList); + } else if (gd::ParameterMetadata::IsBehavior(parameterType)) { + const auto objectParameterIndex = + gd::ParameterMetadataTools::GetObjectParameterIndexFor( + metadata.GetParameters(), parameterIndex); + if (objectParameterIndex == gd::String::npos) { + return result; // No object to check against: consider it valid. + } + const auto &objectOrGroupName = + instruction.GetParameter(objectParameterIndex).GetPlainString(); + const auto &objectsContainersList = + projectScopedContainers.GetObjectsContainersList(); + const auto &behaviorType = parameterMetadata.GetExtraInfo(); + // The object must have a behavior with this name and, when the parameter + // declares a behavior type (most behavior instructions), of this type. + // Untyped behavior parameters (like "De/activate a behavior") accept any + // behavior of the object. + result.isValid = + behaviorType.empty() + ? objectsContainersList.HasBehaviorInObjectOrGroup( + objectOrGroupName, value) + : objectsContainersList.GetTypeOfBehaviorInObjectOrGroup( + objectOrGroupName, value, + /** searchInGroups = */ true) == behaviorType; } else if (gd::ParameterMetadata::IsExpression("resource", parameterType)) { const auto &resourceName = instruction.GetParameter(parameterIndex).GetPlainString(); @@ -191,6 +215,7 @@ bool InstructionValidator::HasRequiredBehaviors( } const auto &behaviorType = behaviorParameter.GetExtraInfo(); if (behaviorType.empty()) { + // The behavior name is checked by the behavior parameter itself. continue; } if (index >= instruction.GetParametersCount()) { diff --git a/GDevelop.js/__tests__/Core.js b/GDevelop.js/__tests__/Core.js index 90b85b34c0..4472f6f28e 100644 --- a/GDevelop.js/__tests__/Core.js +++ b/GDevelop.js/__tests__/Core.js @@ -2822,6 +2822,73 @@ describe('libGD.js', function () { expect(validateVolumeParameter('1 +')).toBe(false); expect(validateVolumeParameter('"Not a number"')).toBe(false); }); + + it('checks that the object has the behavior of an untyped behavior parameter', function () { + // `ActivateBehavior` accepts any behavior: its "behavior" parameter has + // no behavior type, but the object must still have the behavior. + const objectWithBehavior = layout + .getObjects() + .insertNewObject(project, 'Sprite', 'ObjectWithBehavior', 0); + objectWithBehavior.addNewBehavior( + project, + 'PlatformBehavior::PlatformerObjectBehavior', + 'PlatformerObject' + ); + layout + .getObjects() + .insertNewObject(project, 'Sprite', 'ObjectWithoutBehavior', 1); + + const validateParameter = (objectName, behaviorName, parameterIndex) => { + const action = new gd.Instruction(); + action.setType('ActivateBehavior'); + action.setParametersCount(3); + action.setParameter(0, objectName); + action.setParameter(1, behaviorName); + action.setParameter(2, 'yes'); + const result = gd.InstructionValidator.validateParameter( + gd.JsPlatform.get(), + projectScopedContainers, + action, + gd.MetadataProvider.getActionMetadata( + gd.JsPlatform.get(), + 'ActivateBehavior' + ), + parameterIndex + ); + const isValid = result.isValid(); + action.delete(); + return isValid; + }; + const behaviorParameterIndex = 1; + const objectParameterIndex = 0; + + expect( + validateParameter( + 'ObjectWithBehavior', + 'PlatformerObject', + behaviorParameterIndex + ) + ).toBe(true); + expect( + validateParameter( + 'ObjectWithoutBehavior', + 'PlatformerObject', + behaviorParameterIndex + ) + ).toBe(false); + expect( + validateParameter('ObjectWithBehavior', '', behaviorParameterIndex) + ).toBe(false); + + // The error is on the behavior parameter: the object stays valid. + expect( + validateParameter( + 'ObjectWithoutBehavior', + 'PlatformerObject', + objectParameterIndex + ) + ).toBe(true); + }); }); describe('EventsRefactorer', function () { diff --git a/newIDE/app/src/EventsSheet/ParameterFields/BehaviorField.js b/newIDE/app/src/EventsSheet/ParameterFields/BehaviorField.js index 4882838d2a..417a1efb1f 100644 --- a/newIDE/app/src/EventsSheet/ParameterFields/BehaviorField.js +++ b/newIDE/app/src/EventsSheet/ParameterFields/BehaviorField.js @@ -54,9 +54,6 @@ export default (React.forwardRef( const { parameterMetadata } = props; - const [errorText, setErrorText] = React.useState(null); - const [behaviorNames, setBehaviorNames] = React.useState>([]); - const description = parameterMetadata ? parameterMetadata.getDescription() : undefined; @@ -69,58 +66,54 @@ export default (React.forwardRef( ? parameterMetadata.getExtraInfo() : ''; - const updateBehaviorsList = React.useCallback( - () => { - const { - instructionMetadata, - instruction, - expressionMetadata, - expression, - parameterIndex, - projectScopedContainersAccessor, - } = props; - const objectName = getLastObjectParameterValue({ - instructionMetadata, - instruction, - expressionMetadata, - expression, - parameterIndex, - }); - if (!objectName) return; + // Computed synchronously (and not in an effect) so that the first render + // already knows the behaviors of the object: otherwise the field would + // briefly display an error before the list is filled. + const objectName = getLastObjectParameterValue({ + instructionMetadata: props.instructionMetadata, + instruction: props.instruction, + expressionMetadata: props.expressionMetadata, + expression: props.expression, + parameterIndex: props.parameterIndex, + }); + const behaviorNames = React.useMemo( + () => + objectName + ? getSelectableBehavior( + props.projectScopedContainersAccessor, + objectName, + allowedBehaviorType + ) + : [], + // Recompute on each props change: the object (not in the props) may + // have been changed in the instruction, or a behavior may have been + // added to it (from the instruction editor dialog). + // eslint-disable-next-line react-hooks/exhaustive-deps + [props, objectName, allowedBehaviorType] + ); - const newBehaviorNames = getSelectableBehavior( - projectScopedContainersAccessor, - objectName, - allowedBehaviorType - ); - setBehaviorNames(newBehaviorNames); + React.useEffect( + () => { if ( + objectName && !allowedBehaviorType && !!props.value && - newBehaviorNames.length === 0 + behaviorNames.length === 0 ) { // Force emptying the current value if there is no behavior. // Useful when the object is changed to one without behaviors. props.onChange(''); } }, - [props, allowedBehaviorType] + [props, objectName, allowedBehaviorType, behaviorNames] ); - const getError = (value?: string) => { - if (!value && !props.value) return null; - - const isValidChoice = - behaviorNames.filter(choice => props.value === choice).length !== 0; - - if (!isValidChoice) return 'This behavior is not attached to the object'; - - return null; - }; - - const doValidation = (value?: string) => { - setErrorText(getError(value)); - }; + // Derived from the current value so that the error is visible as soon as + // the field is displayed (and not only after it lost focus). + const errorText = + !!props.value && !behaviorNames.includes(props.value) + ? 'This behavior is not attached to the object' + : null; const forceChooseBehavior = React.useCallback( () => { @@ -145,13 +138,6 @@ export default (React.forwardRef( [forceChooseBehavior] ); - React.useEffect( - () => { - updateBehaviorsList(); - }, - [updateBehaviorsList] - ); - const noBehaviorErrorText = allowedBehaviorType ? ( The behavior is not attached to this object. Please select another @@ -178,9 +164,6 @@ export default (React.forwardRef( onChange={props.onChange} onRequestClose={props.onRequestClose} onApply={props.onApply} - onBlur={event => { - doValidation(event.currentTarget.value); - }} dataSource={behaviorNames.map(behaviorName => ({ text: behaviorName, value: behaviorName,