Page MenuHomeWildfire Games

Support for arbitrary attack types.
Needs ReviewPublic

Authored by Freagarach on Jun 19 2019, 10:37 AM.

Details

Reviewers
bb
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Trac Tickets
#252
Summary

This is but a start, to start discussion and receive feedback.
So note: this is far from compleat! (One can play with the current diff though.)
ToDo:

  • Let "Ranged" in "Attack.js" be a consequence of the existence of a "Projectile" node (with appropriate names).
  • Do not hardcode any attack types. (Lots of work probably.)
Test Plan

None yet.

Diff Detail

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

Event Timeline

Freagarach created this revision.Jun 19 2019, 10:37 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.
|----|    | /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
| 505| 505| 
| 506| 506| 		let horizSpeed = +this.template[type].Projectile.Speed;
| 507| 507| 		let gravity = +this.template[type].Projectile.Gravity;
| 508|    |-		//horizSpeed /= 2; gravity /= 2; // slow it down for testing
|    | 508|+		// horizSpeed /= 2; gravity /= 2; // slow it down for testing
| 509| 509| 
| 510| 510| 		let cmpPosition = Engine.QueryInterface(this.entity, IID_Position);
| 511| 511| 		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
| 552| 552| 		// TODO: Use unit rotation to implement x/z offsets.
| 553| 553| 		let deltaLaunchPoint = new Vector3D(0, this.template[type].Projectile.LaunchPoint["@y"], 0.0);
| 554| 554| 		let launchPoint = Vector3D.add(selfPosition, deltaLaunchPoint);
| 555|    |-		
|    | 555|+
| 556| 556| 		let cmpVisual = Engine.QueryInterface(this.entity, IID_Visual);
| 557| 557| 		if (cmpVisual)
| 558| 558| 		{
|    | [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
| 624| 624| 			});
| 625| 625| 	}
| 626| 626| 	else
| 627|    |-	{
|    | 627|+	
| 628| 628| 		// Melee attack - hurt the target immediately
| 629| 629| 		cmpDamage.CauseDamage({
| 630| 630| 			"strengths": this.GetAttackStrengths(type),
| 634| 634| 			"type": type,
| 635| 635| 			"attackerOwner": attackerOwner
| 636| 636| 		});
| 637|    |-	}
|    | 637|+	
| 638| 638| };
| 639| 639| 
| 640| 640| /**

binaries/data/mods/public/simulation/components/Attack.js
| 495| ·»   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
| 599| »   »   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/1780/display/redirect

What does it do? Won't different attacks conflict? And how do units know which one to select?
Also, wasn't @bb working on something similar?

In D2002#83231, @Nescio wrote:

What does it do? Won't different attacks conflict? And how do units know which one to select?
Also, wasn't @bb working on something similar?

According to the ticket, @bb was working on it three years ago. I think this needs to be seen in conjunction with D1595 and #252.

Freagarach updated the Trac tickets for this revision.Jun 19 2019, 10:49 AM
Freagarach added a subscriber: Angen.
In D2002#83231, @Nescio wrote:

What does it do? Won't different attacks conflict? And how do units know which one to select?

The idea is that now we hardcode "Melee", "Ranged", "Slaughter" and "Capture". This would let one have multiple melee attacks, multiple ranged attacks, or just name them differently.
It is a prerequisite to having multiple attacks (see #252) - by itself it could change nothing.
It's a prerequisite for, for example, a magic unit that has several types of spell doing different types of damage (along with D1938)

Also, wasn't @bb working on something similar?

I believe @bb has a branch and was working more on https://trac.wildfiregames.com/ticket/252, which is related but not entirely so.

wraitii updated the Trac tickets for this revision.Jun 19 2019, 10:52 AM

Thank you for the clarification, that sounds great!

bb added a comment.Jun 19 2019, 11:05 AM

I indeed have a branch for the secondary attacks, which also generalizes the attackTypes. That branch is pretty much a more or less complete version of this revision, I will update that branch to current svn soon, but that will come after my exams of next week...

bb added a reviewer: bb.Jun 19 2019, 11:05 AM
In D2002#83250, @bb wrote:

(,,,) which also generalizes the attackTypes. (...)

Hmm, I did not get that from your diff. I might overlook it I guess.
Feel free to take over this diff if you want!

In D2002#83250, @bb wrote:

I indeed have a branch for the secondary attacks, which also generalizes the attackTypes. That branch is pretty much a more or less complete version of this revision, I will update that branch to current svn soon, but that will come after my exams of next week...

Sounds good. I suggest you commandeer this revision then - would be great if you could split the "secondary attack" part from the genericizing part.
@Freagarach Sounds like you might find it more useful to work on something else for now and review @bb's work later :)

bb added a comment.Jun 19 2019, 11:35 AM

For reference this was a WIP while I was working on the full system https://github.com/0ad/0ad/compare/master...bb-bb:t252_secondAttack. I have a little better version iirc, but that needs a rebase before uploading.