Page MenuHomeWildfire Games

Fix "GetBestAttackAgainst"-function in "Attack.js".
ClosedPublic

Authored by Freagarach on Jul 23 2019, 11:02 AM.

Details

Reviewers
wraitii
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP22569: Improve Attack's GetBestAttackAgainst by only considering attack types that…
Summary

This fixes the GetBestAttackAgainst-function from Attack.js. As can be seen in the test (which seems to be mistyped earlier (rP19528)), it now correctly returns the best attack when it can. This is done by using CanAttack in the function.
Split from D2044.
@bb, Please correct me if I'm wrong (since you're the author of D122).

Test Plan

Verify that this is indeed correct and does not break anything.

Diff Detail

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

Event Timeline

Freagarach created this revision.Jul 23 2019, 11:02 AM

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (spaced-comment):
|    | Expected space or tab after '//' in comment.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
| 524| 524| 
| 525| 525| 		let horizSpeed = +this.template[type].Projectile.Speed;
| 526| 526| 		let gravity = +this.template[type].Projectile.Gravity;
| 527|    |-		//horizSpeed /= 2; gravity /= 2; // slow it down for testing
|    | 527|+		// horizSpeed /= 2; gravity /= 2; // slow it down for testing
| 528| 528| 
| 529| 529| 		let cmpPosition = Engine.QueryInterface(this.entity, IID_Position);
| 530| 530| 		if (!cmpPosition || !cmpPosition.IsInWorld())
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
| 571| 571| 		// TODO: Use unit rotation to implement x/z offsets.
| 572| 572| 		let deltaLaunchPoint = new Vector3D(0, this.template[type].Projectile.LaunchPoint["@y"], 0.0);
| 573| 573| 		let launchPoint = Vector3D.add(selfPosition, deltaLaunchPoint);
| 574|    |-		
|    | 574|+
| 575| 575| 		let cmpVisual = Engine.QueryInterface(this.entity, IID_Visual);
| 576| 576| 		if (cmpVisual)
| 577| 577| 		{

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

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

This looks good, see inline.

binaries/data/mods/public/simulation/components/tests/test_Attack.js
267–270

I think you should pass two attack types, one for an enemy and one for an ally, so you can remove the attack workaround above.

Freagarach updated this revision to Diff 9156.Jul 28 2019, 3:21 PM
Freagarach marked an inline comment as done.

Removed attack-workaround in test.

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (spaced-comment):
|    | Expected space or tab after '//' in comment.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
| 524| 524| 
| 525| 525| 		let horizSpeed = +this.template[type].Projectile.Speed;
| 526| 526| 		let gravity = +this.template[type].Projectile.Gravity;
| 527|    |-		//horizSpeed /= 2; gravity /= 2; // slow it down for testing
|    | 527|+		// horizSpeed /= 2; gravity /= 2; // slow it down for testing
| 528| 528| 
| 529| 529| 		let cmpPosition = Engine.QueryInterface(this.entity, IID_Position);
| 530| 530| 		if (!cmpPosition || !cmpPosition.IsInWorld())
|    | [NORMAL] ESLintBear (no-trailing-spaces):
|    | Trailing spaces not allowed.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
| 571| 571| 		// TODO: Use unit rotation to implement x/z offsets.
| 572| 572| 		let deltaLaunchPoint = new Vector3D(0, this.template[type].Projectile.LaunchPoint["@y"], 0.0);
| 573| 573| 		let launchPoint = Vector3D.add(selfPosition, deltaLaunchPoint);
| 574|    |-		
|    | 574|+
| 575| 575| 		let cmpVisual = Engine.QueryInterface(this.entity, IID_Visual);
| 576| 576| 		if (cmpVisual)
| 577| 577| 		{

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

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

wraitii accepted this revision.EditedJul 28 2019, 4:48 PM

Fix looks sensible, works correctly.

Thanks for the patch!

This revision is now accepted and ready to land.Jul 28 2019, 4:48 PM
This revision was automatically updated to reflect the committed changes.
bb added a comment.Aug 4 2019, 12:36 PM

This probably wasn't done at the time since I didn't want to change code twice, but the change is in principle correct