randInt changes from randomPatch excluding unknown* those require more changes all over those files, so needs to be done separately.
refs D121
Details
Details
- Reviewers
elexis - Commits
- rP19443: Replace deprecated randInt calls (besides those in the Unknown maps).
- Trac Tickets
- #4326
Load maps in atlas.
See that the new and old code is equivalent.
grep on "randInt(" and see that all remaining ones are in unkown* (and function declaration).
Diff Detail
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Branch
- /ps/trunk
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 951 Build 1499: Vulcan Build Jenkins Build 1498: arc lint + arc unit
Event Timeline
There are a very large number of changes, so older changes are hidden. Show Older Changes
Comment Actions
Build is green
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!
http://jw:8080/job/phabricator/643/ for more details.
Comment Actions
Build is green
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!
http://jw:8080/job/phabricator/644/ for more details.
| binaries/data/mods/public/maps/random/canyon.js | ||
|---|---|---|
| 227 | whitespace | |
| 246 | One might wonder whether the - 1 was an accident, but looks very much like it was intentional, considering that we also have +1 at the start | |
| binaries/data/mods/public/maps/random/cycladic_archipelago.js | ||
| 342 | good | |
| binaries/data/mods/public/maps/random/deep_forest.js | ||
| 90 | While this is nicely readable, I'm a duplication avoider fetishist | |
| binaries/data/mods/public/maps/random/deep_forest.js | ||
|---|---|---|
| 90 | What do you suggest, storing the Math.cos/sin under a variable or using randFloat and Math.round? | |
Comment Actions
Also tested a bunch of maps in atlas, some of those that were not obvious, deep forest, latium, kerala. flood.
So it seems the only thing that is remaining for the random function file to be closed is the duplication removal of the unknown* maps.
-> Thanks
| binaries/data/mods/public/maps/random/african_plains.js | ||
|---|---|---|
| 317 | ✓ | |
| binaries/data/mods/public/maps/random/alpine_lakes.js | ||
| 378 | ✓ | |
| binaries/data/mods/public/maps/random/alpine_valley.js | ||
| 254 | ✓ | |
| 570 | ✓ | |
| binaries/data/mods/public/maps/random/anatolian_plateau.js | ||
| 316 | ✓ | |
| binaries/data/mods/public/maps/random/archipelago.js | ||
| 194 | ✓ | |
| binaries/data/mods/public/maps/random/ardennes_forest.js | ||
| 569 | ✓ | |
| binaries/data/mods/public/maps/random/atlas_mountains.js | ||
| 276 | ✓ | |
| binaries/data/mods/public/maps/random/belgian_uplands.js | ||
| 233 ↗ | (On Diff #1014) | ✓ |
| binaries/data/mods/public/maps/random/caledonian_meadows.js | ||
| 312 | ✓ | |
| 385 | ✓ | |
| binaries/data/mods/public/maps/random/cantabrian_highlands.js | ||
| 324 | ✓ | |
| binaries/data/mods/public/maps/random/canyon.js | ||
| 231 | ✓, ix, iz, placer still defined globally, ix2, iz2 not refered to anywhere else | |
| 248 | randIntInclusive 1, ... -1 seems better to read here as it highlights that we remove 1 tile from both sides | |
| 506–507 | ✓ | |
| 507 | ✓ (whitespace fix only) | |
| 509–510 | ✓ | |
| 510 | ✓ (whitespace fix only) | |
| binaries/data/mods/public/maps/random/cycladic_archipelago.js | ||
| 342 | ✓ | |
| binaries/data/mods/public/maps/random/deep_forest.js | ||
| 90 | The latter IMO | |
| binaries/data/mods/public/maps/random/empire.js | ||
| 10 | ✓ | |
| binaries/data/mods/public/maps/random/english_channel.js | ||
| 374 | ✓ | |
| binaries/data/mods/public/maps/random/flood.js | ||
| 227 | ✓, randMountains not refered to anywhere else | |
| 230 | randX, randY not refered to anywhere else | |
| 238 | ✓ | |
| 239 | ✓ | |
| 252 | ✓ | |
| 262 | ✓ | |
| 264 | ✓ | |
| binaries/data/mods/public/maps/random/fortress.js | ||
| 344 | ✓ | |
| binaries/data/mods/public/maps/random/frontier.js | ||
| 13 | ✓, guess could remain 30 without making a big difference, but keeping the same function here is good too | |
| 15 | ✓ | |
| binaries/data/mods/public/maps/random/guadalquivir_river.js | ||
| 383 | ✓ | |
| binaries/data/mods/public/maps/random/gulf_of_bothnia.js | ||
| 4 | ✓ | |
| 456 | ✓ | |
| binaries/data/mods/public/maps/random/heightmap/heightmap.js | ||
| 142 | ✓ | |
| binaries/data/mods/public/maps/random/hyrcanian_shores.js | ||
| 456 | ✓ | |
| binaries/data/mods/public/maps/random/islands.js | ||
| 495 | ✓ | |
| binaries/data/mods/public/maps/random/kerala.js | ||
| 207 | One function call less, one factor more, well, fine | |
| 436 | ✓ | |
| binaries/data/mods/public/maps/random/latium.js | ||
| 510 | With this whitespace, it isn't entirely obvious what the first and second argument of that function is | |
| 518 | ✓ | |
| binaries/data/mods/public/maps/random/lorraine_plain.js | ||
| 213 | ✓ | |
| 430 | ✓ | |
| binaries/data/mods/public/maps/random/migration.js | ||
| 423 | ✓ | |
| binaries/data/mods/public/maps/random/northern_lights.js | ||
| 189 | fine | |
| binaries/data/mods/public/maps/random/persian_highlands.js | ||
| 247 | ✓ | |
| 398 | ✓ | |
| binaries/data/mods/public/maps/random/phoenician_levant.js | ||
| 446 | ✓ | |
| binaries/data/mods/public/maps/random/pyrenean_sierra.js | ||
| 748 | ✓ | |
| binaries/data/mods/public/maps/random/rhine_marshlands.js | ||
| 384 | ✓ | |
| binaries/data/mods/public/maps/random/rmgen/library.js | ||
| 138 | ✓ | |
| binaries/data/mods/public/maps/random/rmgen/misc.js | ||
| 639–640 | ✓ | |
| 788 | After 10+ minutes of computing things back and forth, reverting this and adding whitespace and Math only | |
| binaries/data/mods/public/maps/random/rmgen/placer.js | ||
| 161 | ✓ | |
| 366 | same as below | |
| 437 | ✓ count not used anywhere else | |
| 454 | This should be done in an isolated patch, it looks very much like it fixes a bug | |
| binaries/data/mods/public/maps/random/rmgen/randombiome.js | ||
| 65 | ✓ | |
| 151 | ✓ | |
| 168 | ✓ | |
| 448 | ✓ | |
| binaries/data/mods/public/maps/random/rmgen/utilityfunctions.js | ||
| 73 | ✓ | |
| binaries/data/mods/public/maps/random/rmgen/wall_builder.js | ||
| 864 | ✓ | |
| binaries/data/mods/public/maps/random/rmgen2/gaia.js | ||
| 66 | ✓ | |
| 393 | this appears wrong. Example: before it was 1 + randIntExcl(3-1) = 1 + randIntExcl(2) i.e. in (1, 2), now it is in (1, 3) | |
| 595 | ✓ | |
| 1266 | ✓ | |
| binaries/data/mods/public/maps/random/sahel.js | ||
| 216–217 | ✓ | |
| 312 | ✓ | |
| binaries/data/mods/public/maps/random/sahel_watering_holes.js | ||
| 479 | ✓ | |