Page MenuHomeWildfire Games

Let "GetBestAttackAgainst" depend on DPS.
Needs ReviewPublic

Authored by Freagarach on Thu, Jul 4, 8:12 PM.

Details

Reviewers
None
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Summary

This will let the result of the function "GetBestAttackAgainst" depend on DPS against the target, corrected for armour.
This is not compleat yet (test also fails), but uploaded for feedback.
@Angen, @Stan some questions:

  • Probably a function GetExpectedDamage should be created in Armour.js?
  • What to do with the "preference" of an attack? I guess that should have priority, but I'm not sure.

(Requested in D1965.)

Test Plan

NOT YET: Give an entity more than one attack type, check that the returned type is the one with the highes DPS.

Diff Detail

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

Unit TestsFailed

TimeTest
0 msJenkins > cxxtest_debug.xml::[failed-to-read]
Failed to read test report file /mnt/data/jenkins-phabricator/workspace/differential/cxxtest_debug.xml org.dom4j.DocumentException: Error on line 1 of document : Content is not allowed in prolog. Nested exception: Content is not allowed in prolog. at org.dom4j.io.SAXReader.read(SAXReader.java:482)
0 msJenkins > TestAllocators::test_da
0 msJenkins > TestAtlasObjectXML::test_parse_attributes1
0 msJenkins > TestAtlasObjectXML::test_parse_attributes2
0 msJenkins > TestAtlasObjectXML::test_parse_basic
View Full Test Results (1 Failed · 322 Passed)

Event Timeline

Freagarach created this revision.Thu, Jul 4, 8:12 PM
FeXoR added a subscriber: FeXoR.EditedThu, Jul 4, 8:33 PM

Hi @Freagarach !

Please explain to me the concept you have in mind what should be done with this function.
I don't know this part of the code well and don't have the full source to crawl at my disposal ATM.
I see a lot of potential for this function, just by the name, to break a lot of stuff.
Let's start with:

When is this function called?

A) If by the unitAI to determine what attack to use: B) Does this function take into account range? (I can't see that)
And if A and not B then won't it cause e.g. units to run towards an enemy and just die instead of attacking if the melee attack does happen to be more damaging than the ranged attack to the chosen target?
And won't that break the unit AI even further to have such a function in the first place?

(So I really hope that A is false!)

Vulcan added a comment.Thu, Jul 4, 8:33 PM

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
| 431| 431| 		warn(uneval(type));
| 432| 432| 		let damage = this.GetAttackStrengths(type);
| 433| 433| 		warn(uneval(damage));
| 434|    |-		let damageBonus = GetDamageBonus(this.entity, target, type, this.GetBonusTemplate(type))
|    | 434|+		let damageBonus = GetDamageBonus(this.entity, target, type, this.GetBonusTemplate(type));
| 435| 435| 		warn(uneval(damageBonus));
| 436| 436| 		Object.keys(damage).map(function(type, index) {
| 437| 437| 			damage[type] *= damageBonus;
|    | [NORMAL] ESLintBear (comma-spacing):
|    | A space is required after ','.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
| 442| 442| 		warn(uneval(expectedDamage));
| 443| 443| 		expectedDamagePerType[type] = expectedDamage;
| 444| 444| 	}
| 445|    |-	let typesSorted = Object.keys(expectedDamagePerType).sort(function(a,b){
|    | 445|+	let typesSorted = Object.keys(expectedDamagePerType).sort(function(a, b){
| 446| 446| 		return expectedDamagePerType[a] - expectedDamagePerType[b]
| 447| 447| 	});
| 448| 448| 	warn(uneval(typesSorted));
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
| 443| 443| 		expectedDamagePerType[type] = expectedDamage;
| 444| 444| 	}
| 445| 445| 	let typesSorted = Object.keys(expectedDamagePerType).sort(function(a,b){
| 446|    |-		return expectedDamagePerType[a] - expectedDamagePerType[b]
|    | 446|+		return expectedDamagePerType[a] - expectedDamagePerType[b];
| 447| 447| 	});
| 448| 448| 	warn(uneval(typesSorted));
| 449| 449| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 0.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
| 452| 452| 	let type = types.sort((a, b) =>
| 453| 453| 		(types.indexOf(a) + (isPreferred(a) ? types.length : 0)) -
| 454| 454| 		(types.indexOf(b) + (isPreferred(b) ? types.length : 0))).pop();
| 455|    |-warn(uneval(type));
|    | 455|+	warn(uneval(type));
| 456| 456| 
| 457| 457| 	return type;
| 458| 458| };
|    | [NORMAL] ESLintBear (spaced-comment):
|    | Expected space or tab after '//' in comment.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
| 559| 559| 
| 560| 560| 		let horizSpeed = +this.template[type].Projectile.Speed;
| 561| 561| 		let gravity = +this.template[type].Projectile.Gravity;
| 562|    |-		//horizSpeed /= 2; gravity /= 2; // slow it down for testing
|    | 562|+		// horizSpeed /= 2; gravity /= 2; // slow it down for testing
| 563| 563| 
| 564| 564| 		let cmpPosition = Engine.QueryInterface(this.entity, IID_Position);
| 565| 565| 		if (!cmpPosition || !cmpPosition.IsInWorld())
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
| 606| 606| 		// TODO: Use unit rotation to implement x/z offsets.
| 607| 607| 		let deltaLaunchPoint = new Vector3D(0, this.template[type].Projectile.LaunchPoint["@y"], 0.0);
| 608| 608| 		let launchPoint = Vector3D.add(selfPosition, deltaLaunchPoint);
| 609|    |-		
|    | 609|+
| 610| 610| 		let cmpVisual = Engine.QueryInterface(this.entity, IID_Visual);
| 611| 611| 		if (cmpVisual)
| 612| 612| 		{
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'else'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
| 678| 678| 			});
| 679| 679| 	}
| 680| 680| 	else
| 681|    |-	{
|    | 681|+	
| 682| 682| 		// Melee attack - hurt the target immediately
| 683| 683| 		cmpDamage.CauseDamage({
| 684| 684| 			"strengths": this.GetAttackStrengths(type),
| 688| 688| 			"type": type,
| 689| 689| 			"attackerOwner": attackerOwner
| 690| 690| 		});
| 691|    |-	}
|    | 691|+	
| 692| 692| };
| 693| 693| 
| 694| 694| /**

binaries/data/mods/public/simulation/components/Attack.js
| 436| »   »   Object.keys(damage).map(function(type,·index)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'type' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/Attack.js
| 440| »   »   for·(let·type·in·damage)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'type' is already declared in the upper scope.

binaries/data/mods/public/simulation/components/Attack.js
| 440| »   »   for·(let·type·in·damage)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'type' is already declared in the upper scope.

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

binaries/data/mods/public/simulation/components/Attack.js
| 434| »   »   let·damageBonus·=·GetDamageBonus(this.entity,·target,·type,·this.GetBonusTemplate(type))
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

binaries/data/mods/public/simulation/components/Attack.js
| 446| »   »   return·expectedDamagePerType[a]·-·expectedDamagePerType[b]
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

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

Link to build: https://jenkins.wildfiregames.com/job/differential/1896/display/redirect

Freagarach edited the summary of this revision. (Show Details)Thu, Jul 4, 8:46 PM
In D2038#84952, @FeXoR wrote:

When is this function called?
A) If by the unitAI to determine what attack to use: B) Does this function take into account range? (I can't see that)
And if A and not B then won't it cause e.g. units to run towards an enemy and just die instead of attacking if the melee attack does happen to be more damaging than the ranged attack to the chosen target?
And won't that break the unit AI even further to have such a function in the first place?
(So I really hope that A is false!)

A. This does not (yet) take range into account.
I was trying to get this:

In D1965#84927, @Angen wrote:

Generally

  1. GetBestAttackAgainst - has to check attack damages vs armour damages
  2. CanAttack - the same as 1

Reason : unit having only pierce damage can be now send to fight unit with infinit pierce armour
The same logic applies for AI, see AttackPlan.js function update there is sorting of tartgets, needs to be changed to filter out entities one cannot harm at all.

working generic.

Angen added a comment.Thu, Jul 4, 8:52 PM

see as it is handled at https://trac.wildfiregames.com/attachment/ticket/252/t252_secondattack_21.2.diff

(I thought just to filter out attack types one cannot deal damage with depending on amrour. It would be faster and more simple to do than dps. But ok :) )

Freagarach added a subscriber: bb.Thu, Jul 4, 9:12 PM
In D2038#84952, @FeXoR wrote:

Hi @Freagarach !
Please explain to me the concept you have in mind what should be done with this function.
I don't know this part of the code well and don't have the full source to crawl at my disposal ATM.
I see a lot of potential for this function, just by the name, to break a lot of stuff.
Let's start with:
When is this function called?
A) If by the unitAI to determine what attack to use: B) Does this function take into account range? (I can't see that)
And if A and not B then won't it cause e.g. units to run towards an enemy and just die instead of attacking if the melee attack does happen to be more damaging than the ranged attack to the chosen target?
And won't that break the unit AI even further to have such a function in the first place?
(So I really hope that A is false!)

Hi @FeXoR!
Thanks for the feedback! I agree that this might be a tough nut to crack.
The concept I have in mind is that, when entities can have multiple attack types, the best attack type against a target is chosen.
Range ought to be taken into account with some factor I guess, but that also seems like throwing in magic numbers.
It should also be made possible for the player to override this function (see in the diff provided by @Angen prefAttackType).

In D2038#84960, @Angen wrote:

Yeah, there it is assumed that only Melee and Ranged are left and if out of range of Ranged the entity will just run towards the target anyways when the Melee DPS is larger ;) (Don't know how much @bb has improved that code in the meantime?)

In D2038#84960, @Angen wrote:

(I thought just to filter out attack types one cannot deal damage with depending on amrour. It would be faster and more simple to do than dps. But ok :) )

I know, but I wanted to be thourough on the first run, since I then already need a lot of this code for D1965 ;)

FeXoR added a comment.Thu, Jul 4, 9:13 PM

Thanks for the link, @Angen !
Please add the ticket and the link to the exact diff the patch you wrote build on to the description @Freagarach.

So I guess this is part of a vast WIP attempt to add secondary attacks?

Is there a concept that outlines in full how this should work and what the result would be?
(I see some minor concerns stated in #252 like "unintended behavior" but for me this has a lot of potential to make 0 A.D. unplayable)

But since this patch is just an addition (If I understand this right) then I should raise my concerns there instead ;)

In D2038#84973, @FeXoR wrote:

Thanks for the link, @Angen !
Please add the ticket and the link to the exact diff the patch you wrote build on to the description @Freagarach.

I didn't wrote this patch on anything if that is what you mean? When I was busy with D1965 (instantkill and immunity) @Angen came with the feedback that I should check whether an entity is actually attackable.

In D2038#84973, @FeXoR wrote:

So I guess this is part of a vast WIP attempt to add secondary attacks?

Whilst working on that noted above I thought that I might as well extend this to what I'm currently trying to achieve (which could be useful for secondary attacks indeed).

In D2038#84973, @FeXoR wrote:

Is there a concept that outlines in full how this should work and what the result would be?

There is not (yet) a concept, I'm just trying stuff and hoping it may become useful ;)

In D2038#84973, @FeXoR wrote:

(I see some minor concerns stated in #252 like "unintended behavior" but for me this has a lot of potential to make 0 A.D. unplayable)

I am very much aware of that.

In D2038#84973, @FeXoR wrote:

But since this patch is just an addition (If I understand this right) then I should raise my concerns there instead ;)

Due to the split just requested in D1965, and the other good points made, I will pause this ;)

Freagarach updated this revision to Diff 8721.Thu, Jul 4, 9:59 PM

Some sort of DPS. Just for reference, I probably won't work on this soonish.

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
| 438| 438| 	{
| 439| 439| 		let expectedDamage = 0;
| 440| 440| 		let damage = this.GetAttackStrengths(attackType);
| 441|    |-		let damageBonus = GetDamageBonus(this.entity, target, attackType, this.GetBonusTemplate(attackType))
|    | 441|+		let damageBonus = GetDamageBonus(this.entity, target, attackType, this.GetBonusTemplate(attackType));
| 442| 442| 		let maxRange = this.GetRange(attackType)["max"];
| 443| 443| 		let rangeBonus = Math.min(maxRange, distance);
| 444| 444| 
|    | [NORMAL] ESLintBear (dot-notation):
|    | ["max"] is better written in dot notation.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
| 439| 439| 		let expectedDamage = 0;
| 440| 440| 		let damage = this.GetAttackStrengths(attackType);
| 441| 441| 		let damageBonus = GetDamageBonus(this.entity, target, attackType, this.GetBonusTemplate(attackType))
| 442|    |-		let maxRange = this.GetRange(attackType)["max"];
|    | 442|+		let maxRange = this.GetRange(attackType).max;
| 443| 443| 		let rangeBonus = Math.min(maxRange, distance);
| 444| 444| 
| 445| 445| 		for (let damageType in damage)
|    | [NORMAL] ESLintBear (dot-notation):
|    | ["repeat"] is better written in dot notation.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
| 445| 445| 		for (let damageType in damage)
| 446| 446| 			expectedDamage += damage[damageType] * rangeBonus * damageBonus * Math.pow(0.9, armourStrengths[damageType] || 0);
| 447| 447| 
| 448|    |-		expectedDPSPerType[attackType] = expectedDamage / this.GetTimers(attackType)["repeat"];
|    | 448|+		expectedDPSPerType[attackType] = expectedDamage / this.GetTimers(attackType).repeat;
| 449| 449| 	}
| 450| 450| warn(uneval(expectedDPSPerType));
| 451| 451| 	let typesDPSSorted = Object.keys(expectedDPSPerType).sort(function(a,b){
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 0.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
| 447| 447| 
| 448| 448| 		expectedDPSPerType[attackType] = expectedDamage / this.GetTimers(attackType)["repeat"];
| 449| 449| 	}
| 450|    |-warn(uneval(expectedDPSPerType));
|    | 450|+	warn(uneval(expectedDPSPerType));
| 451| 451| 	let typesDPSSorted = Object.keys(expectedDPSPerType).sort(function(a,b){
| 452| 452| 		return expectedDPSPerType[a] - expectedDPSPerType[b]
| 453| 453| 	});
|    | [NORMAL] ESLintBear (comma-spacing):
|    | A space is required after ','.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
| 448| 448| 		expectedDPSPerType[attackType] = expectedDamage / this.GetTimers(attackType)["repeat"];
| 449| 449| 	}
| 450| 450| warn(uneval(expectedDPSPerType));
| 451|    |-	let typesDPSSorted = Object.keys(expectedDPSPerType).sort(function(a,b){
|    | 451|+	let typesDPSSorted = Object.keys(expectedDPSPerType).sort(function(a, b){
| 452| 452| 		return expectedDPSPerType[a] - expectedDPSPerType[b]
| 453| 453| 	});
| 454| 454| warn(uneval(typesDPSSorted));
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
| 449| 449| 	}
| 450| 450| warn(uneval(expectedDPSPerType));
| 451| 451| 	let typesDPSSorted = Object.keys(expectedDPSPerType).sort(function(a,b){
| 452|    |-		return expectedDPSPerType[a] - expectedDPSPerType[b]
|    | 452|+		return expectedDPSPerType[a] - expectedDPSPerType[b];
| 453| 453| 	});
| 454| 454| warn(uneval(typesDPSSorted));
| 455| 455| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 0.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
| 451| 451| 	let typesDPSSorted = Object.keys(expectedDPSPerType).sort(function(a,b){
| 452| 452| 		return expectedDPSPerType[a] - expectedDPSPerType[b]
| 453| 453| 	});
| 454|    |-warn(uneval(typesDPSSorted));
|    | 454|+	warn(uneval(typesDPSSorted));
| 455| 455| 
| 456| 456| 	let isPreferred = className => this.GetPreferredClasses(className).some(isTargetClass);
| 457| 457| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 0.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
| 458| 458| 	let chosenType = attackTypes.sort((a, b) =>
| 459| 459| 		(attackTypes.indexOf(a) + (isPreferred(a) ? attackTypes.length : 0)) -
| 460| 460| 		(attackTypes.indexOf(b) + (isPreferred(b) ? attackTypes.length : 0))).pop();
| 461|    |-warn(uneval(chosenType));
|    | 461|+	warn(uneval(chosenType));
| 462| 462| 
| 463| 463| 	return chosenType;
| 464| 464| };
|    | [NORMAL] ESLintBear (spaced-comment):
|    | Expected space or tab after '//' in comment.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
| 565| 565| 
| 566| 566| 		let horizSpeed = +this.template[type].Projectile.Speed;
| 567| 567| 		let gravity = +this.template[type].Projectile.Gravity;
| 568|    |-		//horizSpeed /= 2; gravity /= 2; // slow it down for testing
|    | 568|+		// horizSpeed /= 2; gravity /= 2; // slow it down for testing
| 569| 569| 
| 570| 570| 		let cmpPosition = Engine.QueryInterface(this.entity, IID_Position);
| 571| 571| 		if (!cmpPosition || !cmpPosition.IsInWorld())
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
| 612| 612| 		// TODO: Use unit rotation to implement x/z offsets.
| 613| 613| 		let deltaLaunchPoint = new Vector3D(0, this.template[type].Projectile.LaunchPoint["@y"], 0.0);
| 614| 614| 		let launchPoint = Vector3D.add(selfPosition, deltaLaunchPoint);
| 615|    |-		
|    | 615|+
| 616| 616| 		let cmpVisual = Engine.QueryInterface(this.entity, IID_Visual);
| 617| 617| 		if (cmpVisual)
| 618| 618| 		{
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'else'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/components/Attack.js
| 684| 684| 			});
| 685| 685| 	}
| 686| 686| 	else
| 687|    |-	{
|    | 687|+	
| 688| 688| 		// Melee attack - hurt the target immediately
| 689| 689| 		cmpDamage.CauseDamage({
| 690| 690| 			"strengths": this.GetAttackStrengths(type),
| 694| 694| 			"type": type,
| 695| 695| 			"attackerOwner": attackerOwner
| 696| 696| 		});
| 697|    |-	}
|    | 697|+	
| 698| 698| };
| 699| 699| 
| 700| 700| /**

binaries/data/mods/public/simulation/components/Attack.js
| 422| »   »   return;
|    | [NORMAL] ESLintBear (consistent-return):
|    | Function expected a return value.

binaries/data/mods/public/simulation/components/Attack.js
| 426| »   »   return;
|    | [NORMAL] ESLintBear (consistent-return):
|    | Function expected a return value.

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

binaries/data/mods/public/simulation/components/Attack.js
| 441| »   »   let·damageBonus·=·GetDamageBonus(this.entity,·target,·attackType,·this.GetBonusTemplate(attackType))
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

binaries/data/mods/public/simulation/components/Attack.js
| 442| »   »   let·maxRange·=·this.GetRange(attackType)["max"];
|    | [NORMAL] JSHintBear:
|    | ['max'] is better written in dot notation.

binaries/data/mods/public/simulation/components/Attack.js
| 448| »   »   expectedDPSPerType[attackType]·=·expectedDamage·/·this.GetTimers(attackType)["repeat"];
|    | [NORMAL] JSHintBear:
|    | ['repeat'] is better written in dot notation.

binaries/data/mods/public/simulation/components/Attack.js
| 452| »   »   return·expectedDPSPerType[a]·-·expectedDPSPerType[b]
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

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

Link to build: https://jenkins.wildfiregames.com/job/differential/1898/display/redirect

elexis added a subscriber: elexis.Thu, Jul 4, 10:48 PM

This is not compleat yet (test also fails), but uploaded for feedback.

Performance impact might be considered.
Perhaps it should rather be static (in the sense of only depending on templates, not technologies and auras), then it could be cached per template type.
(But perhaps the function isn't called so often and if, not so expensive, I didn't check)

FeXoR added a comment.Sun, Jul 7, 1:09 PM

@"if an entity is attackable" When does this happen?