GDevelopApp / GDevelop-extensions

Repository of behaviors, actions, conditions and expressions to be used in GDevelop for creating games
https://gdevelop.io
MIT License
131 stars 52 forks source link

Array tools #176

Closed arthuro555 closed 3 years ago

arthuro555 commented 3 years ago

Describe the extension

This adds a lot of of utility functions for working with arrays. It adds the following types of possibilities:

Checklist

Example

I don't have an example file, but I have (arguably) better. Here are some unit tests in a GDevelop project file: arraytools-unit-tests.zip

Extension file

ArrayTools.zip

Bouh commented 3 years ago

Verification list

// Prefered solution: Math.min.apply(null, arr.map(variable => variable.getAsNumber()));

Math.min(...arr.map(variable => variable.getAsNumber()));



- [ ] Join
- [ ] Split
- [ ] Mean
- [ ] Median
- [ ] Get Random Child
- [ ] Fill
- [ ] Concat
- [ ] Append To
- [ ] Pop
- [ ] Shift
- [ ] Insert variable at

I recommend to remove private functions, used for create sections in functions list. It's fine for review and testing it only.
tristanbob commented 3 years ago

In the ShadowClones extension, my current logic is this:

0) Assume N = number of shadow clones * frames between clones 1) Copy all child variables from ObjectHistory.N to ObjectHistory.N+1 (this can result in copying thousands of variables every frame) 2) Create a new child variable at ObjectHistory.1 based on current values of sprite

I think for this logic to work with arrays, I need to use "unshift: Adds new elements to the beginning of an array".

0) Assume N = number of shadow clones * frames between clones 1) Unshift array to create new child based on current values of sprite 2) Pop array until array length = N (this will remove old data that we don't need any longer)

Using arrays should be MUCH more performant compared to copying values of all child variables.

TLDR: Can you please add "unshift"?

arthuro555 commented 3 years ago

I intentionally didn't add unshift as I think it may bloat a bit too much considering the insert variable at with index parameter 0 is equivalent

tristanbob commented 3 years ago

I intentionally didn't add unshift as I think it may bloat a bit too much considering the insert variable at with index parameter 0 is equivalent

Ah, yes, that sounds like it is what I need (and more flexible for other uses). Thanks!

4ian commented 3 years ago

Math.min.apply(Math, arr.map(variable => variable.getAsNumber())); // Prefered solution: Math.min.apply(null, arr.map(variable => variable.getAsNumber())); Math.min(...arr.map(variable => variable.getAsNumber()));

Remember that in the context of JavaScript written for games, it's actually better not to use map/filter as they will create new arrays in memory, and will create pressure on the garbage collector. Using apply with an array is also a risk to crash/slow down terribly the JS engine because you're calling a function with as much parameters as the the array length... imagine if this is an array of 1000 or 10000 objects - JS engines are not ready for these and will deoptimize (at best).

Instead, just do a for loop and keep the minimum or maximum value. It's the most efficient, no slow down, 0 memory allocations, and probably optimized by JS engines.

arthuro555 commented 3 years ago

Instead, just do a for loop and keep the minimum or maximum value. It's the most efficient, no slow down, 0 memory allocations, and probably optimized by JS engines.

Done, here is the modified extension and unit tests: ArrayTools.zip arraytools-unit-tests.zip

Bouh commented 3 years ago

Something interesting during my review, the long description is not show for Boolean type! image image

arthuro555 commented 3 years ago

I was wondering, should an expression that builds an array out of the key of a structure (Object.keys) be part of this extension or of a "Structure tools" extension?

4ian commented 3 years ago

Good question.. Might be the same extension but should this be renamed to "Collection variable tools" then?

arthuro555 commented 3 years ago

🤔 It's ok for me, but if it becomes tools for collections generally it might grow huge quickly, while separating in multiple extensions would allow to have smaller more maintainable packages.

4ian commented 3 years ago

I honestly don't have a strong feeling on this, on the other hand it might be simpler for the user to just install one extension? But feel free to do a separate one if you prefer!

tristanbob commented 3 years ago

My preference is to consolidate extensions that are highly related, but like Florian said it is up to you.

For array functions, is there a reason they make sense as an extension versus provided by default in GDevelop?

4ian commented 3 years ago

For array functions, is there a reason they make sense as an extension versus provided by default in GDevelop?

One reason if any is that it's 1) faster to get these made as extensions now, so that we can try and use them as soon as the extension is ready and 2) safer, in the sense that if the interface (the conditions/actions names, parameters, etc...) are not perfect, we can fix that by releasing an update extension.

On the other hand, everything that is by default in GDevelop is considered to be there "forever", and any breaking change must be either avoided or automatically upgraded. As the article says, APIs, like diamonds, are forever.

This being said, it's also possible that we "graduate" the extension or part of it into a built-in GDevelop extension, after we have proved it's 1) well designed and 2) used by a number of games so large that it's better to have it not as an extension. Finally, all this discussion could maybe be avoided if in the future we push more extensions in the interface, and we could even suggest a set of "default extensions" to be used in new games :)

tristanbob commented 3 years ago

4ian thanks for the great answers to my questions.

Now for more questions:

Are you considering adding support for 2D arrays? This would help with mapping 2D grids. For instance, fog of war opacity values.... ;)

arthuro555 commented 3 years ago

Arrays can already be 2D, a 2D array is just an array of arrays, and arrays can contain any variables including arrays.

tristanbob commented 3 years ago

(For the next update) Add a "unique" function (perhaps as a parameter on sort) that only keeps one instance per number.

Use case: I am using a secondary array to track the indexes of a primary array that need to be modified. This secondary array might contain the same value multiple times. However, I only want to apply the modification one time per value. With a unique function, I could do this.

arthuro555 commented 3 years ago

I'm starting to collect ideas for the next update too. That sounds like a good idea. Some other ideas I had: