Page MenuHomeWildfire Games

Fix limit restriction for training units in ProductionQueue
ClosedPublic

Authored by Angen on May 12 2019, 2:00 PM.

Details

Reviewers
wraitii
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP22375: Calculate entity limit counts correctly when SpawnUnits fails in…
Trac Tickets
#4303
Summary

As reported by elexis, when you train units with limitations and they cannot be spawned you can add more to queue than you should be.
This is caused by lowering entity limits before units are able to spawn and because that, the limits are not corrected by globalonownershipchanged message.

Test Plan

0. No patch

  1. Build dock with ptolemies
  2. Train ptolemy IV
  3. Train maximum number of Juggernauts (do not move them to make a place for spawn)
  4. You can add more Juggernauts to training queue
  1. apply patch
  2. repeat steps 1 - 3
  3. you cannot add more Juggernauts to training queue

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 7602
Build 12396: Vulcan BuildJenkins
Build 12395: arc lint + arc unit

Event Timeline

Angen created this revision.May 12 2019, 2:00 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)
| 576| 576| 			var ent = Engine.AddEntity(templateName);
| 577| 577| 			this.entityCache.push(ent);
| 578| 578| 		}
| 579|    |-	}
|    | 579|+	
| 580| 580| 
| 581| 581| 	let cmpAutoGarrison;
| 582| 582| 	if (cmpRallyPoint)
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /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
| 611| 611| 			let pos = cmpFootprint.PickSpawnPoint(ent);
| 612| 612| 			if (pos.y < 0)
| 613| 613| 				break;
| 614|    |-			
|    | 614|+
| 615| 615| 			let cmpNewPosition = Engine.QueryInterface(ent, IID_Position);
| 616| 616| 			cmpNewPosition.JumpTo(pos.x, pos.z);
| 617| 617| 
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /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
| 626| 626| 		// since it will be increased by EntityLimits.OnGlobalOwnershipChanged function,
| 627| 627| 		// i.e. we replace a 'trained' entity to an 'alive' one
| 628| 628| 		// Do not move it before spawn check
| 629|    |-		
|    | 629|+
| 630| 630| 		let cmpTrainingRestrictions = Engine.QueryInterface(ent, IID_TrainingRestrictions);
| 631| 631| 		if (cmpTrainingRestrictions)
| 632| 632| 		{
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /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
| 634| 634| 			let cmpPlayerEntityLimits = QueryOwnerInterface(this.entity, IID_EntityLimits);
| 635| 635| 			cmpPlayerEntityLimits.ChangeCount(unitCategory, -1);
| 636| 636| 		}
| 637|    |-		
|    | 637|+
| 638| 638| 		cmpNewOwnership.SetOwner(cmpOwnership.GetOwner());
| 639| 639| 
| 640| 640| 		let cmpPlayerStatisticsTracker = QueryOwnerInterface(this.entity, IID_StatisticsTracker);
|    | [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
| 650| 650| 	}
| 651| 651| 
| 652| 652| 	if (spawnedEnts.length > 0 && !cmpAutoGarrison)
| 653|    |-	{
|    | 653|+	
| 654| 654| 		// If a rally point is set, walk towards it (in formation) using a suitable command based on where the
| 655| 655| 		// rally point is placed.
| 656| 656| 		if (cmpRallyPoint)
| 663| 663| 					ProcessCommand(cmpOwnership.GetOwner(), com);
| 664| 664| 			}
| 665| 665| 		}
| 666|    |-	}
|    | 666|+	
| 667| 667| 
| 668| 668| 	if (createdEnts.length > 0)
| 669| 669| 		Engine.PostMessage(this.entity, MT_TrainingFinished, {

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 346| »   »   »   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
| 348| »   »   »   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
| 589| »   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
| 591| »   »   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
| 787| »   »   »   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
| 377| »   »   »   var·cmpTrigger·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_Trigger);
|    | [NORMAL] JSHintBear:
|    | 'cmpTrigger' is already defined.

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

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 784| »   »   »   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/1375/display/redirect

elexis removed a reviewer: elexis.May 12 2019, 2:11 PM

I did one review this release and it was one review too much

Angen added a reviewer: Restricted Owners Package.May 12 2019, 2:13 PM

Tested and its works as expected in relation to the ticket.
Move the decrementation entity limit only when the entity has been successfully spawned and before the ownership attribution seems correct.

Stan added a subscriber: Stan.May 12 2019, 3:58 PM
Stan added inline comments.
binaries/data/mods/public/simulation/components/ProductionQueue.js
306

Comments start with Capitals.

607

Remove tab.

Angen updated this revision to Diff 7983.May 12 2019, 4:07 PM

tabs and capital

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)
| 576| 576| 			var ent = Engine.AddEntity(templateName);
| 577| 577| 			this.entityCache.push(ent);
| 578| 578| 		}
| 579|    |-	}
|    | 579|+	
| 580| 580| 
| 581| 581| 	let cmpAutoGarrison;
| 582| 582| 	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
| 649| 649| 	}
| 650| 650| 
| 651| 651| 	if (spawnedEnts.length > 0 && !cmpAutoGarrison)
| 652|    |-	{
|    | 652|+	
| 653| 653| 		// If a rally point is set, walk towards it (in formation) using a suitable command based on where the
| 654| 654| 		// rally point is placed.
| 655| 655| 		if (cmpRallyPoint)
| 662| 662| 					ProcessCommand(cmpOwnership.GetOwner(), com);
| 663| 663| 			}
| 664| 664| 		}
| 665|    |-	}
|    | 665|+	
| 666| 666| 
| 667| 667| 	if (createdEnts.length > 0)
| 668| 668| 		Engine.PostMessage(this.entity, MT_TrainingFinished, {

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 346| »   »   »   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
| 348| »   »   »   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
| 589| »   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
| 591| »   »   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
| 786| »   »   »   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
| 377| »   »   »   var·cmpTrigger·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_Trigger);
|    | [NORMAL] JSHintBear:
|    | 'cmpTrigger' is already defined.

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

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 783| »   »   »   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/1376/display/redirect

wraitii requested changes to this revision.May 25 2019, 10:18 AM
wraitii added a subscriber: wraitii.
wraitii added inline comments.
binaries/data/mods/public/simulation/components/ProductionQueue.js
621

Explain the why not the what here, so replace
Do not move it before spawn check by something like Must come after all spawn checks so we decrement if and only if the unit actually spawns

This revision now requires changes to proceed.May 25 2019, 10:18 AM
Angen updated this revision to Diff 8138.May 25 2019, 8:28 PM

change comment

Stan added a comment.May 25 2019, 8:39 PM

Some comments.

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

Inline merge ifs.

if (template.Promotion && ApplyValueModificationsToTemplate("Promotion/RequiredXp", +template.Promotion.RequiredXp, cmpPlayer.GetPlayerID(), template) == 0)
{
	this.AddBatch(template.Promotion.Entity, type, count, metadata);
	return;
}
296–297

Compute it in the object line 321 directly, this way it's not computed if we early return.

306

I don't think the formulation is right.

570–571

Inline ?

Angen updated this revision to Diff 8140.May 25 2019, 8:47 PM

inline

Angen updated this revision to Diff 8142.May 25 2019, 8:50 PM

missed one

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| 	let 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 (let i = 0; i < count; ++i)
| 576| 576| 			let ent = Engine.AddEntity(templateName);
| 577| 577| 			this.entityCache.push(ent);
| 578| 578| 		}
| 579|    |-	}
|    | 579|+	
| 580| 580| 
| 581| 581| 	let cmpAutoGarrison;
| 582| 582| 	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
| 649| 649| 	}
| 650| 650| 
| 651| 651| 	if (spawnedEnts.length > 0 && !cmpAutoGarrison)
| 652|    |-	{
|    | 652|+	
| 653| 653| 		// If a rally point is set, walk towards it (in formation) using a suitable command based on where the
| 654| 654| 		// rally point is placed.
| 655| 655| 		if (cmpRallyPoint)
| 662| 662| 					ProcessCommand(cmpOwnership.GetOwner(), com);
| 663| 663| 			}
| 664| 664| 		}
| 665|    |-	}
|    | 665|+	
| 666| 666| 
| 667| 667| 	if (createdEnts.length > 0)
| 668| 668| 		Engine.PostMessage(this.entity, MT_TrainingFinished, {

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 786| »   »   »   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
| 772| »   »   »   »   »   var·cmpPlayer·=·QueryOwnerInterface(this.entity);
|    | [NORMAL] JSHintBear:
|    | 'cmpPlayer' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 783| »   »   »   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/1494/display/redirect

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
| 473| 473| {
| 474| 474| 	var out = [];
| 475| 475| 	for (var item of this.queue)
| 476|    |-	{
|    | 476|+	
| 477| 477| 		out.push({
| 478| 478| 			"id": item.id,
| 479| 479| 			"unitTemplate": item.unitTemplate,
| 484| 484| 			"timeRemaining": item.timeRemaining,
| 485| 485| 			"metadata": item.metadata,
| 486| 486| 		});
| 487|    |-	}
|    | 487|+	
| 488| 488| 	return out;
| 489| 489| };
| 490| 490| 
|    | [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
| 642| 642| 	}
| 643| 643| 
| 644| 644| 	if (spawnedEnts.length > 0 && !cmpAutoGarrison)
| 645|    |-	{
|    | 645|+	
| 646| 646| 		// If a rally point is set, walk towards it (in formation) using a suitable command based on where the
| 647| 647| 		// rally point is placed.
| 648| 648| 		if (cmpRallyPoint)
| 655| 655| 					ProcessCommand(cmpOwnership.GetOwner(), com);
| 656| 656| 			}
| 657| 657| 		}
| 658|    |-	}
|    | 658|+	
| 659| 659| 
| 660| 660| 	if (createdEnts.length > 0)
| 661| 661| 		Engine.PostMessage(this.entity, MT_TrainingFinished, {

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 779| »   »   »   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
| 765| »   »   »   »   »   var·cmpPlayer·=·QueryOwnerInterface(this.entity);
|    | [NORMAL] JSHintBear:
|    | 'cmpPlayer' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 776| »   »   »   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/1496/display/redirect

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
| 468| 468| {
| 469| 469| 	var out = [];
| 470| 470| 	for (var item of this.queue)
| 471|    |-	{
|    | 471|+	
| 472| 472| 		out.push({
| 473| 473| 			"id": item.id,
| 474| 474| 			"unitTemplate": item.unitTemplate,
| 479| 479| 			"timeRemaining": item.timeRemaining,
| 480| 480| 			"metadata": item.metadata,
| 481| 481| 		});
| 482|    |-	}
|    | 482|+	
| 483| 483| 	return out;
| 484| 484| };
| 485| 485| 
|    | [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
| 637| 637| 	}
| 638| 638| 
| 639| 639| 	if (spawnedEnts.length > 0 && !cmpAutoGarrison)
| 640|    |-	{
|    | 640|+	
| 641| 641| 		// If a rally point is set, walk towards it (in formation) using a suitable command based on where the
| 642| 642| 		// rally point is placed.
| 643| 643| 		if (cmpRallyPoint)
| 650| 650| 					ProcessCommand(cmpOwnership.GetOwner(), com);
| 651| 651| 			}
| 652| 652| 		}
| 653|    |-	}
|    | 653|+	
| 654| 654| 
| 655| 655| 	if (createdEnts.length > 0)
| 656| 656| 		Engine.PostMessage(this.entity, MT_TrainingFinished, {

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 774| »   »   »   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
| 760| »   »   »   »   »   var·cmpPlayer·=·QueryOwnerInterface(this.entity);
|    | [NORMAL] JSHintBear:
|    | 'cmpPlayer' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 771| »   »   »   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/1498/display/redirect

wraitii accepted this revision.May 26 2019, 2:34 PM

Still needs a bit of text touchup but that can be done before committing.

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

I think == 0 is more understandable here.

This revision is now accepted and ready to land.May 26 2019, 2:34 PM
Angen updated this revision to Diff 8470.Jun 15 2019, 12:05 PM

removed one not needed comment ( I think it is easy to see from if what is going on there)

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
| 468| 468| {
| 469| 469| 	var out = [];
| 470| 470| 	for (var item of this.queue)
| 471|    |-	{
|    | 471|+	
| 472| 472| 		out.push({
| 473| 473| 			"id": item.id,
| 474| 474| 			"unitTemplate": item.unitTemplate,
| 479| 479| 			"timeRemaining": item.timeRemaining,
| 480| 480| 			"metadata": item.metadata,
| 481| 481| 		});
| 482|    |-	}
|    | 482|+	
| 483| 483| 	return out;
| 484| 484| };
| 485| 485| 
|    | [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
| 637| 637| 	}
| 638| 638| 
| 639| 639| 	if (spawnedEnts.length > 0 && !cmpAutoGarrison)
| 640|    |-	{
|    | 640|+	
| 641| 641| 		// If a rally point is set, walk towards it (in formation) using a suitable command based on where the
| 642| 642| 		// rally point is placed.
| 643| 643| 		if (cmpRallyPoint)
| 650| 650| 					ProcessCommand(cmpOwnership.GetOwner(), com);
| 651| 651| 			}
| 652| 652| 		}
| 653|    |-	}
|    | 653|+	
| 654| 654| 
| 655| 655| 	if (createdEnts.length > 0)
| 656| 656| 		Engine.PostMessage(this.entity, MT_TrainingFinished, {

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 774| »   »   »   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
| 760| »   »   »   »   »   var·cmpPlayer·=·QueryOwnerInterface(this.entity);
|    | [NORMAL] JSHintBear:
|    | 'cmpPlayer' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 771| »   »   »   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/1715/display/redirect

Angen updated this revision to Diff 8471.Jun 15 2019, 12:07 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
| 468| 468| {
| 469| 469| 	var out = [];
| 470| 470| 	for (var item of this.queue)
| 471|    |-	{
|    | 471|+	
| 472| 472| 		out.push({
| 473| 473| 			"id": item.id,
| 474| 474| 			"unitTemplate": item.unitTemplate,
| 479| 479| 			"timeRemaining": item.timeRemaining,
| 480| 480| 			"metadata": item.metadata,
| 481| 481| 		});
| 482|    |-	}
|    | 482|+	
| 483| 483| 	return out;
| 484| 484| };
| 485| 485| 
|    | [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
| 637| 637| 	}
| 638| 638| 
| 639| 639| 	if (spawnedEnts.length > 0 && !cmpAutoGarrison)
| 640|    |-	{
|    | 640|+	
| 641| 641| 		// If a rally point is set, walk towards it (in formation) using a suitable command based on where the
| 642| 642| 		// rally point is placed.
| 643| 643| 		if (cmpRallyPoint)
| 650| 650| 					ProcessCommand(cmpOwnership.GetOwner(), com);
| 651| 651| 			}
| 652| 652| 		}
| 653|    |-	}
|    | 653|+	
| 654| 654| 
| 655| 655| 	if (createdEnts.length > 0)
| 656| 656| 		Engine.PostMessage(this.entity, MT_TrainingFinished, {

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 774| »   »   »   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
| 760| »   »   »   »   »   var·cmpPlayer·=·QueryOwnerInterface(this.entity);
|    | [NORMAL] JSHintBear:
|    | 'cmpPlayer' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 771| »   »   »   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/1716/display/redirect

Angen updated this revision to Diff 8472.Jun 15 2019, 12:52 PM

one more comment

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
| 468| 468| {
| 469| 469| 	var out = [];
| 470| 470| 	for (var item of this.queue)
| 471|    |-	{
|    | 471|+	
| 472| 472| 		out.push({
| 473| 473| 			"id": item.id,
| 474| 474| 			"unitTemplate": item.unitTemplate,
| 479| 479| 			"timeRemaining": item.timeRemaining,
| 480| 480| 			"metadata": item.metadata,
| 481| 481| 		});
| 482|    |-	}
|    | 482|+	
| 483| 483| 	return out;
| 484| 484| };
| 485| 485| 
|    | [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
| 637| 637| 	}
| 638| 638| 
| 639| 639| 	if (spawnedEnts.length > 0 && !cmpAutoGarrison)
| 640|    |-	{
|    | 640|+	
| 641| 641| 		// If a rally point is set, walk towards it (in formation) using a suitable command based on where the
| 642| 642| 		// rally point is placed.
| 643| 643| 		if (cmpRallyPoint)
| 650| 650| 					ProcessCommand(cmpOwnership.GetOwner(), com);
| 651| 651| 			}
| 652| 652| 		}
| 653|    |-	}
|    | 653|+	
| 654| 654| 
| 655| 655| 	if (createdEnts.length > 0)
| 656| 656| 		Engine.PostMessage(this.entity, MT_TrainingFinished, {

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 774| »   »   »   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
| 760| »   »   »   »   »   var·cmpPlayer·=·QueryOwnerInterface(this.entity);
|    | [NORMAL] JSHintBear:
|    | 'cmpPlayer' is already defined.

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 771| »   »   »   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/1717/display/redirect

wraitii added inline comments.Jun 15 2019, 12:56 PM
binaries/data/mods/public/simulation/components/ProductionQueue.js
307

Why add this here?

Angen updated this revision to Diff 8484.Jun 15 2019, 6:46 PM

final changes

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
| 465| 465| {
| 466| 466| 	var out = [];
| 467| 467| 	for (var item of this.queue)
| 468|    |-	{
|    | 468|+	
| 469| 469| 		out.push({
| 470| 470| 			"id": item.id,
| 471| 471| 			"unitTemplate": item.unitTemplate,
| 476| 476| 			"timeRemaining": item.timeRemaining,
| 477| 477| 			"metadata": item.metadata,
| 478| 478| 		});
| 479|    |-	}
|    | 479|+	
| 480| 480| 	return out;
| 481| 481| };
| 482| 482| 
|    | [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
| 634| 634| 	}
| 635| 635| 
| 636| 636| 	if (spawnedEnts.length > 0 && !cmpAutoGarrison)
| 637|    |-	{
|    | 637|+	
| 638| 638| 		// If a rally point is set, walk towards it (in formation) using a suitable command based on where the
| 639| 639| 		// rally point is placed.
| 640| 640| 		if (cmpRallyPoint)
| 647| 647| 					ProcessCommand(cmpOwnership.GetOwner(), com);
| 648| 648| 			}
| 649| 649| 		}
| 650|    |-	}
|    | 650|+	
| 651| 651| 
| 652| 652| 	if (createdEnts.length > 0)
| 653| 653| 		Engine.PostMessage(this.entity, MT_TrainingFinished, {

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

binaries/data/mods/public/simulation/components/ProductionQueue.js
| 768| »   »   »   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/1728/display/redirect

This revision was automatically updated to reflect the committed changes.