Paths

Table of Contentst

Differential D2038

Let "GetBestAttackAgainst" depend on DPS.
Needs ReviewPublic
Actions

Authored by Freagarach on Jul 4 2019, 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 Build	Jenkins
Build 13341: arc lint + arc unit

Unit TestsFailed
View All

	Time	Test
	0 ms	Jenkins > 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 ms	Jenkins > TestAllocators::test_da
	0 ms	Jenkins > TestAtlasObjectXML::test_parse_attributes1
	0 ms	Jenkins > TestAtlasObjectXML::test_parse_attributes2
	0 ms	Jenkins > TestAtlasObjectXML::test_parse_basic
		View Full Test Results (1 Failed · 322 Passed)

Event Timeline

Freagarach created this revision.Jul 4 2019, 8:12 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!)

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

Harbormaster failed remote builds in B8187: Diff 8719!Jul 4 2019, 8:34 PM

Freagarach edited the summary of this revision. (Show Details)Jul 4 2019, 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

GetBestAttackAgainst - has to check attack damages vs armour damages

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.

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 :) )

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:

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

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 ;)

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 ;)

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

Harbormaster failed remote builds in B8189: Diff 8721!Jul 4 2019, 10:33 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)

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

bb mentioned this in D368: Gameplay Scripting: Entity and Actor coding for Secondary Attacks.Jul 16 2019, 3:25 PM

So, I'd side with @bb that this can be done sanely enough.

Computing DPS is easy enough. I would add this as a function to DamageReceiver, to take bonuses and techs into account. It's definitely not the fastest function in the world, but probably OK if we don't call it too often.
The hard question is how to factor range.

One important thing is to take min-range into account.

Basically I suggest weighing Range more or less depending on stance (defensive/passive/stanground would weigh Ranged increasingly higher), based on template value, and based on speed: if we are faster than our target we should prefer Ranged.

Ideas for acceptance tests:

A has a high-DPS melee attack and a low-DPS ranged attack
- B is in range of melee -> pick melee
- B is not in range of melee -> Pick Melee unless in standground/passive/defensive mode (and defensive might be template-dependent)
A has low-DPS melee attack and a High-DPS ranged attack
- Always pick ranged attack.
A has low-DPS melee attack and a High-DPS ranged attack with a min-range.
- A is faster than B
  - Pick Ranged (we'll auto-kite them because of the min-range)
- A is not faster than B
  - B is within min-range of A (i.e. ranged attack cannot be used) -> Pick Melee
  - B is is range of Ranged attack -> Pick Ranged

In D368 DPS has its function in the DamageReciever. Range is added as a multiplication factor:

in range, the factor is 1
inside minrange or outside maxrange we let the factor drop exponentially

This covers 1a, 2, 3b
3a is kindoff covered since it first will be melee (while we are still in range for that) and then will swap to ranged
1b: can be added by setting the factor to 0 in that case

Sounds good @bb! Would you mind/be able to split the DPS part from D368? Feel free to commandeer this.
Not sure about the automatic kiting though, would not like to make the function too smart?

Revision Contents
Changeset List

Path

Size

binaries/

data/

mods/

public/

simulation/

components/

Attack.js

53 lines

Diff 8721

View Options

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

Let "GetBestAttackAgainst" depend on DPS.Needs ReviewPublicActions