Opened 10 years ago
Closed 10 years ago
#2478 closed enhancement (invalid)
[PATCH] Eliminating "for each" statements with Array.forEach
Reported by: | False Vision | Owned by: | |
---|---|---|---|
Priority: | Nice to Have | Milestone: | |
Component: | Core engine | Keywords: | patch |
Cc: | Patch: |
Description
The "for each...in" statement is deprecated as the part of ECMA-357 and currently only SpiderMoney supports it.
People like me want to run the game on other JavaScript engines and for each statements won't run on them.
I am porting the game to another platform so it would be great if we simply eliminate for each statements with Array.forEach.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for_each...in
Patch is attached.
Attachments (1)
Change History (7)
by , 10 years ago
Attachment: | HelperScripts.patch added |
---|
comment:1 by , 10 years ago
follow-up: 3 comment:2 by , 10 years ago
"for each" statement is only supported on Mozilla Firefox so any effort to run the game on a browser will fail.
But good news is that "for of" will probably replace "for each" in ECMAScript 6. So we would have two "for"s:
for(a in b) // for keys for(a of b) // for values
"for of" is currently supported by Spider Monkey (experimental) and i could run it on 0.A.D.
I believe my patch is so dirty because there are lots of "continue", "break" and "return" statements in body of the loops so implementing them with Array.forEach(function func) is a pain and increases the complexity of the code.
I am porting the game to Windows Phone 8.1 and i am using Jint as scripting engine which doesn't support "for each". Hopefully it is open source and i could modify the source code to add support for both "for each" and "for of" in less than 30 minutes.
Now i do not need the patched to be applied and i think it's better to wait for "for of" to officially put in ECMAScript 6. Or you might want to replace "for each" with "for of" now because none of them is standard (yet) and there would be no difference.
Please skip my patch.
comment:3 by , 10 years ago
Keywords: | review removed |
---|
Replying to falsevision:
I am porting the game to Windows Phone 8.1 and i am using Jint as scripting engine which doesn't support "for each".
That's interesting. Someone else was also porting the game to Windows apps, but they were using C# for everything, which I think was a bad idea. Are you still using C++? Is it a complete rewrite or just adapting the existing engine?
I agree about your patch, but we can keep this ticket open as a reminder about the for each
issue.
comment:4 by , 10 years ago
Imo, we should go with for..of in most cases, as that allows breaking, and looks abit nicer. Only the important stuff for performance should use forEach().
But I think, from now on, we'll use for..of in new code, and later on, the old code can also be updated.
Though I warn you, we still only develop for spidermonkey, and only test on that platform. So any compatibility problems will be for you.
comment:5 by , 10 years ago
Performance-wise a while(){} is the fastest. If case of for(i=0;i<a.length;i++){} a.length should be put in a variable upfront as the lookup is too costly on each iteration.
comment:6 by , 10 years ago
Milestone: | Alpha 16 |
---|---|
Resolution: | → invalid |
Status: | new → closed |
We will probably slowly migrate to for..of. But there's no reason to make a patch for it yet. We clearly state in the coding styles that SpiderMonkey extensions can be used.
Replying to falsevision:
I'm just curious, but why? I've always thought it would be interesting if parts of the game could e.g. run in a browser, but I've never seen a practical application of that (other than maybe Philip doing some testing or proof of concepts), and no one has expressed much interest in it.
You should probably know about #2475. Both that and
for each
were discussed in yesterday's staff meeting, you can read about here.