Attacks per second calculation (on poet's pen wand) possible rounding error

aliunwala commented 1 year ago

Check version

[X] I'm running the latest version of Path of Building and I've verified this by checking the changelog

Check for duplicates

[X] I've checked for duplicate issues by using the search function of the issue tracker

Check for support

[X] I've checked that the calculation is supposed to be supported. If it isn't please open a feature request instead (Red text is a feature request).

What is the value from the calculation in-game?

The trade site calculates "attacks per second" to 1.72 See here:

What is the value from the calculation in Path of Building?

If you import/copy item into pob it auto calculates it to be 1.73:

I am guessing this is a rounding error.

How to reproduce the issue

Go to trade:

https://www.pathofexile.com/trade/search/Standard/kX7n23Qi5

Copy item:

Item Class: Wands
Rarity: Unique
The Poet's Pen
Carved Wand
--------
Wand
Quality: +30% (augmented)
Physical Damage: 9-17
Critical Strike Chance: 7.00%
Attacks per Second: 1.72 (augmented)
--------
Requirements:
Level: 12
Int: 47
--------
Sockets: B-B-R 
--------
Item Level: 84
--------
Quality does not increase Physical Damage (enchant)
1% increased Attack Speed per 8% Quality (enchant)
--------
15% increased Spell Damage (implicit)
--------
+1 to Level of Socketed Skill Gems per 25 Player Levels
Trigger a Socketed Spell when you Attack with this Weapon, with a 0.25 second Cooldown
Adds 3 to 5 Physical Damage to Attacks with this Weapon per 3 Player Levels
12% increased Attack Speed
--------
In every piece of prose, lies a tiny spark of magic.
--------
Note: ~price 3 exalted

See that pob thinks that 1.72 should be 1.73

Character build code

No need you can do this with any character as its just the copy/paste of the item.

Screenshots

No response

aliunwala commented 1 year ago

Just a note. What is also makes this seem like a rounding error is that if you lower the attack speed of the weapon by 1. So that it says "11% increased Attack Speed" the attacks per second drops to 1.71.

Essentially skipping over 1.72.

Again not sure if this a problem or expected. But there is definitely a conflict between the POE trade side and POB when calculating the same value.

aliunwala commented 1 year ago

And another side note I would be happy to try and help find the issue and try to make a PR if you can point me to the file that keeps track of "attacks per second" (I am not a lua programmer. I usually do JavaScript/python/java so I might not be much help. But I am willing to try)

aliunwala commented 1 year ago

Did some more digging for my own sanity.

The carved wand base attack speed is 1.5 aps

##############Example 1############## POB's rounding seems to be incorrect in the case of a 12% poets pen with the 3% from the enchantment will then be: 1.5*(1+.12+.03) = 1.725 APS

Looks like PoB is rounding down (1.72) and the trade site is rounding up (1.73)

Not sure which rounding is correct.

But that seems to be the problem. (that or PoB is missing some precision on the calculation and 1.725 is actually being represented at 1.72499999 and that is getting rounded down.)

##############Example 2############## POB's rounding seems to be correct in the case with an 11% unenchanted poets pen: 1.5*(1+.11) = 1.665 APS

Which gets rounded up by both PoB and by the Trade site to 1.67

So that is also somewhat interesting as PoB looks like its rounding correctly in some cases and not in others.

Paliak commented 1 year ago

The breakdowns in pob are generally rounded to the second digit some more some less but 2 is generally the default. Had a quick look and the weapon attack rate seems to be here:

https://github.com/PathOfBuildingCommunity/PathOfBuilding/blob/78cd786c0d9153c2681735f5974ba0044290311e/src/Classes/Item.lua#L1310C2-L1310C2

but the actual rate is likely modified in a bunch of other places.

PathOfBuildingCommunity / PathOfBuilding