Clean up AddBatch (AddItem) in ProductionQueue.

Removes indentation.
Adds return value.
Cache owner instead of querying often.

This function can be cleaned even more later.

Differential revision: D3670
Comments by: @wraitii
This was SVN commit r25063.

binaries/data/mods/public/simulation/components/ProductionQueue.js

@@ -317,15 +317,22 @@ ProductionQueue.prototype.IsTechnologyResearchedOrInProgress = function(tech)
 
 /*
  * Adds a new batch of identical units to train or a technology to research to the production queue.
+ * @param {string} templateName - The template to start production on.
+ * @param {string} type - The type of production (i.e. "unit" or "technology").
+ * @param {number} count - The amount of units to be produced. Ignored for a tech.
+ * @param {any} metadata - Optionally any metadata to be attached to the item.
+ *
+ * @return {boolean} - Whether the addition of the item has succeeded.
  */
-ProductionQueue.prototype.AddBatch = function(templateName, type, count, metadata)
+ProductionQueue.prototype.AddItem = function(templateName, type, count, metadata)
 {
 	// TODO: there should probably be a limit on the number of queued batches.
 	// TODO: there should be a way for the GUI to determine whether it's going
 	// to be possible to add a batch (based on resource costs and length limits).
 	let cmpPlayer = QueryOwnerInterface(this.entity);
 	if (!cmpPlayer)
-		return;
+		return false;
+	let player = cmpPlayer.GetPlayerID();
 
 	if (!this.queue.length)
 	{
@@ -334,177 +341,165 @@ ProductionQueue.prototype.AddBatch = function(templateName, type, count, metadat
 		{
 			let cmpGUIInterface = Engine.QueryInterface(SYSTEM_ENTITY, IID_GuiInterface);
 			cmpGUIInterface.PushNotification({
-				"players": [cmpPlayer.GetPlayerID()],
+				"players": [player],
 				"message": markForTranslation("Entity is being upgraded. Cannot start production."),
 				"translateMessage": true
 			});
-			return;
+			return false;
 		}
 	}
-
-	if (this.queue.length < this.MaxQueueSize)
-	{
-		if (type == "unit")
-		{
-			if (!Number.isInteger(count) || count <= 0)
-			{
-				error("Invalid batch count " + count);
-				return;
-			}
-
-			// Find the template data so we can determine the build costs.
-			let cmpTemplateManager = Engine.QueryInterface(SYSTEM_ENTITY, IID_TemplateManager);
-			let template = cmpTemplateManager.GetTemplate(templateName);
-			if (!template)
-				return;
-			if (template.Promotion &&
-			  !ApplyValueModificationsToTemplate(
-			    "Promotion/RequiredXp",
-			    +template.Promotion.RequiredXp,
-			    cmpPlayer.GetPlayerID(),
-			    template))
-			{
-				this.AddBatch(template.Promotion.Entity, type, count, metadata);
-				return;
-			}
-
-			// We need the costs after tech modifications.
-			// Obviously we don't have the entities yet, so we must use template data.
-			let costs = {};
-			let totalCosts = {};
-
-			for (let res in template.Cost.Resources)
-			{
-				costs[res] = ApplyValueModificationsToTemplate(
-				    "Cost/Resources/" + res,
-				    +template.Cost.Resources[res],
-				    cmpPlayer.GetPlayerID(),
-				    template);
-
-				totalCosts[res] = Math.floor(count * costs[res]);
-			}
-
-			// TrySubtractResources should report error to player (they ran out of resources).
-			if (!cmpPlayer.TrySubtractResources(totalCosts))
-				return;
-
-			// Update entity count in the EntityLimits component.
-			if (template.TrainingRestrictions)
-			{
-				let unitCategory = template.TrainingRestrictions.Category;
-				let cmpPlayerEntityLimits = QueryOwnerInterface(this.entity, IID_EntityLimits);
-				if (cmpPlayerEntityLimits)
-					cmpPlayerEntityLimits.ChangeCount(unitCategory, count);
-				if (template.TrainingRestrictions.MatchLimit)
-					cmpPlayerEntityLimits.ChangeMatchCount(templateName, count);
-			}
-
-			let buildTime = ApplyValueModificationsToTemplate(
-			    "Cost/BuildTime",
-			    +template.Cost.BuildTime,
-			    cmpPlayer.GetPlayerID(),
-			    template);
-			// Apply a time discount to larger batches.
-			let time = this.GetBatchTime(count) * buildTime * 1000;
-			this.queue.push({
-				"id": this.nextID++,
-				"player": cmpPlayer.GetPlayerID(),
-				"unitTemplate": templateName,
-				"count": count,
-				"metadata": metadata,
-				"resources": costs,
-				"population": ApplyValueModificationsToTemplate(
-				    "Cost/Population",
-				    +template.Cost.Population,
-				    cmpPlayer.GetPlayerID(),
-				    template),
-				"productionStarted": false,
-				"timeTotal": time,
-				"timeRemaining": time
-			});
-
-			// Call the related trigger event.
-			let cmpTrigger = Engine.QueryInterface(SYSTEM_ENTITY, IID_Trigger);
-			cmpTrigger.CallEvent("TrainingQueued", {
-			    "playerid": cmpPlayer.GetPlayerID(),
-			    "unitTemplate": templateName,
-			    "count": count,
-			    "metadata": metadata,
-			    "trainerEntity": this.entity
-			});
-		}
-		else if (type == "technology")
-		{
-			if (!TechnologyTemplates.Has(templateName))
-				return;
-
-			if (!this.GetTechnologiesList().some(tech =>
-				tech &&
-					(tech == templateName ||
-						tech.pair &&
-						(tech.top == templateName || tech.bottom == templateName))))
-			{
-				error("This entity cannot research " + templateName);
-				return;
-			}
-
-			let template = TechnologyTemplates.Get(templateName);
-			let techCostMultiplier = this.GetTechCostMultiplier();
-
-			let cost = {};
-			if (template.cost)
-				for (let res in template.cost)
-					cost[res] = Math.floor((techCostMultiplier[res] || 1) * template.cost[res]);
-
-			// TrySubtractResources should report error to player (they ran out of resources).
-			if (!cmpPlayer.TrySubtractResources(cost))
-				return;
-
-			// Tell the technology manager that we have started researching this
-			// such that people can't research the same thing twice.
-			let cmpTechnologyManager = QueryOwnerInterface(this.entity, IID_TechnologyManager);
-			cmpTechnologyManager.QueuedResearch(templateName, this.entity);
-
-			let time = techCostMultiplier.time * (template.researchTime || 0) * 1000;
-			this.queue.push({
-			    "id": this.nextID++,
-			    "player": cmpPlayer.GetPlayerID(),
-			    "count": 1,
-			    "technologyTemplate": templateName,
-			    "resources": cost,
-			    "productionStarted": false,
-			    "timeTotal": time,
-			    "timeRemaining": time
-			});
-
-			// Call the related trigger event.
-			let cmpTrigger = Engine.QueryInterface(SYSTEM_ENTITY, IID_Trigger);
-			cmpTrigger.CallEvent("ResearchQueued", {
-			    "playerid": cmpPlayer.GetPlayerID(),
-			    "technologyTemplate": templateName,
-			    "researcherEntity": this.entity
-			});
-		}
-		else
-		{
-			warn("Tried to add invalid item of type \"" + type + "\" and template \"" + templateName + "\" to a production queue");
-			return;
-		}
-
-		Engine.PostMessage(this.entity, MT_ProductionQueueChanged, null);
-
-		if (!this.timer)
-			this.StartTimer();
-	}
-	else
+	else if (this.queue.length >= this.MaxQueueSize)
 	{
 		let cmpGUIInterface = Engine.QueryInterface(SYSTEM_ENTITY, IID_GuiInterface);
 		cmpGUIInterface.PushNotification({
-		    "players": [cmpPlayer.GetPlayerID()],
+		    "players": [player],
 		    "message": markForTranslation("The production queue is full."),
 		    "translateMessage": true,
 		});
+		return false;
 	}
+
+	// ToDo: Lots of duplication here, much can probably be combined,
+	// but requires some more refactoring.
+	if (type == "unit")
+	{
+		if (!Number.isInteger(count) || count <= 0)
+		{
+			error("Invalid batch count " + count);
+			return false;
+		}
+
+		// Find the template data so we can determine the build costs.
+		let cmpTemplateManager = Engine.QueryInterface(SYSTEM_ENTITY, IID_TemplateManager);
+		let template = cmpTemplateManager.GetTemplate(this.GetUpgradedTemplate(templateName));
+		if (!template)
+			return false;
+
+		let costs = {};
+		let totalCosts = {};
+
+		for (let res in template.Cost.Resources)
+		{
+			costs[res] = ApplyValueModificationsToTemplate(
+			    "Cost/Resources/" + res,
+			    +template.Cost.Resources[res],
+			    player,
+			    template);
+
+			totalCosts[res] = Math.floor(count * costs[res]);
+		}
+
+		// TrySubtractResources should report error to player (they ran out of resources).
+		if (!cmpPlayer.TrySubtractResources(totalCosts))
+			return false;
+
+		if (template.TrainingRestrictions)
+		{
+			let unitCategory = template.TrainingRestrictions.Category;
+			let cmpPlayerEntityLimits = QueryPlayerIDInterface(player, IID_EntityLimits);
+			if (cmpPlayerEntityLimits)
+			{
+				cmpPlayerEntityLimits.ChangeCount(unitCategory, count);
+				if (template.TrainingRestrictions.MatchLimit)
+					cmpPlayerEntityLimits.ChangeMatchCount(templateName, count);
+			}
+		}
+
+		let buildTime = ApplyValueModificationsToTemplate(
+		    "Cost/BuildTime",
+		    +template.Cost.BuildTime,
+		    player,
+		    template);
+		let time = this.GetBatchTime(count) * buildTime * 1000;
+		this.queue.push({
+			"id": this.nextID++,
+			"player": player,
+			"unitTemplate": templateName,
+			"count": count,
+			"metadata": metadata,
+			"resources": costs,
+			"population": ApplyValueModificationsToTemplate(
+			    "Cost/Population",
+			    +template.Cost.Population,
+			    player,
+			    template),
+			"productionStarted": false,
+			"timeTotal": time,
+			"timeRemaining": time
+		});
+
+		let cmpTrigger = Engine.QueryInterface(SYSTEM_ENTITY, IID_Trigger);
+		cmpTrigger.CallEvent("TrainingQueued", {
+		    "playerid": player,
+		    "unitTemplate": templateName,
+		    "count": count,
+		    "metadata": metadata,
+		    "trainerEntity": this.entity
+		});
+	}
+	else if (type == "technology")
+	{
+		if (!TechnologyTemplates.Has(templateName))
+			return false;
+
+		if (!this.GetTechnologiesList().some(tech =>
+			tech &&
+				(tech == templateName ||
+					tech.pair &&
+					(tech.top == templateName || tech.bottom == templateName))))
+		{
+			error("This entity cannot research " + templateName);
+			return false;
+		}
+
+		let template = TechnologyTemplates.Get(templateName);
+		let techCostMultiplier = this.GetTechCostMultiplier();
+
+		let cost = {};
+		if (template.cost)
+			for (let res in template.cost)
+				cost[res] = Math.floor((techCostMultiplier[res] || 1) * template.cost[res]);
+
+		// TrySubtractResources should report error to player (they ran out of resources).
+		if (!cmpPlayer.TrySubtractResources(cost))
+			return false;
+
+		// Tell the technology manager that we have started researching this
+		// such that players can't research the same thing twice.
+		let cmpTechnologyManager = QueryOwnerInterface(this.entity, IID_TechnologyManager);
+		cmpTechnologyManager.QueuedResearch(templateName, this.entity);
+
+		let time = techCostMultiplier.time * (template.researchTime || 0) * 1000;
+		this.queue.push({
+		    "id": this.nextID++,
+		    "player": player,
+		    "count": 1,
+		    "technologyTemplate": templateName,
+		    "resources": cost,
+		    "productionStarted": false,
+		    "timeTotal": time,
+		    "timeRemaining": time
+		});
+
+		// Call the related trigger event.
+		let cmpTrigger = Engine.QueryInterface(SYSTEM_ENTITY, IID_Trigger);
+		cmpTrigger.CallEvent("ResearchQueued", {
+		    "playerid": player,
+		    "technologyTemplate": templateName,
+		    "researcherEntity": this.entity
+		});
+	}
+	else
+	{
+		warn("Tried to add invalid item of type \"" + type + "\" and template \"" + templateName + "\" to a production queue");
+		return false;
+	}
+
+	Engine.PostMessage(this.entity, MT_ProductionQueueChanged, null);
+
+	if (!this.timer)
+		this.StartTimer();
+	return true;
 };
 
 /*

binaries/data/mods/public/simulation/components/tests/test_ProductionQueue.js

@@ -271,7 +271,7 @@ function regression_test_d1879()
 		"PickSpawnPoint": () => ({ "x": 0, "y": 1, "z": 0 })
 	});
 
-	cmpProdQueue.AddBatch("some_template", "unit", 3);
+	cmpProdQueue.AddItem("some_template", "unit", 3);
 
 	TS_ASSERT_EQUALS(cmpEntLimits.GetCounts().some_limit, 3);
 	TS_ASSERT_EQUALS(cmpEntLimits.GetMatchCounts().some_template, 3);
@@ -290,7 +290,7 @@ function regression_test_d1879()
 		"IsUpgrading": () => false
 	});
 
-	cmpProdQueue.AddBatch("some_template", "unit", 3);
+	cmpProdQueue.AddItem("some_template", "unit", 3);
 	cmpProdQueue.ProgressTimeout(null, 0);
 
 	TS_ASSERT_EQUALS(cmpProdQueue.GetQueue().length, 1);
@@ -373,14 +373,14 @@ function test_batch_adding()
 		"IsUpgrading": () => true
 	});
 
-	cmpProdQueue.AddBatch("some_template", "unit", 3);
+	cmpProdQueue.AddItem("some_template", "unit", 3);
 	TS_ASSERT_EQUALS(cmpProdQueue.GetQueue().length, 0);
 
 	AddMock(testEntity, IID_Upgrade, {
 		"IsUpgrading": () => false
 	});
 
-	cmpProdQueue.AddBatch("some_template", "unit", 3);
+	cmpProdQueue.AddItem("some_template", "unit", 3);
 	TS_ASSERT_EQUALS(cmpProdQueue.GetQueue().length, 1);
 }
 
@@ -449,12 +449,12 @@ function test_batch_removal()
 		"BatchTimeModifier": 1
 	});
 
-	cmpProdQueue.AddBatch("some_template", "unit", 3);
+	cmpProdQueue.AddItem("some_template", "unit", 3);
 	TS_ASSERT_EQUALS(cmpProdQueue.GetQueue().length, 1);
 	cmpTimer.OnUpdate({ "turnLength": 1 });
 	TS_ASSERT_EQUALS(cmpPlayerBlockSpy._called, 1);
 
-	cmpProdQueue.AddBatch("some_template", "unit", 2);
+	cmpProdQueue.AddItem("some_template", "unit", 2);
 	TS_ASSERT_EQUALS(cmpProdQueue.GetQueue().length, 2);
 
 	cmpProdQueue.RemoveItem(1);
@@ -466,8 +466,8 @@ function test_batch_removal()
 	cmpTimer.OnUpdate({ "turnLength": 1 });
 	TS_ASSERT_EQUALS(cmpPlayerUnblockSpy._called, 2);
 
-	cmpProdQueue.AddBatch("some_template", "unit", 3);
-	cmpProdQueue.AddBatch("some_template", "unit", 3);
+	cmpProdQueue.AddItem("some_template", "unit", 3);
+	cmpProdQueue.AddItem("some_template", "unit", 3);
 	cmpPlayer.TryReservePopulationSlots = () => false;
 	cmpProdQueue.RemoveItem(3);
 	TS_ASSERT_EQUALS(cmpPlayerUnblockSpy._called, 3);
@@ -521,8 +521,8 @@ function test_token_changes()
 		cmpProductionQueue.GetTechnologiesList(), ["a", "b_generic", "c_generic"]
 	);
 	// Add a unit of each type to our queue, validate.
-	cmpProductionQueue.AddBatch("units/test/a", "unit", 1, {});
-	cmpProductionQueue.AddBatch("units/test/b", "unit", 1, {});
+	cmpProductionQueue.AddItem("units/test/a", "unit", 1, {});
+	cmpProductionQueue.AddItem("units/test/b", "unit", 1, {});
 	TS_ASSERT_EQUALS(cmpProductionQueue.GetQueue()[0].unitTemplate, "units/test/a");
 	TS_ASSERT_EQUALS(cmpProductionQueue.GetQueue()[1].unitTemplate, "units/test/b");
 

binaries/data/mods/public/simulation/helpers/Commands.js

@@ -352,9 +352,9 @@ var g_Commands = {
 			}
 			if (queue && queue.GetEntitiesList().indexOf(cmd.template) != -1)
 				if ("metadata" in cmd)
-					queue.AddBatch(cmd.template, "unit", +cmd.count, cmd.metadata);
+					queue.AddItem(cmd.template, "unit", +cmd.count, cmd.metadata);
 				else
-					queue.AddBatch(cmd.template, "unit", +cmd.count);
+					queue.AddItem(cmd.template, "unit", +cmd.count);
 		}
 	},
 
@@ -370,7 +370,7 @@ var g_Commands = {
 
 		var queue = Engine.QueryInterface(cmd.entity, IID_ProductionQueue);
 		if (queue)
-			queue.AddBatch(cmd.template, "technology");
+			queue.AddItem(cmd.template, "technology");
 	},
 
 	"stop-production": function(player, cmd, data)