Clean up RemoveBatch -> RemoveItem code in ProductionQueue.

- Rename `RemoveBatch` to `RemoveItem`.
- Refactor that function. (Don't clear the cached entities when *any*
item is removed.)

Fixes the bug that the training is still blocked when a tech is the next
in the queue and the blocking item is removed.

Based on a patch by: @Polakrity
Differential revision: D1843
Comments by: @Angen, @bb
This was SVN commit r25040.

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

@@ -117,7 +117,7 @@ ProductionQueue.prototype.CalculateEntitiesMap = function()
 		let template = this.entitiesMap.get(token);
 		for (let item of queue)
 			if (item.unitTemplate && item.unitTemplate === template)
-				this.RemoveBatch(item.id);
+				this.RemoveItem(item.id);
 	};
 	let updateAllQueuedTemplate = (token, updateTo) => {
 		let template = this.entitiesMap.get(token);
@@ -508,74 +508,74 @@ ProductionQueue.prototype.AddBatch = function(templateName, type, count, metadat
 };
 
 /*
- * Removes an existing batch of units from the production queue.
+ * Removes an item from the queue.
  * Refunds resource costs and population reservations.
+ * item.player is used as this.entity's owner may have changed.
  */
-ProductionQueue.prototype.RemoveBatch = function(id)
+ProductionQueue.prototype.RemoveItem = function(id)
 {
-	// Destroy any cached entities (those which didn't spawn for some reason).
-	for (let ent of this.entityCache)
-		Engine.DestroyEntity(ent);
-
-	this.entityCache = [];
-
-	for (let i = 0; i < this.queue.length; ++i)
-	{
-		// Find the item to remove.
-		let item = this.queue[i];
-		if (item.id != id)
-			continue;
-
-		// Update entity count in the EntityLimits component.
-		if (item.unitTemplate)
-		{
-			let cmpTemplateManager = Engine.QueryInterface(SYSTEM_ENTITY, IID_TemplateManager);
-			let template = cmpTemplateManager.GetTemplate(item.unitTemplate);
-			if (template.TrainingRestrictions)
-			{
-				let cmpPlayerEntityLimits = QueryPlayerIDInterface(item.player, IID_EntityLimits);
-				if (cmpPlayerEntityLimits)
-					cmpPlayerEntityLimits.ChangeCount(template.TrainingRestrictions.Category, -item.count);
-				if (template.TrainingRestrictions.MatchLimit)
-					cmpPlayerEntityLimits.ChangeMatchCount(item.unitTemplate, -item.count);
-			}
-		}
-
-		// Refund the resource cost for this batch.
-		let totalCosts = {};
-		let cmpStatisticsTracker = QueryPlayerIDInterface(item.player, IID_StatisticsTracker);
-		for (let r in item.resources)
-		{
-			totalCosts[r] = Math.floor(item.count * item.resources[r]);
-			if (cmpStatisticsTracker)
-				cmpStatisticsTracker.IncreaseResourceUsedCounter(r, -totalCosts[r]);
-		}
-
-		let cmpPlayer = QueryPlayerIDInterface(item.player);
-		if (cmpPlayer)
-		{
-			cmpPlayer.AddResources(totalCosts);
-
-			// Remove reserved population slots if necessary.
-			if (item.productionStarted && item.unitTemplate)
-				cmpPlayer.UnReservePopulationSlots(item.population * item.count);
-		}
-
-		// Mark the research as stopped if we cancel it.
-		if (item.technologyTemplate)
-		{
-			// item.player is used as this.entity's owner may be invalid (deletion, etc.)
-			let cmpTechnologyManager = QueryPlayerIDInterface(item.player, IID_TechnologyManager);
-			if (cmpTechnologyManager)
-				cmpTechnologyManager.StoppedResearch(item.technologyTemplate, true);
-			this.SetAnimation("idle");
-		}
-
-		this.queue.splice(i, 1);
-		Engine.PostMessage(this.entity, MT_ProductionQueueChanged, null);
-
+	let itemIndex = this.queue.findIndex(item => item.id == id);
+	if (itemIndex == -1)
 		return;
+
+	// Destroy any cached entities (those which didn't spawn for some reason).
+	if (itemIndex == 0 && this.entityCache.length)
+	{
+		for (let ent of this.entityCache)
+			Engine.DestroyEntity(ent);
+
+		this.entityCache = [];
 	}
+
+	let item = this.queue[itemIndex];
+
+	// Update entity count in the EntityLimits component.
+	if (item.unitTemplate)
+	{
+		let cmpTemplateManager = Engine.QueryInterface(SYSTEM_ENTITY, IID_TemplateManager);
+		let template = cmpTemplateManager.GetTemplate(item.unitTemplate);
+		if (template.TrainingRestrictions)
+		{
+			let cmpPlayerEntityLimits = QueryPlayerIDInterface(item.player, IID_EntityLimits);
+			if (cmpPlayerEntityLimits)
+				cmpPlayerEntityLimits.ChangeCount(template.TrainingRestrictions.Category, -item.count);
+			if (template.TrainingRestrictions.MatchLimit)
+				cmpPlayerEntityLimits.ChangeMatchCount(item.unitTemplate, -item.count);
+		}
+	}
+
+	let totalCosts = {};
+	let cmpStatisticsTracker = QueryPlayerIDInterface(item.player, IID_StatisticsTracker);
+	for (let resource in item.resources)
+	{
+		totalCosts[resource] = Math.floor(item.count * item.resources[resource]);
+		if (cmpStatisticsTracker)
+			cmpStatisticsTracker.IncreaseResourceUsedCounter(resource, -totalCosts[resource]);
+	}
+
+	let cmpPlayer = QueryPlayerIDInterface(item.player);
+	if (cmpPlayer)
+	{
+		cmpPlayer.AddResources(totalCosts);
+		if (item.productionStarted && item.unitTemplate)
+			cmpPlayer.UnReservePopulationSlots(item.population * item.count);
+		if (itemIndex == 0)
+			cmpPlayer.UnBlockTraining();
+	}
+
+	if (item.technologyTemplate)
+	{
+		let cmpTechnologyManager = QueryPlayerIDInterface(item.player, IID_TechnologyManager);
+		if (cmpTechnologyManager)
+			cmpTechnologyManager.StoppedResearch(item.technologyTemplate, true);
+		this.SetAnimation("idle");
+	}
+
+	this.queue.splice(itemIndex, 1);
+	Engine.PostMessage(this.entity, MT_ProductionQueueChanged, null);
+
+	if (!this.queue.length)
+		this.StopTimer();
 };
 
 ProductionQueue.prototype.SetAnimation = function(name)
@@ -612,7 +612,7 @@ ProductionQueue.prototype.ResetQueue = function()
 	// buildings' queues when they're about to be destroyed or captured.)
 
 	while (this.queue.length)
-		this.RemoveBatch(this.queue[0].id);
+		this.RemoveItem(this.queue[0].id);
 };
 
 /*
@@ -629,13 +629,6 @@ ProductionQueue.prototype.GetBatchTime = function(batchSize)
 
 ProductionQueue.prototype.OnOwnershipChanged = function(msg)
 {
-	if (msg.from != INVALID_PLAYER)
-	{
-		// Unset flag that previous owner's training may be blocked.
-		let cmpPlayer = QueryPlayerIDInterface(msg.from);
-		if (cmpPlayer && this.queue.length)
-			cmpPlayer.UnBlockTraining();
-	}
 	if (msg.to != INVALID_PLAYER)
 		this.CalculateEntitiesMap();
 
@@ -656,7 +649,6 @@ ProductionQueue.prototype.OnDestroy = function()
 {
 	// Reset the queue to refund any resources.
 	this.ResetQueue();
-	this.StopTimer();
 };
 
 /*
@@ -897,17 +889,11 @@ ProductionQueue.prototype.ProgressTimeout = function(data, lateness)
 
 		time -= item.timeRemaining;
 		this.queue.shift();
-		Engine.PostMessage(this.entity, MT_ProductionQueueChanged, {});
+		Engine.PostMessage(this.entity, MT_ProductionQueueChanged, null);
 	}
 
 	if (!this.queue.length)
-	{
 		this.StopTimer();
-
-		// Unset flag that training is blocked.
-		// (This might happen when the player unqueues all batches.)
-		cmpPlayer.UnBlockTraining();
-	}
 };
 
 ProductionQueue.prototype.PauseProduction = function()

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

@@ -298,7 +298,7 @@ function regression_test_d1879()
 	TS_ASSERT_EQUALS(cmpEntLimits.GetMatchCounts().some_template, 6);
 
 	// Check that when the batch is removed the counts are subtracted again.
-	cmpProdQueue.RemoveBatch(cmpProdQueue.GetQueue()[0].id);
+	cmpProdQueue.RemoveItem(cmpProdQueue.GetQueue()[0].id);
 	TS_ASSERT_EQUALS(cmpEntLimits.GetCounts().some_limit, 3);
 	TS_ASSERT_EQUALS(cmpEntLimits.GetMatchCounts().some_template, 3);
 }
@@ -406,7 +406,7 @@ function test_batch_removal()
 		"CallEvent": () => {}
 	});
 
-	ConstructComponent(SYSTEM_ENTITY, "Timer", null);
+	let cmpTimer = ConstructComponent(SYSTEM_ENTITY, "Timer", null);
 
 	AddMock(SYSTEM_ENTITY, IID_TemplateManager, {
 		"TemplateExists": () => true,
@@ -451,28 +451,28 @@ function test_batch_removal()
 
 	cmpProdQueue.AddBatch("some_template", "unit", 3);
 	TS_ASSERT_EQUALS(cmpProdQueue.GetQueue().length, 1);
-	cmpProdQueue.ProgressTimeout(null, 0);
+	cmpTimer.OnUpdate({ "turnLength": 1 });
 	TS_ASSERT_EQUALS(cmpPlayerBlockSpy._called, 1);
 
 	cmpProdQueue.AddBatch("some_template", "unit", 2);
 	TS_ASSERT_EQUALS(cmpProdQueue.GetQueue().length, 2);
 
-	cmpProdQueue.RemoveBatch(1);
+	cmpProdQueue.RemoveItem(1);
 	TS_ASSERT_EQUALS(cmpProdQueue.GetQueue().length, 1);
-	TS_ASSERT_EQUALS(cmpPlayerUnblockSpy._called, 0);
-
-	cmpProdQueue.RemoveBatch(2);
-	TS_ASSERT_EQUALS(cmpProdQueue.GetQueue().length, 0);
-	cmpProdQueue.ProgressTimeout(null, 0);
 	TS_ASSERT_EQUALS(cmpPlayerUnblockSpy._called, 1);
 
+	cmpProdQueue.RemoveItem(2);
+	TS_ASSERT_EQUALS(cmpProdQueue.GetQueue().length, 0);
+	cmpTimer.OnUpdate({ "turnLength": 1 });
+	TS_ASSERT_EQUALS(cmpPlayerUnblockSpy._called, 2);
+
 	cmpProdQueue.AddBatch("some_template", "unit", 3);
 	cmpProdQueue.AddBatch("some_template", "unit", 3);
 	cmpPlayer.TryReservePopulationSlots = () => false;
-	cmpProdQueue.RemoveBatch(3);
-	cmpProdQueue.ProgressTimeout(null, 0);
-	TS_ASSERT_EQUALS(cmpPlayerUnblockSpy._called, 2);
-
+	cmpProdQueue.RemoveItem(3);
+	TS_ASSERT_EQUALS(cmpPlayerUnblockSpy._called, 3);
+	cmpTimer.OnUpdate({ "turnLength": 1 });
+	TS_ASSERT_EQUALS(cmpPlayerUnblockSpy._called, 4);
 }
 
 function test_token_changes()

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

@@ -375,9 +375,9 @@ var g_Commands = {
 
 	"stop-production": function(player, cmd, data)
 	{
-		var queue = Engine.QueryInterface(cmd.entity, IID_ProductionQueue);
-		if (queue)
-			queue.RemoveBatch(cmd.id);
+		let cmpProductionQueue = Engine.QueryInterface(cmd.entity, IID_ProductionQueue);
+		if (cmpProductionQueue)
+			cmpProductionQueue.RemoveItem(cmd.id);
 	},
 
 	"construct": function(player, cmd, data)