GDevelop
GDevelop copied to clipboard
4ian
Return objects from events based extensions
This adds an action to return objects from an events based extension. This will set the return value to true and pick the currently picked instances of the object back to where the function is used.
Example: pick.zip
Finally getting to this! Very interesting! Is this supporting a few interesting use cases like objects created in the function, can you return them?
I've identified multiple edge cases. This is not an easy problem, this can be very powerful but we must be sure that we handle everything properly and be 100% correct so that we don't discover later issues when using groups/passing groups/passing multiple objects/calling this action multiple times :)
I think the key is being very clear about what is inside the object lists.
Whoops! While I indeed tested passing groups, I did not test for those edge cases. I fixed the first and second one, though I am not sure I understand the last one. If the object is passed twice, shouldn't modifying its picked objects still be fine? The only other alternative would be to not pick them but that sounds more confusing.
I think I understand now what you meant with that last edge case (what if i am using a group in the function that consists of two times the same object that was passed?), and my current implementation while being a little slower on that edge case should be resilient against this (e.g. it will not duplicate objects in the list, since even if it fills it a second time it will clear it before doing so).
As the edge cases seem fixed, I am removing the draft.
Sorry about the delay on this, it's hard to review because I'm not sure if we're missing a case or not.
Can we somehow create a few exhaustive "test cases"? I'm thinking either of testing in GDJSCodeGenerationIntegrationTests or "just" a game that would use this in multiple settings:
- With a group or not at the scene level
- With groups or not inside the function
- Passing groups with multiple times the same object
- Maybe a final test case where a function calls another functions, and both use the action (to ensure picked objects can traverse 2 layers of functions? And that there is no problem of naming, especially because we're pushing objects using their names in the object lists).
I added some cases in the example project: https://www.dropbox.com/s/4gk4gpzcho2ohve/PickTest.zip?dl=0
I observed that:
-
It doesn't feel intuitive with behavior functions When this action is used in a behavior function, it will be called for each owner instance. The last instance executed will decide what is picked. Most of the time, extension users will surround the behavior action with a for each to handle the picked instances with knowledge of the behavior owner. But, in case they don't need to, the function will seem broken.
-
An event-based condition A that call another event-based B won't work because the condition B doesn't find the object lists.
-
Maybe not related to this PR: when an object is created in the picked instances are reset One use-case is the bullet extension. It kind of warp a create action and I guess users will expect it to work the same. This means adding the picked object to the picking list and keeping objects that were already picked.
To answer my younger self:
- It doesn't feel intuitive with behavior functions When this action is used in a behavior function, it will be called for each owner instance. The last instance executed will decide what is picked. Most of the time, extension users will surround the behavior action with a for each to handle the picked instances with knowledge of the behavior owner. But, in case they don't need to, the function will seem broken.
This is actually not a real use-case because behavior Object picking is handled by the generated code according to returned boolean from the condition already. The UI should just hide the "Return object" action from within behaviors.
- An event-based condition A that call another event-based B won't work because the condition B doesn't find the object lists.
The issue was the use of object.getName() which gives the object name in the scene but it didn't work if the event-function caller is not a scene event.
I tried a different implementation to fix this issue:
- https://github.com/4ian/GDevelop/pull/5823
It will work for simple picking but the events may need to use imbricated "for each instance" to check some conditions on each object tuples. It won't be possible with just this new action.
Maybe, it could require a new concept like named picking list:
- An action
Add Object to the object list "MyPickingList" - An action
Remove Object from the object list "MyPickingList" - A condition
Pick all Object from the object list "MyPickingList"
The idea would be to build an object list in "for each instance" loops and use the condition to get the result and give it to the "Return" action. Something like this:
Clear the object list "MyPickingList"
for each instance of Player
for each instance of Obstacle
A condition on Player and Obstacle --> Add Object to the object list "MyPickingList"
--> Add Obstacle to the object list "MyPickingList"
Pick all Player from the object list "MyPickingList" --> Set objects to return to Player
Pick all Obstacle from the object list "MyPickingList" --> Set objects to return to Obstacle
Though, it would be better if we can found a simpler solution.