Page MenuHomeWildfire Games

Fix SimpleGroup and RandomGroup placement retries
ClosedPublic

Authored by Imarok on Mar 22 2017, 11:02 PM.

Details

Summary

Because of a bug the place function didn't retry for SimpleGroup and RandomGroup.
Because just fixing this bug would need us to fix the most numbers of all random maps, for the changed function a deprecated variant is introduced and replaces the function in all old map as long as someone changes it.
As example this also fixes the number of metal mines in latium.

Test Plan

Test everything works

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

Imarok created this revision.Mar 22 2017, 11:02 PM
Owners added a subscriber: Restricted Owners Package.Mar 22 2017, 11:02 PM
Imarok updated the Trac tickets for this revision.Mar 22 2017, 11:03 PM
FeXoR requested changes to this revision.Mar 23 2017, 1:01 AM
FeXoR added a subscriber: FeXoR.

elexis and me agreed on:

This patch misses a way to test the new functions.
Best to fix one or a few maps so all new functions are used at least once for testing.
(That will also make this patch actually fix at least something)

Also before committing this patch a ticket with a list of all random map files using the deprecated functions should be created.
That way we can fix the maps one by one or in smaller bunches or whatever.

This revision now requires changes to proceed.Mar 23 2017, 1:01 AM
Vulcan added a subscriber: Vulcan.Mar 23 2017, 1:11 AM

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/576/ for more details.

In D249#9488, @FeXoR wrote:

elexis and me agreed on:

This patch misses a way to test the new functions.
Best to fix one or a few maps so all new functions are used at least once for testing.
(That will also make this patch actually fix at least something)

Also before committing this patch a ticket with a list of all random map files using the deprecated functions should be created.
That way we can fix the maps one by one or in smaller bunches or whatever.

I think I forgot to say it, but I already fixed the metal mines on Latium..
A trac ticket was planned, but I think it should be done when/after committing.

elexis added a subscriber: elexis.Mar 23 2017, 12:21 PM

Since there are > 100 occurances of the deprecated randInt function in there and since this patch can easily be recreated with an automated rename, we should wait until bb's megacleanup is completed, also I have 2 or 3 commits scheduled: https://github.com/elexis1/0ad/tree/randomPatch

FeXoR added a comment.Apr 18 2017, 4:29 PM

@elexis: Sounds wise.
Could you please add links for all patches that should go in first (if not only that git branch you already linked)?

Imarok updated this revision to Diff 1882.May 12 2017, 9:03 PM
Imarok edited edge metadata.

Rebase

Imarok edited the summary of this revision. (Show Details)May 12 2017, 9:04 PM
Imarok updated this revision to Diff 1885.May 12 2017, 9:29 PM

Don't include the latium fix

Imarok updated this revision to Diff 1888.May 12 2017, 9:46 PM

move default into function head

Imarok updated this revision to Diff 1889.May 12 2017, 9:54 PM

Move the other default

elexis added inline comments.May 12 2017, 10:01 PM
binaries/data/mods/public/maps/random/rmgen/library.js
109 ↗(On Diff #1882)

So actually we only have to call the *Deprecated functions if the place is one of those 2 types (and potentially display a warning if deprecated is called from another type). (That will make the diff shorter, not easier to review, but better reviewing this once than changing every occurance and then still having to review the revert)

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/1168/ for more details.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/1171/ for more details.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/1173/ for more details.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/1174/ for more details.

Imarok updated this revision to Diff 1901.May 13 2017, 5:51 PM

only warn when using behaveDeprecated and the wrong placer. Remove all unneeded Deprecateds. Wrong -> Deprecated

(To test the warning, you can e.g. change L236 in kerala.js)

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/1184/ for more details.

FeXoR added a comment.EditedMay 21 2017, 12:54 PM

Thanks much @Imarok for this patch!
(Not so much for @elexis for retiring as reviewer after making this patch big...)

What exactly happens if we just use the new functions?

IMO it might be good to commit this right after release (if at all) and try fixing all that deprecated stuff until the next one.
Opinions?

I checked:

  • Read through the changed libs (I still would prefer to always return arrays and check for their length).
  • Latium and all maps from A to Hell's Pass (Normal, 4 Players, Seed 42). No problems really related to this patch I guess, just mentioning:
    • Alpine Lakes: Unfair expansion mine distribution
    • Alpine Valley: Unfair expansion mine (and player) distribution
    • Empire: Unfair expansion mine distribution
  • Checked for usage of functions:
    • createAreasDeprecated: Unused
    • createAreasInAreasDeprecated: Unused
In D249#21364, @FeXoR wrote:

What exactly happens if we just use the new functions?

The new function will mostly place more objects than the old one, so this will change the map in a in general undesired way.

IMO it might be good to commit this right after release (if at all) and try fixing all that deprecated stuff until the next one.
Opinions?

what speaks against committing it before the release?

  • Checked for usage of functions:
    • createAreasDeprecated: Unused
    • createAreasInAreasDeprecated: Unused

Oh, I forgot to remove them :D

FeXoR added a comment.May 21 2017, 9:14 PM

what speaks against committing it before the release?

To be honest I'd rather fix the glitch where it happens (the functions, as you already did) and adjust the maps afterwards (even if that means a bit strange maps along the way) than add new functions that are by design not meant to stay.

But I agree it's not really an option to make all maps unplayable. Thus actually testing how bad things get if we go with the new functions only would be preferable IMO.

I haven't done that yet, though you, @Imarok may have done it before putting this much time into a maybe unneeded task ;p

So next step IMO: test how maps turn out with the new functions just replacing the old ones.

FeXoR added a comment.EditedMay 21 2017, 9:59 PM

createAreas() argument "behaveDeprecated" can be removed since it's only changed by createAreasDeprecated() ... which then again is unused ;p

EDIT: Same goes for createAreasInAreas() and createAreasInAreasDeprecated() x)

EDIT 2: Also "WARNING: Deprecated version of createFoo should only be used for SimpleGroup and RandomGroup placers!" on Neareastern Badlands though createObjectGroupsByAreasDeprecated() argument "placer" is a SimpleGroup (and behaveDeprecated = true). Huh?

Imarok updated this revision to Diff 2096.May 21 2017, 11:37 PM

Fix a use of createAreas and remove the deprecated Area functions.

Imarok updated this revision to Diff 2097.May 21 2017, 11:40 PM

Don't add newline.

Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (no-extra-semi):
|    | Unnecessary semicolon.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/maps/random/oasis.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/maps/random/oasis.js
|   7|   7| const tForestFloor = "desert_forestfloor_palms";
|   8|   8| const tHill = ["desert_dirt_rocks_1", "desert_dirt_rocks_2", "desert_dirt_rocks_3"];
|   9|   9| const tDirt = ["desert_dirt_rough","desert_dirt_rough","desert_dirt_rough", "desert_dirt_rough_2", "desert_dirt_rocks_2"];
|  10|    |-const tRoad = "desert_city_tile";;
|    |  10|+const tRoad = "desert_city_tile";
|  11|  11| const tRoadWild = "desert_city_tile";;
|  12|  12| const tShoreBlend = "desert_sand_wet";
|  13|  13| const tShore = "dirta";
|    | [NORMAL] ESLintBear (no-extra-semi):
|    | Unnecessary semicolon.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/maps/random/oasis.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/maps/random/oasis.js
|   8|   8| const tHill = ["desert_dirt_rocks_1", "desert_dirt_rocks_2", "desert_dirt_rocks_3"];
|   9|   9| const tDirt = ["desert_dirt_rough","desert_dirt_rough","desert_dirt_rough", "desert_dirt_rough_2", "desert_dirt_rocks_2"];
|  10|  10| const tRoad = "desert_city_tile";;
|  11|    |-const tRoadWild = "desert_city_tile";;
|    |  11|+const tRoadWild = "desert_city_tile";
|  12|  12| const tShoreBlend = "desert_sand_wet";
|  13|  13| const tShore = "dirta";
|  14|  14| const tWater = "desert_sand_wet";
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'placer' to undefined.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/maps/random/oasis.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/maps/random/oasis.js
|  92|  92| 	playerX[i] = 0.5 + 0.35*cos(playerAngle[i]);
|  93|  93| 	playerZ[i] = 0.5 + 0.35*sin(playerAngle[i]);
|  94|  94| }
|  95|    |-var placer = undefined;
|    |  95|+var placer;
|  96|  96| var fx = 0; var fz = 0;
|  97|  97| var ix =0; var iz = 0;
|  98|  98| for (var i = 0; i < numPlayers; i++)
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'avoidClasses'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/maps/random/oasis.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/maps/random/oasis.js
| 200| 200| 		createObjectGroup(group, 0);
| 201| 201| 		group = new SimpleGroup( [new SimpleObject(aReedsA, 1,3, 0,0)], true, undefined, round(forestX + 5 * cos(watAngle)),round(forestY + 5 * sin(watAngle)) );
| 202| 202| 		createObjectGroup(group, 0);
| 203|    |-	} while (createArea( placer, [terrainPainter, painter],  avoidClasses(clBaseResource,0) ) === undefined);
|    | 203|+	} while (createArea( placer, [terrainPainter, painter], avoidClasses(clBaseResource,0) ) === undefined);
| 204| 204| 
| 205| 205| 	// TODO: add a few random trees here and there
| 206| 206| }
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '-'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/maps/random/oasis.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/maps/random/oasis.js
| 240| 240| iz = round(fz);
| 241| 241| placer = new ClumpPlacer(size*1.1, 0.8, 0.2, 10, ix, iz);
| 242| 242| terrainPainter = new LayeredPainter( [pOasisForestLight,tShoreBlend, tWater, tWater, tWater], [scaleByMapSize(6,20),3, 5, 2] );
| 243|    |-var elevationPainter = new SmoothElevationPainter(ELEVATION_SET,  -3,  15 );
|    | 243|+var elevationPainter = new SmoothElevationPainter(ELEVATION_SET, -3,  15 );
| 244| 244| createArea(placer, [terrainPainter, elevationPainter, paintClass(clWater)], null);
| 245| 245| RMS.SetProgress(50);
| 246| 246| if (mapSize > 150 && randBool())
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '15'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/maps/random/oasis.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/maps/random/oasis.js
| 240| 240| iz = round(fz);
| 241| 241| placer = new ClumpPlacer(size*1.1, 0.8, 0.2, 10, ix, iz);
| 242| 242| terrainPainter = new LayeredPainter( [pOasisForestLight,tShoreBlend, tWater, tWater, tWater], [scaleByMapSize(6,20),3, 5, 2] );
| 243|    |-var elevationPainter = new SmoothElevationPainter(ELEVATION_SET,  -3,  15 );
|    | 243|+var elevationPainter = new SmoothElevationPainter(ELEVATION_SET,  -3, 15 );
| 244| 244| createArea(placer, [terrainPainter, elevationPainter, paintClass(clWater)], null);
| 245| 245| RMS.SetProgress(50);
| 246| 246| if (mapSize > 150 && randBool())
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before ')'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/maps/random/oasis.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/maps/random/oasis.js
| 258| 258| }
| 259| 259| log("Creating some straggler trees around the Passage...");
| 260| 260| group = new SimpleGroup([new SimpleObject(ePalmTall, 1,1, 0,0),new SimpleObject(ePalmShort, 1,2, 1,2), new SimpleObject(aBushA, 0,2, 1,3)], true, clForest);
| 261|    |-createObjectGroupsDeprecated(group, 0, stayClasses(clPassage,1), scaleByMapSize(60,250), 100  );
|    | 261|+createObjectGroupsDeprecated(group, 0, stayClasses(clPassage,1), scaleByMapSize(60,250), 100 );
| 262| 262| 
| 263| 263| log("Creating stone mines...");
| 264| 264| // create large stone quarries

binaries/data/mods/public/maps/random/oasis.js
|  10| const·tRoad·=·"desert_city_tile";;
|    | [NORMAL] JSHintBear:
|    | Unnecessary semicolon.

binaries/data/mods/public/maps/random/oasis.js
|  11| const·tRoadWild·=·"desert_city_tile";;
|    | [NORMAL] JSHintBear:
|    | Unnecessary semicolon.

binaries/data/mods/public/maps/random/oasis.js
|  40| »   »   »   »   »   ,tForestFloor,tForestFloor,tForestFloor,tForestFloor];
|    | [INFO] JSHintBear:
|    | Comma warnings can be turned off with 'laxcomma'.

binaries/data/mods/public/maps/random/oasis.js
|  39| const·pOasisForestLight·=·[tForestFloor·+·TERRAIN_SEPARATOR·+·ePalmShort,·tForestFloor·+·TERRAIN_SEPARATOR·+·ePalmTall,·tForestFloor,tForestFloor,tForestFloor
|    | [NORMAL] JSHintBear:
|    | Misleading line break before ','; readers may interpret this as an expression boundary.

binaries/data/mods/public/maps/random/oasis.js
|  95| var·placer·=·undefined;
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'placer' to 'undefined'.

binaries/data/mods/public/maps/random/oasis.js
| 220| var·terrainPainter·=·new·TerrainPainter(tDirt);
|    | [NORMAL] JSHintBear:
|    | 'terrainPainter' was used before it was defined.

binaries/data/mods/public/maps/random/oasis.js
| 265| group·=·new·SimpleGroup([new·SimpleObject(eStoneMine,·1,1,·0,0),new·SimpleObject(ePalmShort,·1,2,·3,3),new·SimpleObject(ePalmTall,·0,1,·3,3)
|    | [NORMAL] JSHintBear:
|    | Misleading line break before ','; readers may interpret this as an expression boundary.

binaries/data/mods/public/maps/random/oasis.js
| 274| group·=·new·SimpleGroup([new·SimpleObject(eMetalMine,·1,1,·0,0),new·SimpleObject(ePalmShort,·1,2,·2,3),new·SimpleObject(ePalmTall,·0,1,·2,2)
|    | [NORMAL] JSHintBear:
|    | Misleading line break before ','; readers may interpret this as an expression boundary.
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/maps/random/ardennes_forest.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/m

http://jw:8080/job/phabricator_lint/6/ for more details.

Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/maps/random/ardennes_forest.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/maps/random/ardennes_forest.js
| 153| 153| 	{
| 154| 154| 		return (d-13)/(19-13);
| 155| 155| 	}
| 156|    |-	else
| 157|    |-	{
|    | 156|+	
| 158| 157| 		return 1;
| 159|    |-	}
|    | 158|+	
| 160| 159| }
| 161| 160| 
| 162| 161| RMS.SetProgress(10);

binaries/data/mods/public/maps/random/ardennes_forest.js
| 308| »   »   var·group·=·new·RandomGroup(
|    | [NORMAL] JSHintBear:
|    | 'group' was used before it was defined.

binaries/data/mods/public/maps/random/ardennes_forest.js
| 390| »   »   if(h·>·15·&&·h·<·45·&&·playerClass.countMembersInRadius(ix,·iz,·1)·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/maps/random/ardennes_forest.js
| 400| »   »   ····h·<·15·&&·g_Map.validT(ix,·iz)·&&·randBool(0.05)·&&·hillDecoClass.countMembersInRadius(ix,·iz,·1)·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/maps/random/danubius.js
| 172| »   »   let·gX·=·i·==·0·?·gaulCityBorderDistance·:·mapSize·-·gaulCityBorderDistance;
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/maps/random/danubius.js
| 181| »   »   »   let·mX·=·i·==·0·?
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/maps/random/danubius.js
| 192| »   »   »   »   gX·+·gaulCityRadius·*·(i·==·0·?·1·:·-1),
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/maps/random/danubius.js
| 242| »   »   »   placeCustomFortress(mX,·mZ,·new·Fortress("celt·ritual·males",·new·Array(maleCount).fill(0).map(i·=>
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/maps/random/danubius.js
| 325| »   if·(numPlayers·%·2·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/maps/random/danubius.js
| 612| »   »   i·==·0·?
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/maps/random/danubius.js
| 767| »   »   »   »   i·==·0·?·triggerPointShipUnloadLeft·:·triggerPointShipUnloadRight,
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/maps/random/danubius.js
| 783| »   »   »   »   i·==·0·?·triggerPointLandPatrolLeft·:·triggerPointLandPatrolRight,
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/maps/random/cycladic_archipelago.js
| 132| »   var·placer·=·new·ClumpPlacer(islandBottom,·.7,·.1,·10,·ix,·iz);
|    | [NORMAL] JSHintBear:
|    | A leading decimal point can be confused with a dot: '.7'.

binaries/data/mods/public/maps/random/cycladic_archipelago.js
| 132| »   var·placer·=·new·ClumpPlacer(islandBottom,·.7,·.1,·10,·ix,·iz);
|    | [NORMAL] JSHintBear:
|    | A leading decimal point can be confused with a dot: '.1'.

binaries/data/mods/public/maps/random/cycladic_archipelago.js
| 161| »   var·placer·=·new·ClumpPlacer(islandSize,·.7,·.1,·10,·ix,·iz);
|    | [NORMAL] JSHintBear:
|    | A leading decimal point can be confused with a dot: '.7'.

binaries/data/mods/public/maps/random/cycladic_archipelago.js
| 161| »   var·placer·=·new·ClumpPlacer(islandSize,·.7,·.1,·10,·ix,·iz);
|    | [NORMAL] JSHintBear:
|    | A leading decimal point can be confused with a dot: '.1'.

binaries/data/mods/public/maps/random/cycladic_archipelago.js
| 277| »   var·placer·=·new·ClumpPlacer(islandBottom,·.7,·.1,·10,·ix,·iz);
|    | [NORMAL] JSHintBear:
|    | A leading decimal point can be confused with a dot: '.7'.

binaries/data/mods/public/maps/random/cycladic_archipelago.js
| 277| »   var·placer·=·new·ClumpPlacer(islandBottom,·.7,·.1,·10,·ix,·iz);
|    | [NORMAL] JSHintBear:
|    | A leading decimal point can be confused with a dot: '.1'.

binaries/data/mods/public/maps/random/cycladic_archipelago.js
| 285| »   var·placer·=·new·ClumpPlacer(islandSize,·.7,·.1,·10,·ix,·iz);
|    | [NORMAL] JSHintBear:
|    | A leading decimal point can be confused with a dot: '.7'.

binaries/data/mods/public/maps/random/cycladic_archipelago.js
| 285| »   var·placer·=·new·ClumpPlacer(islandSize,·.7,·.1,·10,·ix,·iz);
|    | [NORMAL] JSHintBear:
|    | A leading decimal point can be confused with a dot: '.1'.

binaries/data/mods/public/maps/random/unknown_land.js
| 204| »   »   »   if·(!(numPlayers%2)){
|    | [NORMAL] JSHintBear:
|    | Confusing use of '!'.

binaries/data/mods/public/maps/random/unknown_land.js
| 227| »   »   »   if·(!(numPlayers%2)){
|    | [NORMAL] JSHintBear:
|    | Confusing use of '!'.

binaries/data/mods/public/maps/random/unknown_land.js
| 442| »   »   »   if·(!(numPlayers%2)){
|    | [NORMAL] JSHintBear:
|    | Confusing use of '!'.

binaries/data/mods/public/maps/random/unknown_land.js
| 466| »   »   »   if·(!(numPlayers%2)){
|    | [NORMAL] JSHintBear:
|    | Confusing use of '!'.

binaries/data/mods/public/maps/random/unknown_land.js
| 765| »   »   »   var·distance·=·randFloat(0.,·0.1);
|    | [NORMAL] JSHintBear:
|    | A trailing decimal point can be confused with a dot: '0.'.

binaries/data/mods/public/maps/random/unknown_land.js
| 790| »   »   »   var·distance·=·randFloat(0.,·0.1);
|    | [NORMAL] JSHintBear:
|    | A trailing decimal point can be confused with a dot: '0.'.

binaries/data/mods/public/maps/random/unknown_land.js
| 817| »   »   »   var·distance·=·randFloat(0.,·0.1);
|    | [NORMAL] JSHintBear:
|    | A trailing decimal point can be confused with a dot: '0.'.

binaries/data/mods/public/maps/random/unknown_land.js
| 841| »   »   »   var·distance·=·randFloat(0.,·0.1);
|    | [NORMAL] JSHintBear:
|    | A trailing decimal point can be confused with a dot: '0.'.

binaries/data/mods/public/maps/random/unknown.js
| 365| »   »   »   if·(!(numPlayers%2)){
|    | [NORMAL] JSHintBear:
|    | Confusing use of '!'.

binaries/data/mods/public/maps/random/unknown.js
| 388| »   »   »   if·(!(numPlayers%2)){
|    | [NORMAL] JSHintBear:
|    | Confusing use of '!'.

binaries/data/mods/public/maps/random/unknown.js
| 604| »   »   »   if·(!(numPlayers%2)){
|    | [NORMAL] JSHintBear:
|    | Confusing use of '!'.

binaries/data/mods/public/maps/random/unknown.js
| 628| »   »   »   if·(!(numPlayers%2)){
|    | [NORMAL] JSHintBear:
|    | Confusing use of '!'.

binaries/data/mods/public/maps/random/unknown.js
| 986| »   »   »   var·distance·=·randFloat(0.,·0.1);
|    | [NORMAL] JSHintBear:
|    | A trailing decimal point can be confused with a dot: '0.'.

binaries/data/mods/public/maps/random/unknown.js
|1026| »   »   »   var·distance·=·randFloat(0.,·0.1);
|    | [NORMAL] JSHintBear:
|    | A trailing decimal point can be confused with a dot: '0.'.

binaries/data/mods/public/maps/random/unknown.js
|1068| »   »   »   var·distance·=·randFloat(0.,·0.1);
|    | [NORMAL] JSHintBear:
|    | A trailing decimal point can be confused with a dot: '0.'.

binaries/data/mods/public/maps/random/unknown.js
|1092| »   »   »   var·distance·=·randFloat(0.,·0.1);
|    | [NORMAL] JSHintBear:
|    | A trailing decimal point can be confused with a dot: '0.'.

binaries/data/mods/public/maps/random/migration.js
| 198| var·fx·=·fractionToTiles(0.12);
|    | [NORMAL] JSHintBear:
|    | 'fx' was used before it was defined.

binaries/data/mods/public/maps/random/migration.js
| 199| var·fz·=·fractionToTiles(0.5);
|    | [NORMAL] JSHintBear:
|    | 'fz' was used before it was defined.

binaries/data/mods/public/maps/random/migration.js
| 200| var·ix·=·round(fx);
|    | [NORMAL] JSHintBear:
|    | 'ix' was used before it was defined

http://jw:8080/job/phabricator_lint/7/ for more details.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/1294/ for more details.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/1295/ for more details.

Will need to take D465 into account after review.

FeXoR accepted this revision.Jul 17 2017, 4:26 PM
This revision is now accepted and ready to land.Jul 17 2017, 4:26 PM
Imarok updated this revision to Diff 2923.Jul 17 2017, 5:40 PM

Rebase and fix new maps.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jw:8080/job/phabricator/1750/ for more details.

Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/maps/random/canyon.js
| 506| for·(let·i·=·0;·i·<·randIntInclusive(3,·8);·++i)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/maps/random/canyon.js
| 509| for·(let·i·=·0;·i·<·randIntInclusive(3,·8);·++i)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/maps/random/rmgen/library.js
|  40| function·scaleByMapSize(min,·max)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'min' is already declared in the upper scope.

binaries/data/mods/public/maps/random/rmgen/library.js
|  40| function·scaleByMapSize(min,·max)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'max' is already declared in the upper scope.

binaries/data/mods/public/maps/random/alpine_valley.js
| 248| »   »   matrix[i].push(i·<·numPlayers·&&·j·<·numPlayers·&&·i·!=·j·&&·(i·==·j·-·1·||·i·==·j·+·1))
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

binaries/data/mods/public/maps/random/alpine_valley.js
| 321| »   var·placer·=·new·PathPlacer(ix,·iz,·ix2,·iz2,·scaleByMapSize(9,15),·0.4,·3*(scaleByMapSize(1,4)),·0.1,·0.1,·0.1);
|    | [NORMAL] JSHintBear:
|    | 'placer' was used before it was defined.

binaries/data/mods/public/maps/random/alpine_valley.js
| 333| »   if·(accept·==·null)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with 'null'.

binaries/data/mods/public/maps/random/alpine_valley.js
| 387| »   »   »   <·scaleByMapSize(9,15)·+·scaleByMapSize(10,15))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '<'; readers may interpret this as an expression boundary.

binaries/data/mods/public/maps/random/alpine_valley.js
| 398| »   »   »   <·scaleByMapSize(9,15)·+·scaleByMapSize(10,15))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '<'; readers may interpret this as an expression boundary.
|    | [NORMAL] ESLintBear (no-extra-semi):
|    | Unnecessary semicolon.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/maps/random/syria.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/maps/random/syria.js
|  10|  10| const tCliff = ["desert_cliff_persia_1", "desert_cliff_persia_2"];
|  11|  11| const tHill = ["desert_dirt_rocks_1", "desert_dirt_rocks_2", "desert_dirt_rocks_3"];
|  12|  12| const tDirt = ["desert_dirt_rough", "desert_dirt_rough_2"];
|  13|    |-const tRoad = "desert_shore_stones";;
|    |  13|+const tRoad = "desert_shore_stones";
|  14|  14| const tRoadWild = "desert_grass_a_stones";;
|  15|  15| 
|  16|  16| // gaia entities
|    | [NORMAL] ESLintBear (no-extra-semi):
|    | Unnecessary semicolon.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/maps/random/syria.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/maps/random/syria.js
|  11|  11| const tHill = ["desert_dirt_rocks_1", "desert_dirt_rocks_2", "desert_dirt_rocks_3"];
|  12|  12| const tDirt = ["desert_dirt_rough", "desert_dirt_rough_2"];
|  13|  13| const tRoad = "desert_shore_stones";;
|  14|    |-const tRoadWild = "desert_grass_a_stones";;
|    |  14|+const tRoadWild = "desert_grass_a_stones";
|  15|  15| 
|  16|  16| // gaia entities
|  17|  17| const oTamarix = "gaia/flora_tree_tamarix";

binaries/data/mods/public/maps/random/syria.js
|  13| const·tRoad·=·"desert_shore_stones";;
|    | [NORMAL] JSHintBear:
|    | Unnecessary semicolon.

binaries/data/mods/public/maps/random/syria.js
|  14| const·tRoadWild·=·"desert_grass_a_stones";;
|    | [NORMAL] JSHintBear:
|    | Unnecessary semicolon.

binaries/data/mods/public/maps/random/unknown_nomad.js
| 175| »   var·placer·=·new·ClumpPlacer(mapArea·*·0.45,·0.9,·0.09,·10,·ix,·iz);
|    | [NORMAL] JSHintBear:
|    | 'placer' was used before it was defined.

binaries/data/mods/public/maps/random/unknown_nomad.js
| 176| »   var·terrainPainter·=·new·LayeredPainter(
|    | [NORMAL] JSHintBear:
|    | 'terrainPainter' was used before it was defined.

binaries/data/mods/public/maps/random/unknown_nomad.js
| 180| »   var·elevationPainter·=·new·SmoothElevationPainter(
|    | [NORMAL] JSHintBear:
|    | 'elevationPainter' was used before it was defined.

binaries/data/mods/public/maps/random/unknown_nomad.js
| 682| »   »   »   var·distance·=·randFloat(0.,·0.1);
|    | [NORMAL] JSHintBear:
|    | A trailing decimal point can be confused with a dot: '0.'.

binaries/data/mods/public/maps/random/unknown_nomad.js
| 722| »   »   »   var·distance·=·randFloat(0.,·0.1);
|    | [NORMAL] JSHintBear:
|    | A trailing decimal point can be confused with a dot: '0.'.

binaries/data/mods/public/maps/random/unknown_nomad.js
| 764| »   »   »   var·distance·=·randFloat(0.,·0.1);
|    | [NORMAL] JSHintBear:
|    | A trailing decimal point can be confused with a dot: '0.'.

binaries/data/mods/public/maps/random/unknown_nomad.js
| 803| »   »   »   var·distance·=·randFloat(0.,·0.1);
|    | [NORMAL] JSHintBear:
|    | A trailing decimal point can be confused with a dot: '0.'.

binaries/data/mods/public/maps/random/unknown_land.js
| 204| »   »   »   if·(!(numPlayers%2)){
|    | [NORMAL] JSHintBear:
|    | Confusing use of '!'.

binaries/data/mods/public/maps/random/unknown_land.js
| 227| »   »   »   if·(!(numPlayers%2)){
|    | [NORMAL] JSHintBear:
|    | Confusing use of '!'.

binaries/data/mods/public/maps/random/unknown_land.js
| 442| »   »   »   if·(!(numPlayers%2)){
|    | [NORMAL] JSHintBear:
|    | Confusing use of '!'.

binaries/data/mods/public/maps/random/unknown_land.js
| 466| »   »   »   if·(!(numPlayers%2)){
|    | [NORMAL] JSHintBear:
|    | Confusing use of '!'.

binaries/data/mods/public/maps/random/unknown_land.js
| 765| »   »   »   var·distance·=·randFloat(0.,·0.1);
|    | [NORMAL] JSHintBear:
|    | A trailing decimal point can be confused with a dot: '0.'.

binaries/data/mods/public/maps/random/unknown_land.js
| 790| »   »   »   var·distance·=·randFloat(0.,·0.1);
|    | [NORMAL] JSHintBear:
|    | A trailing decimal point can be confused with a dot: '0.'.

binaries/data/mods/public/maps/random/unknown_land.js
| 817| »   »   »   var·distance·=·randFloat(0.,·0.1);
|    | [NORMAL] JSHintBear:
|    | A trailing decimal point can be confused with a dot: '0.'.

binaries/data/mods/public/maps/random/unknown_land.js
| 841| »   »   »   var·distance·=·randFloat(0.,·0.1);
|    | [NORMAL] JSHintBear:
|    | A trailing decimal point can be confused with a dot: '0.'.
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'null'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/maps/random/corsica.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/maps/random/corsica.js
| 234| 234| // Let's make it generally cliffy
| 235| 235| placer = new ClumpPlacer(fractionToSize(0.3)*1.8, 1.0, 0.2, 4,round((CorsicaX * 5 + fractionToTiles(0.5)) / 6.0),round(fractionToTiles(0.8)));
| 236| 236| elevationPainter = new SmoothElevationPainter(ELEVATION_MODIFY, 30,fractionToTiles(0.45));
| 237|    |-createArea( placer, [elevationPainter],  null);
|    | 237|+createArea( placer, [elevationPainter], null);
| 238| 238| placer = new ClumpPlacer(fractionToSize(0.3)*1.8, 1.0, 0.2, 4,round((SardiniaX * 5 + fractionToTiles(0.5)) / 6.0),round(fractionToTiles(0.2)));
| 239| 239| elevationPainter = new SmoothElevationPainter(ELEVATION_MODIFY, 30,fractionToTiles(0.45));
| 240| 240| createArea( placer, [elevationPainter],  null);
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'null'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/maps/random/corsica.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/maps/random/corsica.js
| 237| 23

http://jw:8080/job/phabricator_lint/320/ for more details.

FeXoR accepted this revision.Jul 29 2017, 9:25 PM
This revision was automatically updated to reflect the committed changes.

created ticket to remove the deprecated variants: #4695