Page MenuHomeWildfire Games

Improve support of classes of bonuses.
ClosedPublic

Authored by Freagarach on Jul 23 2019, 10:26 PM.

Details

Reviewers
wraitii
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP22769: Make damage bonuses, attack preferred classes and garrisonHolder ejectable…
Summary

This makes use of the globalscript MatchesClassList from Templates.js, thus allowing operators to be used in the bonus classes list (i.e. +, ! and " " for AND, NOT and OR, respectively).

Test Plan

Verify that it works and is indeed a wanted feature.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Freagarach created this revision.Jul 23 2019, 10:26 PM

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (spaced-comment):
|    | Expected space or tab after '//' in comment.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
| 535| 535| 
| 536| 536| 		let horizSpeed = +this.template[type].Projectile.Speed;
| 537| 537| 		let gravity = +this.template[type].Projectile.Gravity;
| 538|    |-		//horizSpeed /= 2; gravity /= 2; // slow it down for testing
|    | 538|+		// horizSpeed /= 2; gravity /= 2; // slow it down for testing
| 539| 539| 
| 540| 540| 		let cmpPosition = Engine.QueryInterface(this.entity, IID_Position);
| 541| 541| 		if (!cmpPosition || !cmpPosition.IsInWorld())
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
| 582| 582| 		// TODO: Use unit rotation to implement x/z offsets.
| 583| 583| 		let deltaLaunchPoint = new Vector3D(0, this.template[type].Projectile.LaunchPoint["@y"], 0.0);
| 584| 584| 		let launchPoint = Vector3D.add(selfPosition, deltaLaunchPoint);
| 585|    |-		
|    | 585|+
| 586| 586| 		let cmpVisual = Engine.QueryInterface(this.entity, IID_Visual);
| 587| 587| 		if (cmpVisual)
| 588| 588| 		{

binaries/data/mods/public/simulation/components/Attack.js
| 525| ·»   let·cmpDamage·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_Damage);
|    | [NORMAL] ESLintBear (no-mixed-spaces-and-tabs):
|    | Mixed spaces and tabs.

binaries/data/mods/public/simulation/components/Attack.js
| 629| »   »   cmpTimer.SetTimeout(SYSTEM_ENTITY,·IID_Damage,·"MissileHit",·timeToTarget·*·1000·+·+this.template[type].Delay,·data);
|    | [NORMAL] JSHintBear:
|    | Confusing plusses.
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/210/display/redirect

Freagarach updated this revision to Diff 9126.Jul 26 2019, 8:54 AM
  • Classes => AllClasses for clarity.
  • Emphesise *all* and *any* in the help string.

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (spaced-comment):
|    | Expected space or tab after '//' in comment.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
| 535| 535| 
| 536| 536| 		let horizSpeed = +this.template[type].Projectile.Speed;
| 537| 537| 		let gravity = +this.template[type].Projectile.Gravity;
| 538|    |-		//horizSpeed /= 2; gravity /= 2; // slow it down for testing
|    | 538|+		// horizSpeed /= 2; gravity /= 2; // slow it down for testing
| 539| 539| 
| 540| 540| 		let cmpPosition = Engine.QueryInterface(this.entity, IID_Position);
| 541| 541| 		if (!cmpPosition || !cmpPosition.IsInWorld())
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
| 582| 582| 		// TODO: Use unit rotation to implement x/z offsets.
| 583| 583| 		let deltaLaunchPoint = new Vector3D(0, this.template[type].Projectile.LaunchPoint["@y"], 0.0);
| 584| 584| 		let launchPoint = Vector3D.add(selfPosition, deltaLaunchPoint);
| 585|    |-		
|    | 585|+
| 586| 586| 		let cmpVisual = Engine.QueryInterface(this.entity, IID_Visual);
| 587| 587| 		if (cmpVisual)
| 588| 588| 		{

binaries/data/mods/public/simulation/components/Attack.js
| 525| ·»   let·cmpDamage·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_Damage);
|    | [NORMAL] ESLintBear (no-mixed-spaces-and-tabs):
|    | Mixed spaces and tabs.

binaries/data/mods/public/simulation/components/Attack.js
| 629| »   »   cmpTimer.SetTimeout(SYSTEM_ENTITY,·IID_Damage,·"MissileHit",·timeToTarget·*·1000·+·+this.template[type].Delay,·data);
|    | [NORMAL] JSHintBear:
|    | Confusing plusses.
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/237/display/redirect

Silier added a subscriber: Silier.Jul 26 2019, 9:23 AM
Silier added inline comments.
binaries/data/mods/public/simulation/templates/units/kush_champion_infantry_apedemak.xml
15 ↗(On Diff #9126)

with the AnyClasses these two could be merged

Would it at all be possible to implement simple boolean logic there? Hero Elephant would be both, Hero/Elephant would be either Hero or Elephant

just for record there is already similar logic:
Hero Elephant = Hero or Elephant
Hero+Elephant = Hero and Elephant
!Hero = no Hero

globalscript/Templates MatchesClasses

Freagarach updated this revision to Diff 9144.Jul 28 2019, 7:31 AM
Freagarach retitled this revision from Allow support for "AnyClass" in the bonus template. to Improve support of classes of bonuses.
Freagarach edited the summary of this revision. (Show Details)
Freagarach added a subscriber: bb.

Thanks @Angen for the reference to this! This was probably also what @bb meant.

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (quote-props):
|    | Unquoted property 'turnLength' found.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|  25|  25| 
|  26|  26| 	let cmpDamage = ConstructComponent(SYSTEM_ENTITY, "Damage");
|  27|  27| 	let cmpTimer = ConstructComponent(SYSTEM_ENTITY, "Timer");
|  28|    |-	cmpTimer.OnUpdate({ turnLength: 1 });
|    |  28|+	cmpTimer.OnUpdate({ "turnLength": 1 });
|  29|  29| 	let attacker = 11;
|  30|  30| 	let atkPlayerEntity = 1;
|  31|  31| 	let attackerOwner = 6;
|    | [NORMAL] ESLintBear (comma-spacing):
|    | A space is required after ','.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|  63|  63| 		"position": targetPos,
|  64|  64| 		"isSplash": false,
|  65|  65| 		"projectileId": 9,
|  66|    |-		"direction": new Vector3D(1,0,0)
|    |  66|+		"direction": new Vector3D(1, 0,0)
|  67|  67| 	};
|  68|  68| 
|  69|  69| 	AddMock(atkPlayerEntity, IID_Player, {
|    | [NORMAL] ESLintBear (comma-spacing):
|    | A space is required after ','.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|  63|  63| 		"position": targetPos,
|  64|  64| 		"isSplash": false,
|  65|  65| 		"projectileId": 9,
|  66|    |-		"direction": new Vector3D(1,0,0)
|    |  66|+		"direction": new Vector3D(1,0, 0)
|  67|  67| 	};
|  68|  68| 
|  69|  69| 	AddMock(atkPlayerEntity, IID_Player, {
|    | [NORMAL] ESLintBear (quote-props):
|    | Unquoted property 'turnLength' found.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 114| 114| 
| 115| 115| 	function TestDamage()
| 116| 116| 	{
| 117|    |-		cmpTimer.OnUpdate({ turnLength: 1 });
|    | 117|+		cmpTimer.OnUpdate({ "turnLength": 1 });
| 118| 118| 		TS_ASSERT(damageTaken);
| 119| 119| 		damageTaken = false;
| 120| 120| 	}
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'hack'.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 156| 156| 		"origin": origin,
| 157| 157| 		"radius": 10,
| 158| 158| 		"shape": "Linear",
| 159|    |-		"strengths": { "hack" : 100, "pierce" : 0, "crush": 0 },
|    | 159|+		"strengths": { "hack": 100, "pierce" : 0, "crush": 0 },
| 160| 160| 		"direction": new Vector3D(1, 747, 0),
| 161| 161| 		"playersToDamage": [2],
| 162| 162| 		"type": "Ranged",
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'pierce'.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 156| 156| 		"origin": origin,
| 157| 157| 		"radius": 10,
| 158| 158| 		"shape": "Linear",
| 159|    |-		"strengths": { "hack" : 100, "pierce" : 0, "crush": 0 },
|    | 159|+		"strengths": { "hack" : 100, "pierce": 0, "crush": 0 },
| 160| 160| 		"direction": new Vector3D(1, 747, 0),
| 161| 161| 		"playersToDamage": [2],
| 162| 162| 		"type": "Ranged",
|    | [NORMAL] ESLintBear (comma-spacing):
|    | A space is required after ','.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 163| 163| 		"attackerOwner": attackerOwner
| 164| 164| 	};
| 165| 165| 
| 166|    |-	let fallOff = function(x,y)
|    | 166|+	let fallOff = function(x, y)
| 167| 167| 	{
| 168| 168| 		return (1 - x * x / (data.radius * data.radius)) * (1 - 25 * y * y / (data.radius * data.radius));
| 169| 169| 	};
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'hack'.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 315| 315| 		"origin": new Vector2D(3, 4),
| 316| 316| 		"radius": radius,
| 317| 317| 		"shape": "Circular",
| 318|    |-		"strengths": { "hack" : 100, "pierce" : 0, "crush": 0 },
|    | 318|+		"strengths": { "hack": 100, "pierce" : 0, "crush": 0 },
| 319| 319| 		"playersToDamage": [2],
| 320| 320| 		"type": "Ranged",
| 321| 321| 		"attackerOwner": 1
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'pierce'.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 315| 315| 		"origin": new Vector2D(3, 4),
| 316| 316| 		"radius": radius,
| 317| 317| 		"shape": "Circular",
| 318|    |-		"strengths": { "hack" : 100, "pierce" : 0, "crush": 0 },
|    | 318|+		"strengths": { "hack" : 100, "pierce": 0, "crush": 0 },
| 319| 319| 		"playersToDamage": [2],
| 320| 320| 		"type": "Ranged",
| 321| 321| 		"attackerOwner": 1

binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 125| »   type·=·data.type·=·"Ranged";
|    | [NORMAL] ESLintBear (no-multi-assign):
|    | Unexpected chained assignment.
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/250/display/redirect

Freagarach updated this revision to Diff 9149.Jul 28 2019, 11:49 AM
Freagarach retitled this revision from Improve support of classes of bonuses to Improve support of classes of bonuses..

Messed up a condition.

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (quote-props):
|    | Unquoted property 'turnLength' found.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|  25|  25| 
|  26|  26| 	let cmpDamage = ConstructComponent(SYSTEM_ENTITY, "Damage");
|  27|  27| 	let cmpTimer = ConstructComponent(SYSTEM_ENTITY, "Timer");
|  28|    |-	cmpTimer.OnUpdate({ turnLength: 1 });
|    |  28|+	cmpTimer.OnUpdate({ "turnLength": 1 });
|  29|  29| 	let attacker = 11;
|  30|  30| 	let atkPlayerEntity = 1;
|  31|  31| 	let attackerOwner = 6;
|    | [NORMAL] ESLintBear (comma-spacing):
|    | A space is required after ','.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|  63|  63| 		"position": targetPos,
|  64|  64| 		"isSplash": false,
|  65|  65| 		"projectileId": 9,
|  66|    |-		"direction": new Vector3D(1,0,0)
|    |  66|+		"direction": new Vector3D(1, 0,0)
|  67|  67| 	};
|  68|  68| 
|  69|  69| 	AddMock(atkPlayerEntity, IID_Player, {
|    | [NORMAL] ESLintBear (comma-spacing):
|    | A space is required after ','.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|  63|  63| 		"position": targetPos,
|  64|  64| 		"isSplash": false,
|  65|  65| 		"projectileId": 9,
|  66|    |-		"direction": new Vector3D(1,0,0)
|    |  66|+		"direction": new Vector3D(1,0, 0)
|  67|  67| 	};
|  68|  68| 
|  69|  69| 	AddMock(atkPlayerEntity, IID_Player, {
|    | [NORMAL] ESLintBear (quote-props):
|    | Unquoted property 'turnLength' found.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 114| 114| 
| 115| 115| 	function TestDamage()
| 116| 116| 	{
| 117|    |-		cmpTimer.OnUpdate({ turnLength: 1 });
|    | 117|+		cmpTimer.OnUpdate({ "turnLength": 1 });
| 118| 118| 		TS_ASSERT(damageTaken);
| 119| 119| 		damageTaken = false;
| 120| 120| 	}
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'hack'.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 156| 156| 		"origin": origin,
| 157| 157| 		"radius": 10,
| 158| 158| 		"shape": "Linear",
| 159|    |-		"strengths": { "hack" : 100, "pierce" : 0, "crush": 0 },
|    | 159|+		"strengths": { "hack": 100, "pierce" : 0, "crush": 0 },
| 160| 160| 		"direction": new Vector3D(1, 747, 0),
| 161| 161| 		"playersToDamage": [2],
| 162| 162| 		"type": "Ranged",
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'pierce'.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 156| 156| 		"origin": origin,
| 157| 157| 		"radius": 10,
| 158| 158| 		"shape": "Linear",
| 159|    |-		"strengths": { "hack" : 100, "pierce" : 0, "crush": 0 },
|    | 159|+		"strengths": { "hack" : 100, "pierce": 0, "crush": 0 },
| 160| 160| 		"direction": new Vector3D(1, 747, 0),
| 161| 161| 		"playersToDamage": [2],
| 162| 162| 		"type": "Ranged",
|    | [NORMAL] ESLintBear (comma-spacing):
|    | A space is required after ','.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 163| 163| 		"attackerOwner": attackerOwner
| 164| 164| 	};
| 165| 165| 
| 166|    |-	let fallOff = function(x,y)
|    | 166|+	let fallOff = function(x, y)
| 167| 167| 	{
| 168| 168| 		return (1 - x * x / (data.radius * data.radius)) * (1 - 25 * y * y / (data.radius * data.radius));
| 169| 169| 	};
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'hack'.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 315| 315| 		"origin": new Vector2D(3, 4),
| 316| 316| 		"radius": radius,
| 317| 317| 		"shape": "Circular",
| 318|    |-		"strengths": { "hack" : 100, "pierce" : 0, "crush": 0 },
|    | 318|+		"strengths": { "hack": 100, "pierce" : 0, "crush": 0 },
| 319| 319| 		"playersToDamage": [2],
| 320| 320| 		"type": "Ranged",
| 321| 321| 		"attackerOwner": 1
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'pierce'.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 315| 315| 		"origin": new Vector2D(3, 4),
| 316| 316| 		"radius": radius,
| 317| 317| 		"shape": "Circular",
| 318|    |-		"strengths": { "hack" : 100, "pierce" : 0, "crush": 0 },
|    | 318|+		"strengths": { "hack" : 100, "pierce": 0, "crush": 0 },
| 319| 319| 		"playersToDamage": [2],
| 320| 320| 		"type": "Ranged",
| 321| 321| 		"attackerOwner": 1

binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 125| »   type·=·data.type·=·"Ranged";
|    | [NORMAL] ESLintBear (no-multi-assign):
|    | Unexpected chained assignment.
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/254/display/redirect

wraitii requested changes to this revision.Jul 28 2019, 1:23 PM

Looks much better like that, cheers @Angen for the exact function.
This is working for damage types since we weren't actually using this 'always and' behaviour.

Since you're fixing this, you could also fix:

  • Preferred Classes and Restricted Classes in Attack.js (in fact restricted classes is inconsistent).
  • GarrisonHolder's EjectClassesOnDestroy

Haven't found another obvious place to improve this, but check the AI perhaps.

This revision now requires changes to proceed.Jul 28 2019, 1:23 PM
Freagarach updated this revision to Diff 9164.Jul 28 2019, 10:20 PM
Freagarach edited the summary of this revision. (Show Details)

Extended scope to AI.
I'm not sure about common-api's technology.js, but I'm unable unable to test since it is not called.

Owners added a subscriber: Restricted Owners Package.Jul 28 2019, 10:20 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/263/display/redirect

Freagarach updated this revision to Diff 9165.Jul 28 2019, 10:36 PM

Got mixed up due to most recent commit.

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

Linter detected issues:
Executing section Source...
Executing section JS...

binaries/data/mods/public/simulation/ai/petra/queue.js
| 167| }(PETRA);
|    | [MAJOR] ESLintBear (no-use-before-define):
|    | 'PETRA' was used before it was defined.

binaries/data/mods/public/simulation/ai/petra/entityExtend.js
| 421| }(PETRA);
|    | [MAJOR] ESLintBear (no-use-before-define):
|    | 'PETRA' was used before it was defined.

binaries/data/mods/public/simulation/ai/common-api/gamestate.js
| 269| »   »   switch·(entity.check)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/simulation/ai/common-api/gamestate.js
| 286| »   »   »   switch·(type)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/simulation/ai/common-api/gamestate.js
| 936| }(API3);
|    | [MAJOR] ESLintBear (no-use-before-define):
|    | 'API3' was used before it was defined.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 0.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/baseManager.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/baseManager.js
| 195| 195| 		medium.sort((r1, r2) => r1.dist - r2.dist);
| 196| 196| 		faraway.sort((r1, r2) => r1.dist - r2.dist);
| 197| 197| 
| 198|    |-/*		let debug = false;
|    | 198|+		/*		let debug = false;
| 199| 199| 		if (debug)
| 200| 200| 		{
| 201| 201| 			faraway.forEach(function(res){

binaries/data/mods/public/simulation/ai/petra/baseManager.js
|1108| }(PETRA);
|    | [MAJOR] ESLintBear (no-use-before-define):
|    | 'PETRA' was used before it was defined.

binaries/data/mods/public/simulation/ai/petra/worker.js
|1114| }(PETRA);
|    | [MAJOR] ESLintBear (no-use-before-define):
|    | 'PETRA' was used before it was defined.
|    | [NORMAL] ESLintBear (comma-spacing):
|    | A space is required after ','.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/queueplanBuilding.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/queueplanBuilding.js
| 743| 743|  * if wantedSea is given, this tile should be inside this sea
| 744| 744|  */
| 745| 745| const around = [[ 1.0, 0.0], [ 0.87, 0.50], [ 0.50, 0.87], [ 0.0, 1.0], [-0.50, 0.87], [-0.87, 0.50],
| 746|    |-	        [-1.0, 0.0], [-0.87,-0.50], [-0.50,-0.87], [ 0.0,-1.0], [ 0.50,-0.87], [ 0.87,-0.50]];
|    | 746|+	        [-1.0, 0.0], [-0.87, -0.50], [-0.50,-0.87], [ 0.0,-1.0], [ 0.50,-0.87], [ 0.87,-0.50]];
| 747| 747| 
| 748| 748| m.ConstructionPlan.prototype.isDockLocation = function(gameState, j, dimension, wantedLand, wantedSea)
| 749| 749| {
|    | [NORMAL] ESLintBear (comma-spacing):
|    | A space is required after ','.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/queueplanBuilding.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/queueplanBuilding.js
| 743| 743|  * if wantedSea is given, this tile should be inside this sea
| 744| 744|  */
| 745| 745| const around = [[ 1.0, 0.0], [ 0.87, 0.50], [ 0.50, 0.87], [ 0.0, 1.0], [-0.50, 0.87], [-0.87, 0.50],
| 746|    |-	        [-1.0, 0.0], [-0.87,-0.50], [-0.50,-0.87], [ 0.0,-1.0], [ 0.50,-0.87], [ 0.87,-0.50]];
|    | 746|+	        [-1.0, 0.0], [-0.87,-0.50], [-0.50, -0.87], [ 0.0,-1.0], [ 0.50,-0.87], [ 0.87,-0.50]];
| 747| 747| 
| 748| 748| m.ConstructionPlan.prototype.isDockLocation = function(gameState, j, dimension, wantedLand, wantedSea)
| 749| 749| {
|    | [NORMAL] ESLintBear (comma-spacing):
|    | A space is required after ','.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/queueplanBuilding.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/queueplanBuilding.js
| 743| 743|  * if wantedSea is given, this tile should be inside this sea
| 744| 744|  */
| 745| 745| const around = [[ 1.0, 0.0], [ 0.87, 0.50], [ 0.50, 0.87], [ 0.0, 1.0], [-0.50, 0.87], [-0.87, 0.50],
| 746|    |-	        [-1.0, 0.0], [-0.87,-0.50], [-0.50,-0.87], [ 0.0,-1.0], [ 0.50,-0.87], [ 0.87,-0.50]];
|    | 746|+	        [-1.0, 0.0], [-0.87,-0.50], [-0.50,-0.87], [ 0.0, -1.0], [ 0.50,-0.87], [ 0.87,-0.50]];
| 747| 747| 
| 748| 748| m.ConstructionPlan.prototype.isDockLocation = function(gameState, j, dimension, wantedLand, wantedSea)
| 749| 749| {
|    | [NORMAL] ESLintBear (comma-spacing):
|    | A space is required after ','.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/queueplanBuilding.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/queueplanBuilding.js
| 743| 743|  * if wantedSea is given, this tile should be inside this sea
| 744| 744|  */
| 745| 745| const around = [[ 1.0, 0.0], [ 0.87, 0.50], [ 0.50, 0.87], [ 0.0, 1.0], [-0.50, 0.87], [-0.87, 0.50],
| 746|    |-	        [-1.0, 0.0], [-0.87,-0.50], [-0.50,-0.87], [ 0.0,-1.0], [ 0.50,-0.87], [ 0.87,-0.50]];
|    | 746|+	        [-1.0, 0.0], [-0.87,-0.50], [-0.50,-0.87], [ 0.0,-1.0], [ 0.50, -0.87], [ 0.87,-0.50]];
| 747| 747| 
| 748| 748| m.ConstructionPlan.prototype.isDockLocation = function(gameState, j, dimension, wantedLand, wantedSea)
| 749| 749| {
|    | [NORMAL] ESLintBear (comma-spacing):
|    | A space is required after ','.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/queueplanBuilding.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/queueplanBuilding.js
| 743| 743|  * if wantedSea is given, this tile should be inside this sea
| 744| 744|  */
| 745| 745| const around = [[ 1.0, 0.0], [ 0.87, 0.50], [ 0.50, 0.87], [ 0.0, 1.0], [-0.50, 0.87], [-0.87, 0.50],
| 746|    |-	        [-1.0, 0.0], [-0.87,-0.50], [-0.50,-0.87], [ 0.0,-1.0], [ 0.50,-0.87], [ 0.87,-0.50]];
|    | 746|+	        [-1.0, 0.0], [-0.87,-0.50], [-0.50,-0.87], [ 0.0,-1.0], [ 0.50,-0.87], [ 0.87, -0.50]];
| 747| 747| 
| 748| 748| m.ConstructionPlan.prototype.isDockLocation = function(gameState, j, dimension, wantedLand, wantedSea)
| 749| 749| {

binaries/data/mods/public/simulation/ai/petra/queueplanBuilding.js
| 950| }(PETRA);
|    | [MAJOR] ESLintBear (no-use-before-define):
|    | 'PETRA' was used before it was defined.
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/headquarters.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/headquarters.js
|1092|1092| 				val += gameState.sharedScript.ccResourceMaps[res].map[j];
|1093|1093| 		val *= norm;
|1094|1094| 
|1095|    |-		// If oversea, be just above threshold to be accepted if nothing else 
|    |1095|+		// If oversea, be just above threshold to be accepted if nothing else
|1096|1096| 		if (oversea)
|1097|1097| 			val = Math.max(val, cut + 0.1);
|1098|1098| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 0.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/headquarters.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/headquarters.js
|2683|2683| 			this.phasing = 0;
|2684|2684| 	}
|2685|2685| 
|2686|    |-/*	if (this.Config.debug > 1)
|    |2686|+	/*	if (this.Config.debug > 1)
|2687|2687| 	{
|2688|2688| 		gameState.getOwnUnits().forEach (function (ent) {
|2689|2689| 			if (!ent.position())
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 0.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/headquarters.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/headquarters.js
|2758|2758| 		this.currentBase %= this.baseManagers.length;
|2759|2759| 		activeBase = this.baseManagers[this.currentBase++].update(gameState, queues, events);
|2760|2760| 		--nbBases;
|2761|    |-// TODO what to do with this.reassignTerritories(this.baseManagers[this.currentBase]);
|    |2761|+		// TODO what to do with this.reassignTerritories(this.baseManagers[this.currentBase]);
|2762|2762| 	}
|2763|2763| 	while (!activeBase && nbBases != 0);
|2764|2764| 

binaries/data/mods/public/simulation/ai/petra/headquarters.js
|2896| }(PETRA);
|    | [MAJOR] ESLintBear (no-use-before-define):
|    | 'PETRA' was used before it was defined.

binaries/data/mods/public/simulation/ai/petra/attackManager.js
| 805| }(PETRA);
|    | [MAJOR] ESLintBear (no-use-before-define):
|    | 'PETRA' was used before it was defined.

binaries/data/mods/public/simulation/ai/common-api/filters.js
| 223| }(API3);
|    | [MAJOR] ESLintBear (no-use-before-define):
|    | 'API3' was used before it was defined.

binaries/data/mods/public/simulation/ai/petra/defenseManager.js
| 957| }(PETRA);
|    | [MAJOR] ESLintBear (no-use-before-define):
|    | 'PETRA' was used before it was defined.
|    | [NORMAL] ESLintBear (spaced-comment):
|    | Expected space or tab after '//' in comment.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
| 522| 522| 
| 523| 523| 		let horizSpeed = +this.template[type].Projectile.Speed;
| 524| 524| 		let gravity = +this.template[type].Projectile.Gravity;
| 525|    |-		//horizSpeed /= 2; gravity /= 2; // slow it down for testing
|    | 525|+		// horizSpeed /= 2; gravity /= 2; // slow it down for testing
| 526| 526| 
| 527| 527| 		let cmpPosition = Engine.QueryInterface(this.entity, IID_Position);
| 528| 528| 		if (!cmpPosition || !cmpPosition.IsInWorld())
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
| 569| 569| 		// TODO: Use unit rotation to implement x/z offsets.
| 570| 570| 		let deltaLaunchPoint = new Vector3D(0, this.template[type].Projectile.LaunchPoint["@y"], 0.0);
| 571| 571| 		let launchPoint = Vector3D.add(selfPosition, deltaLaunchPoint);
| 572|    |-		
|    | 572|+
| 573| 573| 		let cmpVisual = Engine.QueryInterface(this.entity, IID_Visual);
| 574| 574| 		if (cmpVisual)
| 575| 575| 		{

binaries/data/mods/public/simulation/components/Attack.js
| 512| ·»   let·cmpDamage·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_Damage);
|    | [NORMAL] ESLintBear (no-mixed-spaces-and-tabs):
|    | Mixed spaces and tabs.

binaries/data/mods/public/simulation/components/Attack.js
| 616| »   »   cmpTimer.SetTimeout(SYSTEM_ENTITY,·IID_Damage,·"MissileHit",·timeToTarget·*·1000·+·+this.template[type].Delay,·data);
|    | [NORMAL] JSHintBear:
|    | Confusing plusses.

binaries/data/mods/public/simulation/ai/common-api/technology.js
| 148| }(API3);
|    | [MAJOR] ESLintBear (no-use-before-define):
|    | 'API3' was used before it was defined.

binaries/data/mods/public/simulation/ai/petra/navalManager.js
| 896| }(PETRA);
|    | [MAJOR] ESLintBear (no-use-before-define):
|    | 'PETRA' was used before it was defined.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 0.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/defenseArmy.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/defenseArmy.js
| 169| 169| 			plan.removeUnit(gameState, ent);
| 170| 170| 	}
| 171| 171| 
| 172|    |-/*
|    | 172|+	/*
| 173| 173| 	// TODO be sure that all units in the transport need the cancelation
| 174| 174| 	if (!ent.position())	// this unit must still be in a transport plan ... try to cancel it
| 175| 175| 	{

binaries/data/mods/public/simulation/ai/petra/defenseArmy.js
| 651| }(PETRA);
|    | [MAJOR] ESLintBear (no-use-before-define):
|    | 'PETRA' was used before it was defined.
|    | [NORMAL] ESLintBear (quote-props):
|    | Unquoted property 'turnLength' found.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|  25|  25| 
|  26|  26| 	let cmpDamage = ConstructComponent(SYSTEM_ENTITY, "Damage");
|  27|  27| 	let cmpTimer = ConstructComponent(SYSTEM_ENTITY, "Timer");
|  28|    |-	cmpTimer.OnUpdate({ turnLength: 1 });
|    |  28|+	cmpTimer.OnUpdate({ "turnLength": 1 });
|  29|  29| 	let attacker = 11;
|  30|  30| 	let atkPlayerEntity = 1;
|  31|  31| 	let attackerOwner = 6;
|    | [NORMAL] ESLintBear (comma-spacing):
|    | A space is required after ','.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|  63|  63| 		"position": targetPos,
|  64|  64| 		"isSplash": false,
|  65|  65| 		"projectileId": 9,
|  66|    |-		"direction": new Vector3D(1,0,0)
|    |  66|+		"direction": new Vector3D(1, 0,0)
|  67|  67| 	};
|  68|  68| 
|  69|  69| 	AddMock(atkPlayerEntity, IID_Player, {
|    | [NORMAL] ESLintBear (comma-spacing):
|    | A space is required after ','.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|  63|  63| 		"position": targetPos,
|  64|  64| 		"isSplash": false,
|  65|  65| 		"projectileId": 9,
|  66|    |-		"direction": new Vector3D(1,0,0)
|    |  66|+		"direction": new Vector3D(1,0, 0)
|  67|  67| 	};
|  68|  68| 
|  69|  69| 	AddMock(atkPlayerEntity, IID_Player, {
|    | [NORMAL] ESLintBear (quote-props):
|    | Unquoted property 'turnLength' found.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 114| 114| 
| 115| 115| 	function TestDamage()
| 116| 116| 	{
| 117|    |-		cmpTimer.OnUpdate({ turnLength: 1 });
|    | 117|+		cmpTimer.OnUpdate({ "turnLength": 1 });
| 118| 118| 		TS_ASSERT(damageTaken);
| 119| 119| 		damageTaken = false;
| 120| 120| 	}
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'hack'.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 156| 156| 		"origin": origin,
| 157| 157| 		"radius": 10,
| 158| 158| 		"shape": "Linear",
| 159|    |-		"strengths": { "hack" : 100, "pierce" : 0, "crush": 0 },
|    | 159|+		"strengths": { "hack": 100, "pierce" : 0, "crush": 0 },
| 160| 160| 		"direction": new Vector3D(1, 747, 0),
| 161| 161| 		"playersToDamage": [2],
| 162| 162| 		"type": "Ranged",
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'pierce'.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 156| 156| 		"origin": origin,
| 157| 157| 		"radius": 10,
| 158| 158| 		"shape": "Linear",
| 159|    |-		"strengths": { "hack" : 100, "pierce" : 0, "crush": 0 },
|    | 159|+		"strengths": { "hack" : 100, "pierce": 0, "crush": 0 },
| 160| 160| 		"direction": new Vector3D(1, 747, 0),
| 161| 161| 		"playersToDamage": [2],
| 162| 162| 		"type": "Ranged",
|    | [NORMAL] ESLintBear (comma-spacing):
|    | A space is required after ','.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 163| 163| 		"attackerOwner": attackerOwner
| 164| 164| 	};
| 165| 165| 
| 166|    |-	let fallOff = function(x,y)
|    | 166|+	let fallOff = function(x, y)
| 167| 167| 	{
| 168| 168| 		return (1 - x * x / (data.radius * data.radius)) * (1 - 25 * y * y / (data.radius * data.radius));
| 169| 169| 	};
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'hack'.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 315| 315| 		"origin": new Vector2D(3, 4),
| 316| 316| 		"radius": radius,
| 317| 317| 		"shape": "Circular",
| 318|    |-		"strengths": { "hack" : 100, "pierce" : 0, "crush": 0 },
|    | 318|+		"strengths": { "hack": 100, "pierce" : 0, "crush": 0 },
| 319| 319| 		"playersToDamage": [2],
| 320| 320| 		"type": "Ranged",
| 321| 321| 		"attackerOwner": 1
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'pierce'.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 315| 315| 		"origin": new Vector2D(3, 4),
| 316| 316| 		"radius": radius,
| 317| 317| 		"shape": "Circular",
| 318|    |-		"strengths": { "hack" : 100, "pierce" : 0, "crush": 0 },
|    | 318|+		"strengths": { "hack" : 100, "pierce": 0, "crush": 0 },
| 319| 319| 		"playersToDamage": [2],
| 320| 320| 		"type": "Ranged",
| 321| 321| 		"attackerOwner": 1

binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 125| »   type·=·data.type·=·"Ranged";
|    | [NORMAL] ESLintBear (no-multi-assign):
|    | Unexpected chained assignment.

binaries/data/mods/public/simulation/ai/petra/startingStrategy.js
| 578| }(PETRA);
|    | [MAJOR] ESLintBear (no-use-before-define):
|    | 'PETRA' was used before it was defined.
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '='.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/attackPlan.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/attackPlan.js
| 139| 139| 	{
| 140| 140| 		priority = 90;
| 141| 141| 		// basically we want a mix of citizen soldiers so our barracks have a purpose, and champion units.
| 142|    |-		this.unitStat.RangedInfantry    = { "priority": 0.7, "minSize": 5, "targetSize": 20, "batchSize": 5, "classes": ["Infantry", "Ranged", "CitizenSoldier"],
|    | 142|+		this.unitStat.RangedInfantry = { "priority": 0.7, "minSize": 5, "targetSize": 20, "batchSize": 5, "classes": ["Infantry", "Ranged", "CitizenSoldier"],
| 143| 143| 			"interests": [["strength", 3]] };
| 144| 144| 		this.unitStat.MeleeInfantry     = { "priority": 0.7, "minSize": 5, "targetSize": 20, "batchSize": 5, "classes": ["Infantry", "Melee", "CitizenSoldier"],
| 145| 145| 			"interests": [["strength", 3]] };
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '='.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/attackPlan.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/attackPlan.js
| 141| 141| 		// basically we want a mix of citizen soldiers so our barracks have a purpose, and champion units.
| 142| 142| 		this.unitStat.RangedInfantry    = { "priority": 0.7, "minSize": 5, "targetSize": 20, "batchSize": 5, "classes": ["Infantry", "Ranged", "CitizenSoldier"],
| 143| 143| 			"interests": [["strength", 3]] };
| 144|    |-		this.unitStat.MeleeInfantry     = { "priority": 0.7, "minSize": 5, "targetSize": 20, "batchSize": 5, "classes": ["Infantry", "Melee", "CitizenSoldier"],
|    | 144|+		this.unitStat.MeleeInfantry = { "priority": 0.7, "minSize": 5, "targetSize": 20, "batchSize": 5, "classes": ["Infantry", "Melee", "CitizenSoldier"],
| 145| 145| 			"interests": [["strength", 3]] };
| 146| 146| 		this.unitStat.ChampRangedInfantry = { "priority": 1, "minSize": 3, "targetSize": 18, "batchSize": 3, "classes": ["Infantry", "Ranged", "Champion"],
| 147| 147| 			"interests": [["strength", 3]] };
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '='.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/attackPlan.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/attackPlan.js
| 145| 145| 			"interests": [["strength", 3]] };
| 146| 146| 		this.unitStat.ChampRangedInfantry = { "priority": 1, "minSize": 3, "targetSize": 18, "batchSize": 3, "classes": ["Infantry", "Ranged", "Champion"],
| 147| 147| 			"interests": [["strength", 3]] };
| 148|    |-		this.unitStat.ChampMeleeInfantry  = { "priority": 1, "minSize": 3, "targetSize": 18, "batchSize": 3, "classes": ["Infantry", "Melee", "Champion"],
|    | 148|+		this.unitStat.ChampMeleeInfantry = { "priority": 1, "minSize": 3, "targetSize": 18, "batchSize": 3, "classes": ["Infantry", "Melee", "Champion"],
| 149| 149| 			"interests": [["strength", 3]] };
| 150| 150| 		this.unitStat.RangedCavalry     = { "priority": 0.7, "minSize": 4, "targetSize": 20, "batchSize": 4, "classes": ["Cavalry", "Ranged", "CitizenSoldier"],
| 151| 151| 			"interests": [["strength", 2]] };
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '='.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/attackPlan.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/attackPlan.js
| 147| 147| 			"interests": [["strength", 3]] };
| 148| 148| 		this.unitStat.ChampMeleeInfantry  = { "priority": 1, "minSize": 3, "targetSize": 18, "batchSize": 3, "classes": ["Infantry", "Melee", "Champion"],
| 149| 149| 			"interests": [["strength", 3]] };
| 150|    |-		this.unitStat.RangedCavalry     = { "priority": 0.7, "minSize": 4, "targetSize": 20, "batchSize": 4, "classes": ["Cavalry", "Ranged", "CitizenSoldier"],
|    | 150|+		this.unitStat.RangedCavalry = { "priority": 0.7, "minSize": 4, "targetSize": 20, "batchSize": 4, "classes": ["Cavalry", "Ranged", "CitizenSoldier"],
| 151| 151| 			"interests": [["strength", 2]] };
| 152| 152| 		this.unitStat.MeleeCavalry      = { "priority": 0.7, "minSize": 4, "targetSize": 20, "batchSize": 4, "classes": ["Cavalry", "Melee", "CitizenSoldier"],
| 153| 153| 			"interests": [["strength", 2]] };
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '='.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/attackPlan.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/attackPlan.js
| 149| 149| 			"interests": [["strength", 3]] };
| 150| 150| 		this.unitStat.RangedCavalry     = { "priority": 0.7, "minSize": 4, "targetSize": 20, "batchSize": 4, "classes": ["Cavalry", "Ranged", "CitizenSoldier"],
| 151| 151| 			"interests": [["strength", 2]] };
| 152|    |-		this.unitStat.MeleeCavalry      = { "priority": 0.7, "minSize": 4, "targetSize": 20, "batchSize": 4, "classes": ["Cavalry", "Melee", "CitizenSoldier"],
|    | 152|+		this.unitStat.MeleeCavalry = { "priority": 0.7, "minSize": 4, "targetSize": 20, "batchSize": 4, "classes": ["Cavalry", "Melee", "CitizenSoldier"],
| 153| 153| 			"interests": [["strength", 2]] };
| 154| 154| 		this.unitStat.ChampRangedCavalry  = { "priority": 1, "minSize": 3, "targetSize": 15, "batchSize": 3, "classes": ["Cavalry", "Ranged", "Champion"],
| 155| 155| 			"interests": [["strength", 3]] };
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '='.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/attackPlan.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/attackPlan.js
| 151| 151| 			"interests": [["strength", 2]] };
| 152| 152| 		this.unitStat.MeleeCavalry      = { "priority": 0.7, "minSize": 4, "targetSize": 20, "batchSize": 4, "classes": ["Cavalry", "Melee", "CitizenSoldier"],
| 153| 153| 			"interests": [["strength", 2]] };
| 154|    |-		this.unitStat.ChampRangedCavalry  = { "priority": 1, "minSize": 3, "targetSize": 15, "batchSize": 3, "classes": ["Cavalry", "Ranged", "Champion"],
|    | 154|+		this.unitStat.ChampRangedCavalry = { "priority": 1, "minSize": 3, "targetSize": 15, "batchSize": 3, "classes": ["Cavalry", "Ranged", "Champion"],
| 155| 155| 			"interests": [["strength", 3]] };
| 156| 156| 		this.unitStat.ChampMeleeCavalry   = { "priority": 1, "minSize": 3, "targetSize": 15, "batchSize": 3, "classes": ["Cavalry", "Melee", "Champion"],
| 157| 157| 			"interests": [["strength", 2]] };
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '='.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/attackPlan.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/attackPlan.js
| 153| 153| 			"interests": [["strength", 2]] };
| 154| 154| 		this.unitStat.ChampRangedCavalry  = { "priority": 1, "minSize": 3, "targetSize": 15, "batchSize": 3, "classes": ["Cavalry", "Ranged", "Champion"],
| 155| 155| 			"interests": [["strength", 3]] };
| 156|    |-		this.unitStat.ChampMeleeCavalry   = { "priority": 1, "minSize": 3, "targetSize": 15, "batchSize": 3, "classes": ["Cavalry", "Melee", "Champion"],
|    | 156|+		this.unitStat.ChampMeleeCavalry = { "priority": 1, "minSize": 3, "targetSize": 15, "batchSize": 3, "classes": ["Cavalry", "Melee", "Champion"],
| 157| 157| 			"interests": [["strength", 2]] };
| 158| 158| 		this.unitStat.Hero                = { "priority": 1, "minSize": 0, "targetSize":  1, "batchSize": 1, "classes": ["Hero"],
| 159| 159| 			"interests": [["strength", 2]] };
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '='.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/attackPlan.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/attackPlan.js
| 155| 155| 			"interests": [["strength", 3]] };
| 156| 156| 		this.unitStat.ChampMeleeCavalry   = { "priority": 1, "minSize": 3, "targetSize": 15, "batchSize": 3, "classes": ["Cavalry", "Melee", "Champion"],
| 157| 157| 			"interests": [["strength", 2]] };
| 158|    |-		this.unitStat.Hero                = { "priority": 1, "minSize": 0, "targetSize":  1, "batchSize": 1, "classes": ["Hero"],
|    | 158|+		this.unitStat.Hero = { "priority": 1, "minSize": 0, "targetSize":  1, "batchSize": 1, "classes": ["Hero"],
| 159| 159| 			"interests": [["strength", 2]] };
| 160| 160| 		this.neededShips = 5;
| 161| 161| 	}
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space before value for key 'targetSize'.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/attackPlan.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/attackPlan.js
| 155| 155| 			"interests": [["strength", 3]] };
| 156| 156| 		this.unitStat.ChampMeleeCavalry   = { "priority": 1, "minSize": 3, "targetSize": 15, "batchSize": 3, "classes": ["Cavalry", "Melee", "Champion"],
| 157| 157| 			"interests": [["strength", 2]] };
| 158|    |-		this.unitStat.Hero                = { "priority": 1, "minSize": 0, "targetSize":  1, "batchSize": 1, "classes": ["Hero"],
|    | 158|+		this.unitStat.Hero                = { "priority": 1, "minSize": 0, "targetSize": 1, "batchSize": 1, "classes": ["Hero"],
| 159| 159| 			"interests": [["strength", 2]] };
| 160| 160| 		this.neededShips = 5;
| 161| 161| 	}
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '='.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/attackPlan.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/attackPlan.js
| 164| 164| 		priority = 70;
| 165| 165| 		this.unitStat.RangedInfantry = { "priority": 1, "minSize": 6, "targetSize": 16, "batchSize": 3, "classes": ["Infantry", "Ranged"],
| 166| 166| 			"interests": [["canGather", 1], ["strength", 1.6], ["costsResource", 0.3, "stone"], ["costsResource", 0.3, "metal"]] };
| 167|    |-		this.unitStat.MeleeInfantry  = { "priority": 1, "minSize": 6, "targetSize": 16, "batchSize": 3, "classes": ["Infantry", "Melee"],
|    | 167|+		this.unitStat.MeleeInfantry = { "priority": 1, "minSize": 6, "targetSize": 16, "batchSize": 3, "classes": ["Infantry", "Melee"],
| 168| 168| 			"interests": [["canGather", 1], ["strength", 1.6], ["costsResource", 0.3, "stone"], ["costsResource", 0.3, "metal"]] };
| 169| 169| 		this.unitStat.Cavalry = { "priority": 1, "minSize": 2, "targetSize": 6, "batchSize": 2, "classes": ["Cavalry", "CitizenSoldier"],
| 170| 170| 			"interests": [["strength", 1]] };
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 0.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/attackPlan.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/attackPlan.js
|1142|1142| 
|1143|1143| 	if (blocker && blocker.hasClass("StoneWall"))
|1144|1144| 	{
|1145|    |-/*		if (this.hasSiegeUnits())
|    |1145|+		/*		if (this.hasSiegeUnits())
|1146|1146| 		{ */
|1147|1147| 			this.isBlocked = true;
|1148|1148| 			return blocker;
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/attackPlan.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/attackPlan.js
|1144|1144| 	{
|1145|1145| /*		if (this.hasSiegeUnits())
|1146|1146| 		{ */
|1147|    |-			this.isBlocked = true;
|    |1147|+		this.isBlocked = true;
|1148|1148| 			return blocker;
|1149|1149| /*		}
|1150|1150| 		return undefined; */
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/attackPlan.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/attackPlan.js
|1145|1145| /*		if (this.hasSiegeUnits())
|1146|1146| 		{ */
|1147|1147| 			this.isBlocked = true;
|1148|    |-			return blocker;
|    |1148|+		return blocker;
|1149|1149| /*		}
|1150|1150| 		return undefined; */
|1151|1151| 	}
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 0.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/attackPlan.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/attackPlan.js
|1146|1146| 		{ */
|1147|1147| 			this.isBlocked = true;
|1148|1148| 			return blocker;
|1149|    |-/*		}
|    |1149|+		/*		}
|1150|1150| 		return undefined; */
|1151|1151| 	}
|1152|1152| 	else if (blocker)
|    | [NORMAL] ESLintBear (operator-assignment):
|    | Assignment can be replaced with operator assignment.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/attackPlan.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/ai/petra/attackPlan.js
|1538|1538| 				range = 30 + ent.attackRange("Ranged").max;
|1539|1539| 			else if (ent.hasClass("Cavalry"))
|1540|1540| 				range += 30;
|1541|    |-			range = range * range;
|    |1541|+			range *= range;
|1542|1542| 			let entAccess = m.getLandAccess(gameState, ent);
|1543|1543| 			// Checking for gates if we're a siege unit.
|1544|1544| 			if (siegeUnit)

binaries/data/mods/public/simulation/ai/petra/attackPlan.js
|2168| }(PETRA);
|    | [MAJOR] ESLintBear (no-use-before-define):
|    | 'PETRA' was used before it was defined.
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/264/display/redirect

You've been struck by, you've been struck by, a boolean logic error !
Didn't expect that the AI would have so many such checks, and TBH I'm not sure it really improves readability that much :/
Still, there's one case in which you did the conversion wrong:
!A || !B ≠ !(A || B), indeed !(A || B) == !A && !B, and !A || !B == !(A && B)

I think I got all cases where you've changed it wrong.

I think it could be worth splitting this in the AI change the not-AI change, since I'm less convinced about the AI change and it's definitely trickier to review.

binaries/data/mods/public/simulation/ai/common-api/gamestate.js
728 ↗(On Diff #9165)

Not equivalent.
MatchesClassesList returns true if classes matches at least one class of template.classes()
By negating that, you return true if none of the classes are a match.

However the earlier code returned true if at least one class didn't match.

You need something like MatchesClassList(classes, template.classes().join("+")) here.

binaries/data/mods/public/simulation/ai/common-api/technology.js
140 ↗(On Diff #9165)

This isn't equivalent either but I think the earlier code was broken in this instance.

binaries/data/mods/public/simulation/ai/petra/attackPlan.js
761 ↗(On Diff #9165)

Likewise not equivalent, needs a +

1377 ↗(On Diff #9165)

Not equivalent, needs +

1392 ↗(On Diff #9165)

Not equivalent, needs +

binaries/data/mods/public/simulation/ai/petra/entityExtend.js
8 ↗(On Diff #9165)

there's a very similar call above, maybe replace it with a call to this function

binaries/data/mods/public/simulation/ai/petra/headquarters.js
2132 ↗(On Diff #9165)

Not equivalent, needs +

binaries/data/mods/public/simulation/ai/petra/queueplanBuilding.js
238 ↗(On Diff #9165)

Not equivalent, needs +

binaries/data/mods/public/simulation/ai/petra/startingStrategy.js
308 ↗(On Diff #9165)

Not equivalent, needs +

binaries/data/mods/public/simulation/helpers/DamageBonus.js
23 ↗(On Diff #9165)

You could skip the !bonus.Classes if MatchesClassesList returned true if either are empty, which mathematically makes sense (the empty set is a sub-set of all sets). Would need to check all calls to MatchesClassList though.

Freagarach updated this revision to Diff 9238.Aug 6 2019, 9:52 AM
Freagarach edited the summary of this revision. (Show Details)
  • Split PetraAI changes.
  • Added some JSDOC.
Vulcan added a comment.Aug 6 2019, 9:57 AM

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (spaced-comment):
|    | Expected space or tab after '//' in comment.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
| 532| 532| 
| 533| 533| 		let horizSpeed = +this.template[type].Projectile.Speed;
| 534| 534| 		let gravity = +this.template[type].Projectile.Gravity;
| 535|    |-		//horizSpeed /= 2; gravity /= 2; // slow it down for testing
|    | 535|+		// horizSpeed /= 2; gravity /= 2; // slow it down for testing
| 536| 536| 
| 537| 537| 		let cmpPosition = Engine.QueryInterface(this.entity, IID_Position);
| 538| 538| 		if (!cmpPosition || !cmpPosition.IsInWorld())
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
| 579| 579| 		// TODO: Use unit rotation to implement x/z offsets.
| 580| 580| 		let deltaLaunchPoint = new Vector3D(0, this.template[type].Projectile.LaunchPoint["@y"], 0.0);
| 581| 581| 		let launchPoint = Vector3D.add(selfPosition, deltaLaunchPoint);
| 582|    |-		
|    | 582|+
| 583| 583| 		let cmpVisual = Engine.QueryInterface(this.entity, IID_Visual);
| 584| 584| 		if (cmpVisual)
| 585| 585| 		{

binaries/data/mods/public/simulation/components/Attack.js
| 522| ·»   let·cmpDamage·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_Damage);
|    | [NORMAL] ESLintBear (no-mixed-spaces-and-tabs):
|    | Mixed spaces and tabs.

binaries/data/mods/public/simulation/components/Attack.js
| 626| »   »   cmpTimer.SetTimeout(SYSTEM_ENTITY,·IID_Damage,·"MissileHit",·timeToTarget·*·1000·+·+this.template[type].Delay,·data);
|    | [NORMAL] JSHintBear:
|    | Confusing plusses.
|    | [NORMAL] ESLintBear (quote-props):
|    | Unquoted property 'turnLength' found.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|  25|  25| 
|  26|  26| 	let cmpDamage = ConstructComponent(SYSTEM_ENTITY, "Damage");
|  27|  27| 	let cmpTimer = ConstructComponent(SYSTEM_ENTITY, "Timer");
|  28|    |-	cmpTimer.OnUpdate({ turnLength: 1 });
|    |  28|+	cmpTimer.OnUpdate({ "turnLength": 1 });
|  29|  29| 	let attacker = 11;
|  30|  30| 	let atkPlayerEntity = 1;
|  31|  31| 	let attackerOwner = 6;
|    | [NORMAL] ESLintBear (comma-spacing):
|    | A space is required after ','.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|  63|  63| 		"position": targetPos,
|  64|  64| 		"isSplash": false,
|  65|  65| 		"projectileId": 9,
|  66|    |-		"direction": new Vector3D(1,0,0)
|    |  66|+		"direction": new Vector3D(1, 0,0)
|  67|  67| 	};
|  68|  68| 
|  69|  69| 	AddMock(atkPlayerEntity, IID_Player, {
|    | [NORMAL] ESLintBear (comma-spacing):
|    | A space is required after ','.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|  63|  63| 		"position": targetPos,
|  64|  64| 		"isSplash": false,
|  65|  65| 		"projectileId": 9,
|  66|    |-		"direction": new Vector3D(1,0,0)
|    |  66|+		"direction": new Vector3D(1,0, 0)
|  67|  67| 	};
|  68|  68| 
|  69|  69| 	AddMock(atkPlayerEntity, IID_Player, {
|    | [NORMAL] ESLintBear (quote-props):
|    | Unquoted property 'turnLength' found.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 114| 114| 
| 115| 115| 	function TestDamage()
| 116| 116| 	{
| 117|    |-		cmpTimer.OnUpdate({ turnLength: 1 });
|    | 117|+		cmpTimer.OnUpdate({ "turnLength": 1 });
| 118| 118| 		TS_ASSERT(damageTaken);
| 119| 119| 		damageTaken = false;
| 120| 120| 	}
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'hack'.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 156| 156| 		"origin": origin,
| 157| 157| 		"radius": 10,
| 158| 158| 		"shape": "Linear",
| 159|    |-		"strengths": { "hack" : 100, "pierce" : 0, "crush": 0 },
|    | 159|+		"strengths": { "hack": 100, "pierce" : 0, "crush": 0 },
| 160| 160| 		"direction": new Vector3D(1, 747, 0),
| 161| 161| 		"playersToDamage": [2],
| 162| 162| 		"type": "Ranged",
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'pierce'.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 156| 156| 		"origin": origin,
| 157| 157| 		"radius": 10,
| 158| 158| 		"shape": "Linear",
| 159|    |-		"strengths": { "hack" : 100, "pierce" : 0, "crush": 0 },
|    | 159|+		"strengths": { "hack" : 100, "pierce": 0, "crush": 0 },
| 160| 160| 		"direction": new Vector3D(1, 747, 0),
| 161| 161| 		"playersToDamage": [2],
| 162| 162| 		"type": "Ranged",
|    | [NORMAL] ESLintBear (comma-spacing):
|    | A space is required after ','.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 163| 163| 		"attackerOwner": attackerOwner
| 164| 164| 	};
| 165| 165| 
| 166|    |-	let fallOff = function(x,y)
|    | 166|+	let fallOff = function(x, y)
| 167| 167| 	{
| 168| 168| 		return (1 - x * x / (data.radius * data.radius)) * (1 - 25 * y * y / (data.radius * data.radius));
| 169| 169| 	};
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'hack'.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 315| 315| 		"origin": new Vector2D(3, 4),
| 316| 316| 		"radius": radius,
| 317| 317| 		"shape": "Circular",
| 318|    |-		"strengths": { "hack" : 100, "pierce" : 0, "crush": 0 },
|    | 318|+		"strengths": { "hack": 100, "pierce" : 0, "crush": 0 },
| 319| 319| 		"playersToDamage": [2],
| 320| 320| 		"type": "Ranged",
| 321| 321| 		"attackerOwner": 1
|    | [NORMAL] ESLintBear (key-spacing):
|    | Extra space after key 'pierce'.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 315| 315| 		"origin": new Vector2D(3, 4),
| 316| 316| 		"radius": radius,
| 317| 317| 		"shape": "Circular",
| 318|    |-		"strengths": { "hack" : 100, "pierce" : 0, "crush": 0 },
|    | 318|+		"strengths": { "hack" : 100, "pierce": 0, "crush": 0 },
| 319| 319| 		"playersToDamage": [2],
| 320| 320| 		"type": "Ranged",
| 321| 321| 		"attackerOwner": 1

binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 125| »   type·=·data.type·=·"Ranged";
|    | [NORMAL] ESLintBear (no-multi-assign):
|    | Unexpected chained assignment.
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/318/display/redirect

wraitii accepted this revision.Aug 23 2019, 9:25 AM

Thanks for the split, much easier to be confident that we aren't introducing weird bugs now :)

This revision is now accepted and ready to land.Aug 23 2019, 9:25 AM
Freagarach marked an inline comment as done.Aug 24 2019, 10:54 AM

Thanks for the split, much easier to be confident that we aren't introducing weird bugs now :)

Thanks for the review and commit!