Page MenuHomeWildfire Games

Use relative templates for speed
ClosedPublic

Authored by fatherbushido on Sep 25 2017, 6:12 PM.

Details

Reviewers
None
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP20230: Use relative templates for unit speed. No significant value change. It makes…
Trac Tickets
#4352
Summary

Global speed of units can be tweak by changing only one value.
Balance will be easier.
Some intermediates values are used for making balance easier.

That patch does not change walking value (I didn't care of Run value). There could be slight changes due to rounding and I merge support units whose values were between 8.5 and 9.5.

(When that will be done also for Vision, we could do that Vision thing).

Things can be done in another way (I already wrote 3 different patches...)

I let some 1.0 for the one who will try to make values consistent or do some balance.

Test Plan

I already checked that everything was validating.

Here are the speed of all units

Before

After

The diff:

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

fatherbushido created this revision.Sep 25 2017, 6:12 PM

If a balance man wants, he would then have to check consitency and tweak those things:

template_unit_infantry_melee_pikeman.xml:38: <WalkSpeed op="mul">0.8</WalkSpeed>
template_unit_infantry_melee_spearman.xml:33: <WalkSpeed op="mul">0.95</WalkSpeed>
template_unit_infantry_melee_swordsman.xml:30: <WalkSpeed op="mul">1.05</WalkSpeed>
template_unit_infantry_ranged_archer.xml:39: <WalkSpeed op="mul">0.9</WalkSpeed>
template_unit_infantry_ranged_javelinist.xml:33: <WalkSpeed op="mul">1.5</WalkSpeed>
template_unit_infantry_ranged_slinger.xml:34: <WalkSpeed op="mul">1.2</WalkSpeed>

template_unit_champion_infantry_archer.xml:40: <WalkSpeed op="mul">1.2</WalkSpeed>
template_unit_champion_infantry_javelinist.xml:40: <WalkSpeed op="mul">1.75</WalkSpeed>
template_unit_champion_infantry_pikeman.xml:44: <WalkSpeed op="mul">0.8</WalkSpeed>
template_unit_champion_infantry_spearman.xml:42: <WalkSpeed op="mul">1.3</WalkSpeed>
template_unit_champion_infantry_swordsman.xml:36: <WalkSpeed op="mul">1.4</WalkSpeed>

template_unit_hero_infantry.xml:43: <WalkSpeed op="mul">1.0</WalkSpeed>
template_unit_hero_infantry_pikeman.xml:34: <WalkSpeed op="mul">0.95</WalkSpeed>
template_unit_hero_infantry_spearman.xml:34: <WalkSpeed op="mul">1.0</WalkSpeed>
template_unit_hero_infantry_swordsman.xml:29: <WalkSpeed op="mul">1.05</WalkSpeed>

template_unit_cavalry.xml:97: <WalkSpeed op="mul">1.95</WalkSpeed>
template_unit_cavalry_melee_spearman.xml:20: <WalkSpeed op="mul">1.25</WalkSpeed>
template_unit_cavalry_melee_swordsman.xml:31: <WalkSpeed op="mul">1.15</WalkSpeed>

template_unit_champion_cavalry.xml:61: <WalkSpeed op="mul">2.25</WalkSpeed>
template_unit_champion_cavalry_spearman.xml:31: <WalkSpeed op="mul">1.2</WalkSpeed>
template_unit_champion_cavalry_swordsman.xml:32: <WalkSpeed op="mul">1.2</WalkSpeed>

template_unit_hero_cavalry.xml:51: <WalkSpeed op="mul">1.85</WalkSpeed>
template_unit_hero_cavalry_javelinist.xml:34: <WalkSpeed op="mul">1.05</WalkSpeed>
template_unit_hero_cavalry_spearman.xml:23: <WalkSpeed op="mul">1.0</WalkSpeed>
template_unit_hero_cavalry_swordsman.xml:29: <WalkSpeed op="mul">1.0</WalkSpeed>

Don't know if relative numbers are easier to understand than aboslute ones in all cases. I think of rP18014 where numbers for animals were taken from some statistics, which achieved a visually appealing result. If the numbers for walk speed were taken from that, absolute numbers might be easier to read.
For places where inheritance is used (infantry for example), it probably makes sense to use relative numbers, so that one doesn't have to compare and since custom is used there. Something @Grugnas might be interested in.

fatherbushido edited the summary of this revision. (Show Details)Sep 25 2017, 6:32 PM
fatherbushido edited the test plan for this revision. (Show Details)
fatherbushido updated the Trac tickets for this revision.
In D930#36373, @elexis wrote:

Don't know if relative numbers are easier to understand than aboslute ones in all cases. I think of rP18014 where numbers for animals were taken from some statistics, which achieved a visually appealing result. If the numbers for walk speed were taken from that, absolute numbers might be easier to read.
For places where inheritance is used (infantry for example), it probably makes sense to use relative numbers, so that one doesn't have to compare and since custom is used there. Something @Grugnas might be interested in.

I had made the same thought. And finally did that.
The point of the patch is that when you think that vision range is too big and you need to change speed, then you change one value.

And...

Grugnas added a comment.EditedSep 25 2017, 6:57 PM

this is a nice improvement in order to keep things more transparent and easier to compare.
I wonder if it makes sense to have absolute values for "root" unit and building templates like (infantry, cavalry, heroes), (economy, military) while looking at child templates.

fatherbushido edited the test plan for this revision. (Show Details)Sep 25 2017, 7:00 PM
Vulcan added a subscriber: Vulcan.Sep 25 2017, 7:01 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jenkins-master:8080/job/phabricator/2070/ for more details.

Executing section Default...
Executing section Source...
Executing section JS...
Executing section XML GUI...
Executing section Python...
Executing section Perl...

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

oversight for catafalque and flaming pigs noticed with the coloured diff attached in the test part.

oversight for bireme noticed with the coloured diff attached in the test part.

fatherbushido added a comment.EditedSep 25 2017, 7:20 PM

Before:

After:

Diff:

I will commit that if nobody object.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jenkins-master:8080/job/phabricator/2071/ for more details.

Executing section Default...
Executing section Source...
Executing section JS...
Executing section XML GUI...
Executing section Python...
Executing section Perl...

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

(To be more explicit, walk speed, vision (also projectile speed and attack range though that doesn't fit well in inheritence schema as not all units have that) are things which are not absolute ie they are just relative to the scales we give to 0ad world. And people should not mismatch physical values and 0ad values where we can 'decorellate' some stuff).

This revision was automatically updated to reflect the committed changes.
Nescio added a subscriber: Nescio.Sep 25 2017, 8:43 PM

In principle I agree this is a good idea. However, I disagree with some on the choices you've made and I believe there is some room for improvement:

  • Assign easy base values in the template_unit.xml file (e.g. walk speed 10, run speed 20, min range 10, max range 500) to allow easy multiplications and avoid unnecessary rounding changes
  • Have at least two multiplication steps, one for the unit family (e.g. cavalry, infantry, ship, siege, support) and one for the unit type (e.g. archer, trireme, healer); you've done this for some, but not everywhere
  • If the desired value of a specific template happens to be identical to the parent's, assign a 1.0 multiplication nevertheless; not doing so (i.e. removing the tag altogether) makes things less obvious to understand or change in the future or mods (have a look at your changes of the support unit files)
  • Aim at keeping the final run stats effectively unchanged, even if this means multiplying them by different values than the walk speed; you've replaced the old run stats with often very different stats, or removed them; this means an undesirable loss of information.
  • Maybe leave the fauna untouched for now; I suppose there were good reasons for choosing the specific values they currently have; besides, there stats do not really affect balance; and elexis has a point, some of those multiplications could make things more confusing

And yes, there are other stats which could be made relative:

  • vision range: last time I tried I had an error message; 50*1.2 results into 60.0000, not 60, and apparently vision range has to be an integer; using op="add" (e.g. 50+10) instead of op="mul" works flawlessly
  • health: similar to walk speed, both add and mul work, although I'd recommend to use mul only
  • armour, costs, loot: better use add for these
  • attack (damage, range, time, etc), possibly, with disables in childs; could be useful but is more work to implement than most other stats
fatherbushido added a comment.EditedSep 25 2017, 8:55 PM
In D930#36403, @Nescio wrote:
  • vision range: last time I tried I had an error message; 50*1.2 results into 60.0000, not 60, and apparently vision range has to be an integer; using op="add" (e.g. 50+10) instead of op="mul" works flawlessly

Well rtfm first.
edit: ;-)

  • health: similar to walk speed, both add and mul work, although I'd recommend to use mul only
  • armour, costs, loot: better use add for these
  • attack (damage,

[...]

For those ones, I won't be so affirmative (for a global relative inheritence).

  • Assign easy base values in the template_unit.xml file (e.g. walk speed 10, run speed 20, min range 10, max range 500) to allow easy multiplications and avoid unnecessary rounding changes

It was the first thing I did. It wasn't a so good idea.

  • Have at least two multiplication steps, one for the unit family (e.g. cavalry, infantry, ship, siege, support) and one for the unit type (e.g. archer, trireme, healer); you've done this for some, but not everywhere

If I understand what you mean, ack. See https://trac.wildfiregames.com/ticket/4352#comment:3.
Can be done when doing """"balancing"""".

  • If the desired value of a specific template happens to be identical to the parent's, assign a 1.0 multiplication nevertheless; not doing so (i.e. removing the tag altogether) makes things less obvious to understand or change in the future or mods (have a look at your changes of the support unit files)

No strong opinion on that. I can say black or white. Matter of taste. I won't discuss any of that choice (I see pros and cons).
(I hope modders know what they are doing.)

  • Aim at keeping the final run stats effectively unchanged, even if this means multiplying them by different values than the walk speed; you've replaced the old run stats with often very different stats, or removed them; this means an undesirable loss of information.

Look at refs (D13)

  • Maybe leave the fauna untouched for now; I suppose there were good reasons for choosing the specific values they currently have; besides, there stats do not really affect balance; and elexis has a point, some of those multiplications could make things more confusing

4 has more meaning than 0.8? Be serious please ;-)

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jenkins-master:8080/job/phabricator/2072/ for more details.

Executing section Default...
Executing section Source...
Executing section JS...
Executing section XML GUI...
Executing section Python...
Executing section Perl...

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

It seems I was too late; apparently this patch was already committed while I was still typing my previous post. Anyway, it's not that important.

Well rtfm first.
edit: ;-)

Sometimes trial-and-error is more efficient :)

For those ones, I won't be so affirmative (for a global relative inheritence).

Note: “could”, not “should”; it's a possibility, not an imperative.

It was the first thing I did. It wasn't a so good idea.

Out of curiosity, may I ask why not?

If I understand what you mean, ack. See https://trac.wildfiregames.com/ticket/4352#comment:3.
Can be done when doing """"balancing"""".

Yes, but it could also have been done in this patch (e.g. unit_mechanical_ship.xml).

No strong opinion on that. I can say black or white. Matter of taste. I won't discuss any of that choice (I see pros and cons).
(I hope modders know what they are doing.)

In general I dislike unnecessary code and prefer files to be short, clear, and readable; however, if a piece of code is not present, it's easier too overlook and forget. And yes, I do hope anyone who edits a file knows what he's doing.

(Anyway, females and traders had a walk speed of 9.5, slaves of 8.0; you've removed the code from the support files, so both now are as fast as the generic unit, which is 9.0; females used to be 18.75% faster than slaves, this difference is now removed.)

Look at refs (D13)

D13 is not implemented. I'm aware the run feature does not really work yet and is intended to be changed. However, those old run values you've removed where probably not chosen randomly. E.g. several of the fauna files had a walk speed of 2.0 and a run speed of 18.0; you changed both to ×0.3, which might be a decent approximation of the walk speed, but results in a significantly lower run speed.

4 has more meaning than 0.8? Be serious please ;-)

No, it's not more meaningful, however, 3.0 is slightly easier to read than ×0.45, and for fauna it's less important to have relative speed; elexis probably expressed it more clearly:

In D930#36373, @elexis wrote:

Don't know if relative numbers are easier to understand than aboslute ones in all cases. I think of rP18014 where numbers for animals were taken from some statistics, which achieved a visually appealing result. If the numbers for walk speed were taken from that, absolute numbers might be easier to read.
For places where inheritance is used (infantry for example), it probably makes sense to use relative numbers, so that one doesn't have to compare and since custom is used there. Something @Grugnas might be interested in.

those old run values you've removed where probably not chosen randomly

you are optimistic

for fauna it's less important to have relative speed; elexis probably expressed it more clearly:

I understand and I could have say that. But then we ll miss the scope of the patch. Again I speak of speed and vision.

temple added a subscriber: temple.Sep 25 2017, 10:24 PM

Maybe define run speed in terms of walk speed? It seems ridiculous to have four lines in every file when one should do the job.

I don't think this makes it easier to balance one unit against another, since you have to do math to figure out the exact speeds (e.g. spear cav 9 * 1.95 * 1.25 versus champ spear cav 9 * 2.25 * 1.2, do you know which is bigger?), but it will make it easier to have more consistent speeds (e.g. spear cav, champ spear cav, hero spear cav should probably all have the same relative factor).

In D930#36413, @temple wrote:

Maybe define run speed in terms of walk speed? It seems ridiculous to have four lines in every file when one should do the job.

refs D438

I feel like a parrot

I don't think this makes it easier to balance one unit against another, since you have to do math to figure out the exact speeds (e.g. spear cav 9 * 1.95 * 1.25 versus champ spear cav 9 * 2.25 * 1.2, do you know which is bigger?), but it will make it easier to have more consistent speeds (e.g. spear cav, champ spear cav, hero spear cav should probably all have the same relative factor).

See that comment in the ticket...
as if I difn t consider that.

In D930#36413, @temple wrote:

Maybe define run speed in terms of walk speed? It seems ridiculous to have four lines in every file when one should do the job.

That suggests a fixed ratio, which is probably not a good idea; it should be possible to have units with simultaneously a very slow walk speed and a very high run speed.

In D930#36413, @temple wrote:

I don't think this makes it easier to balance one unit against another, since you have to do math to figure out the exact speeds (e.g. spear cav 9 * 1.95 * 1.25 versus champ spear cav 9 * 2.25 * 1.2, do you know which is bigger?), but it will make it easier to have more consistent speeds (e.g. spear cav, champ spear cav, hero spear cav should probably all have the same relative factor).

Things would be easier if champions simply had their citizen counterparts as their parent, instead of being a separate template tree. E.g. instead of current:

unit_champion.xml
unit_champion_cavalry.xml
unit_champion_cavalry_spearman.xml
unit_cavalry_melee_spearman.xml

Only:

unit_cavalry_melee_spear.xml
unit_cavalry_melee_spear_champion.xml

However, this is beyond the scope of this patch and ought to be discussed somewhere else.

In D930#36425, @Nescio wrote:
In D930#36413, @temple wrote:

Maybe define run speed in terms of walk speed? It seems ridiculous to have four lines in every file when one should do the job.

That suggests a fixed ratio, which is probably not a good idea; it should be possible to have units with simultaneously a very slow walk speed and a very high run speed.

I just meant instead of a run speed there'd be a run multiplier, which normally wouldn't need to be changed.

(Btw, you've done a nice job cleaning stuff up. If you want to take over D845 and fix it how you see fit, feel free. :) )

In D930#36425, @Nescio wrote:

That suggests a fixed ratio, which is probably not a good idea; it should be possible to have units with simultaneously a very slow walk speed and a very high run speed.

Still possible

Only:

unit_cavalry_melee_spear.xml
unit_cavalry_melee_spear_champion.xml

However, this is beyond the scope of this patch and ought to be discussed somewhere else.

I wrote that somewhere too. (Though both system have pros and cons).
I won't have write that patch for my own mod. I'd just use absolute values *everywhere*.
Things are different when we take care of a common place, we try to do minimal changes and we are quite limited.
But things are like that, when you go in another house, you take off shoes and you respect the place (aka politeness).

Now there is still that: https://trac.wildfiregames.com/ticket/4352#comment:3
I won't take it over as mehers gonna meh. But good luck for the one which would have a speed cleaning commited ;-)

(NB: I hope people will notice the time I have spent here to discuss.)