Add an alert when event-functions only call themselves (#5324)

Core/GDCore/Events/EventVisitor.h

@@ -66,6 +66,11 @@ class GD_CORE_API ReadOnlyEventVisitor {
    * Note that VisitEvent is also called with this event.
    */
   virtual void VisitLinkEvent(const gd::LinkEvent& linkEvent) = 0;
+
+  /**
+   * @brief Abort the iteration on the events.
+   */
+  virtual void StopAnyEventIteration() = 0;
 };
 
 }

Core/GDCore/IDE/Events/ArbitraryEventsWorker.cpp

@@ -84,6 +84,9 @@ void ReadOnlyArbitraryEventsWorker::VisitEventList(const gd::EventsList& events)
   DoVisitEventList(events);
 
   for (std::size_t i = 0; i < events.size(); ++i) {
+    if (shouldStopIteration) {
+      break;
+    }
     events[i].AcceptVisitor(*this);
 
     if (events[i].CanHaveSubEvents()) {
@@ -98,11 +101,17 @@ void ReadOnlyArbitraryEventsWorker::VisitEvent(const gd::BaseEvent& event) {
   const vector<const gd::InstructionsList*> conditionsVectors =
       event.GetAllConditionsVectors();
   for (std::size_t j = 0; j < conditionsVectors.size(); ++j) {
+    if (shouldStopIteration) {
+      break;
+    }
     VisitInstructionList(*conditionsVectors[j], true);
   }
 
   const vector<const gd::InstructionsList*> actionsVectors = event.GetAllActionsVectors();
   for (std::size_t j = 0; j < actionsVectors.size(); ++j) {
+    if (shouldStopIteration) {
+      break;
+    }
     VisitInstructionList(*actionsVectors[j], false);
   }
 }
@@ -116,6 +125,9 @@ void ReadOnlyArbitraryEventsWorker::VisitInstructionList(
   DoVisitInstructionList(instructions, areConditions);
 
   for (std::size_t i = 0; i < instructions.size(); ++i) {
+    if (shouldStopIteration) {
+      break;
+    }
     VisitInstruction(instructions[i], areConditions);
     if (!instructions[i].GetSubInstructions().empty()) {
       VisitInstructionList(instructions[i].GetSubInstructions(),
@@ -129,6 +141,10 @@ void ReadOnlyArbitraryEventsWorker::VisitInstruction(const gd::Instruction& inst
   DoVisitInstruction(instruction, isCondition);
 }
 
+void ReadOnlyArbitraryEventsWorker::StopAnyEventIteration() {
+  shouldStopIteration = true;
+}
+
 ReadOnlyArbitraryEventsWorkerWithContext::~ReadOnlyArbitraryEventsWorkerWithContext() {}
 
 }  // namespace gd

Core/GDCore/IDE/Events/ArbitraryEventsWorker.h

@@ -144,7 +144,7 @@ class GD_CORE_API ArbitraryEventsWorkerWithContext
  */
 class GD_CORE_API ReadOnlyArbitraryEventsWorker : private ReadOnlyEventVisitor {
  public:
-  ReadOnlyArbitraryEventsWorker(){};
+  ReadOnlyArbitraryEventsWorker() : shouldStopIteration(false) {};
   virtual ~ReadOnlyArbitraryEventsWorker();
 
   /**
@@ -152,6 +152,9 @@ class GD_CORE_API ReadOnlyArbitraryEventsWorker : private ReadOnlyEventVisitor {
    */
   void Launch(const gd::EventsList& events) { VisitEventList(events); };
 
+protected:
+  void StopAnyEventIteration() override;
+
  private:
   void VisitEventList(const gd::EventsList& events);
   void VisitEvent(const gd::BaseEvent& event) override;
@@ -188,6 +191,8 @@ class GD_CORE_API ReadOnlyArbitraryEventsWorker : private ReadOnlyEventVisitor {
    */
   virtual void DoVisitInstruction(const gd::Instruction& instruction,
                                   bool isCondition) {};
+
+  bool shouldStopIteration;
 };
 
 /**

Core/GDCore/IDE/Events/EventsFunctionSelfCallChecker.cpp

@@ -0,0 +1,175 @@
+/*
+ * GDevelop Core
+ * Copyright 2008-present Florian Rival ([email protected]). All rights
+ * reserved. This project is released under the MIT License.
+ */
+#include "EventsFunctionSelfCallChecker.h"
+
+#include "GDCore/Events/Expression.h"
+#include "GDCore/Events/Parsers/ExpressionParser2Node.h"
+#include "GDCore/Events/Parsers/ExpressionParser2NodeWorker.h"
+#include "GDCore/Extensions/Metadata/ExpressionMetadata.h"
+#include "GDCore/Extensions/Metadata/MetadataProvider.h"
+#include "GDCore/Extensions/Metadata/ParameterMetadataTools.h"
+#include "GDCore/Extensions/PlatformExtension.h"
+#include "GDCore/IDE/Events/ArbitraryEventsWorker.h"
+#include "GDCore/Project/EventsBasedBehavior.h"
+#include "GDCore/Project/EventsBasedObject.h"
+#include "GDCore/Project/EventsFunction.h"
+#include "GDCore/Project/EventsFunctionsExtension.h"
+#include "GDCore/Project/Object.h"
+#include "GDCore/Project/Project.h"
+#include "GDCore/String.h"
+#include "GDCore/Tools/Log.h"
+
+namespace gd {
+
+/**
+ * \brief Check the type of the first condition or action.
+ */
+class GD_CORE_API FirstInstructionTypeChecker
+    : public ReadOnlyArbitraryEventsWorker {
+public:
+  FirstInstructionTypeChecker(const gd::String &eventFunctionType_,
+                              const bool isEventFunctionCondition_)
+      : eventFunctionType(eventFunctionType_),
+        isEventFunctionCondition(isEventFunctionCondition_),
+        isOnlyCallingItself(false){};
+  virtual ~FirstInstructionTypeChecker(){};
+
+  void DoVisitInstruction(const gd::Instruction &instruction,
+                          bool isCondition) override {
+    if (isCondition == isEventFunctionCondition) {
+      isOnlyCallingItself = instruction.GetType() == eventFunctionType;
+      StopAnyEventIteration();
+    }
+  };
+
+  bool isOnlyCallingItself;
+
+private:
+  gd::String eventFunctionType;
+  bool isEventFunctionCondition;
+};
+
+/**
+ * \brief Check the type of the first condition or action.
+ */
+class GD_CORE_API FirstActionExpressionTypeChecker
+    : public ReadOnlyArbitraryEventsWorker,
+      ExpressionParser2NodeWorker {
+public:
+  FirstActionExpressionTypeChecker(const gd::Platform &platform_,
+                                   const gd::String &eventFunctionType_)
+      : platform(platform_), eventFunctionType(eventFunctionType_),
+        isOnlyCallingItself(false){};
+  virtual ~FirstActionExpressionTypeChecker(){};
+
+  void DoVisitInstruction(const gd::Instruction &instruction,
+                          bool isCondition) override {
+    // Typically, it should be a "return" action.
+    if (!isCondition) {
+      gd::String lastObjectParameter = "";
+      const gd::InstructionMetadata &instrInfos =
+          MetadataProvider::GetActionMetadata(platform, instruction.GetType());
+      for (std::size_t pNb = 0; pNb < instrInfos.parameters.size(); ++pNb) {
+
+        if (ParameterMetadata::IsExpression(
+                "number", instrInfos.parameters[pNb].GetType()) ||
+            ParameterMetadata::IsExpression(
+                "string", instrInfos.parameters[pNb].GetType())) {
+          auto node = instruction.GetParameter(pNb).GetRootNode();
+          node->Visit(*this);
+        }
+      }
+      StopAnyEventIteration();
+    }
+  }
+
+  // Only check expressions that directly calls a function.
+  void OnVisitFunctionCallNode(FunctionCallNode &node) override {
+    isOnlyCallingItself |= node.functionName == eventFunctionType;
+  }
+
+  // Handle extra parenthesis.
+  void OnVisitSubExpressionNode(SubExpressionNode &node) override {
+    node.expression->Visit(*this);
+  }
+  void OnVisitOperatorNode(OperatorNode &node) override {}
+  // Handle sign that could have been forgotten
+  void OnVisitUnaryOperatorNode(UnaryOperatorNode &node) override {
+    node.factor->Visit(*this);
+  }
+  void OnVisitNumberNode(NumberNode &node) override {}
+  void OnVisitTextNode(TextNode &node) override {}
+  void OnVisitVariableNode(VariableNode &node) override {}
+  void OnVisitVariableAccessorNode(VariableAccessorNode &node) override {}
+  void OnVisitVariableBracketAccessorNode(
+      VariableBracketAccessorNode &node) override {}
+  void OnVisitIdentifierNode(IdentifierNode &node) override {}
+  void OnVisitObjectFunctionNameNode(ObjectFunctionNameNode &node) override {}
+  void OnVisitEmptyNode(EmptyNode &node) override {}
+
+  bool isOnlyCallingItself;
+
+private:
+  const gd::Platform &platform;
+  gd::String eventFunctionType;
+};
+
+bool EventsFunctionSelfCallChecker::IsOnlyCallingItself(
+    const gd::Project &project, const gd::String &functionFullType,
+    const gd::EventsFunction &eventsFunction) {
+  bool isOnlyCallingItself = false;
+
+  if (eventsFunction.IsExpression()) {
+    FirstActionExpressionTypeChecker eventWorker(project.GetCurrentPlatform(),
+                                                 functionFullType);
+    eventWorker.Launch(eventsFunction.GetEvents());
+    isOnlyCallingItself = eventWorker.isOnlyCallingItself;
+  } else {
+    FirstInstructionTypeChecker eventWorker(functionFullType,
+                                            !eventsFunction.IsAction());
+    eventWorker.Launch(eventsFunction.GetEvents());
+    isOnlyCallingItself = eventWorker.isOnlyCallingItself;
+  }
+  return isOnlyCallingItself;
+}
+
+bool EventsFunctionSelfCallChecker::IsFreeFunctionOnlyCallingItself(
+    const gd::Project &project,
+    const gd::EventsFunctionsExtension &eventsFunctionsExtension,
+    const gd::EventsFunction &eventsFunction) {
+  return IsOnlyCallingItself(
+      project,
+      PlatformExtension::GetEventsFunctionFullType(
+          eventsFunctionsExtension.GetName(), eventsFunction.GetName()),
+      eventsFunction);
+}
+
+bool EventsFunctionSelfCallChecker::IsBehaviorFunctionOnlyCallingItself(
+    const gd::Project &project,
+    const gd::EventsFunctionsExtension &eventsFunctionsExtension,
+    const gd::EventsBasedBehavior &eventsBasedBehavior,
+    const gd::EventsFunction &eventsFunction) {
+  return IsOnlyCallingItself(
+      project,
+      PlatformExtension::GetBehaviorEventsFunctionFullType(
+          eventsFunctionsExtension.GetName(), eventsBasedBehavior.GetName(),
+          eventsFunction.GetName()),
+      eventsFunction);
+}
+bool EventsFunctionSelfCallChecker::IsObjectFunctionOnlyCallingItself(
+    const gd::Project &project,
+    const gd::EventsFunctionsExtension &eventsFunctionsExtension,
+    const gd::EventsBasedObject &eventsBasedObject,
+    const gd::EventsFunction &eventsFunction) {
+  return IsOnlyCallingItself(project,
+                             PlatformExtension::GetObjectEventsFunctionFullType(
+                                 eventsFunctionsExtension.GetName(),
+                                 eventsBasedObject.GetName(),
+                                 eventsFunction.GetName()),
+                             eventsFunction);
+}
+
+} // namespace gd

Core/GDCore/IDE/Events/EventsFunctionSelfCallChecker.h

@@ -0,0 +1,64 @@
+/*
+ * GDevelop Core
+ * Copyright 2008-present Florian Rival ([email protected]). All rights
+ * reserved. This project is released under the MIT License.
+ */
+#pragma once
+
+#include "GDCore/String.h"
+#include <vector>
+namespace gd {
+class Project;
+class EventsFunctionsExtension;
+class EventsFunctionsContainer;
+class EventsFunction;
+class EventsBasedBehavior;
+class EventsBasedObject;
+} // namespace gd
+
+namespace gd {
+/**
+ * \brief Check if functions are only calling themselves.
+ *
+ * It allows to detect mistakes when implementing functions for compatibility
+ * after a function renaming.
+ * Infinite loops can happens when the legacy function call itself instead of
+ * the new function.
+ * 
+ * \note Only the first instruction or the expressions of the first action is
+ * checked.
+ */
+class GD_CORE_API EventsFunctionSelfCallChecker {
+public:
+  /**
+   * \brief Check if a free function is only calling itself.
+   */
+  static bool IsFreeFunctionOnlyCallingItself(
+      const gd::Project &project,
+      const gd::EventsFunctionsExtension &eventsFunctionsExtension,
+      const gd::EventsFunction &eventsFunction);
+
+  /**
+   * \brief Check if a behavior function is only calling itself.
+   */
+  static bool IsBehaviorFunctionOnlyCallingItself(
+      const gd::Project &project,
+      const gd::EventsFunctionsExtension &eventsFunctionsExtension,
+      const gd::EventsBasedBehavior &eventsBasedBehavior,
+      const gd::EventsFunction &eventsFunction);
+
+  /**
+   * \brief Check if an object function is only calling itself.
+   */
+  static bool IsObjectFunctionOnlyCallingItself(
+      const gd::Project &project,
+      const gd::EventsFunctionsExtension &eventsFunctionsExtension,
+      const gd::EventsBasedObject &eventsBasedObject,
+      const gd::EventsFunction &eventsFunction);
+
+private:
+  static bool IsOnlyCallingItself(const gd::Project &project,
+                                  const gd::String &functionFullType,
+                                  const gd::EventsFunction &eventsFunction);
+};
+} // namespace gd

GDevelop.js/Bindings/Bindings.idl

@@ -143,6 +143,25 @@ interface EventsIdentifiersFinder {
     //Inherited from ExpressionParser2NodeWorker:
 };
 
+interface EventsFunctionSelfCallChecker {
+  boolean STATIC_IsFreeFunctionOnlyCallingItself(
+      [Const, Ref] Project project,
+      [Const, Ref] EventsFunctionsExtension extension,
+      [Const, Ref] EventsFunction eventsFunction);
+
+  boolean STATIC_IsBehaviorFunctionOnlyCallingItself(
+      [Const, Ref] Project project,
+      [Const, Ref] EventsFunctionsExtension extension,
+      [Const, Ref] EventsBasedBehavior eventsBasedBehavior,
+      [Const, Ref] EventsFunction eventsFunction);
+
+  boolean STATIC_IsObjectFunctionOnlyCallingItself(
+      [Const, Ref] Project project,
+      [Const, Ref] EventsFunctionsExtension extension,
+      [Const, Ref] EventsBasedObject eventsBasedObject,
+      [Const, Ref] EventsFunction eventsFunction);
+};
+
 interface InstructionOrExpressionGroupMetadata {
     void InstructionOrExpressionGroupMetadata();
 

GDevelop.js/Bindings/Wrapper.cpp

@@ -44,6 +44,7 @@
 #include <GDCore/IDE/EventsFunctionTools.h>
 #include <GDCore/IDE/Events/EventsVariablesFinder.h>
 #include <GDCore/IDE/Events/EventsIdentifiersFinder.h>
+#include <GDCore/IDE/Events/EventsFunctionSelfCallChecker.h>
 #include <GDCore/IDE/Project/ArbitraryResourceWorker.h>
 #include <GDCore/IDE/Project/ProjectResourcesAdder.h>
 #include <GDCore/IDE/Project/ProjectResourcesCopier.h>
@@ -620,6 +621,11 @@ typedef ExtensionAndMetadata<ExpressionMetadata> ExtensionAndExpressionMetadata;
 #define STATIC_FindAllLayoutVariables FindAllLayoutVariables
 #define STATIC_FindAllObjectVariables FindAllObjectVariables
 #define STATIC_FindAllIdentifierExpressions FindAllIdentifierExpressions
+
+#define STATIC_IsFreeFunctionOnlyCallingItself IsFreeFunctionOnlyCallingItself
+#define STATIC_IsBehaviorFunctionOnlyCallingItself IsBehaviorFunctionOnlyCallingItself
+#define STATIC_IsObjectFunctionOnlyCallingItself IsObjectFunctionOnlyCallingItself
+
 #define STATIC_SearchInEvents SearchInEvents
 #define STATIC_UnfoldWhenContaining UnfoldWhenContaining
 #define STATIC_FoldAll FoldAll

GDevelop.js/types/gdeventsfunctionselfcallchecker.js

@@ -0,0 +1,8 @@
+// Automatically generated by GDevelop.js/scripts/generate-types.js
+declare class gdEventsFunctionSelfCallChecker {
+  static isFreeFunctionOnlyCallingItself(project: gdProject, extension: gdEventsFunctionsExtension, eventsFunction: gdEventsFunction): boolean;
+  static isBehaviorFunctionOnlyCallingItself(project: gdProject, extension: gdEventsFunctionsExtension, eventsBasedBehavior: gdEventsBasedBehavior, eventsFunction: gdEventsFunction): boolean;
+  static isObjectFunctionOnlyCallingItself(project: gdProject, extension: gdEventsFunctionsExtension, eventsBasedObject: gdEventsBasedObject, eventsFunction: gdEventsFunction): boolean;
+  delete(): void;
+  ptr: number;
+};

GDevelop.js/types/libgdevelop.js

@@ -52,6 +52,7 @@ declare class libGDevelop {
   ProjectHelper: Class<gdProjectHelper>;
   EventsVariablesFinder: Class<gdEventsVariablesFinder>;
   EventsIdentifiersFinder: Class<gdEventsIdentifiersFinder>;
+  EventsFunctionSelfCallChecker: Class<gdEventsFunctionSelfCallChecker>;
   InstructionOrExpressionGroupMetadata: Class<gdInstructionOrExpressionGroupMetadata>;
   VersionWrapper: Class<gdVersionWrapper>;
   Platform: Class<gdPlatform>;

newIDE/app/src/EventsSheet/index.js

@@ -1,5 +1,5 @@
 // @flow
-import { t } from '@lingui/macro';
+import { t, Trans } from '@lingui/macro';
 import { type I18n as I18nType } from '@lingui/core';
 
 import * as React from 'react';
@@ -100,6 +100,8 @@ import LeaderboardContext, {
 } from '../Leaderboard/LeaderboardContext';
 import { TutorialContext } from '../Tutorial/TutorialContext';
 import { type Tutorial } from '../Utils/GDevelopServices/Tutorial';
+import AlertMessage from '../UI/AlertMessage';
+import { Column, Line } from '../UI/Grid';
 
 const gd: libGDevelop = global.gd;
 
@@ -1687,6 +1689,31 @@ export class EventsSheetComponentWithoutHandle extends React.Component<
     // eslint-disable-next-line react-hooks/rules-of-hooks
     const screenType = useScreenType();
 
+    const isFunctionOnlyCallingItself =
+      scope.eventsFunctionsExtension &&
+      scope.eventsFunction &&
+      ((!scope.eventsBasedBehavior &&
+        !scope.eventsBasedObject &&
+        gd.EventsFunctionSelfCallChecker.isFreeFunctionOnlyCallingItself(
+          project,
+          scope.eventsFunctionsExtension,
+          scope.eventsFunction
+        )) ||
+        (scope.eventsBasedBehavior &&
+          gd.EventsFunctionSelfCallChecker.isBehaviorFunctionOnlyCallingItself(
+            project,
+            scope.eventsFunctionsExtension,
+            scope.eventsBasedBehavior,
+            scope.eventsFunction
+          )) ||
+        (scope.eventsBasedObject &&
+          gd.EventsFunctionSelfCallChecker.isObjectFunctionOnlyCallingItself(
+            project,
+            scope.eventsFunctionsExtension,
+            scope.eventsBasedObject,
+            scope.eventsFunction
+          )));
+
     return (
       <ResponsiveWindowMeasurer>
         {windowWidth => (
@@ -1718,6 +1745,19 @@ export class EventsSheetComponentWithoutHandle extends React.Component<
                 ref={this._containerDiv}
                 tabIndex={0}
               >
+                {isFunctionOnlyCallingItself && (
+                  <Line>
+                    <Column expand>
+                      <AlertMessage kind="warning">
+                        <Trans>
+                          This function calls itself (it is "recursive"). Ensure
+                          this is expected and there is a proper condition to
+                          stop it if necessary.
+                        </Trans>
+                      </AlertMessage>
+                    </Column>
+                  </Line>
+                )}
                 <EventsTree
                   ref={eventsTree => (this._eventsTree = eventsTree)}
                   key={events.ptr}