Page MenuHomeWildfire Games

Elevation attack bonus for units.
Needs ReviewPublic

Authored by Freagarach on Aug 11 2017, 1:19 AM.

Details

Reviewers
wraitii
temple
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Trac Tickets
#4028
Summary

Units currently get extra range if they're on a hill, but it would be nice if they got a damage bonus as well. See #4028.

The question is, what shape should the bonus take?

Here's one way to think about it. The damage should be something like the kinetic energy of the projectile when it lands. Of course the real picture is messy, but we can think of energy being conserved so it's the same as the energy when we shoot the projectile, which is the kinetic energy mv^2/2 plus the potential energy mgh, where h is the elevation difference between ourselves and the target. Compared to the case with no elevation bonus (h = 0), this is a ratio of 1 + 2gh/v^2.

So the damage multiplier formula should look like 1 + k * h, where k = 2g/v^2. Here's some values of k for g = 9.81 and various projectile speeds:

  v      k     unit
 37.5  0.0140  catapult
 62.5  0.0050  javelin, slinger
 75    0.0035  archer, tower
150    0.0009  bolt shooter

We could have the bonus depend on the speed of the projectile (so catapults would get a huge elevation bonus and bolt shooters would get a very little one), or we could simplify the situation and choose one value of k for all ranged units. I prefer the second approach.

I did some testing and I think I like k = 0.01. This has a nice english meaning too: an elevation bonus of 10m corresponds to a damage bonus of 10%. We can always change it later if it turns out to be too much (or too little).

Playing on the "Empire" map for example, small hills were 10-20m high, and a 10-20% bonus there seems reasonable. Blacksmith upgrades are 20% damage each, for comparison. Cliffs and huge hills were 30-40m, but I think that's fine too. Note that the bonus becomes a penalty for ranged units that have targets above them, e.g. a unit 20m below their target has a damage multiplier of 0.8. (I added a minimum value, 0.1, otherwise mountains over 100m tall would break the formula.) So there's a double effect for ranged vs ranged battles.

Here's the units that have an elevation bonus.

template_structure_defense_defense_tower.xml:      <ElevationBonus>15</ElevationBonus>
template_structure_defense_sentry_tower.xml:      <ElevationBonus>9.0</ElevationBonus>
template_unit_mechanical_siege_tower.xml:      <ElevationBonus>10</ElevationBonus>

To get the multiplier for them, we just add this height (maybe "ElevationBonus" is a bad name? (D2016)) to the elevation difference before plugging it into the formula. Note that enemies attack the base of the towers, so the damage bonus is only in one direction.

However, for units standing on walls, the bonus goes both ways, since the units are actually ~10m higher (the exact number depends on the civ). (Units on walls also have an aura for +3 armor and +20 vision, and melee units can't attack them.)

The elevation difference uses the y-coordinate of current target position, i.e. where the missile lands.

To get the damage multplier, we multiply the elevation bonus by the attack bonus. The attack bonus seems to be for hard counters, which are pike/spears vs cav, hunted elephants vs cav, and mace hero alexander sword cav vs other heroes. That is, there's currently no ranged hard counters, so the attack bonus here should always be 1.

In #4028, the idea is to then remove the range bonus for units. I'm ambivalent about this at the moment (haven't thought about it enough or gone through the calculations carefully), but anyway it should be done in a separate patch.

Test Plan

See if the damage formula makes sense.
Play on maps with hills and see if the value of k seems reasonable. (Add some print lines to spit out the actual elevation differences, etc.)
See if my understanding of the code is correct.

I'll add tests once the formula is okayed.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

temple created this revision.Aug 11 2017, 1:19 AM
temple edited the summary of this revision. (Show Details)
fatherbushido edited edge metadata.EditedAug 11 2017, 8:49 AM

Units currently get extra range if they're on a hill, but it would be nice if they got a damage bonus as well. See #4028.

The extra range ('citadel bonus') is imo sufficient.
#4028 was about avoiding some computations if we want to place at the 'best' range (reverting unitAI taking that into account) and finding a placeholder.
But last plans are to change unitAI/unitMotion.

Here's one way to think about it. The damage should be something like the kinetic energy of the projectile when it lands. Of course the real picture is messy, but we can think of energy being conserved so it's the same as the energy when we shoot the projectile, which is the kinetic energy mv^2/2 plus the potential energy mgh, where h is the elevation difference between ourselves and the target. Compared to the case with no elevation bonus (h = 0), this is a ratio of 1 + 2gh/v^2

That's exactly the things I thought at when I looked at that ticket. (one of the issue is that the multiplier is not really capped).


Well for the moment, I am for "not needed" :/ (so I won't review).

Stan added a subscriber: Stan.Aug 11 2017, 9:55 AM
Stan added inline comments.
binaries/data/mods/public/simulation/components/Attack.js
464

You should use doxygen style comments. I'm not sure this function even needs a comment as it's self explicit but maybe for parameters.

fatherbushido resigned from this revision.Aug 30 2017, 11:27 PM
fatherbushido requested changes to this revision.Oct 23 2017, 3:49 PM

I set request change as in reviewed but not wanted in the current state.

This revision now requires changes to proceed.Oct 23 2017, 3:49 PM
elexis added a subscriber: elexis.Dec 27 2017, 3:16 PM

Agree that from a gameplay point of view, either range or damage should be extended, but both seem a bit too strong.
But that is a decision for the templates.
Adding support in the code doesn't seem too bad.

It was said that range extension should be removed for performance and decidability reasons (going X to attack Y, then noticing that the range changed),
but I don't know if it's really a performance issue.

Keeping the range extension mechanism for mods or future reconsideration might not be bad.
Perhaps it could be disabled in the templates to address the performance issue without losing the feature.
But seems out of scope.

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

doxygen = C++, JSdoc = JS.

GetElevationDamageBonus would indeed remove the last bit of ambiguity making the comment unneeded.

468

-> Damage component I guess

539

wold be better to only pass atomic values rather than doing any kind of computation (logic) here

This revision now requires review to proceed.Jul 19 2019, 3:38 PM

This revision now requires review to proceed.

Not sure why and how I triggered that.

Freagarach updated this revision to Diff 9633.Sep 5 2019, 10:55 AM
Freagarach edited the summary of this revision. (Show Details)
Freagarach edited the test plan for this revision. (Show Details)
Freagarach added a reviewer: Restricted Owners Package.
  • Rebased.
  • Adressed inlines.
  • Included melee.

Breaks balance, I know.

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

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/77/display/redirect

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

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

Freagarach updated this revision to Diff 9634.Sep 5 2019, 11:20 AM

Fix test.

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

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/78/display/redirect

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

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

The 'AttackerData' change should be put in a different diff I think, if there's not one already.

Stan added inline comments.Sep 5 2019, 11:41 AM
binaries/data/mods/public/simulation/components/Attack.js
480

Not sure we need that temp. variable.

binaries/data/mods/public/simulation/helpers/Attacking.js
291 ↗(On Diff #9634)

Multiplier instead of Bonus ?No strong feeling.

Freagarach added a comment.EditedSep 5 2019, 11:42 AM
This comment has been deleted.
binaries/data/mods/public/simulation/components/Attack.js
480

It is needed further up in the code as well :) For the TTT calculation.

In D781#93951, @wraitii wrote:

The 'AttackerData' change should be put in a different diff I think, if there's not one already.

D2269.

Freagarach commandeered this revision.Oct 12 2019, 8:00 PM
Freagarach added a reviewer: temple.
Freagarach updated this revision to Diff 10133.Oct 12 2019, 8:02 PM
Freagarach marked 5 inline comments as done.

Rebased.

Freagarach retitled this revision from Elevation attack bonus for ranged units to Elevation attack bonus for ranged units..Oct 12 2019, 8:02 PM

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

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/440/display/redirect

Freagarach retitled this revision from Elevation attack bonus for ranged units. to Elevation attack bonus for units..Oct 12 2019, 8:03 PM

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

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