-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ISS-33: Fixed and expanded array functions #36
Conversation
Community Toolbox/scripts/tests_ArrayMaxTests/tests_ArrayMaxTests.gml
Outdated
Show resolved
Hide resolved
|
||
var _max = -infinity; | ||
for (var i = 0; i < _length; i += 10000) { | ||
var _partial_max = script_execute_ext(max, _array, _offset + i, min(10000, _length - i)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Idle thought, but given that we ran into stack overflow issues, it's worth considering. I wonder if the stack size is smaller on mobile. 10,000 is likely to be fine on desktop platforms, but it's possible that this will still overflow the stack on a smaller device. I haven't tested it, but food for thought.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't really been involved with mobile development in GM, so I guess I'll use the "we'll think about it once someone finds the problem" approach. If need be, we might implement some script_execute_max_args()
function that chooses the number depending on the platform.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good :)
As it turned out, using plain
script_execute_ext(...)
with large arrays caused some stack overflow error. More details in issue #33Also, I added extra offset and length parameters, working similarly to same parameters in native array functions (like
array_filter
orarray_map
). Generally: