This fixes the warning introduced by rP22247, as a for-in loop over an array will return the index as a string instead of a number, which the c++ code doesn't expect.
My suggested fix is to switch to forEach.
Details
Details
- Reviewers
Itms - Group Reviewers
Restricted Owners Package (Owns No Changed Paths) - Commits
- rP22254: Fix warning introduced in the mirage cleanup, rP22247
Review code. Check that warning has disappeared.
Diff Detail
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Branch
- temp
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 7391 Build 12043: Vulcan Build Jenkins Build 12042: arc lint + arc unit
Event Timeline
Comment Actions
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Source... Executing section JS... binaries/data/mods/public/simulation/components/Fogging.js | 197| » » » continue; | | [MAJOR] ESLintBear: | | Parsing error: Unsyntactic continue binaries/data/mods/public/simulation/components/Fogging.js | 197| » » » continue; | | [NORMAL] JSHintBear: | | Unexpected 'continue'. binaries/data/mods/public/simulation/components/Fogging.js | 204| » » » continue; | | [NORMAL] JSHintBear: | | Unexpected 'continue'. Executing section cli...
Link to build: https://jenkins.wildfiregames.com/job/differential/1336/display/redirect
Comment Actions
As detected by the linter, you should use return and not continue in a function. The current code doesn't even parse.
I have nothing against forEach but why changing? The original loop over player was fine... I don't see any upside to using forEach and it pushes you into changing almost all the lines in the body of the loop. ?
| binaries/data/mods/public/simulation/components/Fogging.js | ||
|---|---|---|
| 197 | In my experience @vladislavbelov is the most knowledgeable about CCs :D The current style lgtm if you convince me using forEach is better. | |
Comment Actions
The flaws of winging it and not running the code :p
I have nothing against forEach but why changing? The original loop over player was fine... I don't see any upside to using forEach and it pushes you into changing almost all the lines in the body of the loop. ?
In general I've switched to using ForEach/map/any in places of imperative-style loops. We rarely have those in modern C++ code now, so JS might follow suite, and I think only using those where that behaviour is actually necessary would be a nice statement of code intent. This runs something for each line, so "For Each".
For example if I were mapping, you would probably agree that map is better than an imperative style loop.
This also explicits the index and the value, which is nice sometimes.
Comment Actions
Yeah as long as we don't need to break out of the loop, forEach is fine. I don't see a real need for it here, but if it's more to your tastes, why not. It's just lengthening the verification of the fix a bit, because it makes it less trivial ?
Comment Actions
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/differential/1347/display/redirect