From cadcf34eb71609f23d04675e8dec557170dfc18a Mon Sep 17 00:00:00 2001 From: Florian Rival Date: Fri, 29 Nov 2024 10:49:42 +0100 Subject: [PATCH] Fix default behaviors not added properly to objects in functions --- .../Metadata/ParameterMetadataTools.cpp | 20 ++- Core/GDCore/Project/Project.cpp | 62 ++++++-- Core/GDCore/Project/Project.h | 18 ++- GDevelop.js/__tests__/Core.js | 139 +++++++++++++++--- 4 files changed, 189 insertions(+), 50 deletions(-) diff --git a/Core/GDCore/Extensions/Metadata/ParameterMetadataTools.cpp b/Core/GDCore/Extensions/Metadata/ParameterMetadataTools.cpp index 75af1eeb3dd1..d4e949c4b4ae 100644 --- a/Core/GDCore/Extensions/Metadata/ParameterMetadataTools.cpp +++ b/Core/GDCore/Extensions/Metadata/ParameterMetadataTools.cpp @@ -26,7 +26,7 @@ void ParameterMetadataTools::ParametersToObjectsContainer( // Keep track of all objects and their behaviors names, so we can remove // those who are in the container but not in the parameters anymore. std::set allObjectNames; - std::map> allObjectBehaviorNames; + std::map> allObjectNonDefaultBehaviorNames; gd::String lastObjectName; for (std::size_t i = 0; i < parameters.GetParametersCount(); ++i) { @@ -49,11 +49,15 @@ void ParameterMetadataTools::ParametersToObjectsContainer( } if (outputObjectsContainer.HasObjectNamed(objectName)) { - // Keep the existing object - behaviors will be added or removed later. + // Keep the existing object, ensure the default behaviors + // are all present (and no more than required by the object type). + // Non default behaviors coming from parameters will be added or removed later. + project.EnsureObjectDefaultBehaviors(outputObjectsContainer.GetObject(objectName)); } else { + // Create a new object (and its default behaviors) if needed. outputObjectsContainer.InsertNewObject( project, - parameter.GetExtraInfo(), + objectType, objectName, outputObjectsContainer.GetObjectsCount()); } @@ -71,7 +75,7 @@ void ParameterMetadataTools::ParametersToObjectsContainer( const gd::String& behaviorType = parameter.GetExtraInfo(); gd::Object& object = outputObjectsContainer.GetObject(lastObjectName); - allObjectBehaviorNames[lastObjectName].insert(behaviorName); + allObjectNonDefaultBehaviorNames[lastObjectName].insert(behaviorName); // Check if we can keep the existing behavior. if (object.HasBehaviorNamed(behaviorName)) { @@ -108,8 +112,14 @@ void ParameterMetadataTools::ParametersToObjectsContainer( } auto& object = outputObjectsContainer.GetObject(objectName); + const auto& allBehaviorNames = allObjectNonDefaultBehaviorNames[objectName]; for (const auto& behaviorName : object.GetAllBehaviorNames()) { - const auto& allBehaviorNames = allObjectBehaviorNames[objectName]; + if (object.GetBehavior(behaviorName).IsDefaultBehavior()) { + // Default behaviors are already ensured to be all present + // (and no more than required by the object type). + continue; + } + if (allBehaviorNames.find(behaviorName) == allBehaviorNames.end()) { object.RemoveBehavior(behaviorName); } diff --git a/Core/GDCore/Project/Project.cpp b/Core/GDCore/Project/Project.cpp index 4c2c5b4eb968..6eadee16fea9 100644 --- a/Core/GDCore/Project/Project.cpp +++ b/Core/GDCore/Project/Project.cpp @@ -81,13 +81,11 @@ Project::~Project() {} void Project::ResetProjectUuid() { projectUuid = UUID::MakeUuid4(); } -std::unique_ptr Project::CreateObject( - const gd::String& objectType, const gd::String& name) const { - std::unique_ptr object = gd::make_unique( - name, objectType, CreateObjectConfiguration(objectType)); - +void Project::EnsureObjectDefaultBehaviors(gd::Object& object) const { auto& platform = GetCurrentPlatform(); auto& project = *this; + auto& objectType = object.GetType(); + auto addDefaultBehavior = [&platform, &project, &object, &objectType]( const gd::String& behaviorType) { auto& behaviorMetadata = @@ -97,17 +95,47 @@ std::unique_ptr Project::CreateObject( " has an unknown default behavior: " + behaviorType); return; } - auto* behavior = object->AddNewBehavior( - project, behaviorType, behaviorMetadata.GetDefaultName()); - behavior->SetDefaultBehavior(true); + + const gd::String& behaviorName = behaviorMetadata.GetDefaultName(); + + // Check if we can keep a behavior that would have been already set up on the object. + if (object.HasBehaviorNamed(behaviorName)) { + const auto& behavior = object.GetBehavior(behaviorName); + + if (!behavior.IsDefaultBehavior() || behavior.GetTypeName() != behaviorType) { + // Behavior type has changed, remove it so it is re-created. + object.RemoveBehavior(behaviorName); + } + } + + if (!object.HasBehaviorNamed(behaviorName)) { + auto* behavior = object.AddNewBehavior( + project, behaviorType, behaviorName); + behavior->SetDefaultBehavior(true); + } }; auto &objectMetadata = gd::MetadataProvider::GetObjectMetadata(platform, objectType); if (!MetadataProvider::IsBadObjectMetadata(objectMetadata)) { - for (auto &behaviorType : objectMetadata.GetDefaultBehaviors()) { + // Add all default behaviors. + const auto& defaultBehaviorTypes = objectMetadata.GetDefaultBehaviors(); + for (auto &behaviorType : defaultBehaviorTypes) { addDefaultBehavior(behaviorType); } + + // Ensure there are no default behaviors that would not be required left on the object. + for (const auto& behaviorName : object.GetAllBehaviorNames()) { + auto& behavior = object.GetBehavior(behaviorName); + if (!behavior.IsDefaultBehavior()) { + // Non default behaviors are not handled by this function. + continue; + } + + if (defaultBehaviorTypes.find(behavior.GetTypeName()) == defaultBehaviorTypes.end()) { + object.RemoveBehavior(behaviorName); + } + } } // During project deserialization, event-based object metadata are not yet // generated. Default behaviors will be added by @@ -115,6 +143,14 @@ std::unique_ptr Project::CreateObject( else if (!project.HasEventsBasedObject(objectType)) { gd::LogWarning("Object: " + name + " has an unknown type: " + objectType); } +} + +std::unique_ptr Project::CreateObject( + const gd::String& objectType, const gd::String& name) const { + std::unique_ptr object = gd::make_unique( + name, objectType, CreateObjectConfiguration(objectType)); + + EnsureObjectDefaultBehaviors(*object); return std::move(object); } @@ -926,8 +962,8 @@ void Project::UnserializeFrom(const SerializerElement& element) { std::vector Project::GetUnserializingOrderExtensionNames( const gd::SerializerElement &eventsFunctionsExtensionsElement) { - - // Some extension have custom objects, which have child objects coming from other extension. + + // Some extension have custom objects, which have child objects coming from other extension. // These child objects must be loaded completely before the parent custom obejct can be unserialized. // This implies: an order on the extension unserialization (and no cycles). @@ -937,8 +973,8 @@ std::vector Project::GetUnserializingOrderExtensionNames( for (std::size_t i = 0; i < eventsFunctionsExtensions.size(); ++i) { remainingExtensionNames[i] = eventsFunctionsExtensions.at(i)->GetName(); } - - // Helper allowing to find if an extension has an object that depends on + + // Helper allowing to find if an extension has an object that depends on // at least one other object from another extension that is not loaded yet. auto isDependentFromRemainingExtensions = [&remainingExtensionNames]( diff --git a/Core/GDCore/Project/Project.h b/Core/GDCore/Project/Project.h index 1d6e729b3e83..786ece4fabb7 100644 --- a/Core/GDCore/Project/Project.h +++ b/Core/GDCore/Project/Project.h @@ -523,13 +523,7 @@ class GD_CORE_API Project { std::unique_ptr CreateObject(const gd::String& type, const gd::String& name) const; - /** - * Create an object configuration of the given type. - * - * \param type The type of the object - */ - std::unique_ptr CreateObjectConfiguration( - const gd::String& type) const; + void EnsureObjectDefaultBehaviors(gd::Object& object) const; /** * Create an event of the given type. @@ -1078,12 +1072,20 @@ class GD_CORE_API Project { /** * @brief Get the project extensions names in the order they have to be unserialized. - * + * * Child-objects need the event-based objects they use to be loaded completely * before they are unserialized. */ std::vector GetUnserializingOrderExtensionNames(const gd::SerializerElement &eventsFunctionsExtensionsElement); + /** + * Create an object configuration of the given type. + * + * \param type The type of the object + */ + std::unique_ptr CreateObjectConfiguration( + const gd::String& type) const; + gd::String name; ///< Game name gd::String description; ///< Game description gd::String version; ///< Game version number (used for some exports) diff --git a/GDevelop.js/__tests__/Core.js b/GDevelop.js/__tests__/Core.js index 38f06751f8d0..4a1f8d5129da 100644 --- a/GDevelop.js/__tests__/Core.js +++ b/GDevelop.js/__tests__/Core.js @@ -4295,19 +4295,37 @@ describe('libGD.js', function () { expect(objectsContainer.hasObjectNamed('MyObjectWithoutType')).toBe(true); expect(objectsContainer.hasObjectNamed('MySpriteObject')).toBe(true); - const objectWithoutType = objectsContainer.getObject('MyObjectWithoutType'); - expect(objectWithoutType.getType()).toBe( - '' + const objectWithoutType = objectsContainer.getObject( + 'MyObjectWithoutType' ); + expect(objectWithoutType.getType()).toBe(''); const mySpriteObject = objectsContainer.getObject('MySpriteObject'); expect(objectsContainer.getObject('MySpriteObject').getType()).toBe( 'Sprite' ); - // Check that behaviors were also added. - expect(objectsContainer.getObject('MySpriteObject').getAllBehaviorNames().toJSArray()).toEqual( - ['MyFirstBehavior', 'MySecondBehavior'] - ); + // Check that behaviors were also added AND that default behaviors are also present. + expect( + objectsContainer + .getObject('MySpriteObject') + .getAllBehaviorNames() + .toJSArray() + ).toEqual([ + 'Animation', + 'Effect', + 'Flippable', + 'MyFirstBehavior', + 'MySecondBehavior', + 'Opacity', + 'Resizable', + 'Scale', + ]); + expect( + objectsContainer + .getObject('MyObjectWithoutType') + .getAllBehaviorNames() + .toJSArray() + ).toEqual([]); // Call a second time and verify no changes. gd.ParameterMetadataTools.parametersToObjectsContainer( @@ -4318,19 +4336,45 @@ describe('libGD.js', function () { expect(objectsContainer.getObjectsCount()).toBe(2); expect(objectsContainer.hasObjectNamed('MyObjectWithoutType')).toBe(true); expect(objectsContainer.hasObjectNamed('MySpriteObject')).toBe(true); - expect(objectsContainer.getObject('MySpriteObject').getAllBehaviorNames().toJSArray()).toEqual( - ['MyFirstBehavior', 'MySecondBehavior'] - ); + expect( + objectsContainer + .getObject('MySpriteObject') + .getAllBehaviorNames() + .toJSArray() + ).toEqual([ + 'Animation', + 'Effect', + 'Flippable', + 'MyFirstBehavior', + 'MySecondBehavior', + 'Opacity', + 'Resizable', + 'Scale', + ]); + expect( + objectsContainer + .getObject('MyObjectWithoutType') + .getAllBehaviorNames() + .toJSArray() + ).toEqual([]); // Verify that objects are even stable in memory. - expect(objectWithoutType).toBe(objectsContainer.getObject('MyObjectWithoutType')); - expect(gd.getPointer(objectWithoutType)).toBe(gd.getPointer(objectsContainer.getObject('MyObjectWithoutType'))); + expect(objectWithoutType).toBe( + objectsContainer.getObject('MyObjectWithoutType') + ); + expect(gd.getPointer(objectWithoutType)).toBe( + gd.getPointer(objectsContainer.getObject('MyObjectWithoutType')) + ); expect(mySpriteObject).toBe(objectsContainer.getObject('MySpriteObject')); - expect(gd.getPointer(mySpriteObject)).toBe(gd.getPointer(objectsContainer.getObject('MySpriteObject'))); + expect(gd.getPointer(mySpriteObject)).toBe( + gd.getPointer(objectsContainer.getObject('MySpriteObject')) + ); // Change an object type, rename a behavior and add a new object. - parameters.getParameter("MyObjectWithoutType").setExtraInfo('Sprite'); - parameters.getParameter("MySecondBehavior").setName("MyRenamedSecondBehavior"); + parameters.getParameter('MyObjectWithoutType').setExtraInfo('Sprite'); + parameters + .getParameter('MySecondBehavior') + .setName('MyRenamedSecondBehavior'); parameters .addNewParameter('MySpriteObject3') .setType('objectList') @@ -4347,19 +4391,58 @@ describe('libGD.js', function () { expect(objectsContainer.hasObjectNamed('MyObjectWithoutType')).toBe(true); expect(objectsContainer.hasObjectNamed('MySpriteObject')).toBe(true); expect(objectsContainer.hasObjectNamed('MySpriteObject3')).toBe(true); - expect(objectsContainer.getObject('MySpriteObject').getAllBehaviorNames().toJSArray()).toEqual( - ['MyFirstBehavior', 'MyRenamedSecondBehavior'] + expect( + objectsContainer + .getObject('MySpriteObject') + .getAllBehaviorNames() + .toJSArray() + ).toEqual([ + 'Animation', + 'Effect', + 'Flippable', + 'MyFirstBehavior', + 'MyRenamedSecondBehavior', + 'Opacity', + 'Resizable', + 'Scale', + ]); + const updatedObjectWithoutType = objectsContainer.getObject( + 'MyObjectWithoutType' ); - const updatedObjectWithoutType = objectsContainer.getObject('MyObjectWithoutType'); expect(updatedObjectWithoutType.getType()).toEqual('Sprite'); + expect( + updatedObjectWithoutType.getAllBehaviorNames().toJSArray() + ).toEqual([ + 'Animation', + 'Effect', + 'Flippable', + 'Opacity', + 'Resizable', + 'Scale', + ]); + const mySpriteObject3 = objectsContainer.getObject('MySpriteObject3'); expect(mySpriteObject3.getType()).toEqual('Sprite'); + expect( + mySpriteObject3.getAllBehaviorNames().toJSArray() + ).toEqual([ + 'Animation', + 'Effect', + 'Flippable', + 'Opacity', + 'Resizable', + 'Scale', + ]); // Verify that objects are changed in memory if changed in parameters. expect(objectWithoutType).not.toBe(updatedObjectWithoutType); - expect(gd.getPointer(objectWithoutType)).not.toBe(gd.getPointer(updatedObjectWithoutType)); + expect(gd.getPointer(objectWithoutType)).not.toBe( + gd.getPointer(updatedObjectWithoutType) + ); expect(mySpriteObject).toBe(objectsContainer.getObject('MySpriteObject')); - expect(gd.getPointer(mySpriteObject)).toBe(gd.getPointer(objectsContainer.getObject('MySpriteObject'))); + expect(gd.getPointer(mySpriteObject)).toBe( + gd.getPointer(objectsContainer.getObject('MySpriteObject')) + ); // Remove an object and check that it is removed from the objects container. parameters.removeParameter('MySpriteObject'); @@ -4378,10 +4461,18 @@ describe('libGD.js', function () { expect(objectsContainer.hasObjectNamed('MySpriteObject3')).toBe(true); // Verify that objects are still stable in memory. - expect(updatedObjectWithoutType).toBe(objectsContainer.getObject('MyObjectWithoutType')); - expect(gd.getPointer(updatedObjectWithoutType)).toBe(gd.getPointer(objectsContainer.getObject('MyObjectWithoutType'))); - expect(mySpriteObject3).toBe(objectsContainer.getObject('MySpriteObject3')); - expect(gd.getPointer(mySpriteObject3)).toBe(gd.getPointer(objectsContainer.getObject('MySpriteObject3'))); + expect(updatedObjectWithoutType).toBe( + objectsContainer.getObject('MyObjectWithoutType') + ); + expect(gd.getPointer(updatedObjectWithoutType)).toBe( + gd.getPointer(objectsContainer.getObject('MyObjectWithoutType')) + ); + expect(mySpriteObject3).toBe( + objectsContainer.getObject('MySpriteObject3') + ); + expect(gd.getPointer(mySpriteObject3)).toBe( + gd.getPointer(objectsContainer.getObject('MySpriteObject3')) + ); objectsContainer.delete(); eventsFunction.delete();