Fix conflict between variable or property and parameter in variable s…

…etters
4ian · Jan 20, 2025 · cea300a · cea300a
1 parent 093e15e
commit cea300a
Show file tree

Hide file tree

Showing 19 changed files with 340 additions and 66 deletions.
diff --git a/Core/GDCore/Events/CodeGeneration/EventsCodeGenerator.h b/Core/GDCore/Events/CodeGeneration/EventsCodeGenerator.h
@@ -467,7 +467,14 @@ class GD_CORE_API EventsCodeGenerator {
    */
   virtual gd::String GetCodeNamespace() { return ""; };
 
-  enum VariableScope { LAYOUT_VARIABLE = 0, PROJECT_VARIABLE, OBJECT_VARIABLE, ANY_VARIABLE };
+  enum VariableScope {
+    LAYOUT_VARIABLE = 0,
+    PROJECT_VARIABLE,
+    OBJECT_VARIABLE,
+    ANY_VARIABLE,
+    VARIABLE_OR_PROPERTY,
+    VARIABLE_OR_PROPERTY_OR_PARAMETER
+  };
 
   /**
    * Generate a single unique number for the specified instruction.
@@ -579,7 +586,9 @@ class GD_CORE_API EventsCodeGenerator {
 
     } else if (scope == PROJECT_VARIABLE) {
       return "getProjectVariable(" + variableName + ")";
-    } else if (scope == ANY_VARIABLE) {
+    } else if (scope == ANY_VARIABLE || scope == VARIABLE_OR_PROPERTY ||
+               scope == VARIABLE_OR_PROPERTY_OR_PARAMETER) {
+      // TODO Split the 3 cases to make tests stronger.
       return "getAnyVariable(" + variableName + ")";
     }
 

diff --git a/Core/GDCore/Events/CodeGeneration/ExpressionCodeGenerator.cpp b/Core/GDCore/Events/CodeGeneration/ExpressionCodeGenerator.cpp
@@ -133,9 +133,12 @@ void ExpressionCodeGenerator::OnVisitVariableNode(VariableNode& node) {
   if (gd::ParameterMetadata::IsExpression("variable", type)) {
     // The node is a variable inside an expression waiting for a *variable* to be returned, not its value.
     EventsCodeGenerator::VariableScope scope =
-        type == "variable" || type == "variableOrProperty" ||
-                type == "variableOrPropertyOrParameter"
+        type == "variable"
             ? gd::EventsCodeGenerator::ANY_VARIABLE
+        : type == "variableOrProperty"
+            ? gd::EventsCodeGenerator::VARIABLE_OR_PROPERTY
+        : type == "variableOrPropertyOrParameter"
+            ? gd::EventsCodeGenerator::VARIABLE_OR_PROPERTY_OR_PARAMETER
         : type == "globalvar" ? gd::EventsCodeGenerator::PROJECT_VARIABLE
         : type == "scenevar"  ? gd::EventsCodeGenerator::LAYOUT_VARIABLE
                               : gd::EventsCodeGenerator::OBJECT_VARIABLE;
@@ -223,9 +226,12 @@ void ExpressionCodeGenerator::OnVisitIdentifierNode(IdentifierNode& node) {
         codeGenerator.GenerateObject(node.identifierName, type, context);
   } else if (gd::ParameterMetadata::IsExpression("variable", type)) {
     EventsCodeGenerator::VariableScope scope =
-        type == "variable" || type == "variableOrProperty" ||
-                type == "variableOrPropertyOrParameter"
+        type == "variable"
             ? gd::EventsCodeGenerator::ANY_VARIABLE
+        : type == "variableOrProperty"
+            ? gd::EventsCodeGenerator::VARIABLE_OR_PROPERTY
+        : type == "variableOrPropertyOrParameter"
+            ? gd::EventsCodeGenerator::VARIABLE_OR_PROPERTY_OR_PARAMETER
         : type == "globalvar" ? gd::EventsCodeGenerator::PROJECT_VARIABLE
         : type == "scenevar"  ? gd::EventsCodeGenerator::LAYOUT_VARIABLE
                               : gd::EventsCodeGenerator::OBJECT_VARIABLE;
@@ -254,7 +260,7 @@ void ExpressionCodeGenerator::OnVisitIdentifierNode(IdentifierNode& node) {
       output += codeGenerator.GenerateVariableValueAs(type);
     }, [&]() {
       output += codeGenerator.GenerateGetVariable(
-          node.identifierName, gd::EventsCodeGenerator::ANY_VARIABLE, context,
+          node.identifierName, gd::EventsCodeGenerator::VARIABLE_OR_PROPERTY_OR_PARAMETER, context,
           "", !node.childIdentifierName.empty());
       if (!node.childIdentifierName.empty()) {
         output += codeGenerator.GenerateVariableAccessor(node.childIdentifierName);

diff --git a/Core/GDCore/IDE/Events/EventsVariableInstructionTypeSwitcher.cpp b/Core/GDCore/IDE/Events/EventsVariableInstructionTypeSwitcher.cpp
@@ -71,6 +71,11 @@ bool EventsVariableInstructionTypeSwitcher::DoVisitInstruction(gd::Instruction&
                     .GetObjectsContainersList()
                     .GetObjectOrGroupVariablesContainer(lastObjectName);
           }
+        } else if (type == "variableOrProperty") {
+            variablesContainer =
+                &GetProjectScopedContainers()
+                      .GetVariablesContainersList()
+                      .GetVariablesContainerFromVariableNameExcludingParameters(variableName);
         } else {
           if (GetProjectScopedContainers().GetVariablesContainersList().Has(
                   variableName)) {

diff --git a/Core/GDCore/Project/VariablesContainersList.cpp b/Core/GDCore/Project/VariablesContainersList.cpp
@@ -157,6 +157,34 @@ VariablesContainersList::GetVariablesContainerFromVariableName(
   return badVariablesContainer;
 }
 
+const VariablesContainer &VariablesContainersList::
+    GetVariablesContainerFromVariableNameExcludingParameters(
+        const gd::String &variableName) const {
+  for (auto it = variablesContainers.rbegin(); it != variablesContainers.rend();
+       ++it) {
+    if ((*it)->GetSourceType() !=
+            gd::VariablesContainer::SourceType::Parameters &&
+        (*it)->Has(variableName))
+      return **it;
+  }
+  return badVariablesContainer;
+}
+
+const VariablesContainer &VariablesContainersList::
+    GetVariablesContainerFromVariableNameExcludingParametersAndProperties(
+        const gd::String &variableName) const {
+  for (auto it = variablesContainers.rbegin(); it != variablesContainers.rend();
+       ++it) {
+    if ((*it)->GetSourceType() !=
+            gd::VariablesContainer::SourceType::Parameters &&
+        (*it)->GetSourceType() !=
+            gd::VariablesContainer::SourceType::Properties &&
+        (*it)->Has(variableName))
+      return **it;
+  }
+  return badVariablesContainer;
+}
+
 std::size_t
 VariablesContainersList::GetVariablesContainerPositionFromVariableName(
     const gd::String &variableName) const {

diff --git a/Core/GDCore/Project/VariablesContainersList.h b/Core/GDCore/Project/VariablesContainersList.h
@@ -120,6 +120,18 @@ class GD_CORE_API VariablesContainersList {
   const VariablesContainer &
   GetVariablesContainerFromVariableName(const gd::String &variableName) const;
 
+  /**
+   * Get the variables container for a given variable.
+   */
+  const VariablesContainer &
+  GetVariablesContainerFromVariableNameExcludingParameters(const gd::String &variableName) const;
+
+  /**
+   * Get the variables container for a given variable.
+   */
+  const VariablesContainer &
+  GetVariablesContainerFromVariableNameExcludingParametersAndProperties(const gd::String &variableName) const;
+
   /**
    * Get the variables container index for a given variable.
    */

diff --git a/GDJS/GDJS/Events/CodeGeneration/EventsCodeGenerator.cpp b/GDJS/GDJS/Events/CodeGeneration/EventsCodeGenerator.cpp
@@ -1363,12 +1363,21 @@ gd::String EventsCodeGenerator::GenerateGetVariable(
     bool hasChild) {
   gd::String output;
   const gd::VariablesContainer* variables = NULL;
-  if (scope == ANY_VARIABLE) {
+  if (scope == ANY_VARIABLE || scope == VARIABLE_OR_PROPERTY ||
+      scope == VARIABLE_OR_PROPERTY_OR_PARAMETER) {
     const auto variablesContainersList =
         GetProjectScopedContainers().GetVariablesContainersList();
-    const auto& variablesContainer =
-        variablesContainersList.GetVariablesContainerFromVariableName(
-            variableName);
+    const auto &variablesContainer =
+        scope == VARIABLE_OR_PROPERTY_OR_PARAMETER
+            ? variablesContainersList.GetVariablesContainerFromVariableName(
+                  variableName)
+        : scope == VARIABLE_OR_PROPERTY
+            ? variablesContainersList
+                  .GetVariablesContainerFromVariableNameExcludingParameters(
+                      variableName)
+            : variablesContainersList
+                  .GetVariablesContainerFromVariableNameExcludingParametersAndProperties(
+                      variableName);
     const auto sourceType = variablesContainer.GetSourceType();
     if (sourceType == gd::VariablesContainer::SourceType::Scene) {
       variables = &variablesContainer;

diff --git a/GDJS/GDJS/Extensions/Builtin/VariablesExtension.cpp b/GDJS/GDJS/Extensions/Builtin/VariablesExtension.cpp
@@ -181,7 +181,7 @@ VariablesExtension::VariablesExtension() {
         const auto variablesContainersList =
             codeGenerator.GetProjectScopedContainers().GetVariablesContainersList();
         const auto& variablesContainer =
-            variablesContainersList.GetVariablesContainerFromVariableName(
+            variablesContainersList.GetVariablesContainerFromVariableNameExcludingParameters(
                 variableName);
         const auto sourceType = variablesContainer.GetSourceType();
         if (sourceType == gd::VariablesContainer::SourceType::Properties) {
@@ -244,7 +244,7 @@ VariablesExtension::VariablesExtension() {
         const auto variablesContainersList =
             codeGenerator.GetProjectScopedContainers().GetVariablesContainersList();
         const auto& variablesContainer =
-            variablesContainersList.GetVariablesContainerFromVariableName(
+            variablesContainersList.GetVariablesContainerFromVariableNameExcludingParameters(
                 variableName);
         const auto sourceType = variablesContainer.GetSourceType();
         if (sourceType == gd::VariablesContainer::SourceType::Properties) {
@@ -304,7 +304,7 @@ VariablesExtension::VariablesExtension() {
         const auto variablesContainersList =
             codeGenerator.GetProjectScopedContainers().GetVariablesContainersList();
         const auto& variablesContainer =
-            variablesContainersList.GetVariablesContainerFromVariableName(
+            variablesContainersList.GetVariablesContainerFromVariableNameExcludingParameters(
                 variableName);
         const auto sourceType = variablesContainer.GetSourceType();
         if (sourceType == gd::VariablesContainer::SourceType::Properties) {

diff --git a/GDevelop.js/Bindings/Bindings.idl b/GDevelop.js/Bindings/Bindings.idl
@@ -342,6 +342,8 @@ interface VariablesContainersList {
   boolean Has([Const] DOMString name);
   [Const, Ref] Variable Get([Const] DOMString name);
   [Const, Ref] VariablesContainer GetVariablesContainerFromVariableName([Const] DOMString variableName);
+  [Const, Ref] VariablesContainer GetVariablesContainerFromVariableNameExcludingParameters([Const] DOMString variableName);
+  [Const, Ref] VariablesContainer GetVariablesContainerFromVariableNameExcludingParametersAndProperties([Const] DOMString variableName);
 
   [Const, Ref] VariablesContainer GetVariablesContainer(unsigned long index);
   unsigned long GetVariablesContainersCount();

diff --git a/GDevelop.js/__tests__/GDJSBehaviorCodeGenerationIntegrationTests.js b/GDevelop.js/__tests__/GDJSBehaviorCodeGenerationIntegrationTests.js
@@ -260,6 +260,69 @@ describe('libGD.js - GDJS Behavior Code Generation integration tests', function
     expect(behavior._getMyProperty()).toBe(456);
   });
 
+  it('Can use a property in a variable action when a parameter with the same name exits', () => {
+    const project = new gd.ProjectHelper.createNewGDJSProject();
+    const eventsFunctionsExtension = project.insertNewEventsFunctionsExtension(
+      'MyExtension',
+      0
+    );
+    const eventsBasedBehavior = eventsFunctionsExtension
+      .getEventsBasedBehaviors()
+      .insertNew('MyBehavior', 0);
+
+    eventsBasedBehavior
+      .getPropertyDescriptors()
+      .insertNew('MyIdentifier', 0)
+      .setValue('123')
+      .setType('Number');
+
+    const eventsSerializerElement = gd.Serializer.fromJSObject([
+      {
+        type: 'BuiltinCommonInstructions::Standard',
+        conditions: [],
+        actions: [
+          {
+            type: { value: 'SetNumberVariable' },
+            parameters: ['MyIdentifier', '=', '456'],
+          },
+        ],
+      },
+    ]);
+    eventsBasedBehavior
+      .getEventsFunctions()
+      .insertNewEventsFunction('MyFunction', 0)
+      .getEvents()
+      .unserializeFrom(project, eventsSerializerElement);
+    gd.WholeProjectRefactorer.ensureBehaviorEventsFunctionsProperParameters(
+      eventsFunctionsExtension,
+      eventsBasedBehavior
+    );
+    const eventsFunction = eventsBasedBehavior
+      .getEventsFunctions()
+      .insertNewEventsFunction('MyFunction', 0);
+    const parameter = eventsFunction
+      .getParameters()
+      .insertNewParameter(
+        'MyIdentifier',
+        eventsFunction.getParameters().getParametersCount()
+      );
+    parameter.setType('number');
+
+    const { runtimeScene, behavior } = generatedBehavior(
+      gd,
+      project,
+      eventsFunctionsExtension,
+      eventsBasedBehavior,
+      { logCode: false }
+    );
+
+    // Check the default value is set.
+    expect(behavior._getMyIdentifier()).toBe(123);
+
+    behavior.MyFunction();
+    expect(behavior._getMyIdentifier()).toBe(456);
+  });
+
   it('Can use a property in a variable condition', () => {
     const project = new gd.ProjectHelper.createNewGDJSProject();
     const scene = project.insertNewLayout('MyScene', 0);

diff --git a/GDevelop.js/types.d.ts b/GDevelop.js/types.d.ts
@@ -362,6 +362,8 @@ export class VariablesContainersList extends EmscriptenObject {
   has(name: string): boolean;
   get(name: string): Variable;
   getVariablesContainerFromVariableName(variableName: string): VariablesContainer;
+  getVariablesContainerFromVariableNameExcludingParameters(variableName: string): VariablesContainer;
+  getVariablesContainerFromVariableNameExcludingParametersAndProperties(variableName: string): VariablesContainer;
   getVariablesContainer(index: number): VariablesContainer;
   getVariablesContainersCount(): number;
 }

diff --git a/GDevelop.js/types/gdvariablescontainerslist.js b/GDevelop.js/types/gdvariablescontainerslist.js
@@ -3,6 +3,8 @@ declare class gdVariablesContainersList {
   has(name: string): boolean;
   get(name: string): gdVariable;
   getVariablesContainerFromVariableName(variableName: string): gdVariablesContainer;
+  getVariablesContainerFromVariableNameExcludingParameters(variableName: string): gdVariablesContainer;
+  getVariablesContainerFromVariableNameExcludingParametersAndProperties(variableName: string): gdVariablesContainer;
   getVariablesContainer(index: number): gdVariablesContainer;
   getVariablesContainersCount(): number;
   delete(): void;

diff --git a/newIDE/app/src/EventsSheet/EventsTree/Renderers/ForEachChildVariableEvent.js b/newIDE/app/src/EventsSheet/EventsTree/Renderers/ForEachChildVariableEvent.js
@@ -14,10 +14,9 @@ import {
 } from '../ClassNames';
 import InlinePopover from '../../InlinePopover';
 import AnyVariableField from '../../ParameterFields/AnyVariableField';
-import {
-  getVariableSourceIcon,
-  getVariablesContainerSourceType,
-} from '../../ParameterFields/VariableField';
+import { getVariableSourceIcon } from '../../ParameterFields/VariableField';
+import { getVariableSourceFromVariableName } from '../../ParameterFields/AnyVariableField';
+
 import { type ParameterFieldInterface } from '../../ParameterFields/ParameterFieldCommons';
 import { type EventRendererProps } from './EventRenderer';
 import ConditionsActionsColumns from '../ConditionsActionsColumns';
@@ -31,9 +30,9 @@ export const getVariableSourceOrSceneIcon = (
   projectScopedContainersAccessor: ProjectScopedContainersAccessor,
   variableName: string
 ) => {
-  const variablesContainerSourceType = getVariablesContainerSourceType(
-    projectScopedContainersAccessor,
-    variableName
+  const variablesContainerSourceType = getVariableSourceFromVariableName(
+    variableName,
+    projectScopedContainersAccessor.get()
   );
   return getVariableSourceIcon(
     variablesContainerSourceType === gd.VariablesContainer.Unknown

diff --git a/newIDE/app/src/EventsSheet/ParameterFields/AnyVariableField.js b/newIDE/app/src/EventsSheet/ParameterFields/AnyVariableField.js
@@ -73,6 +73,18 @@ export default React.forwardRef<ParameterFieldProps, ParameterFieldInterface>(
       [projectScopedContainersAccessor]
     );
 
+    const getVariableSourceFromVariableName = React.useCallback(
+      variableName =>
+        projectScopedContainersAccessor
+          .get()
+          .getVariablesContainersList()
+          .getVariablesContainerFromVariableNameExcludingParametersAndProperties(
+            variableName
+          )
+          .getSourceType(),
+      [projectScopedContainersAccessor]
+    );
+
     const onVariableEditorApply = React.useCallback(
       (selectedVariableName: string | null) => {
         if (selectedVariableName && selectedVariableName.startsWith(value)) {
@@ -121,6 +133,7 @@ export default React.forwardRef<ParameterFieldProps, ParameterFieldInterface>(
               : undefined
           }
           onInstructionTypeChanged={onInstructionTypeChanged}
+          getVariableSourceFromVariableName={getVariableSourceFromVariableName}
         />
         {editorOpen && (
           <GlobalAndSceneVariablesDialog
@@ -139,5 +152,20 @@ export default React.forwardRef<ParameterFieldProps, ParameterFieldInterface>(
   }
 );
 
+export const getVariableSourceFromVariableName = (
+  variableName: string,
+  projectScopedContainers: gdProjectScopedContainers
+): VariablesContainer_SourceType => {
+  const rootVariableName = getRootVariableName(variableName);
+  const variablesContainersList = projectScopedContainers.getVariablesContainersList();
+  return variablesContainersList.has(rootVariableName)
+    ? variablesContainersList
+        .getVariablesContainerFromVariableNameExcludingParametersAndProperties(
+          rootVariableName
+        )
+        .getSourceType()
+    : gd.VariablesContainer.Unknown;
+};
+
 export const renderInlineAnyVariable = (props: ParameterInlineRendererProps) =>
-  renderVariableWithIcon(props, 'variable');
+  renderVariableWithIcon(props, 'variable', getVariableSourceFromVariableName);