Page MenuHomeWildfire Games

Allow specifying the gravity applied to projectiles.
ClosedPublic

Authored by leper on Sep 3 2017, 1:43 AM.

Details

Reviewers
fatherbushido
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP20120: Allow specifying the gravity affecting projectiles.
Summary

This allows for differently shaped flight curves for projectiles.
E.g. mortars, magical projectiles.

Test Plan

Add <Gravity>0.0</Gravity> to some ranged unit template (e.g. template_unit_cavalry_ranged), then modify that to eg 700.0 to get the mortar effect.

Deciding whether this should be optional would be nice. There aren't that many templates in the public mod that would need to be changed (~30 that have a ranged element, but quite a few of those inherit from others, so the actual amount of changes needed would be lower). Might require some work by mods if those don't use our intermediate templates though.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

leper created this revision.Sep 3 2017, 1:43 AM
Vulcan added a subscriber: Vulcan.Sep 3 2017, 2:35 AM

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://jenkins-master:8080/job/phabricator/1954/ for more details.

Vulcan added a comment.Sep 3 2017, 2:37 AM
Executing section Default...
Executing section Source...
Executing section JS...

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

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

binaries/data/mods/public/simulation/components/Attack.js
| 470| »   »   let·gravity·=·this.template[type].Gravity·!=·undefined·?·+this.template[type].Gravity·:·9.81;
|    | [NORMAL] JSHintBear:
|    | Use '!==' to compare with 'undefined'.

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

binaries/data/mods/public/simulation/components/Attack.js
| 591| »   if·(c·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jenkins-master:8080/job/phabricator_lint/471/ for more details.

bb added a subscriber: bb.EditedSep 4 2017, 9:03 PM

Newton taught us that the gravitational acceleration is not influenced by the projectile in question. However Newton was not present anywhere near our timeframe, the laws of physics didn't change. So setting a projectile dependent gravitational acceleration doesn't seem realistic at all. I did rather have a gravitational acceleration settable per map (why disallow a map on the moon?).
On the contrary under certain assumptions (fixed orientation of the projectile, fixed speed, fixed air density and maybe some others) changing the acceleration has the wanted effect on the curve of the projectile regarding the air resistance. I guess in our game we could assume these (and what would be the difference :P), so imo we should projectiles could have multipliers for the gravitational acceleration but the base value is set per map. (that map setting could ofc be done in another rev)

fatherbushido accepted this revision.Sep 4 2017, 10:21 PM

Nice change.
Now we don t have to change the proj speed to have that purely graphical effect.
I wont expand more.
I m fine with keeping it like that, I bend more for making it mandatory.

binaries/data/mods/public/simulation/components/Attack.js
140 ↗(On Diff #3467)

(one could speak about the height but shape is good enough)

470 ↗(On Diff #3467)

I don t know about != and !== stuff

This revision is now accepted and ready to land.Sep 4 2017, 10:21 PM
elexis added a subscriber: elexis.Sep 4 2017, 11:27 PM

bb is kind of right that the phrasing is a bit misleading. Furthermore, gravity is a force, not an acceleration.
It could just be our brand or colloquial use of gravity.
But perhaps it could be phrased to become a projectile weight? That one schema sentence can hint at that only influencing the vertical speed.
meh, @coin

binaries/data/mods/public/simulation/components/Attack.js
470 ↗(On Diff #3467)

Indeed. != undefined is pointless. If the differentiation betweeen falsy values matters, then it is mandatory to use !==. If it doesn't matter, then we can just as well use !x, no?

(In which case we end up with gravity || 9.81)

bb added a comment.Sep 4 2017, 11:33 PM
In D864#33962, @elexis wrote:

bb is kind of right that the phrasing is a bit misleading. Furthermore, gravity is a force, not an acceleration.

The value here is clearly used as the gravitational acceleration and that one is the same for every projectile, if we want the force in the template we should also specify the mass to get the acceleration (since that is what we are interested in).

It could just be our brand or colloquial use of gravity.
But perhaps it could be phrased to become a projectile weight? That one schema sentence can hint at that only influencing the vertical speed.
meh, @coin

Do notice that the projectile weight (mass) has no influence on the curve (neglecting the air resistance which the code does)

In D864#33969, @bb wrote:

Do notice that the projectile weight (mass) has no influence on the curve

Damn, right, I forgot that. Then I cant help with that feature either lol.

leper updated this revision to Diff 3492.Sep 5 2017, 12:21 AM

Make the template property non-optional.

Owners added a subscriber: Restricted Owners Package.Sep 5 2017, 12:22 AM
Vulcan added a comment.Sep 5 2017, 2:50 AM
Executing section Default...
Executing section Source...
Executing section JS...

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

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

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

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

binaries/data/mods/public/simulation/components/Attack.js
| 589| »   if·(c·==·0)
|    | [NORMAL] JSHintBear:
|    | Use '===' to compare with '0'.
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jenkins-master:8080/job/phabricator_lint/482/ for more details.

Vulcan added a comment.Sep 5 2017, 4:26 AM

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://jenkins-master:8080/job/phabricator/1967/ for more details.

fatherbushido accepted this revision.Sep 5 2017, 1:55 PM

Tested again.
Templates are ok. (checked with scripts and unit demo maps too)

And still fully agree with the spirit of the patch which is relevant in our 2d sim.

This revision was automatically updated to reflect the committed changes.