From ab5d95f08f625a1790ff2a55517ceb464017e4d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Pasteau?= <4895034+ClementPasteau@users.noreply.github.com> Date: Wed, 9 Sep 2026 17:51:19 +0200 Subject: [PATCH] Fix validating object variables too many times on export --- .../CodeGeneration/EventsCodeGenerator.cpp | 32 +++++++++- .../CodeGeneration/EventsCodeGenerator.h | 15 +++++ .../ExpressionCodeGenerator.cpp | 6 ++ GDevelop.js/__tests__/GDJS.js | 58 +++++++++++++++++++ 4 files changed, 109 insertions(+), 2 deletions(-) diff --git a/Core/GDCore/Events/CodeGeneration/EventsCodeGenerator.cpp b/Core/GDCore/Events/CodeGeneration/EventsCodeGenerator.cpp index 147a1b67e3c2..9c114d3c659b 100644 --- a/Core/GDCore/Events/CodeGeneration/EventsCodeGenerator.cpp +++ b/Core/GDCore/Events/CodeGeneration/EventsCodeGenerator.cpp @@ -386,6 +386,7 @@ gd::String EventsCodeGenerator::GenerateConditionCode( GetObjectsContainersList().ExpandObjectName( objectName, context.GetCurrentObject()); for (std::size_t i = 0; i < realObjects.size(); ++i) { + expressionValidationFailed = false; // Set up the context gd::String objectType = GetObjectsContainersList().GetTypeOfObject(realObjects[i]); @@ -405,8 +406,13 @@ gd::String EventsCodeGenerator::GenerateConditionCode( realObjects[i], objInfo, arguments, instrInfos, returnBoolean, condition.IsInverted(), context); } + // Expressions are validated against the group name, not the current + // object, so the result is the same for every object: validate them + // only once. + expressionValidationSkipped = !expressionValidationFailed; context.SetNoCurrentObject(); } + expressionValidationSkipped = false; } } else if (instrInfos.IsBehaviorInstruction()) { if (instrInfos.parameters.GetParametersCount() >= 2) { @@ -424,6 +430,7 @@ gd::String EventsCodeGenerator::GenerateConditionCode( MetadataProvider::GetBehaviorMetadata(platform, actualBehaviorType); for (std::size_t i = 0; i < realObjects.size(); ++i) { + expressionValidationFailed = false; // Setup context AddIncludeFiles(autoInfo.includeFiles); context.SetCurrentObject(realObjects[i]); @@ -438,8 +445,13 @@ gd::String EventsCodeGenerator::GenerateConditionCode( realObjects[i], behaviorName, autoInfo, arguments, instrInfos, returnBoolean, condition.IsInverted(), context); } + // Expressions are validated against the group name, not the current + // object, so the result is the same for every object: validate them + // only once. + expressionValidationSkipped = !expressionValidationFailed; context.SetNoCurrentObject(); } + expressionValidationSkipped = false; } } else { std::vector > @@ -675,6 +687,7 @@ gd::String EventsCodeGenerator::GenerateActionCode( GetObjectsContainersList().ExpandObjectName( objectName, context.GetCurrentObject()); for (std::size_t i = 0; i < realObjects.size(); ++i) { + expressionValidationFailed = false; // Setup context gd::String objectType = GetObjectsContainersList().GetTypeOfObject(realObjects[i]); @@ -694,8 +707,13 @@ gd::String EventsCodeGenerator::GenerateActionCode( realObjects[i], objInfo, functionCallName, arguments, instrInfos, context, optionalAsyncCallbackName, optionalAsyncCallbackId); } + // Expressions are validated against the group name, not the current + // object, so the result is the same for every object: validate them + // only once. + expressionValidationSkipped = !expressionValidationFailed; context.SetNoCurrentObject(); } + expressionValidationSkipped = false; } } else if (instrInfos.IsBehaviorInstruction()) { if (instrInfos.parameters.GetParametersCount() >= 2) { @@ -713,6 +731,7 @@ gd::String EventsCodeGenerator::GenerateActionCode( AddIncludeFiles(autoInfo.includeFiles); for (std::size_t i = 0; i < realObjects.size(); ++i) { + expressionValidationFailed = false; // Setup context context.SetCurrentObject(realObjects[i]); context.ObjectsListNeeded(realObjects[i]); @@ -727,8 +746,13 @@ gd::String EventsCodeGenerator::GenerateActionCode( arguments, instrInfos, context, optionalAsyncCallbackName, optionalAsyncCallbackId); } + // Expressions are validated against the group name, not the current + // object, so the result is the same for every object: validate them + // only once. + expressionValidationSkipped = !expressionValidationFailed; context.SetNoCurrentObject(); } + expressionValidationSkipped = false; } } else { vector arguments = GenerateParametersCodes( @@ -1571,7 +1595,9 @@ EventsCodeGenerator::EventsCodeGenerator(const gd::Project& project_, maxCustomConditionsDepth(0), maxConditionsListsSize(0), eventsListNextUniqueId(0), - diagnosticReport(nullptr) {}; + diagnosticReport(nullptr), + expressionValidationSkipped(false), + expressionValidationFailed(false) {}; EventsCodeGenerator::EventsCodeGenerator( const gd::Platform& platform_, @@ -1586,6 +1612,8 @@ EventsCodeGenerator::EventsCodeGenerator( maxCustomConditionsDepth(0), maxConditionsListsSize(0), eventsListNextUniqueId(0), - diagnosticReport(nullptr) {}; + diagnosticReport(nullptr), + expressionValidationSkipped(false), + expressionValidationFailed(false) {}; } // namespace gd diff --git a/Core/GDCore/Events/CodeGeneration/EventsCodeGenerator.h b/Core/GDCore/Events/CodeGeneration/EventsCodeGenerator.h index 8dbee6643948..21bed5ee12aa 100644 --- a/Core/GDCore/Events/CodeGeneration/EventsCodeGenerator.h +++ b/Core/GDCore/Events/CodeGeneration/EventsCodeGenerator.h @@ -390,6 +390,16 @@ class GD_CORE_API EventsCodeGenerator { gd::DiagnosticReport* GetDiagnosticReport() { return diagnosticReport; } + /** + * \brief Return true if the validation of expressions can be skipped, because + * the instruction is generated again for another object of a group. + */ + bool IsExpressionValidationSkipped() const { + return expressionValidationSkipped; + } + + void SetExpressionValidationFailed() { expressionValidationFailed = true; } + /** * \brief Generate the full name for accessing to a boolean variable used for * conditions. @@ -879,6 +889,11 @@ class GD_CORE_API EventsCodeGenerator { ///< list function name. gd::DiagnosticReport* diagnosticReport; + bool expressionValidationSkipped; ///< True when an instruction is generated + ///< for the next objects of a group, after + ///< its expressions were validated once. + bool expressionValidationFailed; ///< True if an expression validation + ///< failed for the current instruction. }; } // namespace gd diff --git a/Core/GDCore/Events/CodeGeneration/ExpressionCodeGenerator.cpp b/Core/GDCore/Events/CodeGeneration/ExpressionCodeGenerator.cpp index e0d327b6eba0..c556f8ec428e 100644 --- a/Core/GDCore/Events/CodeGeneration/ExpressionCodeGenerator.cpp +++ b/Core/GDCore/Events/CodeGeneration/ExpressionCodeGenerator.cpp @@ -52,6 +52,11 @@ gd::String ExpressionCodeGenerator::GenerateExpressionCode( return generator.GenerateDefaultValue(rootType); } + if (codeGenerator.IsExpressionValidationSkipped()) { + node->Visit(generator); + return generator.GetOutput(); + } + gd::ExpressionValidator validator(codeGenerator.GetPlatform(), codeGenerator.GetProjectScopedContainers(), rootType, @@ -59,6 +64,7 @@ gd::String ExpressionCodeGenerator::GenerateExpressionCode( extraInfo); node->Visit(validator); if (!validator.GetFatalErrors().empty()) { + codeGenerator.SetExpressionValidationFailed(); std::cout << "Error: \"" << validator.GetFatalErrors()[0]->GetMessage() << "\" in: \"" << expression.GetPlainString() << "\" (" << rootType << ")" << std::endl; diff --git a/GDevelop.js/__tests__/GDJS.js b/GDevelop.js/__tests__/GDJS.js index 95ce667a7e2e..1af1cee8990f 100644 --- a/GDevelop.js/__tests__/GDJS.js +++ b/GDevelop.js/__tests__/GDJS.js @@ -331,6 +331,64 @@ describe('libGD.js - GDJS related tests', function () { expect(code).toMatch('elseEventsChainSatisfied'); }); + it('reports an invalid expression for each object of a group', function () { + const project = gd.ProjectHelper.createNewGDJSProject(); + const layout = project.insertNewLayout('Scene', 0); + for (const objectName of ['MyObjectA', 'MyObjectB']) { + const object = layout + .getObjects() + .insertNewObject(project, 'Sprite', objectName, 0); + object.getVariables().insertNew('MyVariable', 0).setValue(0); + } + const group = layout + .getObjects() + .getObjectGroups() + .insertNew('MyGroup', 0); + group.addObject('MyObjectA'); + group.addObject('MyObjectB'); + + const evt = layout + .getEvents() + .insertNewEvent(project, 'BuiltinCommonInstructions::Standard', 0); + const action = new gd.Instruction(); + action.setType('SetNumberObjectVariable'); + action.setParametersCount(4); + action.setParameter(0, 'MyGroup'); + action.setParameter(1, 'MyVariable'); + action.setParameter(2, '='); + action.setParameter(3, 'MyGroup.UndeclaredVariable'); + gd.asStandardEvent(evt).getActions().insert(action, 0); + action.delete(); + + const layoutCodeGenerator = new gd.LayoutCodeGenerator(project); + const diagnosticReport = new gd.DiagnosticReport(); + const code = layoutCodeGenerator.generateLayoutCompleteCode( + layout, + new gd.SetString(), + diagnosticReport, + true + ); + + // The action is generated once per object of the group, and the invalid + // expression is reported and replaced by a default value each time. + expect(diagnosticReport.count()).toBe(2); + expect(diagnosticReport.get(0).getActualValue()).toBe( + 'UndeclaredVariable' + ); + expect(diagnosticReport.get(1).getActualValue()).toBe( + 'UndeclaredVariable' + ); + expect(code).toMatch( + 'gdjs.SceneCode.GDMyObjectAObjects1[i].returnVariable(gdjs.SceneCode.GDMyObjectAObjects1[i].getVariables().get("MyVariable")).setNumber(0);' + ); + expect(code).toMatch( + 'gdjs.SceneCode.GDMyObjectBObjects1[i].returnVariable(gdjs.SceneCode.GDMyObjectBObjects1[i].getVariables().get("MyVariable")).setNumber(0);' + ); + + diagnosticReport.delete(); + layoutCodeGenerator.delete(); + project.delete(); + }); it('does not generate code for improperly set up actions/conditions', function () { const project = gd.ProjectHelper.createNewGDJSProject(); const layout = project.insertNewLayout('Scene', 0);