Splitted from D121, changes including pickRandom in rms.
Details
- Reviewers
elexis - Commits
- rP19305: Use pickRandom in random map scripts.
- Trac Tickets
- #4326
Test changed random maps and scripts.
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Build has FAILED
Link to build: http://jw:8080/job/phabricator/522/
See console output for more information: http://jw:8080/job/phabricator/522/console
30 pickRandom uses in this proposal.
29 uses in my branch that was based on your patch in D121, not sure where that one is coming from, dont care enough to look it up now.
In the last D121 patch you had uploaded, after removing all hunks outside of the maps directory,
subtract the setSkySet changes introduced recently, we find 37 uses cat D121.id488.diff | grep pickRandom | grep -v setSkySet | wc -l.
Looking at that patch, we can find few more occurances: alpine_lakes.js f.e.
binaries/data/mods/public/maps/random/ardennes_forest.js | ||
---|---|---|
399 ↗ | (On Diff #802) | good |
binaries/data/mods/public/maps/random/caledonian_meadows.js | ||
284 ↗ | (On Diff #802) | good |
318 ↗ | (On Diff #802) | good |
358 ↗ | (On Diff #802) | good |
359 ↗ | (On Diff #802) | good |
363 ↗ | (On Diff #802) | As discussed in irc, we pull that dAngle out of that term |
370 ↗ | (On Diff #802) | ✓ |
391 ↗ | (On Diff #802) | ✓ |
421 ↗ | (On Diff #802) | ✓ |
444 ↗ | (On Diff #802) | ✓ |
721 ↗ | (On Diff #802) | all of that ✓ |
750 ↗ | (On Diff #802) | ✓ |
binaries/data/mods/public/maps/random/heightmap/heightmap.js | ||
81 ↗ | (On Diff #802) | ✓ |
binaries/data/mods/public/maps/random/island_stronghold.js | ||
311 ↗ | (On Diff #802) | ✓ |
binaries/data/mods/public/maps/random/islands.js | ||
253 ↗ | (On Diff #802) | ✓ |
binaries/data/mods/public/maps/random/rmgen/library.js | ||
147 ↗ | (On Diff #802) | ✓ |
binaries/data/mods/public/maps/random/rmgen/misc.js | ||
637 ↗ | (On Diff #802) | ✓ |
binaries/data/mods/public/maps/random/rmgen/placer.js | ||
159 ↗ | (On Diff #802) | As discussed in irc, var [cx, cz] = pickRandom(edges); to nuke point Verify that chain placer function by starting anatolian plateau |
478 ↗ | (On Diff #802) | ✓ |
569 ↗ | (On Diff #802) | ✓ |
binaries/data/mods/public/maps/random/rmgen/randombiome.js | ||
533 ↗ | (On Diff #802) | ✓ |
binaries/data/mods/public/maps/random/rmgen/terrain.js | ||
71 ↗ | (On Diff #802) | ✓ |
binaries/data/mods/public/maps/random/rmgen2/setup.js | ||
313 ↗ | (On Diff #802) | ✓ all of that |
binaries/data/mods/public/maps/random/syria.js | ||
166 ↗ | (On Diff #802) | ✓ |
binaries/data/mods/public/maps/random/unknown_nomad.js | ||
1303 ↗ | (On Diff #802) | As discussed in irc, [playerX[i], playerZ[i]] = pickRandom(placableArea); Verify by starting unknown nomad |
binaries/data/mods/public/maps/skirmishes/Gallic Fields (3).js | ||
3 ↗ | (On Diff #802) | check |
Build has FAILED
Link to build: http://jw:8080/job/phabricator/532/
See console output for more information: http://jw:8080/job/phabricator/532/console
binaries/data/mods/public/maps/random/caledonian_meadows.js | ||
---|---|---|
363 ↗ | (On Diff #814) | ✓ |
binaries/data/mods/public/maps/random/rmgen/library.js | ||
147 ↗ | (On Diff #814) | NOPE, destructor Destructuring assignment https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment only works with arrays, not objects with other properties would have to be [placer.x, placer.z] = ... |
Nope they need the surrounding code to be rewritten, can be done, but seems out of scope.
binaries/data/mods/public/maps/random/rmgen/library.js | ||
---|---|---|
147 ↗ | (On Diff #822) | This doesn't work, test cycladic and see the crash, placer = pickRandom(pickRandom(areas).points) does work. (pickRandom returns an object right...) |
Build has FAILED
Link to build: http://jw:8080/job/phabricator/534/
See console output for more information: http://jw:8080/job/phabricator/534/console
Build has FAILED
Link to build: http://jw:8080/job/phabricator/535/
See console output for more information: http://jw:8080/job/phabricator/535/console
Searching for randInt(0, yields no more occurances that can be replaced with pickRandom, and the non-rms part of D121 had been committed already, so the patch is complete too.
binaries/data/mods/public/maps/random/rmgen/library.js | ||
---|---|---|
147 ↗ | (On Diff #822) | right, fail while failing. /** * Helper function for randomly placing areas and groups in the given areas. */ function randomizePlacerCoordinatesFromAreas(placer, areas) { warn("BEFORE"); for (let prop in placer) warn(prop); placer = pickRandom(pickRandom(areas).points); warn("AFTER"); for (let prop in placer) warn(prop); } yields: WARNING: BEFORE WARNING: elements WARNING: tileClass WARNING: avoidSelf WARNING: x WARNING: z WARNING: place WARNING: AFTER WARNING: x WARNING: z |
Reverted the library.js change to a previous one
Also: (22:01:17) bb_: tested changed maps in atlas