Page MenuHomeWildfire Games

Mapgen: library improvements
AbandonedPublic

Authored by aeonios on May 19 2018, 1:46 PM.

Details

Reviewers
niektb
FeXoR
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Summary

This changes a couple of things with the mapgen libraries:

  • The XClasses constraints factory functions have been moved from library into Constraint, which is probably a more appropriate place for them.
  • I added a nearClasses function for generating that particular type of constraint, which did not previously have a factory.
  • I added a mountain range generator library file, which is an improved fork of the mountain range generator from the Alpine Valley script. Depending on how it's configured it can potentially produce highly diverse results, from giant mountains to rolling sand dunes.
  • I changed some of the core object creation convenience functions to use the new object placing behavior instead of the deprecated behavior.

Basically the difference is that, say you try to create 10 metal mines, with the deprecated behavior if you set the retry factor to 70 you might get 700 mines, or you might get none at all. With the new (and actually correct) behavior you can set the retry factor to 1000 and if you asked for 10 mines you'll only get 10 mines, assuming that there are sufficient locations that meet the specified constraints. This is extremely important for getting reliable object placement behavior, both for resources like stone, metal and food and also for placing props. With the new behavior it's much, much easier to ensure that generated maps are well balanced and consistent, and that any randomness is intentional rather than an undesired side-effect of badly designed library functions. The new functions use much higher retry factors; 1000, 500, and 250 for mines, food and decorations, respectively, as opposed to 70, 50, and 20 for the old functions.

I also added an optional "areas" parameter to all of the object creation functions, since placing objects by area is generally much more reliable for things like, for example, placing fish into lakes or placing units in very specific defined areas. Technically the functions with the new behavior could probably do it reliably by brute force, but by-areas requires far fewer retries to get a successful placement, and so should reduce map generation time where applicable.

The optional areas parameter has no effect on existing maps (it's the last argument and optional), but any maps using the existing create mines/food/decorations functions will need to be re-tuned to the newer behavior. As the problematic old deprecated behavior is a major blocker to creating good mapgen scripts, it should be phased out entirely well before a24 to allow for new content to be developed. Preferably all the old deprecated code should be removed before a24 including the dead libraries in rmgen2, and anything useful consolidated into a more centralized library. This is merely the first step towards that goal.

Test Plan

At the moment, none. It will probably break every map, and possibly some libraries as well. I'm working on at least getting the libraries un-broken for this revision.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

aeonios created this revision.May 19 2018, 1:46 PM
Vulcan added a subscriber: Vulcan.May 19 2018, 1:50 PM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen/Constraint.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen/Constraint.js
|   4|   4| 
|   5|   5| /**
|   6|   6|  * The following functions are for easily creating constraints in batches.
|   7|    |- */ 
|    |   7|+ */
|   8|   8| 
|   9|   9| /**
|  10|  10|  * Create an avoid constraint for the given classes by the given distances
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen/Constraint.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen/Constraint.js
|  86|  86| function AndConstraint(constraints)
|  87|  87| {
|  88|  88| 	if (constraints instanceof Array)
|  89|    |-		this.constraints = constraints
|    |  89|+		this.constraints = constraints;
|  90|  90| 	else if (!constraints)
|  91|  91| 		this.constraints = [];
|  92|  92| 	else
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen/Constraint.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen/Constraint.js
| 136| 136| 
| 137| 137| AvoidAreasConstraint.prototype.allows = function(position)
| 138| 138| {
| 139|    |-	return this.areas.every(area => !area.contains(position))
|    | 139|+	return this.areas.every(area => !area.contains(position));
| 140| 140| };
| 141| 141| 
| 142| 142| /**

binaries/data/mods/public/maps/random/rmgen/Constraint.js
|  89| »   »   this.constraints·=·constraints
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

binaries/data/mods/public/maps/random/rmgen/Constraint.js
| 139| »   return·this.areas.every(area·=>·!area.contains(position))
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'for-of'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_entities.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_entities.js
|  72|  72| function createMines(objects, constraint, tileClass, count, areas)
|  73|  73| {
|  74|  74| 	for (let object of objects)
|  75|    |-	{
|    |  75|+	
|  76|  76| 		if (areas)
|  77|  77| 		{
|  78|  78| 			createObjectGroupsByAreas(
|  94|  94| 				1000
|  95|  95| 			);
|  96|  96| 		}
|  97|    |-	}
|    |  97|+	
|  98|  98| }
|  99|  99| 
| 100| 100| /**
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_entities.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_entities.js
|  74|  74| 	for (let object of objects)
|  75|  75| 	{
|  76|  76| 		if (areas)
|  77|    |-		{
|    |  77|+		
|  78|  78| 			createObjectGroupsByAreas(
|  79|  79| 				new SimpleGroup(object, true, tileClass),
|  80|  80| 				0,
|  83|  83| 				1000,
|  84|  84| 				areas
|  85|  85| 			);
|  86|    |-		}
|    |  86|+		
|  87|  87| 		else
|  88|  88| 		{
|  89|  89| 			createObjectGroups(
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'else'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_entities.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_entities.js
|  85|  85| 			);
|  86|  86| 		}
|  87|  87| 		else
|  88|    |-		{
|    |  88|+		
|  89|  89| 			createObjectGroups(
|  90|  90| 				new SimpleGroup(object, true, tileClass),
|  91|  91| 				0,
|  93|  93| 				count || scaleByMapSize(4, 16),
|  94|  94| 				1000
|  95|  95| 			);
|  96|    |-		}
|    |  96|+		
|  97|  97| 	}
|  98|  98| }
|  99|  99| 
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'for' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_entities.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_entities.js
| 123| 123| function createFood(objects, counts, constraint, tileClass, areas)
| 124| 124| {
| 125| 125| 	for (let i = 0; i < objects.length; ++i)
| 126|    |-	{
|    | 126|+	
| 127| 127| 		if (areas)
| 128| 128| 		{
| 129| 129| 			createObjectGroupsByAreas(
| 145| 145| 				500
| 146| 146| 			);
| 147| 147| 		}
| 148|    |-	}
|    | 148|+	
| 149| 149| }
| 150| 150| 
| 151| 151| /**
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_entities.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_entities.js
| 125| 125| 	for (let i = 0; i < objects.length; ++i)
| 126| 126| 	{
| 127| 127| 		if (areas)
| 128|    |-		{
|    | 128|+		
| 129| 129| 			createObjectGroupsByAreas(
| 130| 130| 				new SimpleGroup(objects[i], true, tileClass),
| 131| 131| 				0,
| 134| 134| 				500,
| 135| 135| 				areas
| 136| 136| 			);
| 137|    |-		}
|    | 137|+		
| 138| 138| 		else
| 139| 139| 		{
| 140| 140| 			createObjectGroups(
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'else'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_entities.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_entities.js
| 136| 136| 			);
| 137| 137| 		}
| 138| 138| 		else
| 139|    |-		{
|    | 139|+		
| 140| 140| 			createObjectGroups(
| 141| 141| 				new SimpleGroup(objects[i], true, tileClass),
| 142| 142| 				0,
| 144| 144| 				counts[i] || scaleByMapSize(4, 16),
| 145| 145| 				500
| 146| 146| 			);
| 147|    |-		}
|    | 147|+		
| 148| 148| 	}
| 149| 149| }
| 150| 150| 
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'for' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_entities.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_entities.js
| 154| 154| function createDecoration(objects, counts, constraint, areas)
| 155| 155| {
| 156| 156| 	for (let i = 0; i < objects.length; ++i)
| 157|    |-	{
|    | 157|+	
| 158| 158| 		if (areas)
| 159| 159| 		{
| 160| 160| 			createObjectGroupsByAreas(
| 176| 176| 				250
| 177| 177| 			);
| 178| 178| 		}
| 179|    |-	}
|    | 179|+	
| 180| 180| }
| 181| 181| 
| 182| 182| /**
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_entities.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_entities.js
| 156| 156| 	for (let i = 0; i < objects.length; ++i)
| 157| 157| 	{
| 158| 158| 		if (areas)
| 159|    |-		{
|    | 159|+		
| 160| 160| 			createObjectGroupsByAreas(
| 161| 161| 				new SimpleGroup(objects[i], true),
| 162| 162| 				0,
| 165| 165| 				250,
| 166| 166| 				areas
| 167| 167| 			);
| 168|    |-		}
|    | 168|+		
| 169| 169| 		else
| 170| 170| 		{
| 171| 171| 			createObjectGroups(
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'else'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_entities.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_entities.js
| 167| 167| 			);
| 168| 168| 		}
| 169| 169| 		else
| 170|    |-		{
|    | 170|+		
| 171| 171| 			createObjectGroups(
| 172| 172| 				new SimpleGroup(objects[i], true),
| 173| 173| 				0,
| 175| 175| 				counts[i] || scaleByMapSize(16, 200),
| 176| 176| 				250
| 177| 177| 			);
| 178|    |-		}
|    | 178|+		
| 179| 179| 	}
| 180| 180| }
| 181| 181|

Link to build: https://jenkins.wildfiregames.com/job/differential/513/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/514/display/redirect

I added a nearClasses function for generating that particular type of constraint, which did not previously have a factory.

As mentioned in D1497, I purposefully not introduced that function because these helper functions can be be deleted if the constructors of the Constraints support receiving multiple arguments.
This way the library can become smaller and easier to comprehend for beginners without losing features.

binaries/data/mods/public/maps/random/rmgen-common/gaia_entities.js
72

I'm wondering if we should keep these functions to begin with.
The plan was rather to continue to remove them, they're proxies only, they don't shorten the code significantly.
Instead they provide default values and that is something that ends in more trouble than it helps (see commits on these helper functions in this file, bugfixes because maps didnt specify values that ended up in oversights).

85

There is a ticket to remove the last Deprecated calls.
Changing the code is trivial. Testing that all affected maps don't drastically change their result is the hard part.
(That's why the Deprecated functions were introduced in the first place rather than replacing the function).

binaries/data/mods/public/maps/random/rmgen-common/mountain_range_builder.js
1 ↗(On Diff #6574)

wrong include

2 ↗(On Diff #6574)

Apparently the argument that maps should rather do something unique than copying the mapgen approach of other maps wasn't convincing.
Maybe we can negotiate that this makes it easier for mods to copy the mapgen approach of other maps in any case.

138 ↗(On Diff #6574)

I didn't investigate the algorithm changes, but we might consider if squashing this into a single function would end up in considerably less code.
If so, one might further consider to move it to the existing gaia_terrain.js collection.
Would need some investigation to decide.

In D1501#61476, @elexis wrote:

I added a nearClasses function for generating that particular type of constraint, which did not previously have a factory.

As mentioned in D1497, I purposefully not introduced that function because these helper functions can be be deleted if the constructors of the Constraints support receiving multiple arguments.
This way the library can become smaller and easier to comprehend for beginners without losing features.

You could do something like:

new Constraint({near: "class1, dist1, class2, dist2", avoid: "class1, dist1, class2, dist2"..etc} );

which I suppose might be cleaner and easier to understand without being overly complex or verbose, but it'll be a whole lot more work to implement something like that. That would also have the advantage of not needing to put constraints in an array when you want to use more than one, which is something that I did find confusing. For now the factories work though. That's something I'll have to dig through later, but it may or may not require changes to a lot of other things, and would definitely require updating all the maps, which unfortunately means I can't put it off even though I'd rather be working on actual map code. :P

binaries/data/mods/public/maps/random/rmgen-common/gaia_entities.js
72

IMO they do make things a lot easier and shorten the code by a bit. The easier part is what's important though, since they make batch handling of objects and dealing with tile classes and things very compact, even without the defaults. The added optional areas functionality really demonstrates just how flexible these functions can be made, and having a single interface for doing common stuff like this is really convenient.

Note that I ended up using these functions because they were used in Alpine Lakes and it was significantly cleaner and shorter than the code for Alpine Valley, not to mention easier to understand. I'm not particularly happy with create forests, but I haven't gotten around to testing out ways to simplify and improve it.

85

Well figuring out the subset of maps that will be affected by it isn't difficult. Testing and tuning them will take time, but we've got like a year until a24 and I plan on overhauling a lot of the old mapgen scripts anyway.

binaries/data/mods/public/maps/random/rmgen-common/mountain_range_builder.js
1 ↗(On Diff #6574)

Eh? What include should I use then? It uses Constraint, library, math, TerrainPainter, SmoothElevationPainter, TileClassPainter, PathPlacer, RandomMap and who knows what else. Also it works with this include.

2 ↗(On Diff #6574)

Well the main point is that it can be used in a lot of different ways depending on how it's configured. It could also be extended to be applied to specific areas only. IMO extracting such potentially reusable algorithms and making them configurable is a better way to go about it. After all that's what libraries are for. The ancient stuff in rmgen2 was going in that direction but it wasn't particularly well designed and is no longer compatible with current features. Of course people are free to implement whatever custom functionality they need if they do want a unique map, but that doesn't mean they should have to reinvent the wheel even for really basic stuff. Making it friendlier to new devs is also always a plus.

138 ↗(On Diff #6574)

It's technically possible just a PITA. I guess I'll work on that then. :P

aeonios updated this revision to Diff 6590.May 20 2018, 9:23 AM

Alright I condensed the mountain range builder into a single function and moved it into gaia_terrain.

For constraints I decided to combine the four different factory functions into a single function called getConstraints with an args table as input. The new syntax is moderately verbose but it makes creating constraints very consistent and clean, and it always returns either exactly one constraint object or exactly one AndConstraint object. Calling it 'getConstraints' should also make it clearer exactly what's going on when the function is called. That'll definitely break every freaking map script, but oh well. I updated the library functions in player.js so at least that isn't broken.

I also simplified createForests a bit and made it easier to use. It's now much easier to feed it multiple kinds of trees (ie so that you get mixed forests rather than monotypical groves) and it won't create overdense tree clumps when feeding it multiple tree types (which the old version would). It also should no longer require a secret decoder ring to figure out what arguments to feed it.

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen/Constraint.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen/Constraint.js
|   6|   6| const stay = "stay";
|   7|   7| const near = "near";
|   8|   8| const border = "border";
|   9|    |- 
|    |   9|+
|  10|  10|  /**
|  11|  11|  * This function returns constraints based on the specifications given by the arguments.
|  12|  12|  * 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 0 tabs but found 1 space.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen/Constraint.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen/Constraint.js
|   7|   7| const near = "near";
|   8|   8| const border = "border";
|   9|   9|  
|  10|    |- /**
|    |  10|+/**
|  11|  11|  * This function returns constraints based on the specifications given by the arguments.
|  12|  12|  * 
|  13|  13|  * Arguments:
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen/Constraint.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen/Constraint.js
|   9|   9|  
|  10|  10|  /**
|  11|  11|  * This function returns constraints based on the specifications given by the arguments.
|  12|    |- * 
|    |  12|+ *
|  13|  13|  * Arguments:
|  14|  14|  * @param "avoid": {[tileclass1, mindist1, tileclass2, mindist2..etc]} avoid these tileclasses, by no less than the minimum defined distance in all directions.
|  15|  15|  * @param "stay": {[tileclass1, mindist1, tileclass2, mindist2..etc]} stay within the borders of these tileclasses, by at least the minimum defined distance in all directions.
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen/Constraint.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen/Constraint.js
|  20|  20| {
|  21|  21| 	let constraints = [];
|  22|  22| 	let current;
|  23|    |-	
|    |  23|+
|  24|  24| 	current = args.avoid;
|  25|  25| 	if (current)
|  26|  26| 		for (let i = 0; i < current.length/2; ++i)
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen/Constraint.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen/Constraint.js
|  25|  25| 	if (current)
|  26|  26| 		for (let i = 0; i < current.length/2; ++i)
|  27|  27| 			constraints.push(new AvoidTileClassConstraint(current[2*i], current[2*i+1]));
|  28|    |-		
|    |  28|+
|  29|  29| 	current = args.stay;
|  30|  30| 	if (current)
|  31|  31| 		for (let i = 0; i < current.length/2; ++i)
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen/Constraint.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen/Constraint.js
|  30|  30| 	if (current)
|  31|  31| 		for (let i = 0; i < current.length/2; ++i)
|  32|  32| 			constraints.push(new StayInTileClassConstraint(current[2*i], current[2*i+1]));
|  33|    |-	
|    |  33|+
|  34|  34| 	current = args.near;
|  35|  35| 	if (current)
|  36|  36| 		for (let i = 0; i < current.length/2; ++i)
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen/Constraint.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen/Constraint.js
|  35|  35| 	if (current)
|  36|  36| 		for (let i = 0; i < current.length/2; ++i)
|  37|  37| 			constraints.push(new NearTileClassConstraint(current[2*i], current[2*i+1]));
|  38|    |-	
|    |  38|+
|  39|  39| 	current = args.border;
|  40|  40| 	if (current)
|  41|  41| 		for (let i = 0; i < current.length/3; ++i)
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen/Constraint.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen/Constraint.js
|  40|  40| 	if (current)
|  41|  41| 		for (let i = 0; i < current.length/3; ++i)
|  42|  42| 			constraints.push(new BorderTileClassConstraint(current[3*i], current[3*i+1], current[3*i+2]));
|  43|    |-	
|    |  43|+
|  44|  44| 	if (constraints.length == 1)
|  45|  45| 		return constraints[0];
|  46|  46| 
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_entities.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_entities.js
|  31|  31| 		borderTerrain.push(mainTerrain);
|  32|  32| 		borderTerrain.push(forestFloor);
|  33|  33| 		borderTerrain.push(forestFloor + TERRAIN_SEPARATOR + tree);
|  34|    |-		
|    |  34|+
|  35|  35| 		interiorTerrain.push(forestFloor);
|  36|  36| 		interiorTerrain.push(forestFloor);
|  37|  37| 		interiorTerrain.push(mainTerrain);
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 129| 129| 
| 130| 130| 				let state = gotRet[ix][iz];
| 131| 131| 				if (state == -1)
| 132|    |-				{
|    | 132|+				
| 133| 133| 					gotRet[ix][iz] = -2;
| 134|    |-				}
|    | 134|+				
| 135| 135| 				else if (state >= 0)
| 136| 136| 				{
| 137| 137| 					edges.splice(state, 1);
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 273| 273| 				clLava,
| 274| 274| 				position),
| 275| 275| 			0,
| 276|    |-		stayClasses(tileClass, 1));
|    | 276|+			stayClasses(tileClass, 1));
| 277| 277| 	}
| 278| 278| }
| 279| 279| 
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 281| 281|  * This function creates random lines of mountains ensuring that there are enough breaks to pass between them.
| 282| 282|  *
| 283| 283|  * To determine their location, random points are connected in shortest-cycle order and then trimmed to max length.
| 284|    |- * 
|    | 284|+ *
| 285| 285|  * Arguments:
| 286| 286|  * @param {number} "height": how tall the mountains should be, in absolute height (ie set not modify).
| 287| 287|  * @param {number} "width": How wide the mountains will be.
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 298| 298| 	// These parameters paint the mountainranges after their location was determined.
| 299| 299| 	let height = args.height || scaleByMapSize(60, 120);
| 300| 300| 	let mountainWidth = args.width || height/4;
| 301|    |-	
|    | 301|+
| 302| 302| 	let pathplacer = new PathPlacer(undefined, undefined, undefined, args.bumpiness || 0.4, scaleByMapSize(10, 25), args.waviness || 0.75, 0.2, 0.1);
| 303| 303| 	let painters = [
| 304| 304| 		new TerrainPainter(args.terrain),
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 309| 309| 
| 310| 310| 	// Array of Vector2D locations where a mountainrange can start or end.
| 311| 311| 	let vertices = new Array(2 * (args.count || 16)).fill(0).map(() => g_Map.randomCoordinate(false));
| 312|    |-	
|    | 312|+
| 313| 313| 	// Maximum length that a mountain path can have.
| 314| 314| 	let maxLength = args.maxLength || fractionToTiles(0.6);
| 315| 315| 
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 324| 324| 	{
| 325| 325| 		length += vertices[order[i]].distanceTo(vertices[order[i+1]]);
| 326| 326| 		if (length <= maxLength)
| 327|    |-		{
|    | 327|+		
| 328| 328| 			possibleEdges.push([order[i], order[i+1]]);
| 329|    |-		}else
|    | 329|+		else
| 330| 330| 		{
| 331| 331| 			length = 0;
| 332| 332| 		}
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'else'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 327| 327| 		{
| 328| 328| 			possibleEdges.push([order[i], order[i+1]]);
| 329| 329| 		}else
| 330|    |-		{
|    | 330|+		
| 331| 331| 			length = 0;
| 332|    |-		}
|    | 332|+		
| 333| 333| 	}
| 334| 334| 	
| 335| 335| 	g_Map.log("Creating mountain ranges with " + possibleEdges.length + " possible edges");
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 331| 331| 			length = 0;
| 332| 332| 		}
| 333| 333| 	}
| 334|    |-	
|    | 334|+
| 335| 335| 	g_Map.log("Creating mountain ranges with " + possibleEdges.length + " possible edges");
| 336| 336| 
| 337| 337| 	// Create the mountain ranges.
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 338| 338| 	for (let i = 0; i < possibleEdges.length; ++i)
| 339| 339| 	{
| 340| 340| 		let currentEdge = possibleEdges[i];
| 341|    |-		
|    | 341|+
| 342| 342| 		pathplacer.start = vertices[currentEdge[0]];
| 343| 343| 		pathplacer.end = vertices[currentEdge[1]];
| 344| 344| 		pathplacer.width = mountainWidth;
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 349| 349| 			++vertexDegree[currentEdge[1]];
| 350| 350| 		}
| 351| 351| 	}
| 352|    |-	
|    | 352|+
| 353| 353| 	// Create circular mountains to connect ranges that share vertices. 
| 354| 354| 	for (let i = 0; i < vertexDegree.length; ++i)
| 355| 355| 	{
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 350| 350| 		}
| 351| 351| 	}
| 352| 352| 	
| 353|    |-	// Create circular mountains to connect ranges that share vertices. 
|    | 353|+	// Create circular mountains to connect ranges that share vertices.
| 354| 354| 	for (let i = 0; i < vertexDegree.length; ++i)
| 355| 355| 	{
| 356| 356| 		if (vertexDegree[i] > 1)
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'for' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 352| 352| 	
| 353| 353| 	// Create circular mountains to connect ranges that share vertices. 
| 354| 354| 	for (let i = 0; i < vertexDegree.length; ++i)
| 355|    |-	{
|    | 355|+	
| 356| 356| 		if (vertexDegree[i] > 1)
| 357| 357| 		{
| 358| 358| 			createArea(
| 360| 360| 				painters,
| 361| 361| 				constraint);
| 362| 362| 		}
| 363|    |-	}
|    | 363|+	
| 364| 364| }
| 365| 365| 
| 366| 366| /**
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 354| 354| 	for (let i = 0; i < vertexDegree.length; ++i)
| 355| 355| 	{
| 356| 356| 		if (vertexDegree[i] > 1)
| 357|    |-		{
|    | 357|+		
| 358| 358| 			createArea(
| 359| 359| 				new ClumpPlacer(diskArea(mountainWidth / 2), 0.95, 0.6, Infinity, vertices[i]),
| 360| 360| 				painters,
| 361| 361| 				constraint);
| 362|    |-		}
|    | 362|+		
| 363| 363| 	}
| 364| 364| }
| 365| 365| 
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'tileClass'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 366| 366| /**
| 367| 367|  * Paint the given terrain texture in the given sizes at random places of the map to diversify monotone land texturing.
| 368| 368|  */
| 369|    |-function createPatches(sizes, terrain, constraints, count,  tileClass, failFraction =  0.5)
|    | 369|+function createPatches(sizes, terrain, constraints, count, tileClass, failFraction =  0.5)
| 370| 370| {
| 371| 371| 	for (let size of sizes)
| 372| 372| 		createAreas(
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '0.5'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 366| 366| /**
| 367| 367|  * Paint the given terrain texture in the given sizes at random places of the map to diversify monotone land texturing.
| 368| 368|  */
| 369|    |-function createPatches(sizes, terrain, constraints, count,  tileClass, failFraction =  0.5)
|    | 369|+function createPatches(sizes, terrain, constraints, count,  tileClass, failFraction = 0.5)
| 370| 370| {
| 371| 371| 	for (let size of sizes)
| 372| 372| 		createAreas(

binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 183| »   »   »   »   let·position·=·new·Vector2D(ix,·iz);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'position' is already declared in the upper scope.

binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 330| »   »   {
|    | [NORMAL] ESLintBear (brace-rules/brace-on-same-line):
|    | Closing curly brace appears on the same line as the subsequent block.
|    | [NORMAL] ESLintBear (object-curly-spacing):
|    | A space is required after '{'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/player.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/player.js
| 202| 202|  */
| 203| 203| function getPlayerBaseArgs(playerBaseArgs)
| 204| 204| {
| 205|    |-	let baseResourceConstraint = playerBaseArgs.BaseResourceClass && getConstraints({avoid: [playerBaseArgs.BaseResourceClass, 4]});
|    | 205|+	let baseResourceConstraint = playerBaseArgs.BaseResourceClass && getConstraints({ avoid: [playerBaseArgs.BaseResourceClass, 4]});
| 206| 206| 
| 207| 207| 	if (playerBaseArgs.baseResourceConstraint)
| 208| 208| 		baseResourceConstraint = new AndConstraint([baseResourceConstraint, playerBaseArgs.baseResourceConstraint]);
|    | [NORMAL] ESLintBear (quote-props):
|    | Unquoted property 'avoid' found.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/player.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/player.js
| 202| 202|  */
| 203| 203| function getPlayerBaseArgs(playerBaseArgs)
| 204| 204| {
| 205|    |-	let baseResourceConstraint = playerBaseArgs.BaseResourceClass && getConstraints({avoid: [playerBaseArgs.BaseResourceClass, 4]});
|    | 205|+	let baseResourceConstraint = playerBaseArgs.BaseResourceClass && getConstraints({"avoid": [playerBaseArgs.BaseResourceClass, 4]});
| 206| 206| 
| 207| 207| 	if (playerBaseArgs.baseResourceConstraint)
| 208| 208| 		baseResourceConstraint = new AndConstraint([baseResourceConstraint, playerBaseArgs.baseResourceConstraint]);
|    | [NORMAL] ESLintBear (object-curly-spacing):
|    | A space is required before '}'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/player.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/player.js
| 202| 202|  */
| 203| 203| function getPlayerBaseArgs(playerBaseArgs)
| 204| 204| {
| 205|    |-	let baseResourceConstraint = playerBaseArgs.BaseResourceClass && getConstraints({avoid: [playerBaseArgs.BaseResourceClass, 4]});
|    | 205|+	let baseResourceConstraint = playerBaseArgs.BaseResourceClass && getConstraints({avoid: [playerBaseArgs.BaseResourceClass, 4] });
| 206| 206| 
| 207| 207| 	if (playerBaseArgs.baseResourceConstraint)
| 208| 208| 		baseResourceConstraint = new AndConstraint([baseResourceConstraint, playerBaseArgs.baseResourceConstraint]);
|    | [NORMAL] ESLintBear (object-curly-spacing):
|    | A space is required after '{'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/player.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/player.js
| 477| 477| 		let group = new SimpleGroup(objects, true, playerClass);
| 478| 478| 		let success = false;
| 479| 479| 		for (let distanceFactor of [1, 1/2, 1/4, 0])
| 480|    |-			if (createObjectGroups(group, playerIDs[i], new AndConstraint([constraint, getConstraints({avoid: [playerClass, distance * distanceFactor]})]), 1, 200, false).length)
|    | 480|+			if (createObjectGroups(group, playerIDs[i], new AndConstraint([constraint, getConstraints({ avoid: [playerClass, distance * distanceFactor]})]), 1, 200, false).length)
| 481| 481| 			{
| 482| 482| 				success = true;
| 483| 483| 				playerPosition[i] = group.centerPosition;
|    | [NORMAL] ESLintBear (quote-props):
|    | Unquoted property 'avoid' found.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/player.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/player.js
| 477| 477| 		let group = new SimpleGroup(objects, true, playerClass);
| 478| 478| 		let success = false;
| 479| 479| 		for (let distanceFactor of [1, 1/2, 1/4, 0])
| 480|    |-			if (createObjectGroups(group, playerIDs[i], new AndConstraint([constraint, getConstraints({avoid: [playerClass, distance * distanceFactor]})]), 1, 200, false).length)
|    | 480|+			if (createObjectGroups(group, playerIDs[i], new AndConstraint([constraint, getConstraints({"avoid": [playerClass, distance * distanceFactor]})]), 1, 200, false).length)
| 481| 481| 			{
| 482| 482| 				success = true;
| 483| 483| 				playerPosition[i] = group.centerPosition;
|    | [NORMAL] ESLintBear (object-curly-spacing):
|    | A space is required before '}'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/player.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/player.js
| 477| 477| 		let group = new SimpleGroup(objects, true, playerClass);
| 478| 478| 		let success = false;
| 479| 479| 		for (let distanceFactor of [1, 1/2, 1/4, 0])
| 480|    |-			if (createObjectGroups(group, playerIDs[i], new AndConstraint([constraint, getConstraints({avoid: [playerClass, distance * distanceFactor]})]), 1, 200, false).length)
|    | 480|+			if (createObjectGroups(group, playerIDs[i], new AndConstraint([constraint, getConstraints({avoid: [playerClass, distance * distanceFactor] })]), 1, 200, false).length)
| 481| 481| 			{
| 482| 482| 				success = true;
| 483| 483| 				playerPosition[i] = group.centerPosition;
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 2.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/player.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/player.js
| 561| 561| 		east.length > west.length ?
| 562| 562| 			[east, west.concat(team)] :
| 563| 563| 			[east.concat(team), west],
| 564|    |-		[[], []]);
|    | 564|+	[[], []]);
| 565| 565| }
| 566| 566| 
| 567| 567| /**

Link to build: https://jenkins.wildfiregames.com/job/differential/525/display/redirect

aeonios updated this revision to Diff 6593.May 20 2018, 10:06 AM
aeonios edited the test plan for this revision. (Show Details)

More lint.

aeonios updated this revision to Diff 6594.May 20 2018, 10:11 AM

More lint...

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_entities.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_entities.js
|  31|  31| 		borderTerrain.push(mainTerrain);
|  32|  32| 		borderTerrain.push(forestFloor);
|  33|  33| 		borderTerrain.push(forestFloor + TERRAIN_SEPARATOR + tree);
|  34|    |-		
|    |  34|+
|  35|  35| 		interiorTerrain.push(forestFloor);
|  36|  36| 		interiorTerrain.push(forestFloor);
|  37|  37| 		interiorTerrain.push(mainTerrain);
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen/Constraint.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen/Constraint.js
|   9|   9| 
|  10|  10| /**
|  11|  11|  * This function returns constraints based on the specifications given by the arguments.
|  12|    |- * 
|    |  12|+ *
|  13|  13|  * Arguments:
|  14|  14|  * @param "avoid": {[tileclass1, mindist1, tileclass2, mindist2..etc]} avoid these tileclasses, by no less than the minimum defined distance in all directions.
|  15|  15|  * @param "stay": {[tileclass1, mindist1, tileclass2, mindist2..etc]} stay within the borders of these tileclasses, by at least the minimum defined distance in all directions.
|    | [NORMAL] ESLintBear (quote-props):
|    | Unquoted property 'avoid' found.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/player.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/player.js
| 202| 202|  */
| 203| 203| function getPlayerBaseArgs(playerBaseArgs)
| 204| 204| {
| 205|    |-	let baseResourceConstraint = playerBaseArgs.BaseResourceClass && getConstraints({ avoid: [playerBaseArgs.BaseResourceClass, 4] });
|    | 205|+	let baseResourceConstraint = playerBaseArgs.BaseResourceClass && getConstraints({ "avoid": [playerBaseArgs.BaseResourceClass, 4] });
| 206| 206| 
| 207| 207| 	if (playerBaseArgs.baseResourceConstraint)
| 208| 208| 		baseResourceConstraint = new AndConstraint([baseResourceConstraint, playerBaseArgs.baseResourceConstraint]);
|    | [NORMAL] ESLintBear (quote-props):
|    | Unquoted property 'avoid' found.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/player.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/player.js
| 477| 477| 		let group = new SimpleGroup(objects, true, playerClass);
| 478| 478| 		let success = false;
| 479| 479| 		for (let distanceFactor of [1, 1/2, 1/4, 0])
| 480|    |-			if (createObjectGroups(group, playerIDs[i], new AndConstraint([constraint, getConstraints({ avoid: [playerClass, distance * distanceFactor] })]), 1, 200, false).length)
|    | 480|+			if (createObjectGroups(group, playerIDs[i], new AndConstraint([constraint, getConstraints({ "avoid": [playerClass, distance * distanceFactor] })]), 1, 200, false).length)
| 481| 481| 			{
| 482| 482| 				success = true;
| 483| 483| 				playerPosition[i] = group.centerPosition;
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/player.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/player.js
| 561| 561| 		east.length > west.length ?
| 562| 562| 			[east, west.concat(team)] :
| 563| 563| 			[east.concat(team), west],
| 564|    |-			[[], []]
|    | 564|+	[[], []]
| 565| 565| 	);
| 566| 566| }
| 567| 567| 
|    | [NORMAL] ESLintBear (object-curly-spacing):
|    | A space is required after '{'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 259| 259| 				new SmoothElevationPainter(elevationType, layers[i].elevation, layers[i].steepness),
| 260| 260| 				new TileClassPainter(layers[i].tileClass)
| 261| 261| 			],
| 262|    |-			i == 0 ? null : getConstraints({stay: [layers[i - 1].tileClass, 1]})
|    | 262|+			i == 0 ? null : getConstraints({ stay: [layers[i - 1].tileClass, 1]})
| 263| 263| 		);
| 264| 264| 
| 265| 265| 	if (smoke)
|    | [NORMAL] ESLintBear (quote-props):
|    | Unquoted property 'stay' found.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 259| 259| 				new SmoothElevationPainter(elevationType, layers[i].elevation, layers[i].steepness),
| 260| 260| 				new TileClassPainter(layers[i].tileClass)
| 261| 261| 			],
| 262|    |-			i == 0 ? null : getConstraints({stay: [layers[i - 1].tileClass, 1]})
|    | 262|+			i == 0 ? null : getConstraints({"stay": [layers[i - 1].tileClass, 1]})
| 263| 263| 		);
| 264| 264| 
| 265| 265| 	if (smoke)
|    | [NORMAL] ESLintBear (object-curly-spacing):
|    | A space is required before '}'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 259| 259| 				new SmoothElevationPainter(elevationType, layers[i].elevation, layers[i].steepness),
| 260| 260| 				new TileClassPainter(layers[i].tileClass)
| 261| 261| 			],
| 262|    |-			i == 0 ? null : getConstraints({stay: [layers[i - 1].tileClass, 1]})
|    | 262|+			i == 0 ? null : getConstraints({stay: [layers[i - 1].tileClass, 1] })
| 263| 263| 		);
| 264| 264| 
| 265| 265| 	if (smoke)
|    | [NORMAL] ESLintBear (object-curly-spacing):
|    | A space is required after '{'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 272| 272| 				clLava,
| 273| 273| 				position),
| 274| 274| 			0,
| 275|    |-			getConstraints({stay: [tileClass, 1]})
|    | 275|+			getConstraints({ stay: [tileClass, 1]})
| 276| 276| 		);
| 277| 277| 	}
| 278| 278| }
|    | [NORMAL] ESLintBear (quote-props):
|    | Unquoted property 'stay' found.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 272| 272| 				clLava,
| 273| 273| 				position),
| 274| 274| 			0,
| 275|    |-			getConstraints({stay: [tileClass, 1]})
|    | 275|+			getConstraints({"stay": [tileClass, 1]})
| 276| 276| 		);
| 277| 277| 	}
| 278| 278| }
|    | [NORMAL] ESLintBear (object-curly-spacing):
|    | A space is required before '}'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 272| 272| 				clLava,
| 273| 273| 				position),
| 274| 274| 			0,
| 275|    |-			getConstraints({stay: [tileClass, 1]})
|    | 275|+			getConstraints({stay: [tileClass, 1] })
| 276| 276| 		);
| 277| 277| 	}
| 278| 278| }
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 347| 347| 		}
| 348| 348| 	}
| 349| 349| 
| 350|    |-	// Create circular mountains to connect ranges that share vertices. 
|    | 350|+	// Create circular mountains to connect ranges that share vertices.
| 351| 351| 	for (let i = 0; i < vertexDegree.length; ++i)
| 352| 352| 		if (vertexDegree[i] > 1)
| 353| 353| 			createArea(
|    | [NORMAL] ESLintBear (object-curly-spacing):
|    | A space is required after '{'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 539| 539| 	let mapCenter = g_Map.getCenter();
| 540| 540| 	let mapBounds = g_Map.getBounds();
| 541| 541| 
| 542|    |-	let riverConstraint = getConstraints({avoid: [tributaryRiverTileClass, 3]});
|    | 542|+	let riverConstraint = getConstraints({ avoid: [tributaryRiverTileClass, 3]});
| 543| 543| 	if (shallowTileClass)
| 544| 544| 		riverConstraint = new AndConstraint([riverConstraint, getConstraints({avoid: [shallowTileClass, 2]})]);
| 545| 545| 
|    | [NORMAL] ESLintBear (quote-props):
|    | Unquoted property 'avoid' found.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 539| 539| 	let mapCenter = g_Map.getCenter();
| 540| 540| 	let mapBounds = g_Map.getBounds();
| 541| 541| 
| 542|    |-	let riverConstraint = getConstraints({avoid: [tributaryRiverTileClass, 3]});
|    | 542|+	let riverConstraint = getConstraints({"avoid": [tributaryRiverTileClass, 3]});
| 543| 543| 	if (shallowTileClass)
| 544| 544| 		riverConstraint = new AndConstraint([riverConstraint, getConstraints({avoid: [shallowTileClass, 2]})]);
| 545| 545| 
|    | [NORMAL] ESLintBear (object-curly-spacing):
|    | A space is required before '}'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 539| 539| 	let mapCenter = g_Map.getCenter();
| 540| 540| 	let mapBounds = g_Map.getBounds();
| 541| 541| 
| 542|    |-	let riverConstraint = getConstraints({avoid: [tributaryRiverTileClass, 3]});
|    | 542|+	let riverConstraint = getConstraints({avoid: [tributaryRiverTileClass, 3] });
| 543| 543| 	if (shallowTileClass)
| 544| 544| 		riverConstraint = new AndConstraint([riverConstraint, getConstraints({avoid: [shallowTileClass, 2]})]);
| 545| 545| 
|    | [NORMAL] ESLintBear (object-curly-spacing):
|    | A space is required after '{'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 541| 541| 
| 542| 542| 	let riverConstraint = getConstraints({avoid: [tributaryRiverTileClass, 3]});
| 543| 543| 	if (shallowTileClass)
| 544|    |-		riverConstraint = new AndConstraint([riverConstraint, getConstraints({avoid: [shallowTileClass, 2]})]);
|    | 544|+		riverConstraint = new AndConstraint([riverConstraint, getConstraints({ avoid: [shallowTileClass, 2]})]);
| 545| 545| 
| 546| 546| 	for (let i = 0; i < riverCount; ++i)
| 547| 547| 	{
|    | [NORMAL] ESLintBear (quote-props):
|    | Unquoted property 'avoid' found.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 541| 541| 
| 542| 542| 	let riverConstraint = getConstraints({avoid: [tributaryRiverTileClass, 3]});
| 543| 543| 	if (shallowTileClass)
| 544|    |-		riverConstraint = new AndConstraint([riverConstraint, getConstraints({avoid: [shallowTileClass, 2]})]);
|    | 544|+		riverConstraint = new AndConstraint([riverConstraint, getConstraints({"avoid": [shallowTileClass, 2]})]);
| 545| 545| 
| 546| 546| 	for (let i = 0; i < riverCount; ++i)
| 547| 547| 	{
|    | [NORMAL] ESLintBear (object-curly-spacing):
|    | A space is required before '}'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 541| 541| 
| 542| 542| 	let riverConstraint = getConstraints({avoid: [tributaryRiverTileClass, 3]});
| 543| 543| 	if (shallowTileClass)
| 544|    |-		riverConstraint = new AndConstraint([riverConstraint, getConstraints({avoid: [shallowTileClass, 2]})]);
|    | 544|+		riverConstraint = new AndConstraint([riverConstraint, getConstraints({avoid: [shallowTileClass, 2] })]);
| 545| 545| 
| 546| 546| 	for (let i = 0; i < riverCount; ++i)
| 547| 547| 	{

Link to build: https://jenkins.wildfiregames.com/job/differential/527/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Default...
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen/Constraint.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen/Constraint.js
|   9|   9| 
|  10|  10| /**
|  11|  11|  * This function returns constraints based on the specifications given by the arguments.
|  12|    |- * 
|    |  12|+ *
|  13|  13|  * Arguments:
|  14|  14|  * @param "avoid": {[tileclass1, mindist1, tileclass2, mindist2..etc]} avoid these tileclasses, by no less than the minimum defined distance in all directions.
|  15|  15|  * @param "stay": {[tileclass1, mindist1, tileclass2, mindist2..etc]} stay within the borders of these tileclasses, by at least the minimum defined distance in all directions.
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_entities.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_entities.js
|  31|  31| 		borderTerrain.push(mainTerrain);
|  32|  32| 		borderTerrain.push(forestFloor);
|  33|  33| 		borderTerrain.push(forestFloor + TERRAIN_SEPARATOR + tree);
|  34|    |-		
|    |  34|+
|  35|  35| 		interiorTerrain.push(forestFloor);
|  36|  36| 		interiorTerrain.push(forestFloor);
|  37|  37| 		interiorTerrain.push(mainTerrain);
|    | [NORMAL] ESLintBear (quote-props):
|    | Unquoted property 'stay' found.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 259| 259| 				new SmoothElevationPainter(elevationType, layers[i].elevation, layers[i].steepness),
| 260| 260| 				new TileClassPainter(layers[i].tileClass)
| 261| 261| 			],
| 262|    |-			i == 0 ? null : getConstraints({ stay: [layers[i - 1].tileClass, 1] })
|    | 262|+			i == 0 ? null : getConstraints({ "stay": [layers[i - 1].tileClass, 1] })
| 263| 263| 		);
| 264| 264| 
| 265| 265| 	if (smoke)
|    | [NORMAL] ESLintBear (quote-props):
|    | Unquoted property 'stay' found.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 272| 272| 				clLava,
| 273| 273| 				position),
| 274| 274| 			0,
| 275|    |-			getConstraints({ stay: [tileClass, 1] })
|    | 275|+			getConstraints({ "stay": [tileClass, 1] })
| 276| 276| 		);
| 277| 277| 	}
| 278| 278| }
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 347| 347| 		}
| 348| 348| 	}
| 349| 349| 
| 350|    |-	// Create circular mountains to connect ranges that share vertices. 
|    | 350|+	// Create circular mountains to connect ranges that share vertices.
| 351| 351| 	for (let i = 0; i < vertexDegree.length; ++i)
| 352| 352| 		if (vertexDegree[i] > 1)
| 353| 353| 			createArea(
|    | [NORMAL] ESLintBear (quote-props):
|    | Unquoted property 'avoid' found.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 539| 539| 	let mapCenter = g_Map.getCenter();
| 540| 540| 	let mapBounds = g_Map.getBounds();
| 541| 541| 
| 542|    |-	let riverConstraint = getConstraints({ avoid: [tributaryRiverTileClass, 3] });
|    | 542|+	let riverConstraint = getConstraints({ "avoid": [tributaryRiverTileClass, 3] });
| 543| 543| 	if (shallowTileClass)
| 544| 544| 		riverConstraint = new AndConstraint([riverConstraint, getConstraints({ avoid: [shallowTileClass, 2] })]);
| 545| 545| 
|    | [NORMAL] ESLintBear (quote-props):
|    | Unquoted property 'avoid' found.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
| 541| 541| 
| 542| 542| 	let riverConstraint = getConstraints({ avoid: [tributaryRiverTileClass, 3] });
| 543| 543| 	if (shallowTileClass)
| 544|    |-		riverConstraint = new AndConstraint([riverConstraint, getConstraints({ avoid: [shallowTileClass, 2] })]);
|    | 544|+		riverConstraint = new AndConstraint([riverConstraint, getConstraints({ "avoid": [shallowTileClass, 2] })]);
| 545| 545| 
| 546| 546| 	for (let i = 0; i < riverCount; ++i)
| 547| 547| 	{
|    | [NORMAL] ESLintBear (quote-props):
|    | Unquoted property 'avoid' found.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/player.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/player.js
| 202| 202|  */
| 203| 203| function getPlayerBaseArgs(playerBaseArgs)
| 204| 204| {
| 205|    |-	let baseResourceConstraint = playerBaseArgs.BaseResourceClass && getConstraints({ avoid: [playerBaseArgs.BaseResourceClass, 4] });
|    | 205|+	let baseResourceConstraint = playerBaseArgs.BaseResourceClass && getConstraints({ "avoid": [playerBaseArgs.BaseResourceClass, 4] });
| 206| 206| 
| 207| 207| 	if (playerBaseArgs.baseResourceConstraint)
| 208| 208| 		baseResourceConstraint = new AndConstraint([baseResourceConstraint, playerBaseArgs.baseResourceConstraint]);
|    | [NORMAL] ESLintBear (quote-props):
|    | Unquoted property 'avoid' found.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/player.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/player.js
| 477| 477| 		let group = new SimpleGroup(objects, true, playerClass);
| 478| 478| 		let success = false;
| 479| 479| 		for (let distanceFactor of [1, 1/2, 1/4, 0])
| 480|    |-			if (createObjectGroups(group, playerIDs[i], new AndConstraint([constraint, getConstraints({ avoid: [playerClass, distance * distanceFactor] })]), 1, 200, false).length)
|    | 480|+			if (createObjectGroups(group, playerIDs[i], new AndConstraint([constraint, getConstraints({ "avoid": [playerClass, distance * distanceFactor] })]), 1, 200, false).length)
| 481| 481| 			{
| 482| 482| 				success = true;
| 483| 483| 				playerPosition[i] = group.centerPosition;
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/player.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/random/rmgen-common/player.js
| 561| 561| 		east.length > west.length ?
| 562| 562| 			[east, west.concat(team)] :
| 563| 563| 			[east.concat(team), west],
| 564|    |-			[[], []]
|    | 564|+	[[], []]
| 565| 565| 	);
| 566| 566| }
| 567| 567|

Link to build: https://jenkins.wildfiregames.com/job/differential/528/display/redirect

aeonios updated this revision to Diff 6595.May 20 2018, 11:07 AM

Third time's the charm? :|

aeonios updated this revision to Diff 6691.EditedJun 1 2018, 3:06 AM

I extended the removal of deprecated functions to createStragglerTrees.

I additionally added a convenience function, getAreaFromConstraints() which takes the same arguments as constraints and returns an area containing all the points that meet the given criteria. This is useful for feeding the areas params which were added to the other functions in order to boost placing success and reduce the number of retries required. I further changed the getConstraints function to return a null constraint if it's passed an empty table in args, which is useful for certain other situations and generalizes the function further.

Finally I re-implemented the globalSmoothMap function using a smoothing painter. Compared to the old (and since removed) function it adds more params to control the smoothing. Honestly I don't know why that function was removed in the first place since it's pretty necessary when using diamond-square and possibly in other situations as well. Technically it's only one line but it's an ugly line and the function makes it a lot simpler and more readable.

aeonios updated this revision to Diff 6711.Jun 2 2018, 10:00 PM

Added support for height and slope constraints to getConstraints().

Fixed a dumb error in the createMountainRanges() function.

aeonios updated this revision to Diff 6717.Jun 4 2018, 1:49 AM

I replaced the tileclass member counting function used by constraints with a function that returns either true or false, using early exit for success/failure depending on the type of query. The early exit should moderately reduce the cost of constraints checking, which is probably the largest single cost for generating maps.

Stan added a reviewer: FeXoR.May 28 2019, 10:52 PM
lyv added a subscriber: lyv.May 28 2019, 11:10 PM

I replaced the tileclass member counting function used by constraints with a function that returns either true or false, using early exit for success/failure depending on the type of query. The early exit should moderately reduce the cost of constraints checking, which is probably the largest single cost for generating maps.

For all the people who have spent countless hours messing with this, that is only partly true. The last point is only true in a subset of map designs. (Jebel Berkal a prime counter-example)
Most importantly, early-returning is entirely pointless when you have done other optimizations. See that tileclass thing lying around which have promising results. Those results could be matched and even faster at times without losing a feature that have interesting use cases.

(But most importantly, I like to see how this patch ends up)

In D1501#79867, @smiley wrote:

For all the people who have spent countless hours messing with this, that is only partly true. The last point is only true in a subset of map designs. (Jebel Berkal a prime counter-example)

It's true for most maps that aren't Jebel Berkal.

Most importantly, early-returning is entirely pointless when you have done other optimizations. See that tileclass thing lying around which have promising results. Those results could be matched and even faster at times without losing a feature that have interesting use cases.

I don't think so. Even with all the other optimizations this can still represent a significant speedup. Also there were no features lost by this. The behavior is exactly the same, only faster in a lot of common cases.
I mean literally all the function does is say "yes this location meets the requirements" or "no this location doesn't meet the requirements". That's all it ever did, except that it did so more wastefully before since it would keep calculating even after success or failure had already been established.

(But most importantly, I like to see how this patch ends up)

Meh. :P If I ever manage to get back into it.

lyv added a comment.EditedMay 29 2019, 1:27 PM

(First two is from a significantly optimized tileclass system)
Without early return.

Setting biome generic/alpine.
Generating Mainland of size 512 and 8 players.
Creating playerbases... 0.221s.
Creating bumps... 2.384s.
Creating hills... 0.593s.
Creating forests... 1.514s.
Creating dirt patches... 0.986s.
Creating grass patches... 0.615s.
Creating stone mines... 0.013s.
Creating metal mines... 0.004s.
Creating decoration... 0.213s.
Creating food... 0.016s.
Creating food... 0.008s.
Creating straggler trees... 0.248s.
Total map generation time: 6.848s.
Total entities: 16350, Terrain entities: 12882, Textures: 13.

With early return.

Setting biome generic/alpine.
Generating Mainland of size 512 and 8 players.
Creating playerbases... 0.220s.
Creating bumps... 2.461s.
Creating hills... 0.587s.
Creating forests... 1.460s.
Creating dirt patches... 1.052s.
Creating grass patches... 0.633s.
Creating stone mines... 0.014s.
Creating metal mines... 0.003s.
Creating decoration... 0.201s.
Creating food... 0.016s.
Creating food... 0.006s.
Creating straggler trees... 0.208s.
Total map generation time: 6.895s.
Total entities: 16350, Terrain entities: 12882, Textures: 13.

0AD svn tileclass

Setting biome generic/alpine.
Generating Mainland of size 512 and 8 players.
Creating playerbases... 0.377s.
Creating bumps... 6.081s.
Creating hills... 3.606s.
Creating forests... 18.671s.
Creating dirt patches... 3.885s.
Creating grass patches... 2.392s.
Creating stone mines... 0.035s.
Creating metal mines... 0.013s.
Creating decoration... 0.205s.
Creating food... 0.074s.
Creating food... 0.031s.
Creating straggler trees... 0.735s.
Total map generation time: 36.251s.
Total entities: 16350, Terrain entities: 12882, Textures: 13.

No clue how 0AD rmgen's tileclass performs with the early return. But I suppose what @elexis did in P142 should give some ideas.

elexis removed a reviewer: elexis.May 29 2019, 8:38 PM

(I'm currently not active as a developer, so removing myself from the queue)

Total map generation time: 6.848s.
Total map generation time: 36.251s.

Congratulations!

elexis added a subscriber: elexis.May 29 2019, 8:47 PM
elexis added inline comments.
binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
358

Those were the ones from that one alpine labyrinth replacement map? (That code is justified by an argument unrelated from the possible arguments of the rest of this code, i.e. who wants to review and or commit this code could split it into two patches, which might not be me today)

545

new AvoidClassesConstraint(a1, a2, b1, b2, ...) would be more in line with the rest of the codebase, no?

binaries/data/mods/public/maps/random/rmgen/Constraint.js
10

Not global constants

67

I personally would not recommend proxy functions that are that short and I removed many of them (library.js).

In D1501#79925, @smiley wrote:

(First two is from a significantly optimized tileclass system)

Without early return.
Total map generation time: 6.848s.

With early return.
Total map generation time: 6.895s.

0AD svn tileclass
Total map generation time: 36.251s.

No clue how 0AD rmgen's tileclass performs with the early return. But I suppose what @elexis did in P142 should give some ideas.

That's interesting. It seems like you're doing something completely different though.
It doesn't seem like the difference between early exit and without is very significant in your trials, either.
It was faster at some things and slower at others, but I don't see why it should ever be slower. I don't really understand what you did with tileclasses though, it may work completely differently than what I originally designed early exit for.

lyv added a comment.May 30 2019, 6:15 PM

I just wanted to point out that the changes to tileclass is unneeded and only removes a possibility. The chance of someone needing to count the number of tagged tiles within a radius is not that far off.

Anyway, I am way past the point of caring. As long as whoever will commit this is good with it, you are golden.

In D1501#80080, @smiley wrote:

I just wanted to point out that the changes to tileclass is unneeded and only removes a possibility. The chance of someone needing to count the number of tagged tiles within a radius is not that far off.

I see what you mean here. I mean I guess you could place things based on tile class density, but the current usage only uses geometric measures. If there were an actual need for that we could just add extra functions for it.
At the moment I don't see a real reason for using density-based constraints though.

What I don't understand is what's the difference between the 6-7 second map generation and the 30+ second generation from above? You said "significantly optimized" but this is the first I've heard anything about that.

If you've got a better way to reduce the cost then I'm all for it. That was the only reason for implementing early exit in the first place.

binaries/data/mods/public/maps/random/rmgen-common/gaia_terrain.js
358

yes I probably will end up splitting this off. It's not really related to the changes to constraints.

545

I changed the way it worked to a single constructor getConstraints that's more consistent and which should be more efficient when combining multiple constraints, iirc. Basically it saves you from having to type "new" 50 times when you want more than one type of constraint at a time.

Based on this particular example it might pay to get rid of the AndConstraint as well and just merge it into getConstraints, if it works the way I think it does, anyway.

binaries/data/mods/public/maps/random/rmgen/Constraint.js
10

Why not? I did this purely to avoid having to type quotation marks millions of times, because it's a serious pain as I found out firsthand. I don't see why they shouldn't just be reserved names.

67

It saves on parenthesis. A lot. It's also a pain to sort out the various arguments when you've got all that other mess going on, and this is still way easier to use and learn than that.

lyv added a comment.EditedMay 30 2019, 10:55 PM

Yeah, I found the 6-36 thing hard to believe too. Way less and faster iteration and way more efficient cpu cache usage might be the reason. The significantly JS typed arrays would have something to do with it too.
Anyway, we are now speaking of things which does not matter.

Yes, there is currently no need for finding the exact number of points in a radius. But a density constraint may not be the only use case. Smarter coordinate picking for random placers could also be a thing.
But, I guess if it helps, there is no reason to keep dead code around.
(my words don't have much influence. Take what I write as more of interesting fact of the day or something)

In D1501#80184, @smiley wrote:

Yeah, I found the 6-36 thing hard to believe too. Way less and faster iteration and way more efficient cpu cache usage might be the reason. The significantly JS typed arrays would have something to do with it too.
Anyway, we are now speaking of things which does not matter.

You should submit this. Seriously. Going from 36 to 6 is going from O(n^2) to O(n).
What I don't understand is why early exit is performing worse on certain things. :\ It should generally lead to fewer iterations or else have no effect at all.

Yes, there is currently no need for finding the exact number of points in a radius. But a density constraint may not be the only use case. Smarter coordinate picking for random placers could also be a thing.

They wouldn't be very 'random' then, now would they? Doesn't matter, if there's a good use case for it we can just write a function for it. No sense having the feature for uses that don't require it, particularly if it costs more.

But, I guess if it helps, there is no reason to keep dead code around.
(my words don't have much influence. Take what I write as more of interesting fact of the day or something)

I prefer examples to words. :P

lyv added a comment.May 30 2019, 11:22 PM

You should submit this.

You have done far more valuable things. I guess it doesn't matter if they are here or on a local machine.

In D1501#80194, @smiley wrote:

You should submit this.

You have done far more valuable things. I guess it doesn't matter if they are here or on a local machine.

It's not a competition. Heck, this patch is mostly about increasing convenience and consistency, the performance bit was just incidental.

lyv added a comment.EditedMay 30 2019, 11:33 PM

I have already checked out this branch. I feel the constraints took a right step, but I always favored using the objects directly without proxies. I guess that needs a detailed analysis.

(Agreed. I just wanted to see the point and purpose of doing things. If you think anything fruitful can come out of the endeavors, then great. I just don't think so.)

In D1501#80201, @smiley wrote:

Agreed. I just wanted to see the point and purpose of doing things. If you think anything fruitful can come out of the endeavors, then great. I just don't think so.

Are you kidding me? Slow map loading is a major problem with the current version. If you've got a way to dramatically reduce that and it proves to work then it's practically guaranteed to be accepted.

lyv added a comment.May 30 2019, 11:57 PM

If you've got a way to dramatically reduce that and it proves to work then it's practically guaranteed to be accepted.

https://youtu.be/uCGD9dT12C0

In D1501#80203, @smiley wrote:

If you've got a way to dramatically reduce that and it proves to work then it's practically guaranteed to be accepted.

https://youtu.be/uCGD9dT12C0

Well if you don't submit a patch then it definitely won't be accepted. :|

FeXoR abandoned this revision.Jul 9 2019, 8:44 PM

Due to lack of progress and no reviewer agreeing with all the changes here I'll reject this revision and close it.
Parts of it can be reimplemented in split patches.
Best regards and thanks four all participant's effort!

elexis added a comment.Jul 9 2019, 9:20 PM
  • TileClass is to be changed more deeply, it's on our radar and there are various patches floating around
  • the library.js functions would be better moved to the constraint constructors IMO (new AvoidClassesConstraint)

(First two is from a significantly optimized tileclass system)

That 6s performance is achieved by nuking the 27 already IIRC

In D1501#85699, @elexis wrote:
  • TileClass is to be changed more deeply, it's on our radar and there are various patches floating around

such as?

  • the library.js functions would be better moved to the constraint constructors IMO (new AvoidClassesConstraint)

The main reason I switched things around the way I did was to cut down on verbosity. You no longer have to type "new" all the time and instead of typing "AvoidClassesConstraint" you just type "avoid:". That's also why I made them constants, because nothing is a bigger PITA to type than freaking quotes.

(First two is from a significantly optimized tileclass system)

That 6s performance is achieved by nuking the 27 already IIRC

Do what?

elexis added a comment.Jul 9 2019, 9:48 PM
In D1501#85699, @elexis wrote:
  • TileClass is to be changed more deeply, it's on our radar and there are various patches floating around

such as?

D1637 for instance

  • the library.js functions would be better moved to the constraint constructors IMO (new AvoidClassesConstraint)

The main reason I switched things around the way I did was to cut down on verbosity. You no longer have to type "new" all the time and instead of typing "AvoidClassesConstraint" you just type "avoid:". That's also why I made them constants, because nothing is a bigger PITA to type than freaking quotes.

The object-oriented programming should not be hidden from the author, the author should rather become used to it. It's one the core design aspects of the rmgen system to be able to use different prototype instances implementing the same interface interchangably.
Yes it requires typing some more characters (new AvoidTileClassConstraint() vs avoidClasses()), but the alternative means spamming the libraries with random proxies.
The mapscripts should be as easy to write as possible, but not at the cost of introducing proxies in the library. The library should be as concise as possible while maintaining its features.

That 6s performance is achieved by nuking the 27 already IIRC

Do what?

Replace the number 27 with 0 in TileClass.js so that RangeOp is always used

In D1501#85713, @elexis wrote:

The object-oriented programming should not be hidden from the author, the author should rather become used to it. It's one the core design aspects of the rmgen system to be able to use different prototype instances implementing the same interface interchangably.
Yes it requires typing some more characters (new AvoidTileClassConstraint() vs avoidClasses()), but the alternative means spamming the libraries with random proxies.
The mapscripts should be as easy to write as possible, but not at the cost of introducing proxies in the library. The library should be as concise as possible while maintaining its features.

My preferred language is java. :\ I still prefer the factory methods in this case, but I guess I don't necessarily have to do everything in rmgen. An alternative library seems like a better option at this point, because we're definitely not going to agree on very much.

That 6s performance is achieved by nuking the 27 already IIRC

Do what?

Replace the number 27 with 0 in TileClass.js so that RangeOp is always used

Ok, I can pretty much guess what that involves. Seems like an easy win. I bet that early exit could be used there as well, but it'd need similar tuning to ensure that it's only used where it's likely to be profitable. On the other hand, the difference with early exit only seems to be fractions of a second when using RangeOp so it's probably not worth the effort. :P

lyv added a comment.EditedJul 9 2019, 10:52 PM

That 6s performance is achieved by nuking the 27 already IIRC

It helps, but the best I achieved was 18 seconds on 512 8 player mainland. You might have better luck with nani's patch.

In D1501#85734, @smiley wrote:

It helps, but the best I achieved was 8 seconds on 512 8 player mainland. You might have better luck with nani's patch.

8 seconds vs 30-40 seconds is still a huge improvement.

lyv added a comment.Jul 9 2019, 10:57 PM

typo. 8 -> 18

In D1501#85736, @smiley wrote:

typo. 8 -> 18

Oh. In that case it might be worthwhile to do more tuning with early exit, and possibly find the optimum value for using RangeOp.