From 75663530c595c6d6d777c1df0ebdb24b7f4b0a35 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Davy=20H=C3=A9lard?= <davy.helard@gmail.com>
Date: Tue, 21 May 2024 16:50:20 +0200
Subject: [PATCH] Fix renaming of local variables in sub-events

---
 .../IDE/Events/ArbitraryEventsWorker.cpp      |  7 +-
 ...-ApplyRefactoringForVariablesContainer.cpp | 92 +++++++++++++++++++
 2 files changed, 96 insertions(+), 3 deletions(-)

diff --git a/Core/GDCore/IDE/Events/ArbitraryEventsWorker.cpp b/Core/GDCore/IDE/Events/ArbitraryEventsWorker.cpp
index b1299cc4d0fe..bbc970919f87 100644
--- a/Core/GDCore/IDE/Events/ArbitraryEventsWorker.cpp
+++ b/Core/GDCore/IDE/Events/ArbitraryEventsWorker.cpp
@@ -14,6 +14,7 @@
 #include "GDCore/Events/Expression.h"
 #include "GDCore/Extensions/Metadata/ParameterMetadata.h"
 #include "GDCore/String.h"
+#include "GDCore/Tools/Log.h"
 
 using namespace std;
 
@@ -28,9 +29,6 @@ void AbstractArbitraryEventsWorker::VisitEventList(gd::EventsList& events) {
     if (events[i].AcceptVisitor(*this))
       events.RemoveEvent(i);
     else {
-      if (events[i].CanHaveSubEvents())
-        VisitEventList(events[i].GetSubEvents());
-
       ++i;
     }
   }
@@ -55,6 +53,9 @@ bool AbstractArbitraryEventsWorker::VisitEvent(gd::BaseEvent& event) {
       *expressionAndMetadata.first, expressionAndMetadata.second);
   }
 
+  if (!shouldDelete && event.CanHaveSubEvents()) {
+    VisitEventList(event.GetSubEvents());
+  }
   return shouldDelete;
 }
 
diff --git a/Core/tests/WholeProjectRefactorer-ApplyRefactoringForVariablesContainer.cpp b/Core/tests/WholeProjectRefactorer-ApplyRefactoringForVariablesContainer.cpp
index 2408445bd2e5..0ac8d521040b 100644
--- a/Core/tests/WholeProjectRefactorer-ApplyRefactoringForVariablesContainer.cpp
+++ b/Core/tests/WholeProjectRefactorer-ApplyRefactoringForVariablesContainer.cpp
@@ -770,6 +770,98 @@ TEST_CASE("WholeProjectRefactorer::ApplyRefactoringForVariablesContainer",
     REQUIRE(event.GetActions()[0].GetType() == "SetStringVariable");
   }
 
+  SECTION("Can rename a local variable") {
+    gd::Project project;
+    gd::Platform platform;
+    SetupProjectWithDummyPlatform(project, platform);
+
+    auto &scene = project.InsertNewLayout("Scene", 0);
+
+    gd::StandardEvent &event =
+        dynamic_cast<gd::StandardEvent &>(scene.GetEvents().InsertNewEvent(
+            project, "BuiltinCommonInstructions::Standard"));
+    event.GetVariables().InsertNew("MyLocalVariable").SetValue(123);
+
+    {
+      gd::Instruction action;
+      action.SetType("SetNumberVariable");
+      action.SetParametersCount(3);
+      action.SetParameter(0, gd::Expression("MyLocalVariable"));
+      action.SetParameter(1, gd::Expression("="));
+      action.SetParameter(2, gd::Expression("123"));
+      event.GetActions().Insert(action);
+    }
+
+    // Do the changes and launch the refactoring.
+    event.GetVariables().ResetPersistentUuid();
+    gd::SerializerElement originalSerializedVariables;
+    event.GetVariables().SerializeTo(originalSerializedVariables);
+
+    event.GetVariables().Rename("MyLocalVariable", "MyRenamedLocalVariable");
+    auto changeset =
+        gd::WholeProjectRefactorer::ComputeChangesetForVariablesContainer(
+            originalSerializedVariables, event.GetVariables());
+
+    REQUIRE(changeset.oldToNewVariableNames.find("MyLocalVariable")->second ==
+            "MyRenamedLocalVariable");
+
+    gd::LogMessage("OTHER TEST START");
+
+    gd::WholeProjectRefactorer::ApplyRefactoringForVariablesContainer(
+        project, event.GetVariables(), changeset);
+
+    REQUIRE(event.GetActions()[0].GetParameter(0).GetPlainString() == "MyRenamedLocalVariable");
+  }
+
+  SECTION("Can rename a local variable in sub-events") {
+    gd::Project project;
+    gd::Platform platform;
+    SetupProjectWithDummyPlatform(project, platform);
+
+    auto &scene = project.InsertNewLayout("Scene", 0);
+
+    gd::StandardEvent &event =
+        dynamic_cast<gd::StandardEvent &>(scene.GetEvents().InsertNewEvent(
+            project, "BuiltinCommonInstructions::Standard"));
+    event.GetVariables().InsertNew("MyLocalVariable").SetValue(123);
+
+    gd::StandardEvent &subEvent =
+        dynamic_cast<gd::StandardEvent &>(event.GetSubEvents().InsertNewEvent(
+            project, "BuiltinCommonInstructions::Standard"));
+
+    {
+      gd::Instruction action;
+      action.SetType("SetNumberVariable");
+      action.SetParametersCount(3);
+      action.SetParameter(0, gd::Expression("MyLocalVariable"));
+      action.SetParameter(1, gd::Expression("="));
+      action.SetParameter(2, gd::Expression("123"));
+      subEvent.GetActions().Insert(action);
+    }
+
+    // Do the changes and launch the refactoring.
+    event.GetVariables().ResetPersistentUuid();
+    gd::SerializerElement originalSerializedVariables;
+    event.GetVariables().SerializeTo(originalSerializedVariables);
+
+    event.GetVariables().Rename("MyLocalVariable", "MyRenamedLocalVariable");
+    auto changeset =
+        gd::WholeProjectRefactorer::ComputeChangesetForVariablesContainer(
+            originalSerializedVariables, event.GetVariables());
+
+    REQUIRE(changeset.oldToNewVariableNames.find("MyLocalVariable")->second ==
+            "MyRenamedLocalVariable");
+
+    gd::LogMessage("TEST START");
+
+    gd::WholeProjectRefactorer::ApplyRefactoringForVariablesContainer(
+        project, event.GetVariables(), changeset);
+
+    gd::LogMessage("TEST END");
+
+    REQUIRE(subEvent.GetActions()[0].GetParameter(0).GetPlainString() == "MyRenamedLocalVariable");
+  }
+
   SECTION("Can change the instruction type of variable occurrences (function)") {
     gd::Project project;
     gd::Platform platform;