diff --git a/Core/GDCore/IDE/Events/ExpressionsParameterMover.cpp b/Core/GDCore/IDE/Events/ExpressionsParameterMover.cpp index d03c6d4be5..acc3883840 100644 --- a/Core/GDCore/IDE/Events/ExpressionsParameterMover.cpp +++ b/Core/GDCore/IDE/Events/ExpressionsParameterMover.cpp @@ -87,7 +87,8 @@ class GD_CORE_API ExpressionParameterMover }; if (node.functionName == functionName) { - if (!objectType.empty() && !node.objectName.empty()) { + if (behaviorType.empty() && !objectType.empty() && + !node.objectName.empty()) { // Move parameter of an object function const gd::String& thisObjectType = gd::GetTypeOfObject( globalObjectsContainer, objectsContainer, node.objectName); @@ -103,7 +104,7 @@ class GD_CORE_API ExpressionParameterMover moveParameter(node.parameters); hasDoneMoving = true; } - } else { + } else if (behaviorType.empty() && objectType.empty()) { // Move parameter of a free function moveParameter(node.parameters); hasDoneMoving = true; @@ -119,10 +120,12 @@ class GD_CORE_API ExpressionParameterMover bool hasDoneMoving; const gd::ObjectsContainer& globalObjectsContainer; const gd::ObjectsContainer& objectsContainer; - const gd::String& behaviorType; // The behavior type for which the expression - // must be replaced (optional) - const gd::String& objectType; // The object type for which the expression - // must be replaced (optional) + const gd::String& behaviorType; // The behavior type of the function which + // must have a parameter moved (optional). + const gd::String& objectType; // The object type of the function which + // must have a parameter moved (optional). If + // `behaviorType` is not empty, it takes + // precedence over `objectType`. const gd::String& functionName; std::size_t oldIndex; std::size_t newIndex; diff --git a/Core/GDCore/IDE/Events/ExpressionsRenamer.cpp b/Core/GDCore/IDE/Events/ExpressionsRenamer.cpp index c6da9640a2..b8e743c33c 100644 --- a/Core/GDCore/IDE/Events/ExpressionsRenamer.cpp +++ b/Core/GDCore/IDE/Events/ExpressionsRenamer.cpp @@ -73,7 +73,8 @@ class GD_CORE_API ExpressionFunctionRenamer void OnVisitIdentifierNode(IdentifierNode& node) override {} void OnVisitFunctionNode(FunctionNode& node) override { if (node.functionName == oldFunctionName) { - if (!objectType.empty() && !node.objectName.empty()) { + if (behaviorType.empty() && !objectType.empty() && + !node.objectName.empty()) { // Replace an object function const gd::String& thisObjectType = gd::GetTypeOfObject( globalObjectsContainer, objectsContainer, node.objectName); @@ -89,7 +90,7 @@ class GD_CORE_API ExpressionFunctionRenamer node.functionName = newFunctionName; hasDoneRenaming = true; } - } else { + } else if (behaviorType.empty() && objectType.empty()) { // Replace a free function node.functionName = newFunctionName; hasDoneRenaming = true; @@ -106,9 +107,11 @@ class GD_CORE_API ExpressionFunctionRenamer const gd::ObjectsContainer& globalObjectsContainer; const gd::ObjectsContainer& objectsContainer; const gd::String& behaviorType; // The behavior type for which the expression - // must be replaced (optional) + // must be replaced (optional). const gd::String& objectType; // The object type for which the expression - // must be replaced (optional) + // must be replaced (optional). If + // `behaviorType` is not empty, it takes + // precedence over `objectType`. const gd::String& oldFunctionName; const gd::String& newFunctionName; }; diff --git a/Core/GDCore/IDE/WholeProjectRefactorer.cpp b/Core/GDCore/IDE/WholeProjectRefactorer.cpp index 7de8d88534..0ad73f7d75 100644 --- a/Core/GDCore/IDE/WholeProjectRefactorer.cpp +++ b/Core/GDCore/IDE/WholeProjectRefactorer.cpp @@ -470,7 +470,17 @@ void WholeProjectRefactorer::RenameBehaviorProperty( auto& properties = eventsBasedBehavior.GetPropertyDescriptors(); if (!properties.Has(oldPropertyName)) return; - const auto& property = properties.Get(oldPropertyName); + // Order is important: we first rename the expressions then the instructions, + // to avoid being unable to fetch the metadata (the types of parameters) of + // instructions after they are renamed. + gd::ExpressionsRenamer expressionRenamer = + gd::ExpressionsRenamer(project.GetCurrentPlatform()); + expressionRenamer.SetReplacedBehaviorExpression( + GetBehaviorFullType(eventsFunctionsExtension.GetName(), + eventsBasedBehavior.GetName()), + EventsBasedBehavior::GetPropertyExpressionName(oldPropertyName), + EventsBasedBehavior::GetPropertyExpressionName(newPropertyName)); + ExposeProjectEvents(project, expressionRenamer); gd::InstructionsTypeRenamer actionRenamer = gd::InstructionsTypeRenamer( project, @@ -495,15 +505,6 @@ void WholeProjectRefactorer::RenameBehaviorProperty( eventsBasedBehavior.GetName(), EventsBasedBehavior::GetPropertyConditionName(newPropertyName))); ExposeProjectEvents(project, conditionRenamer); - - gd::ExpressionsRenamer expressionRenamer = - gd::ExpressionsRenamer(project.GetCurrentPlatform()); - expressionRenamer.SetReplacedBehaviorExpression( - GetBehaviorFullType(eventsFunctionsExtension.GetName(), - eventsBasedBehavior.GetName()), - EventsBasedBehavior::GetPropertyExpressionName(oldPropertyName), - EventsBasedBehavior::GetPropertyExpressionName(newPropertyName)); - ExposeProjectEvents(project, expressionRenamer); } void WholeProjectRefactorer::RenameEventsBasedBehavior( diff --git a/Core/GDCore/IDE/WholeProjectRefactorer.h b/Core/GDCore/IDE/WholeProjectRefactorer.h index 5c246b3f08..469ef23856 100644 --- a/Core/GDCore/IDE/WholeProjectRefactorer.h +++ b/Core/GDCore/IDE/WholeProjectRefactorer.h @@ -50,7 +50,10 @@ class GD_CORE_API WholeProjectRefactorer { gd::ArbitraryEventsWorkerWithContext& worker); /** - * \brief Refactor the project after an events function extension is renamed + * \brief Refactor the project **before** an events function extension is renamed. + * + * \warning Do the renaming of the specified extension after calling this. + * This is because the extension is expected to have its old name for the refactoring. */ static void RenameEventsFunctionsExtension( gd::Project& project, @@ -59,7 +62,10 @@ class GD_CORE_API WholeProjectRefactorer { const gd::String& newName); /** - * \brief Refactor the project after an events function is renamed + * \brief Refactor the project **before** an events function is renamed. + * + * \warning Do the renaming of the specified function after calling this. + * This is because the function is expected to have its old name for the refactoring. */ static void RenameEventsFunction( gd::Project& project, @@ -68,8 +74,11 @@ class GD_CORE_API WholeProjectRefactorer { const gd::String& newFunctionName); /** - * \brief Refactor the project after an events function of a behavior is + * \brief Refactor the project **before** an events function of a behavior is * renamed. + * + * \warning Do the renaming of the specified function after calling this. + * This is because the function is expected to have its old name for the refactoring. */ static void RenameBehaviorEventsFunction( gd::Project& project, @@ -79,8 +88,11 @@ class GD_CORE_API WholeProjectRefactorer { const gd::String& newFunctionName); /** - * \brief Refactor the project after an events function parameter - * was moved. + * \brief Refactor the project **before** an events function parameter + * is moved. + * + * \warning Do the move of the specified function parameters after calling this. + * This is because the function is expected to be in its old state for the refactoring. */ static void MoveEventsFunctionParameter( gd::Project& project, @@ -90,8 +102,11 @@ class GD_CORE_API WholeProjectRefactorer { std::size_t newIndex); /** - * \brief Refactor the project after the parmaeter of an events function of a - * behavior was moved. + * \brief Refactor the project **before** the parameter of an events function of a + * behavior is moved. + * + * \warning Do the move of the specified function parameters after calling this. + * This is because the function is expected to be in its old state for the refactoring. */ static void MoveBehaviorEventsFunctionParameter( gd::Project& project, @@ -102,8 +117,11 @@ class GD_CORE_API WholeProjectRefactorer { std::size_t newIndex); /** - * \brief Refactor the project after a property of a behavior is + * \brief Refactor the project **before** a property of a behavior is * renamed. + * + * \warning Do the renaming of the specified property after calling this. + * This is because the property is expected to have its old name for the refactoring. */ static void RenameBehaviorProperty( gd::Project& project, @@ -113,7 +131,10 @@ class GD_CORE_API WholeProjectRefactorer { const gd::String& newPropertyName); /** - * \brief Refactor the project after a behavior is renamed. + * \brief Refactor the project **before** a behavior is renamed. + * + * \warning Do the renaming of the specified behavior after calling this. + * This is because the behavior is expected to have its old name for the refactoring. */ static void RenameEventsBasedBehavior( gd::Project& project, diff --git a/Core/tests/WholeProjectRefactorer.cpp b/Core/tests/WholeProjectRefactorer.cpp index bb73854846..d7c5f4fb2e 100644 --- a/Core/tests/WholeProjectRefactorer.cpp +++ b/Core/tests/WholeProjectRefactorer.cpp @@ -126,8 +126,7 @@ gd::EventsFunctionsExtension &SetupProjectWithEventsFunctionExtension( layout.GetEvents().InsertEvent(event); } - // Create an event in the layout referring to - // MyEventsExtension::MyEventsBasedBehavior::MyBehaviorEventsFunction + // Create an event in the layout using "MyProperty" action { gd::StandardEvent event; gd::Instruction instruction; @@ -138,6 +137,33 @@ gd::EventsFunctionsExtension &SetupProjectWithEventsFunctionExtension( layout.GetEvents().InsertEvent(event); } + // Create an event in the layout using "MyProperty" condition + { + gd::StandardEvent event; + gd::Instruction instruction; + instruction.SetType( + "MyEventsExtension::MyEventsBasedBehavior::" + + gd::EventsBasedBehavior::GetPropertyConditionName("MyProperty")); + event.GetConditions().Insert(instruction); + layout.GetEvents().InsertEvent(event); + } + + // Create an event in the layout using "MyProperty" expression + { + gd::StandardEvent event; + gd::Instruction instruction; + instruction.SetType("MyExtension::DoSomething"); + instruction.SetParametersCount(1); + instruction.SetParameter( + 0, + gd::Expression( + "ObjectWithMyBehavior.MyBehavior::" + + gd::EventsBasedBehavior::GetPropertyExpressionName("MyProperty") + + "()")); + event.GetActions().Insert(instruction); + layout.GetEvents().InsertEvent(event); + } + // Create an event in ExternalEvents1 referring to // MyEventsExtension::MyEventsBasedBehavior::MyBehaviorEventsFunctionExpression { @@ -148,6 +174,22 @@ gd::EventsFunctionsExtension &SetupProjectWithEventsFunctionExtension( instruction.SetParameter( 0, gd::Expression("1 + " + "ObjectWithMyBehavior.MyBehavior::" + "MyBehaviorEventsFunctionExpression(123, 456, 789)")); + event.GetActions().Insert(instruction); + externalEvents.GetEvents().InsertEvent(event); + } + + // Create an event in ExternalEvents1 **wrongly** referring to + // MyEventsExtension::MyEventsBasedBehavior::MyBehaviorEventsFunctionExpression + { + gd::StandardEvent event; + gd::Instruction instruction; + instruction.SetType("MyExtension::DoSomething"); + instruction.SetParametersCount(1); + instruction.SetParameter( + 0, + gd::Expression("2 + " "ObjectWithMyBehavior::MyBehavior." "MyBehaviorEventsFunctionExpression(123, 456, 789)")); event.GetActions().Insert(instruction); @@ -506,7 +548,7 @@ TEST_CASE("WholeProjectRefactorer", "[common]") { .GetBehavior("MyBehavior") .GetTypeName() == "MyRenamedExtension::MyEventsBasedBehavior"); - // Check if events based behaviors functions have been renamed in + // Check if events-based behavior methods have been renamed in // instructions REQUIRE(static_cast( project.GetLayout("LayoutWithBehaviorFunctions") @@ -518,7 +560,7 @@ TEST_CASE("WholeProjectRefactorer", "[common]") { "MyRenamedExtension::MyEventsBasedBehavior::" "MyBehaviorEventsFunction"); - // Check if events based behaviors properties have been renamed in + // Check if events-based behaviors properties have been renamed in // instructions REQUIRE(static_cast( project.GetLayout("LayoutWithBehaviorFunctions") @@ -530,7 +572,7 @@ TEST_CASE("WholeProjectRefactorer", "[common]") { "MyRenamedExtension::MyEventsBasedBehavior::" "SetPropertyMyProperty"); - // Check events based behaviors functions have *not* been renamed in + // Check events-based behavior methods have *not* been renamed in // expressions REQUIRE(static_cast( project.GetExternalEvents("ExternalEventsWithBehaviorFunctions") @@ -541,7 +583,7 @@ TEST_CASE("WholeProjectRefactorer", "[common]") { .GetParameter(0) .GetPlainString() == "1 + " - "ObjectWithMyBehavior::MyBehavior." + "ObjectWithMyBehavior.MyBehavior::" "MyBehaviorEventsFunctionExpression(123, 456, 789)"); } SECTION("(Free) events function renamed") { @@ -586,23 +628,18 @@ TEST_CASE("WholeProjectRefactorer", "[common]") { SetupProjectWithDummyPlatform(project, platform); auto &eventsExtension = SetupProjectWithEventsFunctionExtension(project); - gd::WholeProjectRefactorer::MoveEventsFunctionParameter(project, - eventsExtension, - "MyEventsFunction", - 0, 2); gd::WholeProjectRefactorer::MoveEventsFunctionParameter( - project, - eventsExtension, - "MyEventsFunctionExpression", - 0, 1); + project, eventsExtension, "MyEventsFunction", 0, 2); + gd::WholeProjectRefactorer::MoveEventsFunctionParameter( + project, eventsExtension, "MyEventsFunctionExpression", 0, 1); // Check that events function calls in instructions have been updated - auto& action = static_cast( - project.GetLayout("LayoutWithFreeFunctions") - .GetEvents() - .GetEvent(0)) - .GetActions() - .Get(0); + auto &action = static_cast( + project.GetLayout("LayoutWithFreeFunctions") + .GetEvents() + .GetEvent(0)) + .GetActions() + .Get(0); REQUIRE(action.GetParameter(0).GetPlainString() == "Second parameter"); REQUIRE(action.GetParameter(1).GetPlainString() == "Third parameter"); REQUIRE(action.GetParameter(2).GetPlainString() == "First parameter"); @@ -644,7 +681,7 @@ TEST_CASE("WholeProjectRefactorer", "[common]") { .GetTypeName() == "MyEventsExtension::MyRenamedEventsBasedBehavior"); - // Check if events based behaviors functions have been renamed in + // Check if events-based behavior methods have been renamed in // instructions REQUIRE(static_cast( project.GetLayout("LayoutWithBehaviorFunctions") @@ -656,7 +693,7 @@ TEST_CASE("WholeProjectRefactorer", "[common]") { "MyEventsExtension::MyRenamedEventsBasedBehavior::" "MyBehaviorEventsFunction"); - // Check if events based behaviors properties have been renamed in + // Check if events-based behaviors properties have been renamed in // instructions REQUIRE(static_cast( project.GetLayout("LayoutWithBehaviorFunctions") @@ -668,7 +705,7 @@ TEST_CASE("WholeProjectRefactorer", "[common]") { "MyEventsExtension::MyRenamedEventsBasedBehavior::" "SetPropertyMyProperty"); - // Check events based behaviors functions have *not* been renamed in + // Check events-based behavior methods have *not* been renamed in // expressions REQUIRE(static_cast( project.GetExternalEvents("ExternalEventsWithBehaviorFunctions") @@ -679,7 +716,7 @@ TEST_CASE("WholeProjectRefactorer", "[common]") { .GetParameter(0) .GetPlainString() == "1 + " - "ObjectWithMyBehavior::MyBehavior." + "ObjectWithMyBehavior.MyBehavior::" "MyBehaviorEventsFunctionExpression(123, 456, 789)"); } SECTION("(Events based Behavior) events function renamed") { @@ -703,7 +740,7 @@ TEST_CASE("WholeProjectRefactorer", "[common]") { "MyBehaviorEventsFunctionExpression", "MyRenamedBehaviorEventsFunctionExpression"); - // Check if events based behaviors functions have been renamed in + // Check if events-based behavior methods have been renamed in // instructions REQUIRE(static_cast( project.GetLayout("LayoutWithBehaviorFunctions") @@ -715,7 +752,7 @@ TEST_CASE("WholeProjectRefactorer", "[common]") { "MyEventsExtension::MyEventsBasedBehavior::" "MyRenamedBehaviorEventsFunction"); - // Check events based behaviors functions have been renamed in + // Check events-based behavior methods have been renamed in // expressions REQUIRE(static_cast( project.GetExternalEvents("ExternalEventsWithBehaviorFunctions") @@ -726,8 +763,22 @@ TEST_CASE("WholeProjectRefactorer", "[common]") { .GetParameter(0) .GetPlainString() == "1 + " - "ObjectWithMyBehavior::MyBehavior." + "ObjectWithMyBehavior.MyBehavior::" "MyRenamedBehaviorEventsFunctionExpression(123, 456, 789)"); + + // Check that a ill-named function that looks a bit like a behavior method + // (but which is actually a free function) is *not* renamed. + REQUIRE(static_cast( + project.GetExternalEvents("ExternalEventsWithBehaviorFunctions") + .GetEvents() + .GetEvent(1)) + .GetActions() + .Get(0) + .GetParameter(0) + .GetPlainString() == + "2 + " + "ObjectWithMyBehavior::MyBehavior." + "MyBehaviorEventsFunctionExpression(123, 456, 789)"); } SECTION("(Events based Behavior) events function parameter moved") { gd::Project project; @@ -742,27 +793,29 @@ TEST_CASE("WholeProjectRefactorer", "[common]") { eventsExtension, eventsBasedBehavior, "MyBehaviorEventsFunction", - 0, 2); + 0, + 2); gd::WholeProjectRefactorer::MoveBehaviorEventsFunctionParameter( project, eventsExtension, eventsBasedBehavior, "MyBehaviorEventsFunctionExpression", - 0, 2); + 0, + 2); - // Check if events based behaviors functions have been renamed in + // Check if parameters of events-based behavior methods have been moved in // instructions - auto& action = static_cast( - project.GetLayout("LayoutWithBehaviorFunctions") - .GetEvents() - .GetEvent(0)) - .GetActions() - .Get(0); + auto &action = static_cast( + project.GetLayout("LayoutWithBehaviorFunctions") + .GetEvents() + .GetEvent(0)) + .GetActions() + .Get(0); REQUIRE(action.GetParameter(0).GetPlainString() == "Second parameter"); REQUIRE(action.GetParameter(1).GetPlainString() == "Third parameter"); REQUIRE(action.GetParameter(2).GetPlainString() == "First parameter"); - // Check events based behaviors functions have been renamed in + // Check parameters of events-based behavior methods have been moved in // expressions REQUIRE(static_cast( project.GetExternalEvents("ExternalEventsWithBehaviorFunctions") @@ -773,8 +826,22 @@ TEST_CASE("WholeProjectRefactorer", "[common]") { .GetParameter(0) .GetPlainString() == "1 + " - "ObjectWithMyBehavior::MyBehavior." + "ObjectWithMyBehavior.MyBehavior::" "MyBehaviorEventsFunctionExpression(456, 789, 123)"); + + // Check that a ill-named function that looks a bit like a behavior method + // (but which is actually a free function) has its parameter *not* moved. + REQUIRE(static_cast( + project.GetExternalEvents("ExternalEventsWithBehaviorFunctions") + .GetEvents() + .GetEvent(1)) + .GetActions() + .Get(0) + .GetParameter(0) + .GetPlainString() == + "2 + " + "ObjectWithMyBehavior::MyBehavior." + "MyBehaviorEventsFunctionExpression(123, 456, 789)"); } SECTION("(Events based Behavior) property renamed") { gd::Project project; @@ -790,7 +857,7 @@ TEST_CASE("WholeProjectRefactorer", "[common]") { "MyProperty", "MyRenamedProperty"); - // Check if events based behaviors property has been renamed in + // Check if events-based behaviors property has been renamed in // instructions REQUIRE(static_cast( project.GetLayout("LayoutWithBehaviorFunctions") @@ -801,5 +868,25 @@ TEST_CASE("WholeProjectRefactorer", "[common]") { .GetType() == "MyEventsExtension::MyEventsBasedBehavior::" "SetPropertyMyRenamedProperty"); + + REQUIRE(static_cast( + project.GetLayout("LayoutWithBehaviorFunctions") + .GetEvents() + .GetEvent(2)) + .GetConditions() + .Get(0) + .GetType() == + "MyEventsExtension::MyEventsBasedBehavior::" + "PropertyMyRenamedProperty"); + + REQUIRE(static_cast( + project.GetLayout("LayoutWithBehaviorFunctions") + .GetEvents() + .GetEvent(3)) + .GetActions() + .Get(0) + .GetParameter(0) + .GetPlainString() == + "ObjectWithMyBehavior.MyBehavior::PropertyMyRenamedProperty()"); } }