Page MenuHomeWildfire Games

ProductionQueue - Refactor the remove item function
Needs ReviewPublic

Authored by Polakrity on Apr 22 2019, 5:34 PM.

Details

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

Refactor actual code for remove items in queue.

  • Rename RemoveBatch function in RemoveItem to be more general.
  • Remove the For-loop for find the index to a findIndex.
  • Code style cleanup (var -> let, etc.).

Also clear cached entities when only its first element has removed in ProductionQueue:

  • In the current state, RemoveBatch function cleans the entityCache and destroy the entities kept in memory whatever the element of the queue we want to delete.
  • This entityCache stores the entities freshly created by SpawnUnits function ( that is used when we finish the production of the first batch in the queue).
  • But when we may not be able to spawn all entities then these entities stays in the cache (to avoid being recreated) and reused until successful.

So I managed this to be executed only when we want delete the first element of the queue.

Test Plan

Check if everything works as before (remove an item, refund when destroy the prod.'s entity., etc.)

A way to test the "clear cached entities" thing:

  • Map: Sandbox - The Athenians
  • Unit: 1 Trireme.
  • Technology: Iphicratean Reforms (to be able to create mercenaries in ship).

Without patch:

  1. Stay away from the mainland with the ship.
  2. Add units in many batchs.
  3. Wait the first batch ends and then the units can't spawn.

(Among other things we should be able to spawn units in autogarnison if free slots but it's not the diff for that)

  1. Print the actual entityCache values.
  2. Remove a batch that isn't the first element.
  3. Print the entityCache and notice values aren't the same.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 7319
Build 11919: Vulcan BuildJenkins

Event Timeline

Polakrity created this revision.Apr 22 2019, 5:34 PM
Polakrity edited the test plan for this revision. (Show Details)
Stan added a reviewer: Restricted Owners Package.Apr 22 2019, 5:37 PM

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'for-of'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/ProductionQueue.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/ProductionQueue.js
| 475| 475| {
| 476| 476| 	var out = [];
| 477| 477| 	for (var item of this.queue)
| 478|    |-	{
|    | 478|+	
| 479| 479| 		out.push({
| 480| 480| 			"id": item.id,
| 481| 481| 			"unitTemplate": item.unitTemplate,
| 486| 486| 			"timeRemaining": item.timeRemaining,
| 487| 487| 			"metadata": item.metadata,
| 488| 488| 		});
| 489|    |-	}
|    | 489|+	
| 490| 490| 	return out;
| 491| 491| };
| 492| 492| 
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/ProductionQueue.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/ProductionQueue.js
| 568| 568| 	var spawnedEnts = [];
| 569| 569| 
| 570| 570| 	if (this.entityCache.length == 0)
| 571|    |-	{
|    | 571|+	
| 572| 572| 		// We need entities to test spawning, but we don't want to waste resources,
| 573| 573| 		//	so only create them once and use as needed
| 574| 574| 		for (var i = 0; i < count; ++i)
| 587| 587| 				cmpPlayerEntityLimits.ChangeCount(unitCategory, -1);
| 588| 588| 			}
| 589| 589| 		}
| 590|    |-	}
|    | 590|+	
| 591| 591| 
| 592| 592| 	let cmpAutoGarrison;
| 593| 593| 	if (cmpRallyPoint)
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/ProductionQueue.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/ProductionQueue.js
| 648| 648| 	}
| 649| 649| 
| 650| 650| 	if (spawnedEnts.length > 0 && !cmpAutoGarrison)
| 651|    |-	{
|    | 651|+	
| 652| 652| 		// If a rally point is set, walk towards it (in formation) using a suitable command based on where the
| 653| 653| 		// rally point is placed.
| 654| 654| 		if (cmpRallyPoint)
| 661| 661| 					ProcessCommand(cmpOwnership.GetOwner(), com);
| 662| 662| 			}
| 663| 663| 		}
| 664|    |-	}
|    | 664|+	
| 665| 665| 
| 666| 666| 	if (createdEnts.length > 0)
| 667| 667| 		Engine.PostMessage(this.entity, MT_TrainingFinished, {

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 343| »   »   »   let·template·=·TechnologyTemplates.Get(templateName);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'template' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 345| »   »   »   let·time·=·techCostMultiplier.time·*·template.researchTime·*·cmpPlayer.GetTimeMultiplier();
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'time' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 600| »   for·(let·i·=·0;·i·<·count;·++i)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 602| »   »   let·ent·=·this.entityCache[0];
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'ent' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 785| »   »   »   let·template·=·TechnologyTemplates.Get(item.technologyTemplate);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'template' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 190| »   for·(var·i·in·techList)
|    | [NORMAL] JSHintBear:
|    | 'i' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 192| »   »   var·tech·=·techList[i];
|    | [NORMAL] JSHintBear:
|    | 'tech' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 202| »   for·(var·i·=·0;·i·<·techList.length;·i++)
|    | [NORMAL] JSHintBear:
|    | 'i' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 204| »   »   var·tech·=·techList[i];
|    | [NORMAL] JSHintBear:
|    | 'tech' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 374| »   »   »   var·cmpTrigger·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_Trigger);
|    | [NORMAL] JSHintBear:
|    | 'cmpTrigger' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 771| »   »   »   »   »   var·cmpPlayer·=·QueryOwnerInterface(this.entity);
|    | [NORMAL] JSHintBear:
|    | 'cmpPlayer' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 782| »   »   »   var·cmpTechnologyManager·=·QueryOwnerInterface(this.entity,·IID_TechnologyManager);
|    | [NORMAL] JSHintBear:
|    | 'cmpTechnologyManager' is already defined.
Executing section cli...

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

bb added a subscriber: bb.Apr 26 2019, 9:23 PM

Among other things we should be able to spawn units in autogarnison if free slots but it's not the diff for that)

Careful with Extinct volcano OP-ness (on that map autogarrison would give a significant advantage to some civs)

binaries/data/mods/public/simulation/components/ProductionQueue.js
410–414

Wondering whether we want a findIndex here

420

Second statement is dead code
In principle one could achieve the same with adding a check outside the loop this.queue[0].id == id

Polakrity updated this revision to Diff 7874.Apr 27 2019, 12:43 AM
Polakrity retitled this revision from Clear cached entities when only its first element has removed in ProductionQueue to ProductionQueue - Refactor the remove item function.
Polakrity edited the summary of this revision. (Show Details)
Polakrity edited the test plan for this revision. (Show Details)

Refactor RemoveBatch function.
Then correct the length check entityCache.

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'for-of'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/ProductionQueue.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/ProductionQueue.js
| 469| 469| {
| 470| 470| 	var out = [];
| 471| 471| 	for (var item of this.queue)
| 472|    |-	{
|    | 472|+	
| 473| 473| 		out.push({
| 474| 474| 			"id": item.id,
| 475| 475| 			"unitTemplate": item.unitTemplate,
| 480| 480| 			"timeRemaining": item.timeRemaining,
| 481| 481| 			"metadata": item.metadata,
| 482| 482| 		});
| 483|    |-	}
|    | 483|+	
| 484| 484| 	return out;
| 485| 485| };
| 486| 486| 
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/ProductionQueue.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/ProductionQueue.js
| 558| 558| 	var spawnedEnts = [];
| 559| 559| 
| 560| 560| 	if (this.entityCache.length == 0)
| 561|    |-	{
|    | 561|+	
| 562| 562| 		// We need entities to test spawning, but we don't want to waste resources,
| 563| 563| 		//	so only create them once and use as needed
| 564| 564| 		for (var i = 0; i < count; ++i)
| 577| 577| 				cmpPlayerEntityLimits.ChangeCount(unitCategory, -1);
| 578| 578| 			}
| 579| 579| 		}
| 580|    |-	}
|    | 580|+	
| 581| 581| 
| 582| 582| 	let cmpAutoGarrison;
| 583| 583| 	if (cmpRallyPoint)
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/ProductionQueue.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/ProductionQueue.js
| 638| 638| 	}
| 639| 639| 
| 640| 640| 	if (spawnedEnts.length > 0 && !cmpAutoGarrison)
| 641|    |-	{
|    | 641|+	
| 642| 642| 		// If a rally point is set, walk towards it (in formation) using a suitable command based on where the
| 643| 643| 		// rally point is placed.
| 644| 644| 		if (cmpRallyPoint)
| 651| 651| 					ProcessCommand(cmpOwnership.GetOwner(), com);
| 652| 652| 			}
| 653| 653| 		}
| 654|    |-	}
|    | 654|+	
| 655| 655| 
| 656| 656| 	if (createdEnts.length > 0)
| 657| 657| 		Engine.PostMessage(this.entity, MT_TrainingFinished, {

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 343| »   »   »   let·template·=·TechnologyTemplates.Get(templateName);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'template' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 345| »   »   »   let·time·=·techCostMultiplier.time·*·template.researchTime·*·cmpPlayer.GetTimeMultiplier();
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'time' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 590| »   for·(let·i·=·0;·i·<·count;·++i)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 592| »   »   let·ent·=·this.entityCache[0];
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'ent' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 775| »   »   »   let·template·=·TechnologyTemplates.Get(item.technologyTemplate);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'template' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 190| »   for·(var·i·in·techList)
|    | [NORMAL] JSHintBear:
|    | 'i' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 192| »   »   var·tech·=·techList[i];
|    | [NORMAL] JSHintBear:
|    | 'tech' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 202| »   for·(var·i·=·0;·i·<·techList.length;·i++)
|    | [NORMAL] JSHintBear:
|    | 'i' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 204| »   »   var·tech·=·techList[i];
|    | [NORMAL] JSHintBear:
|    | 'tech' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 374| »   »   »   var·cmpTrigger·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_Trigger);
|    | [NORMAL] JSHintBear:
|    | 'cmpTrigger' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 761| »   »   »   »   »   var·cmpPlayer·=·QueryOwnerInterface(this.entity);
|    | [NORMAL] JSHintBear:
|    | 'cmpPlayer' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 772| »   »   »   var·cmpTechnologyManager·=·QueryOwnerInterface(this.entity,·IID_TechnologyManager);
|    | [NORMAL] JSHintBear:
|    | 'cmpTechnologyManager' is already defined.
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
| 330| 330| 			{
| 331| 331| 				var list = queue.GetEntitiesList();
| 332| 332| 				if (list.indexOf(cmd.template) === -1 && cmd.promoted)
| 333|    |-				{
|    | 333|+				
| 334| 334| 					for (var promoted of cmd.promoted)
| 335| 335| 					{
| 336| 336| 						if (list.indexOf(promoted) === -1)
| 338| 338| 						cmd.template = promoted;
| 339| 339| 						break;
| 340| 340| 					}
| 341|    |-				}
|    | 341|+				
| 342| 342| 			}
| 343| 343| 			if (queue && queue.GetEntitiesList().indexOf(cmd.template) != -1)
| 344| 344| 				if ("metadata" in cmd)
|    | [NORMAL] ESLintBear (operator-linebreak):
|    | '&&' should be placed at the end of the line.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
| 537| 537| 			if (cmpGarrisonHolder)
| 538| 538| 			{
| 539| 539| 				// Only the owner of the garrisonHolder may unload entities from any owners
| 540|    |-				if (!IsOwnedByPlayer(player, garrisonHolder) && !data.controlAllUnits
| 541|    |-				    && player != +cmd.owner)
|    | 540|+				if (!IsOwnedByPlayer(player, garrisonHolder) && !data.controlAllUnits &&
|    | 541|+				    player != +cmd.owner)
| 542| 542| 						continue;
| 543| 543| 
| 544| 544| 				if (!cmpGarrisonHolder.UnloadTemplate(cmd.template, cmd.owner, cmd.all))
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 5 tabs but found 6.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
| 539| 539| 				// Only the owner of the garrisonHolder may unload entities from any owners
| 540| 540| 				if (!IsOwnedByPlayer(player, garrisonHolder) && !data.controlAllUnits
| 541| 541| 				    && player != +cmd.owner)
| 542|    |-						continue;
|    | 542|+					continue;
| 543| 543| 
| 544| 544| 				if (!cmpGarrisonHolder.UnloadTemplate(cmd.template, cmd.owner, cmd.all))
| 545| 545| 					notifyUnloadFailure(player, garrisonHolder);
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'for' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
| 949| 949| 		{
| 950| 950| 			var count = 0;
| 951| 951| 			for (let j = 0; j < length - 1; ++j)
| 952|    |-			{
|    | 952|+			
| 953| 953| 				if ((waterPoints[(i + j) % length] + 1) % numPoints == waterPoints[(i + j + 1) % length])
| 954| 954| 					++count;
| 955| 955| 				else
| 956| 956| 					break;
| 957|    |-			}
|    | 957|+			
| 958| 958| 			consec[i] = count;
| 959| 959| 		}
| 960| 960| 		var start = 0;
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'for-in'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
| 960| 960| 		var start = 0;
| 961| 961| 		var count = 0;
| 962| 962| 		for (var c in consec)
| 963|    |-		{
|    | 963|+		
| 964| 964| 			if (consec[c] > count)
| 965| 965| 			{
| 966| 966| 				start = c;
| 967| 967| 				count = consec[c];
| 968| 968| 			}
| 969|    |-		}
|    | 969|+		
| 970| 970| 
| 971| 971| 		// If we've found a shoreline, stop searching
| 972| 972| 		if (count != numPoints-1)
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'metadata'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|1143|1143| 
|1144|1144| 	// send Metadata info if any
|1145|1145| 	if (cmd.metadata)
|1146|    |-		Engine.PostMessage(ent, MT_AIMetadata, { "id": ent, "metadata" : cmd.metadata, "owner" : player } );
|    |1146|+		Engine.PostMessage(ent, MT_AIMetadata, { "id": ent, "metadata": cmd.metadata, "owner" : player } );
|1147|1147| 
|1148|1148| 	// Tell the units to start building this new entity
|1149|1149| 	if (cmd.autorepair)
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'owner'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|1143|1143| 
|1144|1144| 	// send Metadata info if any
|1145|1145| 	if (cmd.metadata)
|1146|    |-		Engine.PostMessage(ent, MT_AIMetadata, { "id": ent, "metadata" : cmd.metadata, "owner" : player } );
|    |1146|+		Engine.PostMessage(ent, MT_AIMetadata, { "id": ent, "metadata" : cmd.metadata, "owner": player } );
|1147|1147| 
|1148|1148| 	// Tell the units to start building this new entity
|1149|1149| 	if (cmd.autorepair)
|    | [NORMAL] ESLintBear (space-in-parens):
|    | There should be no spaces inside this paren.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|1143|1143| 
|1144|1144| 	// send Metadata info if any
|1145|1145| 	if (cmd.metadata)
|1146|    |-		Engine.PostMessage(ent, MT_AIMetadata, { "id": ent, "metadata" : cmd.metadata, "owner" : player } );
|    |1146|+		Engine.PostMessage(ent, MT_AIMetadata, { "id": ent, "metadata" : cmd.metadata, "owner" : player });
|1147|1147| 
|1148|1148| 	// Tell the units to start building this new entity
|1149|1149| 	if (cmd.autorepair)
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|1147|1147| 
|1148|1148| 	// Tell the units to start building this new entity
|1149|1149| 	if (cmd.autorepair)
|1150|    |-	{
|    |1150|+	
|1151|1151| 		ProcessCommand(player, {
|1152|1152| 			"type": "repair",
|1153|1153| 			"entities": entities,
|1155|1155| 			"autocontinue": cmd.autocontinue,
|1156|1156| 			"queued": cmd.queued
|1157|1157| 		});
|1158|    |-	}
|    |1158|+	
|1159|1159| 
|1160|1160| 	return ent;
|1161|1161| }
|    | [NORMAL] ESLintBear (spaced-comment):
|    | Expected space or tab after '//' in comment.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|1251|1251| 		}
|1252|1252| 
|1253|1253| 		lastTowerControlGroup = cmpSnappedStartObstruction.GetControlGroup();
|1254|    |-		//warn("setting lastTowerControlGroup to control group of start snapped entity " + cmd.startSnappedEntity + ": " + lastTowerControlGroup);
|    |1254|+		// warn("setting lastTowerControlGroup to control group of start snapped entity " + cmd.startSnappedEntity + ": " + lastTowerControlGroup);
|1255|1255| 	}
|1256|1256| 
|1257|1257| 	var i = 0;
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|1268|1268| 		// 'lastTowerControlGroup' must always be defined and valid here, except if we're at the first piece and we didn't do
|1269|1269| 		// start position snapping (implying that the first entity we build must be a tower)
|1270|1270| 		if (lastTowerControlGroup === null || lastTowerControlGroup == INVALID_ENTITY)
|1271|    |-		{
|    |1271|+		
|1272|1272| 			if (!(i == 0 && piece.template == cmd.wallSet.templates.tower && !cmd.startSnappedEntity))
|1273|1273| 			{
|1274|1274|     			error("[TryConstructWall] Expected last tower control group to be available, none found (1st pass, iteration " + i + ")");
|1275|1275|     			break;
|1276|1276| 			}
|1277|    |-		}
|    |1277|+		
|1278|1278| 
|1279|1279| 		var constructPieceCmd = {
|1280|1280| 			"type": "construct",
|    | [NORMAL] ESLintBear (spaced-comment):
|    | Expected space or tab after '//' in comment.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|1315|1315| 
|1316|1316| 				if (i > 0)
|1317|1317| 				{
|1318|    |-					//warn("   updating previous wall piece's secondary control group to " + newTowerControlGroup);
|    |1318|+					// warn("   updating previous wall piece's secondary control group to " + newTowerControlGroup);
|1319|1319| 					var cmpPreviousObstruction = Engine.QueryInterface(pieces[i-1].ent, IID_Obstruction);
|1320|1320| 					// TODO: ensure that cmpPreviousObstruction exists
|1321|1321| 					// TODO: ensure that the previous obstruction does not yet have a secondary control group set
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|1373|1373| 		}
|1374|1374| 
|1375|1375| 		if (piece.template == cmd.wallSet.templates.tower)
|1376|    |-		{
|    |1376|+		
|1377|1377| 			// encountered a tower entity, update the last tower control group
|1378|1378| 			lastTowerControlGroup = cmpPieceObstruction.GetControlGroup();
|1379|    |-		}
|    |1379|+		
|1380|1380| 		else
|1381|1381| 		{
|1382|1382| 			// Encountered a non-tower entity, update its secondary control group to 'lastTowerControlGroup'.
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|1387|1387| 			if (existingSecondaryControlGroup == INVALID_ENTITY)
|1388|1388| 			{
|1389|1389| 				if (lastTowerControlGroup != null && lastTowerControlGroup != INVALID_ENTITY)
|1390|    |-				{
|    |1390|+				
|1391|1391| 					cmpPieceObstruction.SetControlGroup2(lastTowerControlGroup);
|1392|    |-				}
|    |1392|+				
|1393|1393| 			}
|1394|1394| 			else if (existingSecondaryControlGroup != lastTowerControlGroup)
|1395|1395| 			{
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|1462|1462| 	}
|1463|1463| 
|1464|1464| 	if (formedEnts.length == 0)
|1465|    |-	{
|    |1465|+	
|1466|1466| 		// No units support the formation - return all the others
|1467|1467| 		return nonformedUnitAIs;
|1468|    |-	}
|    |1468|+	
|1469|1469| 
|1470|1470| 	// Find what formations the formationable selected entities are currently in
|1471|1471| 	var formation = ExtractFormations(formedEnts);
|    | [NORMAL] ESLintBear (operator-linebreak):
|    | '&&' should be placed at the end of the line.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|1478|1478| 		// Check that all its members are selected
|1479|1479| 		var fid = formationIds[0];
|1480|1480| 		var cmpFormation = Engine.QueryInterface(+fid, IID_Formation);
|1481|    |-		if (cmpFormation && cmpFormation.GetMemberCount() == formation.members[fid].length
|1482|    |-			&& cmpFormation.GetMemberCount() == formation.entities.length)
|    |1481|+		if (cmpFormation && cmpFormation.GetMemberCount() == formation.members[fid].length &&
|    |1482|+			cmpFormation.GetMemberCount() == formation.entities.length)
|1483|1483| 		{
|1484|1484| 			cmpFormation.DeleteTwinFormations();
|1485|1485| 			// The whole formation was selected, so reuse its controller for this command
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|1510|1510| 					{
|1511|1511| 						var template = cmpUnitAI.GetFormationTemplate();
|1512|1512| 						if (lastFormationTemplate === undefined)
|1513|    |-						{
|    |1513|+						
|1514|1514| 							lastFormationTemplate = template;
|1515|    |-						}
|    |1515|+						
|1516|1516| 						else if (lastFormationTemplate != template)
|1517|1517| 						{
|1518|1518| 							lastFormationTemplate = undefined;
|    | [NORMAL] ESLintBear (comma-spacing):
|    | A space is required after ','.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|1586|1586| 		for (var i = matrix.length - 1; i >= 0 && !closeClusters; --i)
|1587|1587| 			for (var j = i - 1; j >= 0 && !closeClusters; --j)
|1588|1588| 				if (matrix[i][j] < distSq)
|1589|    |-					closeClusters = [i,j];
|    |1589|+					closeClusters = [i, j];
|1590|1590| 
|1591|1591| 		// if no more close clusters found, just return all found clusters so far
|1592|1592| 		if (!closeClusters)
|    | [NORMAL] ESLintBear (comma-spacing):
|    | A space is required after ','.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|1608|1608| 		}
|1609|1609| 		// remove the rows and columns in the matrix for the merged clusters,
|1610|1610| 		// and the clusters themselves from the cluster list
|1611|    |-		clusters.splice(closeClusters[0],1);
|    |1611|+		clusters.splice(closeClusters[0], 1);
|1612|1612| 		clusters.splice(closeClusters[1],1);
|1613|1613| 		matrix.splice(closeClusters[0],1);
|1614|1614| 		matrix.splice(closeClusters[1],1);
|    | [NORMAL] ESLintBear (comma-spacing):
|    | A space is required after ','.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|1609|1609| 		// remove the rows and columns in the matrix for the merged clusters,
|1610|1610| 		// and the clusters themselves from the cluster list
|1611|1611| 		clusters.splice(closeClusters[0],1);
|1612|    |-		clusters.splice(closeClusters[1],1);
|    |1612|+		clusters.splice(closeClusters[1], 1);
|1613|1613| 		matrix.splice(closeClusters[0],1);
|1614|1614| 		matrix.splice(closeClusters[1],1);
|1615|1615| 		for (let i = 0; i < matrix.length; ++i)
|    | [NORMAL] ESLintBear (comma-spacing):
|    | A space is required after ','.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|1610|1610| 		// and the clusters themselves from the cluster list
|1611|1611| 		clusters.splice(closeClusters[0],1);
|1612|1612| 		clusters.splice(closeClusters[1],1);
|1613|    |-		matrix.splice(closeClusters[0],1);
|    |1613|+		matrix.splice(closeClusters[0], 1);
|1614|1614| 		matrix.splice(closeClusters[1],1);
|1615|1615| 		for (let i = 0; i < matrix.length; ++i)
|1616|1616| 		{
|    | [NORMAL] ESLintBear (comma-spacing):
|    | A space is required after ','.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|1611|1611| 		clusters.splice(closeClusters[0],1);
|1612|1612| 		clusters.splice(closeClusters[1],1);
|1613|1613| 		matrix.splice(closeClusters[0],1);
|1614|    |-		matrix.splice(closeClusters[1],1);
|    |1614|+		matrix.splice(closeClusters[1], 1);
|1615|1615| 		for (let i = 0; i < matrix.length; ++i)
|1616|1616| 		{
|1617|1617| 			if (matrix[i].length > closeClusters[0])
|    | [NORMAL] ESLintBear (comma-spacing):
|    | A space is required after ','.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|1615|1615| 		for (let i = 0; i < matrix.length; ++i)
|1616|1616| 		{
|1617|1617| 			if (matrix[i].length > closeClusters[0])
|1618|    |-				matrix[i].splice(closeClusters[0],1);
|    |1618|+				matrix[i].splice(closeClusters[0], 1);
|1619|1619| 			if (matrix[i].length > closeClusters[1])
|1620|1620| 				matrix[i].splice(closeClusters[1],1);
|1621|1621| 		}
|    | [NORMAL] ESLintBear (comma-spacing):
|    | A space is required after ','.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/helpers/Commands.js
|1617|1617| 			if (matrix[i].length > closeClusters[0])
|1618|1618| 				matrix[i].splice(closeClusters[0],1);
|1619|1619| 			if (matrix[i].length > closeClusters[1])
|1620|    |-				matrix[i].splice(closeClusters[1],1);
|    |1620|+				matrix[i].splice(closeClusters[1], 1);
|1621|1621| 		}
|1622|1622| 		// add a new row of distances to the matrix and the new cluster
|1623|1623| 		clusters.push(newCluster);

binaries/data/mods/public/simulation/helpers/Commands.js
| 789| »   »   let·ent·=·pickRandom(cmpRangeManager.GetEntitiesByPlayer(cmd.player).filter(ent·=>·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'ent' is already declared in the upper scope.

binaries/data/mods/public/simulation/helpers/Commands.js
|1274| ····»   »   »   error("[TryConstructWall]·Expected·last·tower·control·group·to·be·available,·none·found·(1st·pass,·iteration·"·+·i·+·")");
|    | [NORMAL] ESLintBear (no-mixed-spaces-and-tabs):
|    | Mixed spaces and tabs.

binaries/data/mods/public/simulation/helpers/Commands.js
|1275| ····»   »   »   break;
|    | [NORMAL] ESLintBear (no-mixed-spaces-and-tabs):
|    | Mixed spaces and tabs.

binaries/data/mods/public/simulation/helpers/Commands.js
|1505| »   »   »   »   var·lastFormationTemplate·=·undefined;
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'lastFormationTemplate' to undefined.

binaries/data/mods/public/simulation/helpers/Commands.js
|1584| »   »   var·closeClusters·=·undefined;
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'closeClusters' to undefined.

binaries/data/mods/public/simulation/helpers/Commands.js
|1601| »   »   for·(let·i·=·0;·i·<·clusters.length;·++i)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/simulation/helpers/Commands.js
|1615| »   »   for·(let·i·=·0;·i·<·matrix.length;·++i)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/simulation/helpers/Commands.js
|  53| var·g_Commands·=·{
|    | [NORMAL] JSHintBear:
|    | 'g_Commands' was used before it was defined.

binaries/data/mods/public/simulation/helpers/Commands.js
| 541| »   »   »   »   ····&&·player·!=·+cmd.owner)
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/helpers/Commands.js
| 729| »   »   »   »   var·cmpGUIInterface·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_GuiInterface);
|    | [NORMAL] JSHintBear:
|    | 'cmpGUIInterface' is already defined.

binaries/data/mods/public/simulation/helpers/Commands.js
| 948| »   »   for·(var·i·=·0;·i·<·length;·++i)
|    | [NORMAL] JSHintBear:
|    | 'i' is already defined.

binaries/data/mods/public/simulation/helpers/Commands.js
| 961| »   »   var·count·=·0;
|    | [NORMAL] JSHintBear:
|    | 'count' is already defined.

binaries/data/mods/public/simulation/helpers/Commands.js
|1108| »   »   var·cmpGuiInterface·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_GuiInterface);
|    | [NORMAL] JSHintBear:
|    | 'cmpGuiInterface' is already defined.

binaries/data/mods/public/simulation/helpers/Commands.js
|1360| »   »   var·piece·=·pieces[j];
|    | [NORMAL] JSHintBear:
|    | 'piece' is already defined.

binaries/data/mods/public/simulation/helpers/Commands.js
|1443| »   »   var·cmpUnitAI·=·Engine.QueryInterface(ent,·IID_UnitAI);
|    | [NORMAL] JSHintBear:
|    | 'cmpUnitAI' is already defined.

binaries/data/mods/public/simulation/helpers/Commands.js
|1482| »   »   »   &&·cmpFormation.GetMemberCount()·==·formation.entities.length)
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/helpers/Commands.js
|1508| »   »   »   »   »   var·cmpUnitAI·=·Engine.QueryInterface(ent,·IID_UnitAI);
|    | [NORMAL] JSHintBear:
|    | 'cmpUnitAI' is already defined.

binaries/data/mods/public/simulation/helpers/Commands.js
|1541| »   »   »   var·cmpFormation·=·Engine.QueryInterface(formationEnt,·IID_Formation);
|    | [NORMAL] JSHintBear:
|    | 'cmpFormation' is already defined.
Executing section cli...

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

In D1843#76399, @bb wrote:

Careful with Extinct volcano OP-ness (on that map autogarrison would give a significant advantage to some civs)

For civilizations that can train in ships?