Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to use parameters and properties in the variable action and condition #7124

Merged
merged 24 commits into from
Jan 15, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
94d1768
Expose parameters and properties in VariablesContainerList.
D8H Nov 2, 2024
d7b68da
Add new parameter type variableOrPropertyOrParameter and variableOrPr…
D8H Nov 2, 2024
5e995c3
Add code generation.
D8H Nov 3, 2024
af0e851
Fix expression validator.
D8H Nov 3, 2024
aa33e1b
Fix Core tests.
D8H Nov 3, 2024
e174ab9
Fix GDevelop.js tests.
D8H Nov 3, 2024
3da7836
Add code generation tests.
D8H Nov 16, 2024
e8022d0
Add test with name collisions.
D8H Nov 17, 2024
fb857f2
Fix variable type detection.
D8H Nov 18, 2024
042abb7
Add a refactoring operation for property type change.
D8H Nov 18, 2024
d5b34f4
Add tests for property renaming.
D8H Nov 25, 2024
0ba1e5d
Fix property renaming.
D8H Nov 24, 2024
3e07508
Rename parameters
D8H Nov 25, 2024
8e6cfe6
Add tests for parameter renamed in variable parameters.
D8H Dec 6, 2024
b5d6d33
Add a refactoring operation for parameter type change.
D8H Dec 7, 2024
5e3d045
Fix a regression in code generation for local variables in behavior f…
D8H Dec 7, 2024
725526a
Add tests for free functions.
D8H Dec 7, 2024
cefe3a9
Add a line break in the generated code.
D8H Dec 8, 2024
9dff6aa
Fix boolean variable conditions when the value is neither True or False.
D8H Dec 13, 2024
9c71c06
Fix a typo in tests and rename a function.
D8H Dec 15, 2024
275af93
Add errors for property or parameter with children.
D8H Dec 15, 2024
55c1eba
Clearer comment.
D8H Jan 10, 2025
b2169cd
Fix variables list for actionWithOperator behavior function parameters.
D8H Jan 10, 2025
671290f
Fix typo in test title
D8H Jan 15, 2025
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
18 changes: 18 additions & 0 deletions Core/GDCore/Events/CodeGeneration/EventsCodeGenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1358,12 +1358,30 @@ gd::String EventsCodeGenerator::GeneratePropertyGetter(const gd::PropertiesConta
return "getProperty" + property.GetName() + "As" + type + "()";
}

gd::String EventsCodeGenerator::GeneratePropertyGetterWithoutCasting(
const gd::PropertiesContainer &propertiesContainer,
const gd::NamedPropertyDescriptor &property) {
return "getProperty" + property.GetName() + "()";
}

gd::String EventsCodeGenerator::GeneratePropertySetterWithoutCasting(
const gd::PropertiesContainer &propertiesContainer,
const gd::NamedPropertyDescriptor &property,
const gd::String &operandCode) {
return "setProperty" + property.GetName() + "(" + operandCode + ")";
}

gd::String EventsCodeGenerator::GenerateParameterGetter(const gd::ParameterMetadata& parameter,
const gd::String& type,
gd::EventsCodeGenerationContext& context) {
return "getParameter" + parameter.GetName() + "As" + type + "()";
}

gd::String EventsCodeGenerator::GenerateParameterGetterWithoutCasting(
const gd::ParameterMetadata &parameter) {
return "getParameter" + parameter.GetName() + "()";
}

EventsCodeGenerator::EventsCodeGenerator(const gd::Project& project_,
const gd::Layout& layout,
const gd::Platform& platform_)
Expand Down
15 changes: 14 additions & 1 deletion Core/GDCore/Events/CodeGeneration/EventsCodeGenerator.h
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,11 @@ class GD_CORE_API EventsCodeGenerator {
GenerateAnyOrSceneVariableGetter(const gd::Expression &variableExpression,
EventsCodeGenerationContext &context);

virtual gd::String GeneratePropertySetterWithoutCasting(
const gd::PropertiesContainer &propertiesContainer,
const gd::NamedPropertyDescriptor &property,
const gd::String &operandCode);

protected:
virtual const gd::String GenerateRelationalOperatorCodes(
const gd::String& operatorString);
Expand Down Expand Up @@ -565,7 +570,8 @@ class GD_CORE_API EventsCodeGenerator {
const gd::String& variableName,
const VariableScope& scope,
gd::EventsCodeGenerationContext& context,
const gd::String& objectName) {
const gd::String& objectName,
bool hasChild) {
// This code is only used as a mock.
// See the real implementation in GDJS.
if (scope == LAYOUT_VARIABLE) {
Expand Down Expand Up @@ -627,11 +633,18 @@ class GD_CORE_API EventsCodeGenerator {
const gd::String& type,
gd::EventsCodeGenerationContext& context);

virtual gd::String GeneratePropertyGetterWithoutCasting(
const gd::PropertiesContainer &propertiesContainer,
const gd::NamedPropertyDescriptor &property);

virtual gd::String GenerateParameterGetter(
const gd::ParameterMetadata& parameter,
const gd::String& type,
gd::EventsCodeGenerationContext& context);

virtual gd::String
GenerateParameterGetterWithoutCasting(const gd::ParameterMetadata &parameter);

/**
* \brief Generate the code to reference an object which is
* in an empty/null state.
Expand Down
59 changes: 30 additions & 29 deletions Core/GDCore/Events/CodeGeneration/ExpressionCodeGenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -133,20 +133,19 @@ 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 == "variable" || type == "variableOrProperty" ||
type == "variableOrPropertyOrParameter"
? gd::EventsCodeGenerator::ANY_VARIABLE
: type == "globalvar"
? gd::EventsCodeGenerator::PROJECT_VARIABLE
: type == "scenevar"
? gd::EventsCodeGenerator::LAYOUT_VARIABLE
: gd::EventsCodeGenerator::OBJECT_VARIABLE;
: type == "globalvar" ? gd::EventsCodeGenerator::PROJECT_VARIABLE
: type == "scenevar" ? gd::EventsCodeGenerator::LAYOUT_VARIABLE
: gd::EventsCodeGenerator::OBJECT_VARIABLE;

auto objectName = gd::ExpressionVariableOwnerFinder::GetObjectName(codeGenerator.GetPlatform(),
codeGenerator.GetObjectsContainersList(),
rootObjectName,
node);
output += codeGenerator.GenerateGetVariable(
node.name, scope, context, objectName);
node.name, scope, context, objectName, node.child != nullptr);
if (node.child) node.child->Visit(*this);
} else {
// The node represents a variable or an object variable in an expression waiting for its *value* to be returned.
Expand All @@ -163,7 +162,7 @@ void ExpressionCodeGenerator::OnVisitVariableNode(VariableNode& node) {
output += codeGenerator.GenerateVariableValueAs(type);
}, [&]() {
output += codeGenerator.GenerateGetVariable(
node.name, gd::EventsCodeGenerator::ANY_VARIABLE, context, "");
node.name, gd::EventsCodeGenerator::ANY_VARIABLE, context, "", node.child != nullptr);
if (node.child) node.child->Visit(*this);
output += codeGenerator.GenerateVariableValueAs(type);
}, [&]() {
Expand All @@ -184,8 +183,9 @@ void ExpressionCodeGenerator::OnVisitVariableAccessorNode(
VariableAccessorNode& node) {
if (!objectNameToUseForVariableAccessor.empty()) {
// Use the name of the object passed by the parent, as we need both to access an object variable.
output += codeGenerator.GenerateGetVariable(node.name,
gd::EventsCodeGenerator::OBJECT_VARIABLE, context, objectNameToUseForVariableAccessor);
output += codeGenerator.GenerateGetVariable(
node.name, gd::EventsCodeGenerator::OBJECT_VARIABLE, context,
objectNameToUseForVariableAccessor, node.child != nullptr);

// We have accessed an object variable, from now we can continue accessing the child variables
// (including using the bracket notation).
Expand Down Expand Up @@ -222,24 +222,24 @@ void ExpressionCodeGenerator::OnVisitIdentifierNode(IdentifierNode& node) {
output +=
codeGenerator.GenerateObject(node.identifierName, type, context);
} else if (gd::ParameterMetadata::IsExpression("variable", type)) {
EventsCodeGenerator::VariableScope scope =
type == "variable"
EventsCodeGenerator::VariableScope scope =
type == "variable" || type == "variableOrProperty" ||
type == "variableOrPropertyOrParameter"
? gd::EventsCodeGenerator::ANY_VARIABLE
: type == "globalvar"
? gd::EventsCodeGenerator::PROJECT_VARIABLE
: type == "scenevar"
? gd::EventsCodeGenerator::LAYOUT_VARIABLE
: gd::EventsCodeGenerator::OBJECT_VARIABLE;

auto objectName = gd::ExpressionVariableOwnerFinder::GetObjectName(codeGenerator.GetPlatform(),
codeGenerator.GetObjectsContainersList(),
rootObjectName,
node);
output += codeGenerator.GenerateGetVariable(
node.identifierName, scope, context, objectName);
if (!node.childIdentifierName.empty()) {
output += codeGenerator.GenerateVariableAccessor(node.childIdentifierName);
}
: type == "globalvar" ? gd::EventsCodeGenerator::PROJECT_VARIABLE
: type == "scenevar" ? gd::EventsCodeGenerator::LAYOUT_VARIABLE
: gd::EventsCodeGenerator::OBJECT_VARIABLE;

auto objectName = gd::ExpressionVariableOwnerFinder::GetObjectName(
codeGenerator.GetPlatform(), codeGenerator.GetObjectsContainersList(),
rootObjectName, node);
output += codeGenerator.GenerateGetVariable(
node.identifierName, scope, context, objectName,
!node.childIdentifierName.empty());
if (!node.childIdentifierName.empty()) {
output +=
codeGenerator.GenerateVariableAccessor(node.childIdentifierName);
}
} else {
const auto& variablesContainersList = codeGenerator.GetProjectScopedContainers().GetVariablesContainersList();
const auto& propertiesContainersList = codeGenerator.GetProjectScopedContainers().GetPropertiesContainersList();
Expand All @@ -249,12 +249,13 @@ void ExpressionCodeGenerator::OnVisitIdentifierNode(IdentifierNode& node) {
codeGenerator.GetProjectScopedContainers().MatchIdentifierWithName<void>(node.identifierName, [&]() {
// Generate the code to access the object variable.
output += codeGenerator.GenerateGetVariable(
node.childIdentifierName, gd::EventsCodeGenerator::OBJECT_VARIABLE, context, node.identifierName);
node.childIdentifierName, gd::EventsCodeGenerator::OBJECT_VARIABLE,
context, node.identifierName, !node.childIdentifierName.empty());
output += codeGenerator.GenerateVariableValueAs(type);
}, [&]() {
output += codeGenerator.GenerateGetVariable(
node.identifierName, gd::EventsCodeGenerator::ANY_VARIABLE, context,
"");
"", !node.childIdentifierName.empty());
if (!node.childIdentifierName.empty()) {
output += codeGenerator.GenerateVariableAccessor(node.childIdentifierName);
}
Expand Down
12 changes: 6 additions & 6 deletions Core/GDCore/Extensions/Builtin/VariablesExtension.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ void GD_CORE_API BuiltinExtensionsImplementer::ImplementsVariablesExtension(
"",
"res/conditions/var24.png",
"res/conditions/var.png")
.AddParameter("variable", _("Variable"))
.AddParameter("variableOrPropertyOrParameter", _("Variable"))
.UseStandardRelationalOperatorParameters(
"number", ParameterOptions::MakeNewOptions());

Expand All @@ -45,7 +45,7 @@ void GD_CORE_API BuiltinExtensionsImplementer::ImplementsVariablesExtension(
"",
"res/conditions/var24.png",
"res/conditions/var.png")
.AddParameter("variable", _("Variable"))
.AddParameter("variableOrPropertyOrParameter", _("Variable"))
.UseStandardRelationalOperatorParameters(
"string", ParameterOptions::MakeNewOptions());

Expand All @@ -58,7 +58,7 @@ void GD_CORE_API BuiltinExtensionsImplementer::ImplementsVariablesExtension(
"",
"res/conditions/var24.png",
"res/conditions/var.png")
.AddParameter("variable", _("Variable"))
.AddParameter("variableOrPropertyOrParameter", _("Variable"))
.AddParameter("trueorfalse", _("Check if the value is"))
.SetDefaultValue("true")
// This parameter allows to keep the operand expression
Expand All @@ -73,7 +73,7 @@ void GD_CORE_API BuiltinExtensionsImplementer::ImplementsVariablesExtension(
"",
"res/actions/var24.png",
"res/actions/var.png")
.AddParameter("variable", _("Variable"))
.AddParameter("variableOrProperty", _("Variable"))
.UseStandardOperatorParameters("number",
ParameterOptions::MakeNewOptions());

Expand All @@ -85,7 +85,7 @@ void GD_CORE_API BuiltinExtensionsImplementer::ImplementsVariablesExtension(
"",
"res/actions/var24.png",
"res/actions/var.png")
.AddParameter("variable", _("Variable"))
.AddParameter("variableOrProperty", _("Variable"))
.UseStandardOperatorParameters("string",
ParameterOptions::MakeNewOptions());

Expand All @@ -98,7 +98,7 @@ void GD_CORE_API BuiltinExtensionsImplementer::ImplementsVariablesExtension(
"",
"res/conditions/var24.png",
"res/conditions/var.png")
.AddParameter("variable", _("Variable"))
.AddParameter("variableOrProperty", _("Variable"))
.AddParameter("operator", _("Value"), "boolean")
// This parameter allows to keep the operand expression
// when the editor switch between variable instructions.
Expand Down
5 changes: 3 additions & 2 deletions Core/GDCore/Extensions/Metadata/ParameterMetadataTools.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ void ParameterMetadataTools::ParametersToObjectsContainer(
const auto& parameter = parameters.GetParameter(i);
if (parameter.GetName().empty()) continue;

if (gd::ParameterMetadata::IsObject(parameter.GetType())) {
auto &valueTypeMetadata = parameter.GetValueTypeMetadata();
if (valueTypeMetadata.IsObject()) {
const gd::String& objectName = parameter.GetName();
const gd::String& objectType = parameter.GetExtraInfo();
allObjectNames.insert(objectName);
Expand Down Expand Up @@ -68,7 +69,7 @@ void ParameterMetadataTools::ParametersToObjectsContainer(
// Search "lastObjectName" in the codebase for other place where this
// convention is enforced.
lastObjectName = objectName;
} else if (gd::ParameterMetadata::IsBehavior(parameter.GetType())) {
} else if (valueTypeMetadata.IsBehavior()) {
if (!lastObjectName.empty()) {
if (outputObjectsContainer.HasObjectNamed(lastObjectName)) {
const gd::String& behaviorName = parameter.GetName();
Expand Down
26 changes: 21 additions & 5 deletions Core/GDCore/Extensions/Metadata/ValueTypeMetadata.h
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ class GD_CORE_API ValueTypeMetadata {
}

/**
* \brief Return true if the type of the parameter is a number.
* \brief Return true if the type of the parameter is a variable.
* \note If you had a new type of parameter, also add it in the IDE (
* see EventsFunctionParametersEditor, ParameterRenderingService
* and ExpressionAutocompletion) and in the EventsCodeGenerator.
Expand All @@ -148,6 +148,19 @@ class GD_CORE_API ValueTypeMetadata {
return gd::ValueTypeMetadata::GetPrimitiveValueType(type) == "variable";
}

/**
* \brief Return true if the type of the parameter is a variable and not a
* property or a parameter.
*/
bool IsVariableOnly() const {
return
// Any variable.
name == "variable" ||
// Old, "pre-scoped" variables:
name == "objectvar" || name == "globalvar" ||
name == "scenevar";
}

/**
* \brief Return true if the type is a variable but from a specific scope
* (scene, project or object). In new code, prefer to use the more generic "variable"
Expand Down Expand Up @@ -217,10 +230,13 @@ class GD_CORE_API ValueTypeMetadata {
return parameterType == "yesorno" || parameterType == "trueorfalse";
} else if (type == "variable") {
return
parameterType == "variable" || // Any variable.
// Old, "pre-scoped" variables:
parameterType == "objectvar" || parameterType == "globalvar" ||
parameterType == "scenevar";
// Any variable.
parameterType == "variable" ||
parameterType == "variableOrProperty" ||
parameterType == "variableOrPropertyOrParameter" ||
// Old, "pre-scoped" variables:
parameterType == "objectvar" || parameterType == "globalvar" ||
parameterType == "scenevar";
} else if (type == "resource") {
return parameterType == "fontResource" ||
parameterType == "audioResource" ||
Expand Down
6 changes: 3 additions & 3 deletions Core/GDCore/IDE/Events/EventsParameterReplacer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ class GD_CORE_API ExpressionParameterReplacer
parameterMetadata->GetValueTypeMetadata();
if (gd::EventsParameterReplacer::CanContainParameter(
parameterTypeMetadata)) {
isParentTypeAVariable = parameterTypeMetadata.IsVariable();
isParentTypeAVariable = parameterTypeMetadata.IsVariableOnly();
parameter->Visit(*this);
}
}
Expand Down Expand Up @@ -197,7 +197,7 @@ bool EventsParameterReplacer::DoVisitInstruction(gd::Instruction& instruction,
if (node) {
ExpressionParameterReplacer renamer(
platform, GetProjectScopedContainers(),
parameterMetadata.GetValueTypeMetadata().IsVariable(),
parameterMetadata.GetValueTypeMetadata().IsVariableOnly(),
oldToNewPropertyNames);
node->Visit(renamer);

Expand All @@ -221,7 +221,7 @@ bool EventsParameterReplacer::DoVisitEventExpression(
if (node) {
ExpressionParameterReplacer renamer(
platform, GetProjectScopedContainers(),
metadata.GetValueTypeMetadata().IsVariable(), oldToNewPropertyNames);
metadata.GetValueTypeMetadata().IsVariableOnly(), oldToNewPropertyNames);
node->Visit(renamer);

if (renamer.HasDoneRenaming()) {
Expand Down
6 changes: 3 additions & 3 deletions Core/GDCore/IDE/Events/EventsPropertyReplacer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ class GD_CORE_API ExpressionPropertyReplacer
parameterMetadata->GetValueTypeMetadata();
if (gd::EventsPropertyReplacer::CanContainProperty(
parameterTypeMetadata)) {
isParentTypeAVariable = parameterTypeMetadata.IsVariable();
isParentTypeAVariable = parameterTypeMetadata.IsVariableOnly();
parameter->Visit(*this);
}
}
Expand Down Expand Up @@ -231,7 +231,7 @@ bool EventsPropertyReplacer::DoVisitInstruction(gd::Instruction& instruction,
if (node) {
ExpressionPropertyReplacer renamer(
platform, GetProjectScopedContainers(), targetPropertiesContainer,
parameterMetadata.GetValueTypeMetadata().IsVariable(),
parameterMetadata.GetValueTypeMetadata().IsVariableOnly(),
oldToNewPropertyNames, removedPropertyNames);
node->Visit(renamer);

Expand All @@ -257,7 +257,7 @@ bool EventsPropertyReplacer::DoVisitEventExpression(
if (node) {
ExpressionPropertyReplacer renamer(
platform, GetProjectScopedContainers(), targetPropertiesContainer,
metadata.GetValueTypeMetadata().IsVariable(), oldToNewPropertyNames,
metadata.GetValueTypeMetadata().IsVariableOnly(), oldToNewPropertyNames,
removedPropertyNames);
node->Visit(renamer);

Expand Down
15 changes: 10 additions & 5 deletions Core/GDCore/IDE/Events/ExpressionValidator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -460,10 +460,12 @@ const gd::String& ExpressionValidator::TypeToString(Type type) {
case Type::NumberOrString:
return numberOrStringTypeString;
case Type::Variable:
return variableTypeString;
// This function is only used to display errors.
// Users don't care if it's legacy or not or
// if it allows properties and parameters.
case Type::VariableOrProperty:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a risk that the caller of this function would expect to get back something like "variableOrProperty" or "variableOrPropertyOrParameter"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the same for legacy variables. I moved the comment so it's clearer.

case Type::VariableOrPropertyOrParameter:
case Type::LegacyVariable:
// This function is only used to display error.
// Users don't care if it's legacy or not.
return variableTypeString;
case Type::Object:
return objectTypeString;
Expand Down Expand Up @@ -493,8 +495,11 @@ ExpressionValidator::Type ExpressionValidator::StringToType(
ExpressionValidator::variableTypeString, type)) {
if (gd::ValueTypeMetadata::IsTypeLegacyPreScopedVariable(type)) {
return Type::LegacyVariable;
}
else {
} else if (type == "variableOrProperty") {
return Type::VariableOrProperty;
} else if (type == "variableOrPropertyOrParameter") {
return Type::VariableOrPropertyOrParameter;
} else {
return Type::Variable;
}
}
Expand Down
Loading
Loading