[PATCH] Missing round for walk/run speed

[elexis]

When upgrading the cavarly speed at the corral two times by 10%, the walk speed of persian spear cavalry is not rounded properly, see screenshot attached.

[elexis]

The walk speeds of those cav are:

No upgrades: 22 First upgrade: 24.2000...003 Both upgrades: 26.4000...003

Notice that 22 * 1,1 * 1,1 = 26,6 instead of 26,4 = round(22 * 1,1) * 1,1. So it looks like the upgraded values are rounded in general.

---

Thanks to ffm for reporting it!

[Michał]

patch fixing the issue

[Michał]

proof of concept of the patch based on three cavalry unit 2-step modifications

[Michał]

Hi, I'm new here. Decided to contribute.

I think I've found where the error is. Please see attached patch.

Mainly, the modification to the value of the speed during the second upgrade was made based on the original value, that is:

No upgrades: 22, bonus on upgrade: 22*1.1 = 2.2 1st upgrade: 22 + 2.2 = 24.2 2nd upgrade: 24.2 + 2.2 = 26.4

I assume this is not what is wanted for this specific case.

So previously the multiplication bonus was made based on the original value. My patch changes this to apply based on the current, if a former modification was applied.

I have not the knowledge whether this is the expected calculation for every mod in the game - haven't read through the whole design unfortunately.

I can only imagine that there can exist a case where the additional bonus for a second tier upgrade would be based on the original value, not on the current. If that is the case, my patch does not cover it. Nor I think this both cases can be covered using modification.multiply - they would have to be separated, I presume.

Though, I'm no expert.

Waiting for any feedback on this.

[leper]

Quoting the comment immediately preceding the code you changed

// Nothing is cumulative so that ordering doesn't matter as much as possible 

...

[elexis]

Replying to kravvcu:

Hi, I'm new here. Decided to contribute.

Same here

Mainly, the modification to the value of the speed during the second upgrade was made based on the original value, that is:

No upgrades: 22, bonus on upgrade: 22*1.1 = 2.2 1st upgrade: 22 + 2.2 = 24.2 2nd upgrade: 24.2 + 2.2 = 26.4

The code you address in the patch proves that my computaiton in the first comment was wrong and this one above is correct. There is no rounding involved when computing the units moving speed. Both +10% speed increase upgrades refer to the original speed.

I assume this is not what is wanted for this specific case.

According to the comment it is, so that code change is not required.

What needs to be addressed is the tooltip itself that displays the computed value. It should round to max 3 or 4 decimal places.

Waiting for any feedback on this.

Thanks for working on this!

[Michał]

Comparison of current formatting and proposed .5s and .2f options for speed values.

[Michał]

Current formatting in comparison to formatting options .5s and .2f. Second option's impact in rounding value of speed

[Michał]

Second iteration patch.

[Michał]

I've found the part of code where the formatting is defined. Please see the attached files, I've included my propositions for the fix there, I think the .jpg's speek for themselves.

I assumed we would want at max 2 decimal places.

By going with the '.5s' formatting option we would assume exclusion of speeds over 100 - this would be formatted with only one decimal place as the formatting option is the maximum width of the formatted string (dot included).

We can format the speed value as a float by going with the '.2f' option which always shows two decimal places even if they're 0. The negative side is that the value is rounded to the nearest one, which is shown in the second .jpg. Even so, I included a patch with this options set, as I think determinism is what we want, when it comes to the number of decimal values shown.

Related attachments: 0ad_comp.jpg, 0ad_comp2.jpg, patch_2

[leper]

We don't really have any floats in the gui. Just round it to the next int.

[elexis]

The dog speed also needs a rounding iirc

[Stan]

Any news ?

[Stan]

This only display the two first number, so unless the speed is <10

> 100 that should be fine

[Stan]

Proper Rounding

[Itms]

In 17070:

Fix missing rounding in the GUI. Patch by stanislas69, fixes #3245.