Page MenuHomeWildfire Games

Fix issues with attack bonus
ClosedPublic

Authored by fatherbushido on Aug 15 2017, 10:10 PM.

Details

Reviewers
Mate-86
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP20015: Fix issues with civ and classes bonuses for splash damage. Even if those…
Summary

Currently, the attack bonuses (known as rps hard bonuses) are only taken into account for the main target. It will be the false one if the hitten entity is not the main targeted entity. Worse, there is such a bonus schema for the splash schema which will not be used at all.

In fact, it seems that we should parse those data in the Damage component.

Patch:

  • basically, split the GetAttackBonus function in two parts:
    • parse the bonus template to the damage component (for ranged attack) and compute the actual 'bonus' for the damaged entities.
    • add an helper function (we could put it in the damage component, but it has to be called straightly for Melee and Capture in the Attack component)
  • update and fix (!) some previous tests, add tests about those new things, add a 'complete' test of the MissileHit function
  • remove an useless UnitAI function
  • remove a comment in the AI code which would not be exactly accurate after the patch
Test Plan

Request for comments:

  • is the helper function ok (move it in Damage component?)
  • for the death damage component, imo those kind of bonus are not really relevant, perhaps should we not implement it for that? ---

./binaries/system/test


Check that things works well in game (ranged, splash attack, iberian ship damage on death, that melee and capture attack still works, and check that petra seems ok).

Event Timeline

fatherbushido created this revision.Aug 15 2017, 10:10 PM
Vulcan added a subscriber: Vulcan.Aug 15 2017, 10:57 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jw:8080/job/phabricator/1857/ for more details.

Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/simulation/components/Damage.js
| 221| »   »   »   damageMultiplier·*=·GetDamageBonus(ent,·data.splashBonus)
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
|    | [NORMAL] ESLintBear (no-extra-semi):
|    | Unnecessary semicolon.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 217| 217| 	});
| 218| 218| 
| 219| 219| 	cmpDamage.CauseSplashDamage(data);
| 220|    |-};
|    | 220|+}
| 221| 221| 
| 222| 222| TestLinearSplashDamage();
| 223| 223| 
|    | [NORMAL] ESLintBear (no-extra-semi):
|    | Unnecessary semicolon.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 305| 305| 		"type": "Ranged",
| 306| 306| 		"attackerOwner": 1
| 307| 307| 	});
| 308|    |-};
|    | 308|+}
| 309| 309| 
| 310| 310| TestCircularSplashDamage();

binaries/data/mods/public/simulation/components/tests/test_Damage.js
|  40| »   cmpAttack.GetAttackStrengths·=·(type)·=>·({·"hack":·0,·"pierce":·0,·"crush":·damage·});
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'type' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 167| »   }
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 220| };
|    | [NORMAL] JSHintBear:
|    | Unnecessary semicolon.

binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 234| »   }
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 308| };
|    | [NORMAL] JSHintBear:
|    | Unnecessary semicolon.

binaries/data/mods/public/simulation/ai/petra/entityExtend.js
| 178| »   »   if·(bonus.Classes·&&·bonus.Classes.split(/\s+/).some(cls·=>·!target.hasClass(cls)))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/simulation/components/Attack.js
| 199| »   let·wantedTypesReal·=·wantedTypes.filter(wtype·=>·wtype.indexOf("!")·!=·0);
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

binaries/data/mods/public/simulation/components/Attack.js
| 239| »   ···(!wantedTypes·||·!wantedTypes.filter(wType·=>·wType.indexOf("!")·!=·0).length))
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

binaries/data/mods/public/simulation/components/Attack.js
| 441| »   »   return·clone(template.Bonuses)
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

binaries/data/mods/public/simulation/components/Attack.js
| 534| »   »   if·(!cmpHealth·||·cmpHealth.GetHitpoints()·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/components/Attack.js
| 586| »   if·(c·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/components/Attack.js
| 594| }
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
|    | [NORMAL] ESLintBear (no-extra-semi):
|    | Unnecessary semicolon.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/helpers/DamageBonus.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/helpers/DamageBonus.js
|  21|  21| 	}
|  22|  22| 
|  23|  23| 	return attackBonus;
|  24|    |-};
|    |  24|+}
|  25|  25| 
|  26|  26| Engine.RegisterGlobal("GetDamageBonus", GetDamageBonus);

binaries/data/mods/public/simulation/helpers/DamageBonus.js
|  18| »   »   if·(bonus.Classes·&&·bonus.Classes.split(/\s+/).some(cls·=>·!cmpIdentity.HasClass(cls)))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/simulation/helpers/DamageBonus.js
|  24| };
|    | [NORMAL] JSHintBear:
|    | Unnecessary semicolon.

binaries/data/mods/public/simulation/components/tests/test_Attack.js
| 166| »   attackComponentTest(className,·true,·(attacker,·cmpAttack,·defender)·=>·{
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
| 911| 911| 					this.FinishOrder();
| 912| 912| 					return;
| 913| 913| 				}
| 914|    |-				else
| 915|    |-				{
|    | 914|+				
| 916| 915| 					// Out of range; move there in formation
| 917| 916| 					if (this.MoveToGarrisonRange(msg.data.target))
| 918| 917| 					{
| 919| 918| 						this.SetNextState("GARRISON.APPROACHING");
| 920| 919| 						return;
| 921| 920| 					}
| 922|    |-				}
|    | 921|+				
| 923| 922| 			}
| 924| 923| 
| 925| 924| 			this.SetNextState("GARRISON.GARRISONING");
|    | [NORMAL] ESLintBear (no-unneeded-ternary):
|    | Unnecessary use of boolean literals in conditional expression.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|1958|1958| 					// TODO: we should probably only bother syncing projectile attacks, not melee
|1959|1959| 
|1960|1960| 					// If using a non-default prepare time, re-sync the animation when the timer runs.
|1961|    |-					this.resyncAnimation = (prepare != this.attackTimers.prepare) ? true : false;
|    |1961|+					this.resyncAnimation = (prepare != this.attackTimers.prepare);
|1962|1962| 
|1963|1963| 					this.FaceTowardsTarget(this.order.data.target);
|1964|1964| 
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|2184|2184| 							this.PerformGather(nearby, false, false);
|2185|2185| 							return true;
|2186|2186| 						}
|2187|    |-						else
|2188|    |-						{
|    |2187|+						
|2189|2188| 							// It's probably better in this case, to avoid units getting stuck around a dropsite
|2190|2189| 							// in a "Target is far away, full, nearby are no good resources, return to dropsite" loop
|2191|2190| 							// to order it to GatherNear the resource position.
|2206|2205| 									return true;
|2207|2206| 								}
|2208|2207| 							}
|2209|    |-						}
|    |2208|+						
|2210|2209| 						return true;
|2211|2210| 					}
|2212|2211| 					return false;
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|2196|2196| 								this.GatherNearPosition(pos.x, pos.z, oldType, oldTemplate);
|2197|2197| 								return true;
|2198|2198| 							}
|2199|    |-							else
|2200|    |-							{
|    |2199|+							
|2201|2200| 								// we're kind of stuck here. Return resource.
|2202|2201| 								var nearby = this.FindNearestDropsite(oldType.generic);
|2203|2202| 								if (nearby)
|2205|2204| 									this.PushOrderFront("ReturnResource", { "target": nearby, "force": false });
|2206|2205| 									return true;
|2207|2206| 								}
|2208|    |-							}
|    |2207|+							
|2209|2208| 						}
|2210|2209| 						return true;
|2211|2210| 					}
|    | [NORMAL] ESLintBear (no-unneeded-ternary):
|    | Unnecessary use of boolean literals in conditional expression.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|2580|2580| 					this.StartTimer(prepare, this.healTimers.repeat);
|2581|2581| 
|2582|2582| 					// If using a non-default prepare time, re-sync the animation when the timer runs.
|2583|    |-					this.resyncAnimation = (prepare != this.healTimers.prepare) ? true : false;
|    |2583|+					this.resyncAnimation = (prepare != this.healTimers.prepare);
|2584|2584| 
|2585|2585| 					this.FaceTowardsTarget(this.order.data.target);
|2586|2586| 				},
|    | [NORMAL] ESLintBear (no-unneeded-ternary):
|    | Unnecessary use of boolean literals in conditional expression.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|3396|3396| 
|3397|3397| UnitAI.prototype.IsAnimal = function()
|3398|3398| {
|3399|    |-	return (this.template.NaturalBehaviour ? true : false);
|    |3399|+	return (!!this.template.NaturalBehaviour);
|3400|3400| };
|3401|3401| 
|3402|3402| UnitAI.prototype.IsDangerousAnimal = function()
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|3715|3715| 		// Otherwise we've successfully processed a new order
|3716|3716| 		return true;
|3717|3717| 	}
|3718|    |-	else
|3719|    |-	{
|    |3718|+	
|3720|3719| 		this.SetNextState("IDLE");
|3721|3720| 
|3722|3721| 		Engine.PostMessage(this.entity, MT_UnitAIOrderDataChanged, { "to": this.GetOrderData() });
|3737|3736| 		}
|3738|3737| 
|3739|3738| 		return false;
|3740|    |-	}
|    |3739|+	
|3741|3740| };
|3742|3741| 
|3743|3742| /**
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'Engine'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|4772|4772| 	// If we are guarding/escorting, don't abandon as long as the guarded unit is in target range of the attacker
|4773|4773| 	if (this.isGuardOf)
|4774|4774| 	{
|4775|    |-		var cmpUnitAI =  Engine.QueryInterface(target, IID_UnitAI);
|    |4775|+		var cmpUnitAI = Engine.QueryInterface(target, IID_UnitAI);
|4776|4776| 		var cmpAttack = Engine.QueryInterface(target, IID_Attack);
|4777|4777| 		if (cmpUnitAI && cmpAttack &&
|4778|4778| 		    cmpAttack.GetAttackTypes().some(type => cmpUnitAI.CheckTargetAttackRange(this.isGuardOf, type)))
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'Engine'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|4819|4819| 	// If we are guarding/escorting, chase at least as long as the guarded unit is in target range of the attacker
|4820|4820| 	if (this.isGuardOf)
|4821|4821| 	{
|4822|    |-		var cmpUnitAI =  Engine.QueryInterface(target, IID_UnitAI);
|    |4822|+		var cmpUnitAI = Engine.QueryInterface(target, IID_UnitAI);
|4823|4823| 		var cmpAttack = Engine.QueryInterface(target, IID_Attack);
|4824|4824| 		if (cmpUnitAI && cmpAttack &&
|4825|4825| 		    cmpAttack.GetAttackTypes().some(type => cmpUnitAI.CheckTargetAttackRange(this.isGuardOf, type)))
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|4993|4993| 	{
|4994|4994| 		if (this.isGuardOf == target && this.order && this.order.type == "Guard")
|4995|4995| 			return;
|4996|    |-		else
|4997|    |-			this.RemoveGuard();
|    |4996|+		this.RemoveGuard();
|4998|4997| 	}
|4999|4998| 
|5000|4999| 	this.AddOrder("Guard", { "target": target, "force": false }, queued);
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'lastPos' to undefined.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|5241|5241| 
|5242|5242| 	// Remember the position of our target, if any, in case it disappears
|5243|5243| 	// later and we want to head to its last known position
|5244|    |-	var lastPos = undefined;
|    |5244|+	var lastPos;
|5245|5245| 	var cmpPosition = Engine.QueryInterface(target, IID_Position);
|5246|5246| 	if (cmpPosition && cmpPosition.IsInWorld())
|5247|5247| 		lastPos = cmpPosition.GetPosition();

binaries/data/mods/public/simulation/components/UnitAI.js
|2482| »   »   »   »   »   »   let·nearby·=·this.FindNearestDropsite(resourceType.generic);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'nearby' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|3904| »   var·isWorkType·=·type·=>·type·==·"Gather"·||·type·==·"Trade"·||·type·==·"Repair"·||·type·==·"ReturnResource";
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'type' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4229| »   »   let·cmpOwnership·=·Engine.QueryInterface(ent,·IID_Ownership);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'cmpOwnership' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4694| »   var·target·=·ents.find(target·=>·this.CanAttack(target,·forceResponse));
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'target' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4709| »   var·target·=·ents.find(target·=>
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'target' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4755| »   var·ent·=·ents.find(ent·=>·this.CanHeal(ent));
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'ent' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4778| »   »   ····cmpAttack.GetAttackTypes().some(type·=>·cmpUnitAI.CheckTargetAttackRange(this.isGuardOf,·type)))
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'type' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
| 363| »   »   ····&&·(this.lastShorelinePosition.z·==·cmpPosition.GetPosition().z))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|1951| »   »   »   »   »   »   var·cmpFormation·=·Engine.QueryInterface(this.formationController,·IID_Formation);
|    | [NORMAL] JSHintBear:
|    | 'cmpFormation' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|2177| »   »   »   »   »   »   »   »   ·&&·((type.generic·==·"treasure"·&&·oldType.generic·==·"treasure")
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|21

http://jw:8080/job/phabricator_lint/396/ for more details.

fatherbushido edited the summary of this revision. (Show Details)
fatherbushido edited the test plan for this revision. (Show Details)

Clean lint, add tests, fix some previous tests too

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jw:8080/job/phabricator/1859/ for more details.

Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/simulation/helpers/DamageBonus.js
|  18| »   »   if·(bonus.Classes·&&·bonus.Classes.split(/\s+/).some(cls·=>·!cmpIdentity.HasClass(cls)))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/simulation/ai/petra/entityExtend.js
| 178| »   »   if·(bonus.Classes·&&·bonus.Classes.split(/\s+/).some(cls·=>·!target.hasClass(cls)))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/simulation/components/tests/test_Attack.js
| 166| »   attackComponentTest(className,·true,·(attacker,·cmpAttack,·defender)·=>·{
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
| 911| 911| 					this.FinishOrder();
| 912| 912| 					return;
| 913| 913| 				}
| 914|    |-				else
| 915|    |-				{
|    | 914|+				
| 916| 915| 					// Out of range; move there in formation
| 917| 916| 					if (this.MoveToGarrisonRange(msg.data.target))
| 918| 917| 					{
| 919| 918| 						this.SetNextState("GARRISON.APPROACHING");
| 920| 919| 						return;
| 921| 920| 					}
| 922|    |-				}
|    | 921|+				
| 923| 922| 			}
| 924| 923| 
| 925| 924| 			this.SetNextState("GARRISON.GARRISONING");
|    | [NORMAL] ESLintBear (no-unneeded-ternary):
|    | Unnecessary use of boolean literals in conditional expression.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|1958|1958| 					// TODO: we should probably only bother syncing projectile attacks, not melee
|1959|1959| 
|1960|1960| 					// If using a non-default prepare time, re-sync the animation when the timer runs.
|1961|    |-					this.resyncAnimation = (prepare != this.attackTimers.prepare) ? true : false;
|    |1961|+					this.resyncAnimation = (prepare != this.attackTimers.prepare);
|1962|1962| 
|1963|1963| 					this.FaceTowardsTarget(this.order.data.target);
|1964|1964| 
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|2184|2184| 							this.PerformGather(nearby, false, false);
|2185|2185| 							return true;
|2186|2186| 						}
|2187|    |-						else
|2188|    |-						{
|    |2187|+						
|2189|2188| 							// It's probably better in this case, to avoid units getting stuck around a dropsite
|2190|2189| 							// in a "Target is far away, full, nearby are no good resources, return to dropsite" loop
|2191|2190| 							// to order it to GatherNear the resource position.
|2206|2205| 									return true;
|2207|2206| 								}
|2208|2207| 							}
|2209|    |-						}
|    |2208|+						
|2210|2209| 						return true;
|2211|2210| 					}
|2212|2211| 					return false;
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|2196|2196| 								this.GatherNearPosition(pos.x, pos.z, oldType, oldTemplate);
|2197|2197| 								return true;
|2198|2198| 							}
|2199|    |-							else
|2200|    |-							{
|    |2199|+							
|2201|2200| 								// we're kind of stuck here. Return resource.
|2202|2201| 								var nearby = this.FindNearestDropsite(oldType.generic);
|2203|2202| 								if (nearby)
|2205|2204| 									this.PushOrderFront("ReturnResource", { "target": nearby, "force": false });
|2206|2205| 									return true;
|2207|2206| 								}
|2208|    |-							}
|    |2207|+							
|2209|2208| 						}
|2210|2209| 						return true;
|2211|2210| 					}
|    | [NORMAL] ESLintBear (no-unneeded-ternary):
|    | Unnecessary use of boolean literals in conditional expression.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|2580|2580| 					this.StartTimer(prepare, this.healTimers.repeat);
|2581|2581| 
|2582|2582| 					// If using a non-default prepare time, re-sync the animation when the timer runs.
|2583|    |-					this.resyncAnimation = (prepare != this.healTimers.prepare) ? true : false;
|    |2583|+					this.resyncAnimation = (prepare != this.healTimers.prepare);
|2584|2584| 
|2585|2585| 					this.FaceTowardsTarget(this.order.data.target);
|2586|2586| 				},
|    | [NORMAL] ESLintBear (no-unneeded-ternary):
|    | Unnecessary use of boolean literals in conditional expression.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|3396|3396| 
|3397|3397| UnitAI.prototype.IsAnimal = function()
|3398|3398| {
|3399|    |-	return (this.template.NaturalBehaviour ? true : false);
|    |3399|+	return (!!this.template.NaturalBehaviour);
|3400|3400| };
|3401|3401| 
|3402|3402| UnitAI.prototype.IsDangerousAnimal = function()
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|3715|3715| 		// Otherwise we've successfully processed a new order
|3716|3716| 		return true;
|3717|3717| 	}
|3718|    |-	else
|3719|    |-	{
|    |3718|+	
|3720|3719| 		this.SetNextState("IDLE");
|3721|3720| 
|3722|3721| 		Engine.PostMessage(this.entity, MT_UnitAIOrderDataChanged, { "to": this.GetOrderData() });
|3737|3736| 		}
|3738|3737| 
|3739|3738| 		return false;
|3740|    |-	}
|    |3739|+	
|3741|3740| };
|3742|3741| 
|3743|3742| /**
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'Engine'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|4772|4772| 	// If we are guarding/escorting, don't abandon as long as the guarded unit is in target range of the attacker
|4773|4773| 	if (this.isGuardOf)
|4774|4774| 	{
|4775|    |-		var cmpUnitAI =  Engine.QueryInterface(target, IID_UnitAI);
|    |4775|+		var cmpUnitAI = Engine.QueryInterface(target, IID_UnitAI);
|4776|4776| 		var cmpAttack = Engine.QueryInterface(target, IID_Attack);
|4777|4777| 		if (cmpUnitAI && cmpAttack &&
|4778|4778| 		    cmpAttack.GetAttackTypes().some(type => cmpUnitAI.CheckTargetAttackRange(this.isGuardOf, type)))
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'Engine'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|4819|4819| 	// If we are guarding/escorting, chase at least as long as the guarded unit is in target range of the attacker
|4820|4820| 	if (this.isGuardOf)
|4821|4821| 	{
|4822|    |-		var cmpUnitAI =  Engine.QueryInterface(target, IID_UnitAI);
|    |4822|+		var cmpUnitAI = Engine.QueryInterface(target, IID_UnitAI);
|4823|4823| 		var cmpAttack = Engine.QueryInterface(target, IID_Attack);
|4824|4824| 		if (cmpUnitAI && cmpAttack &&
|4825|4825| 		    cmpAttack.GetAttackTypes().some(type => cmpUnitAI.CheckTargetAttackRange(this.isGuardOf, type)))
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|4993|4993| 	{
|4994|4994| 		if (this.isGuardOf == target && this.order && this.order.type == "Guard")
|4995|4995| 			return;
|4996|    |-		else
|4997|    |-			this.RemoveGuard();
|    |4996|+		this.RemoveGuard();
|4998|4997| 	}
|4999|4998| 
|5000|4999| 	this.AddOrder("Guard", { "target": target, "force": false }, queued);
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'lastPos' to undefined.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|5241|5241| 
|5242|5242| 	// Remember the position of our target, if any, in case it disappears
|5243|5243| 	// later and we want to head to its last known position
|5244|    |-	var lastPos = undefined;
|    |5244|+	var lastPos;
|5245|5245| 	var cmpPosition = Engine.QueryInterface(target, IID_Position);
|5246|5246| 	if (cmpPosition && cmpPosition.IsInWorld())
|5247|5247| 		lastPos = cmpPosition.GetPosition();

binaries/data/mods/public/simulation/components/UnitAI.js
|2482| »   »   »   »   »   »   let·nearby·=·this.FindNearestDropsite(resourceType.generic);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'nearby' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|3904| »   var·isWorkType·=·type·=>·type·==·"Gather"·||·type·==·"Trade"·||·type·==·"Repair"·||·type·==·"ReturnResource";
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'type' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4229| »   »   let·cmpOwnership·=·Engine.QueryInterface(ent,·IID_Ownership);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'cmpOwnership' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4694| »   var·target·=·ents.find(target·=>·this.CanAttack(target,·forceResponse));
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'target' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4709| »   var·target·=·ents.find(target·=>
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'target' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4755| »   var·ent·=·ents.find(ent·=>·this.CanHeal(ent));
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'ent' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4778| »   »   ····cmpAttack.GetAttackTypes().some(type·=>·cmpUnitAI.CheckTargetAttackRange(this.isGuardOf,·type)))
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'type' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
| 363| »   »   ····&&·(this.lastShorelinePosition.z·==·cmpPosition.GetPosition().z))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|1951| »   »   »   »   »   »   var·cmpFormation·=·Engine.QueryInterface(this.formationController,·IID_Formation);
|    | [NORMAL] JSHintBear:
|    | 'cmpFormation' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|2177| »   »   »   »   »   »   »   »   ·&&·((type.generic·==·"treasure"·&&·oldType.generic·==·"treasure")
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|2178| »   »   »   »   »   »   »   »   ·||·(type.specific·==·oldType.specific
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|2179| »   »   »   »   »   »   »   »   ·&&·(type.specific·!=·"meat"·||·oldTemplate·==·template)))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|2202| »   »   »   »   »   »   »   »   var·nearby·=·this.FindNearestDropsite(oldType.generic);
|    | [NORMAL] JSHintBear:
|    | 'nearby' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|2242| »   »   »   »   »   »   »   »   &&·((type.generic·==·"treasure"·&&·oldType.generic·==·"treasure")
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|2243| »   »   »   »   »   »   »   »   ||·(type.specific·==·oldType.specific
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|2244| »   »   »   »   »   »   »   »   &&·(type.specific·!=·"meat"·||·oldTemplate·==·template)))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|2288| »   »   »   »   »   »   »   ||·(type.specific·==·resourceType.specific
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|2289| »   »   »   »   »   »   »   &&·(type.specific·!=·"meat"·||·resourceTemplate·==·template))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|2305| »   »   »   »   »   var·nearby·=·this.FindNearestDropsite(resourceType.generic);
|    | [NORMAL] JSHintBear:
|    | 'nearby' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|2477| »   »   »   »   »   var·cmpResourceGatherer·=·Engine.QueryInterface(this.entity,·IID_ResourceGatherer);
|    | [NORMAL] JSHintBear:
|    | 'cmpResourceGatherer' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|2493| »   »   »   »   »   var·nearby·=·this.FindNearbyResource(function·(ent,·type,·template)·{
|    | [NORMAL] JSHintBear:
|    | 'nearby' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|2496| »   »   »   »   »   »   »   ||·(type.specific·==·resourceType.specific
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|2497| »   »   »   »   »   »   »   &&·(type.specific·!=·"meat"·||·resourceTemplate·==·template))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|2517| »   »   »   »   »   var·nearby·=·this.FindNearestDropsite(resourceType.generic);
|    | [NORMAL] JSHintBear:
|    | 'nearby' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|2701| »   »   »   »   »   var·cmpResourceGat

http://jw:8080/job/phabricator_lint/398/ for more details.

some null/undefined tweaks

fatherbushido retitled this revision from [WIP] Fix issues with attack bonus to Fix issues with attack bonus.Aug 16 2017, 6:21 PM
fatherbushido edited the summary of this revision. (Show Details)
fatherbushido edited the test plan for this revision. (Show Details)
fatherbushido edited the test plan for this revision. (Show Details)
fatherbushido added inline comments.Aug 16 2017, 6:28 PM
binaries/data/mods/public/simulation/components/tests/test_Damage.js
158

need to check that entity are actually hit (use that set).

elexis added a subscriber: elexis.Aug 16 2017, 6:40 PM
elexis added inline comments.
binaries/data/mods/public/simulation/components/Attack.js
442

(If the clone is a pure precaution, it can be removed once templates are frozen in #4257.)

binaries/data/mods/public/simulation/components/UnitAI.js
4694

Agree that it's useless. Confirming that it's currently unused as well (so it could be nuked in a separate commit if one feels like it)

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jw:8080/job/phabricator/1860/ for more details.

Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/simulation/helpers/DamageBonus.js
|  18| »   »   if·(bonus.Classes·&&·bonus.Classes.split(/\s+/).some(cls·=>·!cmpIdentity.HasClass(cls)))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/simulation/components/tests/test_Attack.js
| 166| »   attackComponentTest(className,·true,·(attacker,·cmpAttack,·defender)·=>·{
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '400'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 489| 489| 	data.bonus = bonus;
| 490| 490| 	cmpDamage.MissileHit(data, 0);
| 491| 491| 	TS_ASSERT(hitEnts.has(61));
| 492|    |-	TS_ASSERT_EQUALS(dealtDamage,  400 * 100 + 200);
|    | 492|+	TS_ASSERT_EQUALS(dealtDamage, 400 * 100 + 200);
| 493| 493| 	dealtDamage = 0;
| 494| 494| 	hitEnts.clear();
| 495| 495| 
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '400'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 496| 496| 	data.splashBonus = splashBonus;
| 497| 497| 	cmpDamage.MissileHit(data, 0);
| 498| 498| 	TS_ASSERT(hitEnts.has(61));
| 499|    |-	TS_ASSERT_EQUALS(dealtDamage,  400 * 100 + 10000 * 200);
|    | 499|+	TS_ASSERT_EQUALS(dealtDamage, 400 * 100 + 10000 * 200);
| 500| 500| 	dealtDamage = 0;
| 501| 501| 	hitEnts.clear();
| 502| 502| 
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '100'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 503| 503| 	data.bonus = undefined;
| 504| 504| 	cmpDamage.MissileHit(data, 0);
| 505| 505| 	TS_ASSERT(hitEnts.has(61));
|    | 506|+	TS_ASSERT_EQUALS(dealtDamage, 100 + 10000 * 200);
|    | 507|+	dealtDamage = 0;
|    | 508|+	hitEnts.clear();
|    | 509|+
|    | 510|+	data.bonus = null;
|    | 511|+	cmpDamage.MissileHit(data, 0);
|    | 512|+	TS_ASSERT(hitEnts.has(61));
| 506| 513| 	TS_ASSERT_EQUALS(dealtDamage,  100 + 10000 * 200);
| 507| 514| 	dealtDamage = 0;
| 508| 515| 	hitEnts.clear();
| 509| 516| 
| 510|    |-	data.bonus = null;
|    | 517|+	data.bonus = {};
| 511| 518| 	cmpDamage.MissileHit(data, 0);
| 512| 519| 	TS_ASSERT(hitEnts.has(61));
| 513| 520| 	TS_ASSERT_EQUALS(dealtDamage,  100 + 10000 * 200);
| 514| 521| 	dealtDamage = 0;
| 515| 522| 	hitEnts.clear();
| 516|    |-
| 517|    |-	data.bonus = {};
| 518|    |-	cmpDamage.MissileHit(data, 0);
| 519|    |-	TS_ASSERT(hitEnts.has(61));
| 520|    |-	TS_ASSERT_EQUALS(dealtDamage,  100 + 10000 * 200);
| 521|    |-	dealtDamage = 0;
| 522|    |-	hitEnts.clear();
| 523| 523| }
| 524| 524| 
| 525| 525| Test_MissileHit();
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '100'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 510| 510| 	data.bonus = null;
| 511| 511| 	cmpDamage.MissileHit(data, 0);
| 512| 512| 	TS_ASSERT(hitEnts.has(61));
|    | 513|+	TS_ASSERT_EQUALS(dealtDamage, 100 + 10000 * 200);
|    | 514|+	dealtDamage = 0;
|    | 515|+	hitEnts.clear();
|    | 516|+
|    | 517|+	data.bonus = {};
|    | 518|+	cmpDamage.MissileHit(data, 0);
|    | 519|+	TS_ASSERT(hitEnts.has(61));
| 513| 520| 	TS_ASSERT_EQUALS(dealtDamage,  100 + 10000 * 200);
| 514| 521| 	dealtDamage = 0;
| 515| 522| 	hitEnts.clear();
| 516|    |-
| 517|    |-	data.bonus = {};
| 518|    |-	cmpDamage.MissileHit(data, 0);
| 519|    |-	TS_ASSERT(hitEnts.has(61));
| 520|    |-	TS_ASSERT_EQUALS(dealtDamage,  100 + 10000 * 200);
| 521|    |-	dealtDamage = 0;
| 522|    |-	hitEnts.clear();
| 523| 523| }
| 524| 524| 
| 525| 525| Test_MissileHit();
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '100'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/tests/test_Damage.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 517| 517| 	data.bonus = {};
| 518| 518| 	cmpDamage.MissileHit(data, 0);
| 519| 519| 	TS_ASSERT(hitEnts.has(61));
| 520|    |-	TS_ASSERT_EQUALS(dealtDamage,  100 + 10000 * 200);
|    | 520|+	TS_ASSERT_EQUALS(dealtDamage, 100 + 10000 * 200);
| 521| 521| 	dealtDamage = 0;
| 522| 522| 	hitEnts.clear();
| 523| 523| }

binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 330| »   }
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

binaries/data/mods/public/simulation/components/Attack.js
| 199| »   let·wantedTypesReal·=·wantedTypes.filter(wtype·=>·wtype.indexOf("!")·!=·0);
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

binaries/data/mods/public/simulation/components/Attack.js
| 239| »   ···(!wantedTypes·||·!wantedTypes.filter(wType·=>·wType.indexOf("!")·!=·0).length))
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

binaries/data/mods/public/simulation/components/Attack.js
| 534| »   »   if·(!cmpHealth·||·cmpHealth.GetHitpoints()·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/components/Attack.js
| 586| »   if·(c·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/ai/petra/entityExtend.js
| 178| »   »   if·(bonus.Classes·&&·bonus.Classes.split(/\s+/).some(cls·=>·!target.hasClass(cls)))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
| 911| 911| 					this.FinishOrder();
| 912| 912| 					return;
| 913| 913| 				}
| 914|    |-				else
| 915|    |-				{
|    | 914|+				
| 916| 915| 					// Out of range; move there in formation
| 917| 916| 					if (this.MoveToGarrisonRange(msg.data.target))
| 918| 917| 					{
| 919| 918| 						this.SetNextState("GARRISON.APPROACHING");
| 920| 919| 						return;
| 921| 920| 					}
| 922|    |-				}
|    | 921|+				
| 923| 922| 			}
| 924| 923| 
| 925| 924| 			this.SetNextState("GARRISON.GARRISONING");
|    | [NORMAL] ESLintBear (no-unneeded-ternary):
|    | Unnecessary use of boolean literals in conditional expression.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|1958|1958| 					// TODO: we should probably only bother syncing projectile attacks, not melee
|1959|1959| 
|1960|1960| 					// If using a non-default prepare time, re-sync the animation when the timer runs.
|1961|    |-					this.resyncAnimation = (prepare != this.attackTimers.prepare) ? true : false;
|    |1961|+					this.resyncAnimation = (prepare != this.attackTimers.prepare);
|1962|1962| 
|1963|1963| 					this.FaceTowardsTarget(this.order.data.target);
|1964|1964| 
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|2184|2184| 							this.PerformGather(nearby, false, false);
|2185|2185| 							return true;
|2186|2186| 						}
|2187|    |-						else
|2188|    |-						{
|    |2187|+						
|2189|2188| 							// It's probably better in this case, to avoid units getting stuck around a dropsite
|2190|2189| 							// in a "Target is far away, full, nearby are no good resources, return to dropsite" loop
|2191|2190| 							// to order it to GatherNear the resource position.
|2206|2205| 									return true;
|2207|2206| 								}
|2208|2207| 							}
|2209|    |-						}
|    |2208|+						
|2210|2209| 						return true;
|2211|2210| 					}
|2212|2211| 					return false;
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|2196|2196| 								this.GatherNearPosition(pos.x, pos.z, oldType, oldTemplate);
|2197|2197| 								return true;
|2198|2198| 							}
|2199|    |-							else
|2200|    |-							{
|    |2199|+							
|2201|2200| 								// we're kind of stuck here. Return resource.
|2202|2201| 								var nearby = this.FindNearestDropsite(oldType.generic);
|2203|2202| 								if (nearby)
|2205|2204| 									this.PushOrderFront("ReturnResource", { "target": nearby, "force": false });
|2206|2205| 									return true;
|2207|2206| 								}
|2208|    |-							}
|    |2207|+							
|2209|2208| 						}
|2210|2209| 						return true;
|2211|2210| 					}
|    | [NORMAL] ESLintBear (no-unneeded-ternary):
|    | Unnecessary use of boolean literals in conditional expression.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|2580|2580| 					this.StartTimer(prepare, this.healTimers.repeat);
|2581|2581| 
|2582|2582| 					// If using a non-default prepare time, re-sync the animation when the timer runs.
|2583|    |-					this.resyncAnimation = (prepare != this.healTimers.prepare) ? true : false;
|    |2583|+					this.resyncAnimation = (prepare != this.healTimers.prepare);
|2584|2584| 
|2585|2585| 					this.FaceTowardsTarget(this.order.data.target);
|2586|2586| 				},
|    | [NORMAL] ESLintBear (no-unneeded-ternary):
|    | Unnecessary use of boolean literals in conditional expression.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|3396|3396| 
|3397|3397| UnitAI.prototype.IsAnimal = function()
|3398|3398| {
|3399|    |-	return (this.template.NaturalBehaviour ? true : false);
|    |3399|+	return (!!this.template.NaturalBehaviour);
|3400|3400| };
|3401|3401| 
|3402|3402| UnitAI.prototype.IsDangerousAnimal = function()
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|3715|3715| 		// Otherwise we've successfully processed a new order
|3716|3716| 		return true;
|3717|3717| 	}
|3718|    |-	else
|3719|    |-	{
|    |3718|+	
|3720|3719| 		this.SetNextState("IDLE");
|3721|3720| 
|3722|3721| 		Engine.PostMessage(this.entity, MT_UnitAIOrderDataChanged, { "to": this.GetOrderData() });
|3737|3736| 		}
|3738|3737| 
|3739|3738| 		return false;
|3740|    |-	}
|    |3739|+	
|3741|3740| };
|3742|3741| 
|3743|3742| /**
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'Engine'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|4772|4772| 	// If we are guarding/escorting, don't abandon as long as the guarded unit is in target range of the attacker
|4773|4773| 	if (this.isGuardOf)
|4774|4774| 	{
|4775|    |-		var cmpUnitAI =  Engine.QueryInterface(target, IID_UnitAI);
|    |4775|+		var cmpUnitAI = Engine.QueryInterface(target, IID_UnitAI);
|4776|4776| 		var cmpAttack = Engine.QueryInterface(target, IID_Attack);
|4777|4777| 		if (cmpUnitAI && cmpAttack &&
|4778|4778| 		    cmpAttack.GetAttackTypes().some(type => cmpUnitAI.CheckTargetAttackRange(this.isGuardOf, type)))
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'Engine'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|4819|4819| 	// If we are guarding/escorting, chase at least as long as the guarded unit is in target range of the attacker
|4820|4820| 	if (this.isGuardOf)
|4821|4821| 	{
|4822|    |-		var cmpUnitAI =  Engine.QueryInterface(target, IID_UnitAI);
|    |4822|+		var cmpUnitAI = Engine.QueryInterface(target, IID_UnitAI);
|4823|4823| 		var cmpAttack = Engine.QueryInterface(target, IID_Attack);
|4824|4824| 		if (cmpUnitAI && cmpAttack &&
|4825|4825| 		    cmpAttack.GetAttackTypes().some(type => cmpUnitAI.CheckTargetAttackRange(this.isGuardOf, type)))
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|4993|4993| 	{
|4994|4994| 		if (this.isGuardOf == target && this.order && this.order.type == "Guard")
|4995|4995| 			return;
|4996|    |-		else
|4997|    |-			this.RemoveGuard();
|    |4996|+		this.RemoveGuard();
|4998|4997| 	}
|4999|4998| 
|5000|4999| 	this.AddOrder("Guard", { "target": target, "force": false }, queued);
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'lastPos' to undefined.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|5241|5241| 
|5242|5242| 	// Remember the position of our target, if any, in case it disappears
|5243|5243| 	// later and we want to head to its last known position
|5244|    |-	var lastPos = undefined;
|    |5244|+	var lastPos;
|5245|5245| 	var cmpPosition = Engine.QueryInterface(target, IID_Position);
|5246|5246| 	if (cmpPosition && cmpPosition.IsInWorld())
|5247|5247| 		lastPos = cmpPosition.GetPosition();

binaries/data/mods/public/simulation/components/UnitAI.js
|2482| »   »   »   »   »   »   let·nearby·=·this.FindNearestDropsite(resourceType.generic);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'nearby' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|3904| »   var·isWorkType·=·type·=>·type·==·"Gather"·||·type·==·"Trade"·||·type·==·"Repair"·||·type·==·"ReturnResource";
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'type' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4229| »   »   let·cmpOwnership·=·Engine.QueryInterface(ent,·IID_Ownership);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'cmpOwnership' is already declared in the

http://jw:8080/job/phabricator_lint/399/ for more details.

fatherbushido marked an inline comment as done.

complete tests

fatherbushido added reviewers: Mate-86, Restricted Owners Package.Aug 16 2017, 10:02 PM
fatherbushido added inline comments.Aug 16 2017, 10:04 PM
binaries/data/mods/public/simulation/components/Attack.js
442

currently it's not pure precaution: there would be a data leak (see the related test).
Thx for recalling #4257. I am not sure it would exactly prevent that.

case consistency in a comment

Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/simulation/ai/petra/entityExtend.js
| 178| »   »   if·(bonus.Classes·&&·bonus.Classes.split(/\s+/).some(cls·=>·!target.hasClass(cls)))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/simulation/components/tests/test_Attack.js
| 166| »   attackComponentTest(className,·true,·(attacker,·cmpAttack,·defender)·=>·{
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/simulation/components/Attack.js
| 199| »   let·wantedTypesReal·=·wantedTypes.filter(wtype·=>·wtype.indexOf("!")·!=·0);
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

binaries/data/mods/public/simulation/components/Attack.js
| 239| »   ···(!wantedTypes·||·!wantedTypes.filter(wType·=>·wType.indexOf("!")·!=·0).length))
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

binaries/data/mods/public/simulation/components/Attack.js
| 534| »   »   if·(!cmpHealth·||·cmpHealth.GetHitpoints()·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/components/Attack.js
| 586| »   if·(c·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/helpers/DamageBonus.js
|  18| »   »   if·(bonus.Classes·&&·bonus.Classes.split(/\s+/).some(cls·=>·!cmpIdentity.HasClass(cls)))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 328| »   }
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
| 911| 911| 					this.FinishOrder();
| 912| 912| 					return;
| 913| 913| 				}
| 914|    |-				else
| 915|    |-				{
|    | 914|+				
| 916| 915| 					// Out of range; move there in formation
| 917| 916| 					if (this.MoveToGarrisonRange(msg.data.target))
| 918| 917| 					{
| 919| 918| 						this.SetNextState("GARRISON.APPROACHING");
| 920| 919| 						return;
| 921| 920| 					}
| 922|    |-				}
|    | 921|+				
| 923| 922| 			}
| 924| 923| 
| 925| 924| 			this.SetNextState("GARRISON.GARRISONING");
|    | [NORMAL] ESLintBear (no-unneeded-ternary):
|    | Unnecessary use of boolean literals in conditional expression.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|1958|1958| 					// TODO: we should probably only bother syncing projectile attacks, not melee
|1959|1959| 
|1960|1960| 					// If using a non-default prepare time, re-sync the animation when the timer runs.
|1961|    |-					this.resyncAnimation = (prepare != this.attackTimers.prepare) ? true : false;
|    |1961|+					this.resyncAnimation = (prepare != this.attackTimers.prepare);
|1962|1962| 
|1963|1963| 					this.FaceTowardsTarget(this.order.data.target);
|1964|1964| 
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|2184|2184| 							this.PerformGather(nearby, false, false);
|2185|2185| 							return true;
|2186|2186| 						}
|2187|    |-						else
|2188|    |-						{
|    |2187|+						
|2189|2188| 							// It's probably better in this case, to avoid units getting stuck around a dropsite
|2190|2189| 							// in a "Target is far away, full, nearby are no good resources, return to dropsite" loop
|2191|2190| 							// to order it to GatherNear the resource position.
|2206|2205| 									return true;
|2207|2206| 								}
|2208|2207| 							}
|2209|    |-						}
|    |2208|+						
|2210|2209| 						return true;
|2211|2210| 					}
|2212|2211| 					return false;
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|2196|2196| 								this.GatherNearPosition(pos.x, pos.z, oldType, oldTemplate);
|2197|2197| 								return true;
|2198|2198| 							}
|2199|    |-							else
|2200|    |-							{
|    |2199|+							
|2201|2200| 								// we're kind of stuck here. Return resource.
|2202|2201| 								var nearby = this.FindNearestDropsite(oldType.generic);
|2203|2202| 								if (nearby)
|2205|2204| 									this.PushOrderFront("ReturnResource", { "target": nearby, "force": false });
|2206|2205| 									return true;
|2207|2206| 								}
|2208|    |-							}
|    |2207|+							
|2209|2208| 						}
|2210|2209| 						return true;
|2211|2210| 					}
|    | [NORMAL] ESLintBear (no-unneeded-ternary):
|    | Unnecessary use of boolean literals in conditional expression.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|2580|2580| 					this.StartTimer(prepare, this.healTimers.repeat);
|2581|2581| 
|2582|2582| 					// If using a non-default prepare time, re-sync the animation when the timer runs.
|2583|    |-					this.resyncAnimation = (prepare != this.healTimers.prepare) ? true : false;
|    |2583|+					this.resyncAnimation = (prepare != this.healTimers.prepare);
|2584|2584| 
|2585|2585| 					this.FaceTowardsTarget(this.order.data.target);
|2586|2586| 				},
|    | [NORMAL] ESLintBear (no-unneeded-ternary):
|    | Unnecessary use of boolean literals in conditional expression.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|3396|3396| 
|3397|3397| UnitAI.prototype.IsAnimal = function()
|3398|3398| {
|3399|    |-	return (this.template.NaturalBehaviour ? true : false);
|    |3399|+	return (!!this.template.NaturalBehaviour);
|3400|3400| };
|3401|3401| 
|3402|3402| UnitAI.prototype.IsDangerousAnimal = function()
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|3715|3715| 		// Otherwise we've successfully processed a new order
|3716|3716| 		return true;
|3717|3717| 	}
|3718|    |-	else
|3719|    |-	{
|    |3718|+	
|3720|3719| 		this.SetNextState("IDLE");
|3721|3720| 
|3722|3721| 		Engine.PostMessage(this.entity, MT_UnitAIOrderDataChanged, { "to": this.GetOrderData() });
|3737|3736| 		}
|3738|3737| 
|3739|3738| 		return false;
|3740|    |-	}
|    |3739|+	
|3741|3740| };
|3742|3741| 
|3743|3742| /**
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'Engine'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|4772|4772| 	// If we are guarding/escorting, don't abandon as long as the guarded unit is in target range of the attacker
|4773|4773| 	if (this.isGuardOf)
|4774|4774|

http://jw:8080/job/phabricator_lint/402/ for more details.

fatherbushido added inline comments.Aug 16 2017, 11:07 PM
binaries/data/mods/public/simulation/components/tests/test_Damage.js
328

semicolon

Stan added a subscriber: Stan.Aug 16 2017, 11:08 PM
Stan added inline comments.
binaries/data/mods/public/simulation/components/Attack.js
434–435

GetTemplateBonus ?

binaries/data/mods/public/simulation/components/tests/test_Damage.js
274

What are you testing here ?

449

trailing coma

463

trailing coma

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jw:8080/job/phabricator/1864/ for more details.

Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/simulation/components/tests/test_Attack.js
| 166| »   attackComponentTest(className,·true,·(attacker,·cmpAttack,·defender)·=>·{
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/simulation/components/Attack.js
| 199| »   let·wantedTypesReal·=·wantedTypes.filter(wtype·=>·wtype.indexOf("!")·!=·0);
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

binaries/data/mods/public/simulation/components/Attack.js
| 239| »   ···(!wantedTypes·||·!wantedTypes.filter(wType·=>·wType.indexOf("!")·!=·0).length))
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

binaries/data/mods/public/simulation/components/Attack.js
| 534| »   »   if·(!cmpHealth·||·cmpHealth.GetHitpoints()·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/components/Attack.js
| 586| »   if·(c·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 342| »   }
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
| 911| 911| 					this.FinishOrder();
| 912| 912| 					return;
| 913| 913| 				}
| 914|    |-				else
| 915|    |-				{
|    | 914|+				
| 916| 915| 					// Out of range; move there in formation
| 917| 916| 					if (this.MoveToGarrisonRange(msg.data.target))
| 918| 917| 					{
| 919| 918| 						this.SetNextState("GARRISON.APPROACHING");
| 920| 919| 						return;
| 921| 920| 					}
| 922|    |-				}
|    | 921|+				
| 923| 922| 			}
| 924| 923| 
| 925| 924| 			this.SetNextState("GARRISON.GARRISONING");
|    | [NORMAL] ESLintBear (no-unneeded-ternary):
|    | Unnecessary use of boolean literals in conditional expression.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|1958|1958| 					// TODO: we should probably only bother syncing projectile attacks, not melee
|1959|1959| 
|1960|1960| 					// If using a non-default prepare time, re-sync the animation when the timer runs.
|1961|    |-					this.resyncAnimation = (prepare != this.attackTimers.prepare) ? true : false;
|    |1961|+					this.resyncAnimation = (prepare != this.attackTimers.prepare);
|1962|1962| 
|1963|1963| 					this.FaceTowardsTarget(this.order.data.target);
|1964|1964| 
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|2184|2184| 							this.PerformGather(nearby, false, false);
|2185|2185| 							return true;
|2186|2186| 						}
|2187|    |-						else
|2188|    |-						{
|    |2187|+						
|2189|2188| 							// It's probably better in this case, to avoid units getting stuck around a dropsite
|2190|2189| 							// in a "Target is far away, full, nearby are no good resources, return to dropsite" loop
|2191|2190| 							// to order it to GatherNear the resource position.
|2206|2205| 									return true;
|2207|2206| 								}
|2208|2207| 							}
|2209|    |-						}
|    |2208|+						
|2210|2209| 						return true;
|2211|2210| 					}
|2212|2211| 					return false;
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|2196|2196| 								this.GatherNearPosition(pos.x, pos.z, oldType, oldTemplate);
|2197|2197| 								return true;
|2198|2198| 							}
|2199|    |-							else
|2200|    |-							{
|    |2199|+							
|2201|2200| 								// we're kind of stuck here. Return resource.
|2202|2201| 								var nearby = this.FindNearestDropsite(oldType.generic);
|2203|2202| 								if (nearby)
|2205|2204| 									this.PushOrderFront("ReturnResource", { "target": nearby, "force": false });
|2206|2205| 									return true;
|2207|2206| 								}
|2208|    |-							}
|    |2207|+							
|2209|2208| 						}
|2210|2209| 						return true;
|2211|2210| 					}
|    | [NORMAL] ESLintBear (no-unneeded-ternary):
|    | Unnecessary use of boolean literals in conditional expression.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|2580|2580| 					this.StartTimer(prepare, this.healTimers.repeat);
|2581|2581| 
|2582|2582| 					// If using a non-default prepare time, re-sync the animation when the timer runs.
|2583|    |-					this.resyncAnimation = (prepare != this.healTimers.prepare) ? true : false;
|    |2583|+					this.resyncAnimation = (prepare != this.healTimers.prepare);
|2584|2584| 
|2585|2585| 					this.FaceTowardsTarget(this.order.data.target);
|2586|2586| 				},
|    | [NORMAL] ESLintBear (no-unneeded-ternary):
|    | Unnecessary use of boolean literals in conditional expression.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|3396|3396| 
|3397|3397| UnitAI.prototype.IsAnimal = function()
|3398|3398| {
|3399|    |-	return (this.template.NaturalBehaviour ? true : false);
|    |3399|+	return (!!this.template.NaturalBehaviour);
|3400|3400| };
|3401|3401| 
|3402|3402| UnitAI.prototype.IsDangerousAnimal = function()
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|3715|3715| 		// Otherwise we've successfully processed a new order
|3716|3716| 		return true;
|3717|3717| 	}
|3718|    |-	else
|3719|    |-	{
|    |3718|+	
|3720|3719| 		this.SetNextState("IDLE");
|3721|3720| 
|3722|3721| 		Engine.PostMessage(this.entity, MT_UnitAIOrderDataChanged, { "to": this.GetOrderData() });
|3737|3736| 		}
|3738|3737| 
|3739|3738| 		return false;
|3740|    |-	}
|    |3739|+	
|3741|3740| };
|3742|3741| 
|3743|3742| /**
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'Engine'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|4772|4772| 	// If we are guarding/escorting, don't abandon as long as the guarded unit is in target range of the attacker
|4773|4773| 	if (this.isGuardOf)
|4774|4774| 	{
|4775|    |-		var cmpUnitAI =  Engine.QueryInterface(target, IID_UnitAI);
|    |4775|+		var cmpUnitAI = Engine.QueryInterface(target, IID_UnitAI);
|4776|4776| 		var cmpAttack = Engine.QueryInterface(target, IID_Attack);
|4777|4777| 		if (cmpUnitAI && cmpAttack &&
|4778|4778| 		    cmpAttack.GetAttackTypes().some(type => cmpUnitAI.CheckTargetAttackRange(this.isGuardOf, type)))
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'Engine'.
|----|

http://jw:8080/job/phabricator_lint/403/ for more details.

elexis added inline comments.Aug 16 2017, 11:14 PM
binaries/data/mods/public/simulation/components/Attack.js
442

(If that is cloned because some later use of that template data modifies the template and then some later later use of that template data expects to use the original template while processing the modified template data, then this wouldn't silently process unintentional values anymore but throw an error if #4257 is implemented, while either the clone or creating a new object from scratch is still needed to fix that)

fatherbushido added inline comments.Aug 16 2017, 11:42 PM
binaries/data/mods/public/simulation/components/Attack.js
442

in fact, I don't really understand what's the plan for that ticket. But it s indeed straightly linked.

fatherbushido added inline comments.Aug 16 2017, 11:50 PM
binaries/data/mods/public/simulation/components/tests/test_Damage.js
274

my bread toaster?

449

One likes it, another doesn't.
I have no personal taste about that.

Stan added inline comments.Aug 16 2017, 11:54 PM
binaries/data/mods/public/simulation/components/tests/test_Damage.js
274

Sounds fine keep me some bread to hold for the night

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jw:8080/job/phabricator/1865/ for more details.

Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/simulation/helpers/DamageBonus.js
|  18| »   »   if·(bonus.Classes·&&·bonus.Classes.split(/\s+/).some(cls·=>·!cmpIdentity.HasClass(cls)))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/simulation/components/tests/test_Attack.js
| 166| »   attackComponentTest(className,·true,·(attacker,·cmpAttack,·defender)·=>·{
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/simulation/components/Attack.js
| 199| »   let·wantedTypesReal·=·wantedTypes.filter(wtype·=>·wtype.indexOf("!")·!=·0);
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

binaries/data/mods/public/simulation/components/Attack.js
| 239| »   ···(!wantedTypes·||·!wantedTypes.filter(wType·=>·wType.indexOf("!")·!=·0).length))
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

binaries/data/mods/public/simulation/components/Attack.js
| 534| »   »   if·(!cmpHealth·||·cmpHealth.GetHitpoints()·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/components/Attack.js
| 586| »   if·(c·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
| 911| 911| 					this.FinishOrder();
| 912| 912| 					return;
| 913| 913| 				}
| 914|    |-				else
| 915|    |-				{
|    | 914|+				
| 916| 915| 					// Out of range; move there in formation
| 917| 916| 					if (this.MoveToGarrisonRange(msg.data.target))
| 918| 917| 					{
| 919| 918| 						this.SetNextState("GARRISON.APPROACHING");
| 920| 919| 						return;
| 921| 920| 					}
| 922|    |-				}
|    | 921|+				
| 923| 922| 			}
| 924| 923| 
| 925| 924| 			this.SetNextState("GARRISON.GARRISONING");
|    | [NORMAL] ESLintBear (no-unneeded-ternary):
|    | Unnecessary use of boolean literals in conditional expression.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|1958|1958| 					// TODO: we should probably only bother syncing projectile attacks, not melee
|1959|1959| 
|1960|1960| 					// If using a non-default prepare time, re-sync the animation when the timer runs.
|1961|    |-					this.resyncAnimation = (prepare != this.attackTimers.prepare) ? true : false;
|    |1961|+					this.resyncAnimation = (prepare != this.attackTimers.prepare);
|1962|1962| 
|1963|1963| 					this.FaceTowardsTarget(this.order.data.target);
|1964|1964| 
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|2184|2184| 							this.PerformGather(nearby, false, false);
|2185|2185| 							return true;
|2186|2186| 						}
|2187|    |-						else
|2188|    |-						{
|    |2187|+						
|2189|2188| 							// It's probably better in this case, to avoid units getting stuck around a dropsite
|2190|2189| 							// in a "Target is far away, full, nearby are no good resources, return to dropsite" loop
|2191|2190| 							// to order it to GatherNear the resource position.
|2206|2205| 									return true;
|2207|2206| 								}
|2208|2207| 							}
|2209|    |-						}
|    |2208|+						
|2210|2209| 						return true;
|2211|2210| 					}
|2212|2211| 					return false;
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|2196|2196| 								this.GatherNearPosition(pos.x, pos.z, oldType, oldTemplate);
|2197|2197| 								return true;
|2198|2198| 							}
|2199|    |-							else
|2200|    |-							{
|    |2199|+							
|2201|2200| 								// we're kind of stuck here. Return resource.
|2202|2201| 								var nearby = this.FindNearestDropsite(oldType.generic);
|2203|2202| 								if (nearby)
|2205|2204| 									this.PushOrderFront("ReturnResource", { "target": nearby, "force": false });
|2206|2205| 									return true;
|2207|2206| 								}
|2208|    |-							}
|    |2207|+							
|2209|2208| 						}
|2210|2209| 						return true;
|2211|2210| 					}
|    | [NORMAL] ESLintBear (no-unneeded-ternary):
|    | Unnecessary use of boolean literals in conditional expression.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|2580|2580| 					this.StartTimer(prepare, this.healTimers.repeat);
|2581|2581| 
|2582|2582| 					// If using a non-default prepare time, re-sync the animation when the timer runs.
|2583|    |-					this.resyncAnimation = (prepare != this.healTimers.prepare) ? true : false;
|    |2583|+					this.resyncAnimation = (prepare != this.healTimers.prepare);
|2584|2584| 
|2585|2585| 					this.FaceTowardsTarget(this.order.data.target);
|2586|2586| 				},
|    | [NORMAL] ESLintBear (no-unneeded-ternary):
|    | Unnecessary use of boolean literals in conditional expression.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|3396|3396| 
|3397|3397| UnitAI.prototype.IsAnimal = function()
|3398|3398| {
|3399|    |-	return (this.template.NaturalBehaviour ? true : false);
|    |3399|+	return (!!this.template.NaturalBehaviour);
|3400|3400| };
|3401|3401| 
|3402|3402| UnitAI.prototype.IsDangerousAnimal = function()
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|3715|3715| 		// Otherwise we've successfully processed a new order
|3716|3716| 		return true;
|3717|3717| 	}
|3718|    |-	else
|3719|    |-	{
|    |3718|+	
|3720|3719| 		this.SetNextState("IDLE");
|3721|3720| 
|3722|3721| 		Engine.PostMessage(this.entity, MT_UnitAIOrderDataChanged, { "to": this.GetOrderData() });
|3737|3736| 		}
|3738|3737| 
|3739|3738| 		return false;
|3740|    |-	}
|    |3739|+	
|3741|3740| };
|3742|3741| 
|3743|3742| /**
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'Engine'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|4772|4772| 	// If we are guarding/escorting, don't abandon as long as the guarded unit is in target range of the attacker
|4773|4773| 	if (this.isGuardOf)
|4774|4774| 	{
|4775|    |-		var cmpUnitAI =  Engine.QueryInterface(target, IID_UnitAI);
|    |4775|+		var cmpUnitAI = Engine.QueryInterface(target, IID_UnitAI);
|4776|4776| 		var cmpAttack = Engine.QueryInterface(target, IID_Attack);
|4777|4777| 		if (cmpUnitAI && cmpAttack &&
|4778|4778| 		    cmpAttack.GetAttackTypes().some(type => cmpUnitAI.CheckTargetAttackRange(this.isGuardOf, type)))
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'Engine'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|4819|4819| 	// If we are guarding/escorting, chase at least as long as the guarded unit is in target range of the attacker
|4820|4820| 	if (this.isGuardOf)
|4821|4821| 	{
|4822|    |-		var cmpUnitAI =  Engine.QueryInterface(target, IID_UnitAI);
|    |4822|+		var cmpUnitAI = Engine.QueryInterface(target, IID_UnitAI);
|4823|4823| 		var cmpAttack = Engine.QueryInterface(target, IID_Attack);
|4824|4824| 		if (cmpUnitAI && cmpAttack &&
|4825|4825| 		    cmpAttack.GetAttackTypes().some(type => cmpUnitAI.CheckTargetAttackRange(this.isGuardOf, type)))
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|4993|4993| 	{
|4994|4994| 		if (this.isGuardOf == target && this.order && this.order.type == "Guard")
|4995|4995| 			return;
|4996|    |-		else
|4997|    |-			this.RemoveGuard();
|    |4996|+		this.RemoveGuard();
|4998|4997| 	}
|4999|4998| 
|5000|4999| 	this.AddOrder("Guard", { "target": target, "force": false }, queued);
|    | [NORMAL] ESLintBear (no-undef-init):
|    | It's not necessary to initialize 'lastPos' to undefined.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|5241|5241| 
|5242|5242| 	// Remember the position of our target, if any, in case it disappears
|5243|5243| 	// later and we want to head to its last known position
|5244|    |-	var lastPos = undefined;
|    |5244|+	var lastPos;
|5245|5245| 	var cmpPosition = Engine.QueryInterface(target, IID_Position);
|5246|5246| 	if (cmpPosition && cmpPosition.IsInWorld())
|5247|5247| 		lastPos = cmpPosition.GetPosition();

binaries/data/mods/public/simulation/components/UnitAI.js
|2482| »   »   »   »   »   »   let·nearby·=·this.FindNearestDropsite(resourceType.generic);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'nearby' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|3904| »   var·isWorkType·=·type·=>·type·==·"Gather"·||·type·==·"Trade"·||·type·==·"Repair"·||·type·==·"ReturnResource";
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'type' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4229| »   »   let·cmpOwnership·=·Engine.QueryInterface(ent,·IID_Ownership);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'cmpOwnership' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4694| »   var·target·=·ents.find(target·=>·this.CanAttack(target,·forceResponse));
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'target' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4709| »   var·target·=·ents.find(target·=>
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'target' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4755| »   var·ent·=·ents.find(ent·=>·this.CanHeal(ent));
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'ent' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4778| »   »   ····cmpAttack.GetAttackTypes().some(type·=>·cmpUnitAI.CheckTargetAttackRange(this.isGuardOf,·type)))
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'type' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/UnitAI.js
| 363| »   »   ····&&·(this.lastShorelinePosition.z·==·cmpPosition.GetPosition().z))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|1951| »   »   »   »   »   »   var·cmpFormation·=·Engine.QueryInterface(this.formationController,·IID_Formation);
|    | [NORMAL] JSHintBear:
|    | 'cmpFormation' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|2177| »   »   »   »   »   »   »   »   ·&&·((type.generic·==·"treasure"·&&·oldType.generic·==·"treasure")
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|2178| »   »   »   »   »   »   »   »   ·||·(type.specific·==·oldType.specific
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|2179| »   »   »   »   »   »   »   »   ·&&·(type.specific·!=·"meat"·||·oldTemplate·==·template)))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|2202| »   »   »   »   »   »   »   »   var·nearby·=·this.FindNearestDropsite(oldType.generic);
|    | [NORMAL] JSHintBear:
|    | 'nearby' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|2242| »   »   »   »   »   »   »   »   &&·((type.generic·==·"treasure"·&&·oldType.generic·==·"treasure")
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|2243| »   »   »   »   »   »   »   »   ||·(type.specific·==·oldType.specific
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|2244| »   »   »   »   »   »   »   »   &&·(type.specific·!=·"meat"·||·oldTemplate·==·template)))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|2288| »   »   »   »   »   »   »   ||·(type.specific·==·resourceType.specific
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|2289| »   »   »   »   »   »   »   &&·(type.specific·!=·"meat"·||·resourceTemplate·==·template))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|2305| »   »   »   »   »   var·nearby·=·this.FindNearestDropsite(resourceType.generic);
|    | [NORMAL] JSHintBear:
|    | 'nearby' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|2477| »   »   »   »   »   var·cmpResourceGatherer·=·Engine.QueryInterface(this.entity,·IID_ResourceGatherer);
|    | [NORMAL] JSHintBear:
|    | 'cmpResourceGatherer' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|2493| »   »   »   »   »   var·nearby·=·this.FindNearbyResource(function·(ent,·type,·template)·{
|    | [NORMAL] JSHintBear:
|    | 'nearby' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|2496| »   »   »   »   »   »   »   ||·(type.specific·==·resourceType.specific
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|2497| »   »   »   »   »   »   »   &&·(type.specific·!=·"meat"·||·resourceTemplate·==·template))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|2517| »   »   »   »   »   var·nearby·=·this.FindNearestDropsite(resourceType.generic);
|    | [NORMAL] JSHintBear:
|    | 'nearby' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|2701| »   »   »   »   »   var·cmpResourceGatherer·=·Engine.QueryInterface(this.entity,·IID_ResourceGatherer);
|    | [NORMAL] JSHintBear:
|    | 'cmpResourceGatherer' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|2896| »   »   »   »   »   var·cmpResourceDropsite·=·Engine.QueryInterface(msg.data.newentity,·IID_ResourceDropsite);
|    | [NORMAL] JSHintBear:
|    | 'cmpResourceDropsite' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|2991| »   »   »   »   »   »   var·target·=·this.alertGarrisoningTarget;
|    | [NORMAL] JSHintBear:
|    | 'target' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|2995| »   »   »   »   »   if·(this.CanGarrison(target))
|    | [NORMAL] JSHintBear:
|    | 'target' used out of scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|2998| »   »   »   »   »   »   if·(this.CheckGarrisonRange(target))
|    | [NORMAL] JSHintBear:
|    | 'target' used out of scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|3000| »   »   »   »   »   »   »   var·cmpGarrisonHolder·=·Engine.QueryInterface(target,·IID_GarrisonHolder);
|    | [NORMAL] JSHintBear:
|    | 'target' used out of scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|3022| »   »   »   »   »   »   »   »   var·cmpResourceDropsite·=·Engine.QueryInterface(target,·IID_ResourceDropsite);
|    | [NORMAL] JSHintBear:
|    | 'target' used out of scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|3023| »   »   »   »   »   »   »   »   if·(cmpResourceDropsite·&&·this.CanReturnResource(target,·true))
|    | [NORMAL] JSHintBear:
|    | 'target' used out of scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|3038| »   »   »   »   »   »   »   »   »   var·cmpHolderPosition·=·Engine.QueryInterface(target,·IID_Position);
|    | [NORMAL] JSHintBear:
|    | 'target' used out of scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|3039| »   »   »   »   »   »   »   »   »   var·cmpHolderUnitAI·=·Engine.QueryInterface(target,·IID_UnitAI);
|    | [NORMAL] JSHintBear:
|    | 'target' used out of scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|3066| »   »   »   »   »   »   »   if·(this.MoveToTarget(target))
|    | [NORMAL] JSHintBear:
|    | 'target' used out of scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|3363| »   return·this.alertRaiser·!=·undefined;
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'undefined'.

binaries/data/mods/public/simulation/components/UnitAI.js
|3505| »   »   »   ||·this.orderQueue[0].type·==·"Pack"·||·this.orderQueue[0].type·==·"Unpack")))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|3569| »   »   if·(i·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/components/UnitAI.js
|3862| »   »   var·order·=·{·"type":·type,·"data":·data·};
|    | [NORMAL] JSHintBear:
|    | 'order' is already defined.

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

binaries/data/mods/public/simulation/components/UnitAI.js
|3947| »   if·(this.workOrders.length·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/components/UnitAI.js
|4133| »   return·cmpHealth·&&·cmpHealth.GetHitpoints()·!=·0;
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

binaries/data/mods/public/simulation/components/UnitAI.js
|4472| »   »   var·parabolicMaxRange·=·0;
|    | [NORMAL] JSHintBear:
|    | 'parabolicMaxRange' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|4476| »   var·guessedMaxRange·=·(range.max·+·parabolicMaxRange)/2;
|    | [NORMAL] JSHintBear:
|    | 'parabolicMaxRange' used out of scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4483| »   return·cmpUnitMotion.MoveToTargetRange(target,·range.min,·Math.min(range.max,·parabolicMaxRange));
|    | [NORMAL] JSHintBear:
|    | 'parabolicMaxRange' used out of scope.

binaries/data/mods/public/simulation/components/UnitAI.js
|4539| »   »   »   &&·cmpFormationUnitAI.order.data.target·==·target)
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|4711| »   »   &&·this.CheckTargetDistanceFromHeldPosition(target,·IID_Attack,·this.GetBestAttackAgainst(target,·true))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|4712| »   »   &&·(this.GetStance().respondChaseBeyondVision·||·this.CheckTargetIsInVisionRange(target))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|5244| »   var·lastPos·=·undefined;
|    | [NORMAL] JSHintBear:
|    | It's not necessary to initialize 'lastPos' to 'undefined'.

binaries/data/mods/public/simulation/components/UnitAI.js
|5583| »   »   »   »   »   »   &&·!MatchesClassList(cmpIdentity.GetClassesList(),·targetClasses.attack))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|5586| »   »   »   »   »   »   &&·MatchesClassList(cmpIdentity.GetClassesList(),·targetClasses.avoid))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|5599| »   var·targets·=·this.GetTargetsFromUnit();
|    | [NORMAL] JSHintBear:
|    | 'targets' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5600| »   for·(var·targ·of·targets)
|    | [NORMAL] JSHintBear:
|    | 'targ' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5606| »   »   »   var·cmpIdentity·=·Engine.QueryInterface(targ,·IID_Identity);
|    | [NORMAL] JSHintBear:
|    | 'cmpIdentity' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5607| »   »   »   var·targetClasses·=·this.order.data.targetClasses;
|    | [NORMAL] JSHintBear:
|    | 'targetClasses' is already defined.

binaries/data/mods/public/simulation/components/UnitAI.js
|5609| »   »   »   »   &&·!MatchesClassList(cmpIdentity.GetClassesList(),·targetClasses.attack))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|5612| »   »   »   »   &&·MatchesClassList(cmpIdentity.GetClassesList(),·targetClasses.avoid))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '&&'; readers may interpret this as an expression boundary.

binaries/data/mods/public/simulation/components/UnitAI.js
|5612| »   »   »   »   &&·MatchesClassList(cmpIdentity.GetClassesList(),·targetClasses.avoid))
|    | [MAJOR] JSHintBear:
|    | Too many errors. (91% scanned).

binaries/data/mods/public/simulation/ai/petra/entityExtend.js
| 178| »   »   if·(bonus.Classes·&&·bonus.Classes.split(/\s+/).some(cls·=>·!target.hasClass(cls)))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/simulation/components/tests/test_Damage.js
| 342| »   }
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/404/ for more details.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jw:8080/job/phabricator/1868/ for more details.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jw:8080/job/phabricator/1869/ for more details.

Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/simulation/helpers/DamageBonus.js
|  18| »   »   if·(bonus.Classes·&&·bonus.Classes.split(/\s+/).some(cls·=>·!cmpIdentity.HasClass(cls)))
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/simulation/components/tests/test_Attack.js
| 166| »   attackComponentTest(className,·true,·(attacker,·cmpAttack,·defender)·=>·{
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.

binaries/data/mods/public/simulation/components/Attack.js
| 199| »   let·wantedTypesReal·=·wantedTypes.filter(wtype·=>·wtype.indexOf("!")·!=·0);
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

binaries/data/mods/public/simulation/components/Attack.js
| 239| »   ···(!wantedTypes·||·!wantedTypes.filter(wType·=>·wType.indexOf("!")·!=·0).length))
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with '0'.

binaries/data/mods/public/simulation/components/Attack.js
| 534| »   »   if·(!cmpHealth·||·cmpHealth.GetHitpoints()·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.

binaries/data/mods/public/simulation/components/Attack.js
| 586| »   if·(c·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
| 911| 911| 					this.FinishOrder();
| 912| 912| 					return;
| 913| 913| 				}
| 914|    |-				else
| 915|    |-				{
|    | 914|+				
| 916| 915| 					// Out of range; move there in formation
| 917| 916| 					if (this.MoveToGarrisonRange(msg.data.target))
| 918| 917| 					{
| 919| 918| 						this.SetNextState("GARRISON.APPROACHING");
| 920| 919| 						return;
| 921| 920| 					}
| 922|    |-				}
|    | 921|+				
| 923| 922| 			}
| 924| 923| 
| 925| 924| 			this.SetNextState("GARRISON.GARRISONING");
|    | [NORMAL] ESLintBear (no-unneeded-ternary):
|    | Unnecessary use of boolean literals in conditional expression.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|1958|1958| 					// TODO: we should probably only bother syncing projectile attacks, not melee
|1959|1959| 
|1960|1960| 					// If using a non-default prepare time, re-sync the animation when the timer runs.
|1961|    |-					this.resyncAnimation = (prepare != this.attackTimers.prepare) ? true : false;
|    |1961|+					this.resyncAnimation = (prepare != this.attackTimers.prepare);
|1962|1962| 
|1963|1963| 					this.FaceTowardsTarget(this.order.data.target);
|1964|1964| 
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|2184|2184| 							this.PerformGather(nearby, false, false);
|2185|2185| 							return true;
|2186|2186| 						}
|2187|    |-						else
|2188|    |-						{
|    |2187|+						
|2189|2188| 							// It's probably better in this case, to avoid units getting stuck around a dropsite
|2190|2189| 							// in a "Target is far away, full, nearby are no good resources, return to dropsite" loop
|2191|2190| 							// to order it to GatherNear the resource position.
|2206|2205| 									return true;
|2207|2206| 								}
|2208|2207| 							}
|2209|    |-						}
|    |2208|+						
|2210|2209| 						return true;
|2211|2210| 					}
|2212|2211| 					return false;
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|2196|2196| 								this.GatherNearPosition(pos.x, pos.z, oldType, oldTemplate);
|2197|2197| 								return true;
|2198|2198| 							}
|2199|    |-							else
|2200|    |-							{
|    |2199|+							
|2201|2200| 								// we're kind of stuck here. Return resource.
|2202|2201| 								var nearby = this.FindNearestDropsite(oldType.generic);
|2203|2202| 								if (nearby)
|2205|2204| 									this.PushOrderFront("ReturnResource", { "target": nearby, "force": false });
|2206|2205| 									return true;
|2207|2206| 								}
|2208|    |-							}
|    |2207|+							
|2209|2208| 						}
|2210|2209| 						return true;
|2211|2210| 					}
|    | [NORMAL] ESLintBear (no-unneeded-ternary):
|    | Unnecessary use of boolean literals in conditional expression.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|2580|2580| 					this.StartTimer(prepare, this.healTimers.repeat);
|2581|2581| 
|2582|2582| 					// If using a non-default prepare time, re-sync the animation when the timer runs.
|2583|    |-					this.resyncAnimation = (prepare != this.healTimers.prepare) ? true : false;
|    |2583|+					this.resyncAnimation = (prepare != this.healTimers.prepare);
|2584|2584| 
|2585|2585| 					this.FaceTowardsTarget(this.order.data.target);
|2586|2586| 				},
|    | [NORMAL] ESLintBear (no-unneeded-ternary):
|    | Unnecessary use of boolean literals in conditional expression.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|3396|3396| 
|3397|3397| UnitAI.prototype.IsAnimal = function()
|3398|3398| {
|3399|    |-	return (this.template.NaturalBehaviour ? true : false);
|    |3399|+	return (!!this.template.NaturalBehaviour);
|3400|3400| };
|3401|3401| 
|3402|3402| UnitAI.prototype.IsDangerousAnimal = function()
|    | [NORMAL] ESLintBear (no-else-return):
|    | Unnecessary 'else' after 'return'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|3715|3715| 		// Otherwise we've successfully processed a new order
|3716|3716| 		return true;
|3717|3717| 	}
|3718|    |-	else
|3719|    |-	{
|    |3718|+	
|3720|3719| 		this.SetNextState("IDLE");
|3721|3720| 
|3722|3721| 		Engine.PostMessage(this.entity, MT_UnitAIOrderDataChanged, { "to": this.GetOrderData() });
|3737|3736| 		}
|3738|3737| 
|3739|3738| 		return false;
|3740|    |-	}
|    |3739|+	
|3741|3740| };
|3742|3741| 
|3743|3742| /**
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'Engine'.
|----|    | /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/phabricator_lint/binaries/data/mods/public/simulation/components/UnitAI.js
|4772|4772| 	// If we are guarding/escorting, don't abandon as long as the guarded unit is in target range of the attacker
|4773|4773| 	if (this.isGuardOf)
|4774|4774| 	{
|4775|    |-		var cmpUnitAI =  Engine.QueryInterface(target, IID_UnitAI);
|    |4775|+		var cmpUnitAI = Engine.QueryInterface(target, IID_UnitAI);
|4776|4776| 		var cmpAttack = Engine.QueryInterface(target, IID_Attack);
|4777|4777| 		if (cmpUnitAI && cmpAttack &&
|4778|4778| 		    cmpAttack.GetAttackTypes().some(type => cmpUnitAI.CheckTargetAttackRange(this.isGuardOf, type

http://jw:8080/job/phabricator_lint/405/ for more details.

fatherbushido added inline comments.Aug 17 2017, 9:50 AM
binaries/data/mods/public/simulation/components/tests/test_Damage.js
274

;-)

Mate-86 edited edge metadata.Aug 17 2017, 12:39 PM

If we want to have suicide units like in AoE, it may worth to implement the bonus for DeathDamage also (eg. petard deals extra damage against buildings but not against units): http://ageofempires.wikia.com/wiki/Suicide_Units

binaries/data/mods/public/simulation/components/tests/test_Damage.js
449
521

This function is 200+ lines long and does multiple things. Does that make sense to split it?

binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js
68

shouldn't this multiplier used during the bonus damage calculation?

72

Shouldn't an assertion come after this call? Eg. checking that cmpDamage.CauseSplashDamage called with the right parameters?

binaries/data/mods/public/simulation/helpers/DamageBonus.js
19

Shouldn't this condition be the other way around? Eg. bonus.Classes = "Infantry", "Cavarly" and when the target has class Infantry then the attack should remove the bonus. Or did I misunderstand this feature?

fatherbushido marked an inline comment as done.Aug 17 2017, 1:15 PM

Thanks for the comments. (Reviewing other code has benefits for the reviewed guy and for the reviewer.)

binaries/data/mods/public/simulation/components/tests/test_Damage.js
521

No real sense to split it. I guess we could concatenate stuff a bit but we'll lost readability - though that's mostly a matter of tastes). We've also many lines due to not inlined object identation.

binaries/data/mods/public/simulation/components/tests/test_DeathDamage.js
68

Not exactly for damage calculation, but that should be parsed to CauseSplashDamage.
And the test check that (for example remove L70).

72

It is (L52).
(It's a good question as it's indeed the purpose of the test).

binaries/data/mods/public/simulation/helpers/DamageBonus.js
19

It's the bonus of the attacker against entities which have one of those classes.

(@Mate-86: I would like that one to be fixed/commited before adressing the other damage things.)

Mate-86 accepted this revision.Aug 20 2017, 9:06 PM

I reviewed the changed:

  • all my questions were answered satisfactorily (I even learnt new things, eg. placing the assertion in the mock function)
  • I've tested the changes locally by setting different kind of DeathDamage bonuses for the Fireship
  • all the changes are logical and properly covered with unit tests
This revision is now accepted and ready to land.Aug 20 2017, 9:06 PM
This revision was automatically updated to reflect the committed changes.
elexis added inline comments.Sep 3 2017, 6:10 PM
binaries/data/mods/public/simulation/components/Attack.js
442

The exact issue (of this.template being unintentionally modifyable) is documented in #4759. If it was fixed, we would get an JS error instead of a wrong number in a template. Same for rP19623.