diff --git a/Core/GDCore/Events/EventVisitor.h b/Core/GDCore/Events/EventVisitor.h index 3468d4c617..a976f1b43b 100644 --- a/Core/GDCore/Events/EventVisitor.h +++ b/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; }; } diff --git a/Core/GDCore/IDE/Events/ArbitraryEventsWorker.cpp b/Core/GDCore/IDE/Events/ArbitraryEventsWorker.cpp index 71956b167f..10d88d5a2f 100644 --- a/Core/GDCore/IDE/Events/ArbitraryEventsWorker.cpp +++ b/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 conditionsVectors = event.GetAllConditionsVectors(); for (std::size_t j = 0; j < conditionsVectors.size(); ++j) { + if (shouldStopIteration) { + break; + } VisitInstructionList(*conditionsVectors[j], true); } const vector 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 diff --git a/Core/GDCore/IDE/Events/ArbitraryEventsWorker.h b/Core/GDCore/IDE/Events/ArbitraryEventsWorker.h index 63c5699b85..dde7e9cf48 100644 --- a/Core/GDCore/IDE/Events/ArbitraryEventsWorker.h +++ b/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; }; /** diff --git a/Core/GDCore/IDE/Events/EventsFunctionSelfCallChecker.cpp b/Core/GDCore/IDE/Events/EventsFunctionSelfCallChecker.cpp new file mode 100644 index 0000000000..805924b49a --- /dev/null +++ b/Core/GDCore/IDE/Events/EventsFunctionSelfCallChecker.cpp @@ -0,0 +1,175 @@ +/* + * GDevelop Core + * Copyright 2008-present Florian Rival (Florian.Rival@gmail.com). 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 diff --git a/Core/GDCore/IDE/Events/EventsFunctionSelfCallChecker.h b/Core/GDCore/IDE/Events/EventsFunctionSelfCallChecker.h new file mode 100644 index 0000000000..dd27e14198 --- /dev/null +++ b/Core/GDCore/IDE/Events/EventsFunctionSelfCallChecker.h @@ -0,0 +1,64 @@ +/* + * GDevelop Core + * Copyright 2008-present Florian Rival (Florian.Rival@gmail.com). All rights + * reserved. This project is released under the MIT License. + */ +#pragma once + +#include "GDCore/String.h" +#include +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 diff --git a/GDevelop.js/Bindings/Bindings.idl b/GDevelop.js/Bindings/Bindings.idl index 915319d24a..795b778d31 100644 --- a/GDevelop.js/Bindings/Bindings.idl +++ b/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(); diff --git a/GDevelop.js/Bindings/Wrapper.cpp b/GDevelop.js/Bindings/Wrapper.cpp index a50867d94c..abe2221f55 100644 --- a/GDevelop.js/Bindings/Wrapper.cpp +++ b/GDevelop.js/Bindings/Wrapper.cpp @@ -44,6 +44,7 @@ #include #include #include +#include #include #include #include @@ -620,6 +621,11 @@ typedef ExtensionAndMetadata 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 diff --git a/GDevelop.js/types/gdeventsfunctionselfcallchecker.js b/GDevelop.js/types/gdeventsfunctionselfcallchecker.js new file mode 100644 index 0000000000..649d1f433e --- /dev/null +++ b/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; +}; \ No newline at end of file diff --git a/GDevelop.js/types/libgdevelop.js b/GDevelop.js/types/libgdevelop.js index cdbc73967f..a42e52d143 100644 --- a/GDevelop.js/types/libgdevelop.js +++ b/GDevelop.js/types/libgdevelop.js @@ -52,6 +52,7 @@ declare class libGDevelop { ProjectHelper: Class; EventsVariablesFinder: Class; EventsIdentifiersFinder: Class; + EventsFunctionSelfCallChecker: Class; InstructionOrExpressionGroupMetadata: Class; VersionWrapper: Class; Platform: Class; diff --git a/newIDE/app/src/EventsSheet/index.js b/newIDE/app/src/EventsSheet/index.js index e3bc7a1d5a..207a280d6f 100644 --- a/newIDE/app/src/EventsSheet/index.js +++ b/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 ( {windowWidth => ( @@ -1718,6 +1745,19 @@ export class EventsSheetComponentWithoutHandle extends React.Component< ref={this._containerDiv} tabIndex={0} > + {isFunctionOnlyCallingItself && ( + + + + + This function calls itself (it is "recursive"). Ensure + this is expected and there is a proper condition to + stop it if necessary. + + + + + )} (this._eventsTree = eventsTree)} key={events.ptr}