createObjectGroups should not break when receiving an empty areas array

[elexis]

On this tiny Nile map, I got an error message instead of a finished mapgeneration.

Creating reeds...ERROR: JavaScript error: maps/random/rmgen/library.js line 185
TypeError: pickRandom(...) is undefined
  createObjectGroupsByAreas/placeFunc@maps/random/rmgen/library.js:185:38
  retryPlacing@maps/random/rmgen/library.js:113:16
  createObjectGroupsByAreas@maps/random/rmgen/library.js:189:9
  createObjectGroupsByAreasDeprecated@maps/random/rmgen/library.js:133:9
  @maps/random/the_nile.js:198:1

Alpha 23 / r21655, replay:

start {"settings":{"PlayerData":[{"Name":"elexis","Civ":"pers","Color":{"r":150,"g":20,"b":20},"AI":"","AIDiff":5,"AIBehavior":"defensive","Team":-1},{"Name":"Player 2","Civ":"kush","Color":{"r":21,"g":55,"b":149},"AI":"","AIDiff":3,"AIBehavior":"random","Team":-1},{"Name":"Player 3","Civ":"gaul","Color":{"r":86,"g":180,"b":31},"AI":"","AIDiff":3,"AIBehavior":"random","Team":-1},{"Name":"Indibil","Civ":"iber","Color":{"r":231,"g":200,"b":5},"AI":"petra","AIDiff":0,"AIBehavior":"balanced","Team":-1},{"Name":"Player 5","Civ":"rome","Color":{"r":50,"g":170,"b":170},"AI":"","AIDiff":3,"AIBehavior":"random","Team":-1},{"Name":"Kashta","Civ":"kush","Color":{"r":160,"g":80,"b":200},"AI":"petra","AIDiff":3,"AIBehavior":"random","Team":-1},{"Name":"Player 7","Civ":"pers","Color":{"r":220,"g":115,"b":16},"AI":"","AIDiff":3,"AIBehavior":"random","Team":-1},{"Name":"Player 8","Civ":"athen","Color":{"r":64,"g":64,"b":64},"AI":"","AIDiff":3,"AIBehavior":"random","Team":-1}],"VictoryConditions":["conquest","wonder","regicide"],"Size":128,"PopulationCap":300,"StartingResources":1000,"Ceasefire":0,"RelicCount":2,"RelicDuration":20,"WonderDuration":0,"RegicideGarrison":false,"Nomad":false,"RevealMap":true,"ExploreMap":true,"DisableTreasures":false,"DisableSpies":false,"LockTeams":false,"LastManStanding":true,"CheatsEnabled":true,"Name":"The Nile","Script":"the_nile.js","Description":"A calm wide river, representing the Nile River in Egypt, divides the map into western and eastern parts.","Keywords":["naval"],"RatingEnabled":false,"Preview":"the_nile.png","Seed":1647258984,"AISeed":2821901706,"BaseTerrain":"grass1_spring","BaseHeight":0,"XXXXXX":"Optionally define other things here, like we would for a scenario","GameType":"conquest","VictoryScripts":["scripts/TriggerHelper.js","scripts/ConquestCommon.js","scripts/Conquest.js","scripts/WonderVictory.js","scripts/Regicide.js"],"VictoryDuration":20,"CircularMap":true,"TriggerScripts":["scripts/TriggerHelper.js","scripts/ConquestCommon.js","scripts/Conquest.js","scripts/WonderVictory.js","scripts/Regicide.js"],"mapType":"random"},"mapType":"random","mapPath":"maps/random/","mapFilter":"all","gameSpeed":1,"script":"the_nile.js","map":"maps/random/the_nile","matchID":"9DA6FCAD95C9B1A8","timestamp":1522858830,"engine_version":"0.0.23","mods":[["public","0.0.23"]]}

It throws the error message when wanting to pick a random point of a random area. But this fails in case the areas is an empty array.

Instead it should fail silently and just not place anything.

[elexis]

Thoughts:

Either createObjectGroupsByAreas checks not only for an empty array of areas given but also for the provided area items being undefined or empty arrays, or it errors out when picking the random point for a random area, or every single call to createObjectGroupsByAreas requires a safeguard, which would at a lot of indentation:

g_Map.log("Creating island metal mines");
createObjectGroupsByAreas(
	new SimpleGroup([new SimpleObject(oMetalLarge, 1, 1, 0, 4)], true, clMetal),
	0,
	[avoidClasses(clMetal, 50, clRock, 10), stayClasses(clIsland, 5)],
	scaleByMapSize(3, 10),
	20,
	areaIslands);

Also the empty area test may not be performed in the retry loop but only once, also copies should be avoided.

Hence:

 function createObjectGroupsByAreas(group, player, constraints, amount, retryFactor, areas)
 {
+       areas = areas.filter(area => area.getPoints().length);
+       if (!areas.length)
+               return [];
+

or the safeguard spam

or implementing some new prototype like MultiArea with a getRandomPoint() function, which seems overkill for that job.

The N-th alternative is to have random map scripts deliberately error out if there is no point available, but then Danubius should throw an error message once the createArea call produced no result or an empty result.

This could actually be added to the end of createArea (but then it wouldnt be possible to create something only because it might fail in some RNG dependent instance), or the map scripts would have to throw new Errors() whenever they store an area that they will later reuse and want to rely on or use if-statements.

[FeXoR]

In 24525:

Avoid an error when calling createObjectGroupsByAreas or createAreasInAreas with areas argument being empty or containing an empty (sub) area.

Differential Revision: D1492
Fixes #5112
Original patch by smiley
Commandeered, edited and tested by @FeXoR
Discussed by @smiley, @elexis, @vladislavbelov and @FeXoR

[FeXoR]

In 24632:

Remove heightmap grascale pictures and corresponding scenarios due to uncertain copyright status

Differential Revision: D2777
Fixes #5112
Patch by @Nescio
Accepted by @FeXoR